Support for idempotent installs/only install if missing?

[knzai]

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.

[taiki-e]

We currently have this only for binstall.

install-action/main.sh

Lines 318 to 326 in b28eee2

	binstall_version=$(call_jq -r '.latest.version' "${manifest_dir}/cargo-binstall.json")
	local install_binstall='1'
	_binstall_version=$("cargo-binstall${exe}" binstall -V 2>/dev/null || echo "")
	if [[ -n "${_binstall_version}" ]]; then
	if [[ "${_binstall_version}" == "${binstall_version}" ]]; then
	info "cargo-binstall already installed at ${cargo_bin}/cargo-binstall${exe}"
	install_binstall=''
	else
	info "cargo-binstall already installed at ${cargo_bin}/cargo-binstall${exe}, but is not compatible version with install-action, upgrading"

The difficult point is that we need to handle the different formats of the version output well. For example, binstall changed it once.

install-action/.github/workflows/ci.yml

Lines 107 to 114 in b28eee2

	# We use the version output to check the version of cargo-binstall, but they
	# several times change the version output format in the past so we need to
	# check it with CI. (e.g., 0.14.0->0.16.0 update change it
	# from "cargo-binstall <version>" to "<version>")
	- run: |
	if [[ "$(cargo binstall -V)" != "$(jq -r '.latest.version' manifests/cargo-binstall.json)" ]]; then
	exit 1
	fi

That said, <tool> --version | head -1 | grep -Eq "(^|[^0-9])${version_to_install//\./\\.}($|[^0-9])" may actually be sufficient for most tools.

[taiki-e]

(There are also a number of tools that do not support version output...)

install-action/main.sh

Lines 786 to 811 in 3e71e71

	# cargo-udeps 0.1.30 and wasm-pack 0.12.0 do not support --version flag.
	case "${tool_bin_stem}" in
	# biome up to 1.2.2 exits with 1 on both --version and --help flags.
	# cargo-machete up to 0.6.0 does not support --version flag.
	# wait-for-them 0.4.0 exits with 1 on both --version and --help flags.
	biome | cargo-machete | wait-for-them) rx "${tool_bin_stem}" --version || true ;;
	# these packages support neither --version nor --help flag.
	cargo-careful | wasm-bindgen-test-runner) ;;
	# wasm2es6js does not support --version flag and --help flag doesn't contains version info.
	wasm2es6js) ;;
	cargo-*)
	case "${tool_bin_stem}" in
	# cargo-valgrind 2.1.0's --version flag just calls cargo's --version flag
	cargo-valgrind) rx "${tool_bin_stem}" "${tool_bin_stem#cargo-}" --help ;;
	*)
	if ! rx "${tool_bin_stem}" "${tool_bin_stem#cargo-}" --version; then
	rx "${tool_bin_stem}" "${tool_bin_stem#cargo-}" --help
	fi
	;;
	esac
	;;
	*)
	if ! rx "${tool_bin_stem}" --version; then
	rx "${tool_bin_stem}" --help
	fi
	;;

[knzai]

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

[taiki-e]

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
• Older Version Installed #348

[knzai]

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. :)

[knzai]

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)

[taiki-e]

--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 #577 (comment) grep approach.

Perhaps that information could also be used to simplify the branches mentioned above.

[taiki-e]

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/

[knzai]

Oooh, the CI logs make for such a nice easy sanity check!

[knzai]

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

[knzai]

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

1. cargo-careful doesn't even handle --help so just the command -v cargo-careful or which cargo-careful
2. cargo-zigbuild zigbuild supports --help but still no version string to be found
3. 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)
4. 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)

1. cargo-careful is the one sure get effed case
2. info: verifying sha256 checksum for cargo-zigbuild-v0.19.1.x86_64-unknown-linux-musl.tar.gz
3. https://github.com/DeepSourceCorp/cli/releases/download/v0.8.6/deepsource_0.8.6_linux_amd64.tar.gz
4. 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.

[taiki-e]

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.)

[knzai]

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 :)

[taiki-e]

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: shenek/wait-for-them#53

[jayvdb]

I also created DeepSourceCorp/cli#246 because I didnt realise it had a version subcommand.

[jayvdb]

Just now created rust-cross/cargo-zigbuild#272 , but the cargo search ... approach is a good fallback. But that means cargo becomes necessary, if it wasnt already?

[jayvdb]

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`.

[zanieb]

Instead of parsing the version output can you just see if the existing binary matches the checksum of the target version?

[taiki-e]

The checksum is for the archive, not the binary.
Anyway, I think it will be slower than just calling version command.