-
Notifications
You must be signed in to change notification settings - Fork 751
releasing workflow: get rid of ./
in tar archives
#7689
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ilyagr
wants to merge
1
commit into
main
Choose a base branch
from
ig/tar-dot-slash
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can do
* .*
if we want to also include hidden files in the future. Do think we want that?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not AFAIK! That would include
..
and./
again 😛 . (I haven't tested every shell, but this seems to be the case in Dash. Fish avoids this.)Currently, the best "probably correct" ways to do that in my mind are https://stackoverflow.com/questions/939982/how-do-i-tar-a-directory-of-files-and-folders-without-including-the-directory-it/39530409#comment140787449_39530409 (
ls -A | tar -T -
) or the comment that's attached to (using the more reliablefind
) or maybe https://stackoverflow.com/a/9776491/563359 (which tries to form a glob that excludes.
and..
).There is also a good Bash-specific option involving setting an option to make
*
include hidden files (but not.
or..
).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fwiw, tarballs typically have
<basename>/
prefix. Is there a reason to omit$staging/
path?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would mildly break any packagers that rely on our current scheme (Update: One specific thing to test would be whether
cargo binstall
still works. I'm not sure whether or not there are many more tools that would depend on the path). Also, I find this convention to be weird (I'm guessing the limitation we're discussing is the major reason for this convention; I want to get a time machine and tell them to fix their UI instead of introducing a weird convention to work around the weird limitation of the UI).We could still follow it, switching to it wouldn't be the end of the world.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we could use a saner tool, like
7zip
, to make the tarball. (I'm mostly joking, but7zip
is smart enough to skip./
, and I think it can make a tarball)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we don't have a time machine, I would just follow the convention. It might be also good to change the archive format to e.g.
.zip
to break existing packaging explicitly and make it clear that we don't follow the naming convention.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I packaged an application a few weeks ago, and I'm quite sure I had to not put the files in a base dir, as I usually do, to make it compatible with cargo binstall.
I wouldn't use zip, because it is much more difficult to install a tool packaged in a zip file with a one-liner, as it's frequently done in docker images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually one can use
* .?*
to include hidden files but omit.
and..
.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My plan was to test this PR and then merge it as an easy solution that can't break anything. Perhaps unwisely (would it really be more work than writing this comment?), I'm not sure I have the energy/desire to do much more than that at the moment.
Instead of using
*
, I could also list the three files we want in the archive explicitly, if people prefer.It seems that Thomas and Yuya would both prefer to create a subdirectory. I'm fine with that, as long as it works1 with
binstall
, but I don't want to do that myself (mainly because I cowardly don't want to be on the lookout for the unlikely possibility that this will break something for someone, and I'm lazy about remembering how to test thatcargo binstall
still works). One decision that would have to be made is whether to make this change to .tar.gz-s only or to zip-s as well. I might also be more up to it, or more up to helping others test such a change, at some later point.Another easy option would be to archive the binary file only, skipping the license and the README.
jj --help
already points to our website (though not to the place you could download the release).I am not sure about to switching to zip archives for the Linux binaries; this seems like it'd be unusual in a way that triggers my sense of conventionality. IIUC Gaetan's point, there's also the issue
unzip
cannot unzip standard input (I haven't double-checked, but it seems likely).OTOH, if we did switch to ZIP, this would solve the
./
problem without us having to do anything else. Either7zip
is clever about not creating those, or perhaps the ZIP format doesn't support those, but we already do not have the./
problem on Windows where we use ZIPs.Would that 't were so simple. (This is a scene about a silent movie actor moving to speaking cinema)
This trick seems to include
..
in less-sophisticated shells.I keep coming to the conclusion that
ls -A
or afind
command are easiest, unless we want to just settle on assumingbash (not sure why I refuse)recent bash. I think one of the answers I linked to in #7689 (comment) had a more complicated glob they thought worked.Update: This seems to work,
but we can make it harder:
So, I think this is probably the correct answer for anyone crazy enough to have files starting with two dots:
🙃
Oh, this doesn't work in fish shell. Fortunately, in fish shell, like in bash,
* .*
works.Footnotes
If
cargo binstall
doesn't work if.tar.gz
has a subdirectory, as Gaetan suggested (or did he mean the opposite?), it can probably be fixed by settingbin-dir
, but that's more work I'm not currently excited about doing. We could also try to fix it upstream; if a subdir is a common convention, perhapsbinstall
should support it out of the box. ↩There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I must have been even less clear than usual. Sorry for that :)
Yes. I should have said "less convenient" than "much more difficult" though. Here is an example of oneliner to install a command line tool from a zip file (with a subdir):
to be compared with a tar.gz with a subdir:
or a tar.gz without subdir:
That's what I meant. I didn't find the cargo binstall specific configuration. Nice to know it exists :)