Packages: [GitHub Actions] Content-Disposition header?
In the GitHub workflow ci.yml, there's an invocation of wget with an experimental (according to wget docs) --content-disposition option. It should instruct wget to "honor the Content-Disposition header when choosing local file names".
Either I'm missing something, or the server's response doesn't even contain such header:
HTTP/1.1 200 OK
Server: nginx
Date: Wed, 14 Oct 2020 13:58:52 GMT
Content-Type: application/octet-stream
Content-Length: 1118840
Last-Modified: Mon, 14 Sep 2020 06:18:34 GMT
Connection: keep-alive
ETag: "5f5f0b3a-111278"
Accept-Ranges: bytes
...rest is tar.gz binary content...
On the other hand, why using this option at all? Automation scripts are no place for random filenames dictated by the server. It would be much more robust approach to use deterministic file names, e.g. with -O/--output-document option:
wget -O st_syntax_tests.tar.xz https://url...
ratijas
All 9 comments
Using -O option would also solve the problem of parsing output from ls two lines later:
This approach is flawed, because in general ls can not be trusted to do globbing for later argument expansion. Even if right now there is a reason to believe there would only ever be one matching file — such assumption might simply break in different environment (such as a future evolution of this ci flow).
ratijas
on 14 Oct 2020
I'm not sure whether you're concerned about this repo or one you work on yourself. If it's one of yours, you may be interested in https://github.com/marketplace/actions/sublime-text-syntax-tests
michaelblyons
on 14 Oct 2020
Hi @michaelblyons,
I am about to make new syntax package, specifically Roff & friends (which is used mainly for man pages these days). So, naturally, I looked into how tests and CI can be configured, and I started from the official Packages repo.
Thanks for pointing out GitHub marketplace. Never used before. Gotta check it out.
Still, this issue is worth fixing, in my opinion. The patch is already on its way.
ratijas
on 14 Oct 2020
I have no opinion - the GitHub action was just copied from the previous Travis config. PRs welcome!
wbond
on 14 Oct 2020
Well, SublimeText/syntax-test-action also used --content-disposition for some undocumented reason. But at least they are downloading and extracting into a temporary directory (through mktemp).
Anyway, why use globbing when you can avoid it?
ratijas
on 14 Oct 2020
That’s a community project - you are welcome to inquire there. I don’t know the reason off of the top of my head.
wbond
on 14 Oct 2020
I found out the reason why --content-disposition flag is used in SublimeText/syntax-test-action project. It's because there are several different links to various *.tar.{bz2,gz,etc...} archives, plus one for the latest endpoint which redirects to, well, actually to the latest archive — format of which is not known in advance.
I feel like it needs a better solution that globbing. Extension reported by the server should be preserved, so that tar knows which kind of archive it is, but guessing a filename via st_*_*.tar.* just doesn't feel right.
ratijas
on 15 Oct 2020
I have no opinion - the GitHub action was just copied from the previous Travis config. PRs welcome!
Hi @wbond :eyes:
This PR is stalling.
ratijas
on 21 Oct 2020
The origin of the --content-disposition parameter was, as you correctly found out, the usage of a latest endpoint for ST3 syntax test builds. Since ST4 is still in semi-private alpha, there is also no latest endpoint for ST4 yet and the syntax test build numbers need to be adjusted manually, if desired.
I decided to stick with globs in the action because they are simple and work.
FichteFoll
on 22 Oct 2020
Related issues
philipbordallo
·
3Comments
ghost
·
3Comments
darkred
·
4Comments
jfcherng
·
3Comments
MiChAeLoKGB
·
4Comments