-
Notifications
You must be signed in to change notification settings - Fork 750
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
base: main
Are you sure you want to change the base?
Conversation
cp "$outdir/jj" "$staging/" | ||
tar czf "$staging.tar.gz" -C "$staging" . | ||
cd "$staging" | ||
tar czf "../$staging.tar.gz" * |
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?
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 reliable find
) 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?
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, but 7zip
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.
I think we can do
* .*
if we want to also include hidden files in the future. Do think we want that?
Usually one can use * .?*
to include hidden files but omit .
and ..
.
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 that cargo 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. Either 7zip
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.
Usually one can use * .?* to include hidden files but omit . and ...
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.
$ ls -a
./ ../ .a .ab aa
$ bash -c 'echo * .?*'
aa .a .ab
$ dash -c 'echo * .?*'
aa .. .a .ab
$ ls -A |cat
.a
.ab
aa
$ # **Update:**
$ /bin/bash -c 'echo * .?*' # Old version shipped with macOS
aa .. .a .ab
I keep coming to the conclusion that ls -A
or a find
command are easiest, unless we want to just settle on assuming bash (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,
$ dash -c 'echo * .[!.]*'
aa .a .ab
but we can make it harder:
$ touch ..aa
$ ls -A
..aa .a .ab aa
$ dash -c 'echo * .[!.]*'
aa .a .ab
So, I think this is probably the correct answer for anyone crazy enough to have files starting with two dots:
$ bash -c 'echo * .[!.]* ..?*'
aa .a .ab ..aa
$ dash -c 'echo * .[!.]* ..?*'
aa .a .ab ..aa
$ /bin/bash -c 'echo * .[!.]* ..?*' # Old version shipped with macOS
aa .a .ab ..aa
🙃
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 :)
IIUC Gaetan's point, there's also the issue unzip cannot unzip standard input
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):
wget https://github.com/sxyazi/yazi/releases/download/v25.5.31/yazi-x86_64-unknown-linux-musl.zip \
&& unzip -j -d ~/.local/bin/ yazi-x86_64-unknown-linux-musl.zip '*/yazi' \
&& rm yazi-x86_64-unknown-linux-musl.zip
to be compared with a tar.gz with a subdir:
curl -sSL https://github.com/glehmann/yage/releases/download/0.5.0/yage-0.5.0-linux-amd64.tar.gz \
| tar -xzC ~/.local/bin --strip-components=1 '*/yage'
or a tar.gz without subdir:
curl -sSL https://github.com/xcp-ng/randstream/releases/download/0.3.1/randstream-0.3.1-x86_64-unknown-linux-musl.tar.gz \
| tar -xzC ~/.local/bin/ ./randstream
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 setting bin-dir
That's what I meant. I didn't find the cargo binstall specific configuration. Nice to know it exists :)
264bd2e
to
c20a732
Compare
I did now test that this works, see https://github.com/ilyagr/jj/releases/tag/test-tar for the resulting archives. So, I'll unmark this as draft. I also made the same change to the docs archive, which I forgot to do before. There, using I realized that the docs archives benefit more from being packaged in a subdir, and it's less likely that anything depends on that directory structure, but I'm not sure whether packaging them differently is worth the inconsistency. See https://github.com/ilyagr/jj/releases/tag/test-tar2 and main...ilyagr:jj:tar-dot-slash-extra-docs-dir for that alternative. To be clear, I'm not insisting we merge this as-is, though (barring any simple mistakes I made or simple style changes) I'm not sure there's much gain to be had without a significant amount of additional work. But let me know. It's possible Yuya or someone else had (or will have) some insight I missed or misunderstood. |
Actually, I do have one alternative solution: do something explicitly bash-specific that will error outside bash:
In other words, we would do I think I prefer |
Fixes #6497 The new version would skip any hidden files in the dir, but we don't have any. See also the discussion in the linked issue. I didn't modify the `7zip` invocation as it does not create a `./` entry in the archive.
c20a732
to
70b2bbe
Compare
Fixes #6497
The new version would skip any hidden files in the dir, but we don't have any. See also the discussion in the linked issue.
I didn't modify the
7zip
invocation as it does not create a./
entry in the archive.This is a draft because I haven't fully tested it yet. We could also merge it before the next release and test it as we're releasing the next version.