Support for idempotent installs/only install if missing?
This doesn't have any support for not reinstalling each time run does it? When working on my action I was inspired by your action@command usage, so I have a zolacti.on@check and build that are run independently, and each installs zola if needed. I hacked in a quick
- name: check and store install state of zola for idempotency
shell: bash
run: |
if command -v zola > /dev/null 2>&1; then
echo "zola_installed=true" >> $GITHUB_ENV
fi
- name: Install zola
if: true && ! env.zola_installed
uses: taiki-e/install-action@zola
but I'll probably expand it to store the version instead of true, so I can pass that down to your script. But really at this point I'm just wrapping your install with my own with a check, and it made me wonder if I missed something in your action.
If not, any interested in adding it? I'll try and see if I can make a PR if so.
We currently have this only for binstall. https://github.com/taiki-e/install-action/blob/b28eee2bb643cb4606a986e802770206f53c5259/main.sh#L318-L326
The difficult point is that we need to handle the different formats of the version output well. For example, binstall changed it once.
https://github.com/taiki-e/install-action/blob/b28eee2bb643cb4606a986e802770206f53c5259/.github/workflows/ci.yml#L107-L114
That said, <tool> --version | head -1 | grep -Eq "(^|[^0-9])${version_to_install//\./\\.}($|[^0-9])" may actually be sufficient for most tools.
(There are also a number of tools that do not support version output...) https://github.com/taiki-e/install-action/blob/3e71e7135de310b70bc22dccb4d275acde8e055a/main.sh#L786-L811
Yeah I ended up using if command -v zola > /dev/null 2>&1; as the likely simplest and most robust. I considered parsing which, but command -v and check the exit code is even simpler maybe.
Getting in the edge cases of what to do if a version is requested, and it's a tool that doesn't have version output, is a bit thornier for sure. The simplest case of "don't install if any X are installed" isn't so bad with command -v
if command -v zola > /dev/null 2>&1; as the likely simplest and most robust.
Well, this action cannot adopt this way because it is insufficient, at least for use cases where people actually want to install the latest version.
- https://github.com/openrr/openrr/blob/d288bef2a386eb6b11522e1588f381f0435ab2f6/.github/workflows/ci.yml#L306
- https://github.com/taiki-e/install-action/issues/348
For sure, as per my second para I did consider the complexities that apply for this tool (to some small degree, obviously not as in-depth you do). If a tool is requested at a version, and the tool has no method of checking version, [re]install must happen, of course.
I have my own personal use case covered separately already, but will continue the discussion as long as you are interested re the broader case for the tool
Some sort of "don't force reinstall if we have sufficient information to not" flag is theoretically possible in the broader not just binstall case? Unless I'm missing something if a specific version is not requested, or the tool supports version info and that version is installed, you can noop across the board?
The install@tool usage then has a very easy skip check on command without even worrying about version format.
If you weren't already handling the logic for different tools not supporting -v it'd be more a pain for the next parts, but if you already know which do what, the logic you specified covers it I think?
I hope nobody supports --version without following the practice of script_name version_num on line one- wait I should go double check that one-liner works on my scripts. I threw the fuller version history in --version (but -v does the normal short script_name version_num). Lol, of course I didn't do it that way
If you are interested I could play around with a --skip_reinstall flag that does the above? I won't if it's not sufficient for the use cases or you aren't interested. :)
Yeah I gotta fix my script to be less pretty and do the more standard thing of outputting the basics on the first line. lmao.
I wonder if every --version follows $script_name white space with either a : - or wrapping () $version_num appearing somewhere in it's text if not line 1. If not sigh
ETA: yay shell scripting being loosely held conventions, heh. At least that was a quick fix (not that I expect anyone to use this script, but good to keep practices)
--skip_reinstall flag
The "don't reinstall if we know it's the same version" is perfectly reasonable, and ideally I would like to implement that for all tools by default.
And I believe it is at least possible to implement it for the tool you want to use (zola).
The install@tool ussage
This is a shorthand of tool@latest which install the latest version...
It would probably be reasonable to identify tools that can output the version from CI logs, make it recognizable from the tool side via manifest, and implement https://github.com/taiki-e/install-action/issues/577#issuecomment-2219713430's grep approach.
Perhaps that information could also be used to simplify the branches mentioned above.
implement #577 (comment) grep approach
<tool> --version | head -1 | grep -Eq "(^|[^0-9])${version_to_install//\./\\.}($|[^0-9])" ^^^^^^^
Hmm, head -1 before grep doesn't work with shellcheck (head -2 or another is needed)...
+ shellcheck --version
ShellCheck - shell script analysis tool
version: 0.10.0
license: GNU General Public License, version 3
website: https://www.shellcheck.net/
Oooh, the CI logs make for such a nice easy sanity check!
common cases I've seen in shell commands over the years:
- tool v1
- tool: 1.2
- tool - .2 #boo hiss why
- tool (0.2.3-alphatag)
- v[ersion] 1
- v[ersion] : 1.2
- v[ersion] - .2
- v[ersion] (0.2.3-alphatag)
But you can ignore the end parens and just stop after the first digit not followed by a .?
So I think the broadest possible all cases of version string, even tools you don't have supported yet might be grep with no head:
[only pseudo-code regex for discussion]
[version|$toolname]\s*[:-(]\s*v+\s*[regex for 0, 0.0, or 0.0.0 and ignore anything after like-alphajunk
There's very few edge cases in those logs that I could find, and most of them can be handled via output during install, so in theory it is available from somewhere? Not as idempotent as checking for installs from other sources via --version, but that's the edgest of cases? Who bothers implement help and doesn't even slip in a version number. sigh.
Currently not handled with --version according to CI output, but let's look at options
- cargo-careful doesn't even handle --help so just the
command -v cargo-carefulorwhich cargo-careful - cargo-zigbuild zigbuild supports --help but still no version string to be found
- deepsource requires a subcommand version, not a flag (so should be easy, it just needs special casing on running version check already, even for not doing this enhancement)
- wait-for-them supports --help but still no version string to be found
Okay, how about info from the install process itself (need to poke at which install path to see if available before installing)
- cargo-careful is the one sure get effed case
- info: verifying sha256 checksum for cargo-zigbuild-v0.19.1.x86_64-unknown-linux-musl.tar.gz
- https://github.com/DeepSourceCorp/cli/releases/download/v0.8.6/deepsource_0.8.6_linux_amd64.tar.gz
- https://github.com/shenek/wait-for-them/releases/download/v0.4.0/wait-for-them-linux
Hmm, maybe I guess maybe check https://crates.io/api/v1/crates/cargo-careful/versions before installing and use that version number and all 4 are covered?
Oh, or just
cargo search cargo-careful
cargo-careful = "0.4.2" # Execute Rust code carefully, with extra checking along the way
--
So I guess stash the parsed version from the install in some nice name scoped (taiki_e_install_$toolname) variable into > $GITHUB_ENV for the 4 cases that can't be figured out from the tool itself? These will only be idempotent if installed through the install-action itself, but that seems like a reasonable fallback.
Probably wan't a -f/--force-reinstall option if there isn't one already.
As for tools that do not provide a way to output the version, I think we can ignore (always reinstall) them for now. (I feel it would make more sense to send request to implement it than to look for a hack to get around it on our part.)
I feel like that's reasonable, and it's only 3 tools (if we consider deepsource just needing to be called differently).
But it's also easy to snag during install, if you want. But it is your lib, so :)
4. wait-for-them supports --help but still no version string to be found
I found there is an open feature request to add --version flag: https://github.com/shenek/wait-for-them/issues/53
I also created https://github.com/DeepSourceCorp/cli/issues/246 because I didnt realise it had a version subcommand.
Just now created https://github.com/rust-cross/cargo-zigbuild/issues/272 , but the cargo search ... approach is a good fallback. But that means cargo becomes necessary, if it wasnt already?
cargo-zigbuild --version works. I double checked and cargo-careful v0.4.3 still doesnt have a version anywhere
> cargo-careful --version
fatal error: `cargo-careful` called with bad first argument; please only invoke this binary through `cargo careful`
> cargo careful --version
fatal error: `cargo careful` supports the following subcommands: `run`, `test`, `build`, `nextest`, and `setup`
> cargo careful --help
fatal error: `cargo careful` supports the following subcommands: `run`, `test`, `build`, `nextest`, and `setup`.
Instead of parsing the version output can you just see if the existing binary matches the checksum of the target version?
The checksum is for the archive, not the binary. Anyway, I think it will be slower than just calling version command.