search

taiki-e / install-action

GitHub Action for installing development tools (mainly from GitHub Releases).
Apache License 2.0
251 stars 32 forks source link

Support for idempotent installs/only install if missing? #577

Open knzai opened 3 weeks ago

knzai commented 3 weeks ago

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 commented 2 weeks ago

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.

taiki-e commented 2 weeks ago

(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

knzai commented 2 weeks ago

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 commented 2 weeks ago

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.

knzai commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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

taiki-e commented 2 weeks ago

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 commented 2 weeks ago

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

knzai commented 2 weeks ago

common cases I've seen in shell commands over the years:

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 commented 2 weeks ago

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_einstall$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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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