chshersh / tool-sync

🧰 Download pre-built binaries of all your favourite tools with a single command
https://crates.io/crates/tool-sync
Mozilla Public License 2.0
68 stars 16 forks source link

Can't sync starship (multiple name matches found) #116

Open slyshykO opened 1 year ago

slyshykO commented 1 year ago

https://github.com/starship/starship/releases

[starship] 
owner = "starship"
repo = "starship"
exe_name = "starship"
asset_name.windows = "starship-x86_64-pc-windows-msvc.zip"
>tool sync
x  starship
Multiple name matches found for this asset:

         * starship-x86_64-pc-windows-msvc.zip
         * starship-x86_64-pc-windows-msvc.zip.sha256

Please add one of these to the config.
MitchellBerend commented 1 year ago

Related to #102 and #70

There seems to be a naming convention where the name of the binary including the compression format is appended with the sha checksum in some projects.

chshersh commented 1 year ago

Indeed, that's a problem. Currently, there's no way in tool-sync to specify the exact asset when things like this happen.

The quick fix by @MitchellBerend in #117 solves this particular problem. But it's a bit ad-hoc. Someone down the road stores checksum with .md5 suffix and we'll need to introduce another dirty quick fix.

We can merge the quick fix to support a popular Rust tool. starship is already desired in:

I would like to include the implementation of the following issue in the next release:

Once it's done, I expect users to never need to specify custom rules for matching asset names. I have a design sketch in my mind but I didn't have time to write it explicitly yet 😞


Alternatively, we could support regexes in matching logic. So this would look like just:

asset_name.windows = "starship-x86_64-pc-windows-msvc.zip$"

It's an extra dependency and we probably will need to support regexes at some point. Not sure yet though 🤔

MitchellBerend commented 1 year ago

Oh I totally forgot md5, I would very much like regex support but iirc regex is a huge crate.

slyshykO commented 1 year ago

can we start with wildmatch ?

MitchellBerend commented 1 year ago

Unless I'm reading the docs wrong this doesn't solve the problem of having an extra suffix.

Another option is to treat the asset_name as the suffix that needs to be matched but that would exclude the option of having the asset name match on any substring.

MitchellBerend commented 1 year ago

asset_name.windows = "(starship)-x86_64-pc-windows-msvc.zip$"

Is the exe_name option even required anymore if regex is added like this?

Edit: Im not sure how the store_dir option works on windows, but we could probably figure out a way to make the config portable.

chshersh commented 1 year ago

Is the exe_name option even required anymore if regex is added like this?

It's required. This option is used to find the executable inside the asset archive. It's currently not used in the matching phase.

Im not sure how the store_dir option works on windows, but we could probably figure out a way to make the config portable.

Indeed, portable file paths is a problem I've encountered before. But it looks like not a problem?.. At least, I'm using the same config and / paths on CI for Windows, macOS and Linux and seem to work 🤔 Maybe because of PowerShell, it can handle them.

MitchellBerend commented 1 year ago

Thinking about this more another option came to mind. We can also implementing out own line terminator that way we can use the $ and we don't add another dependency.

chshersh commented 1 year ago

@MitchellBerend This feels like reinventing the wheel. Maybe we need the ability to match the asset name exactly, without any substring searches. And then people can use it when the logic for guessing an asset name is not enough.

Asset names cannot change within a single version, so people can update their configurations if they change in future names.

MitchellBerend commented 1 year ago

This feels like reinventing the wheel.

It definitely is.

Maybe a config option to enable/disable the name guessing functionality per tool is a good approach? I would have it enabled by default where the user can choose this for specific tools that pose problems.

chshersh commented 1 year ago

My point is that maybe giving too much flexibility and choice for the user will make the tool more difficult to use. My guess is that the following will be enough:

  1. Support automatic guess. Ideally, it should handle 99% of use cases. And we can always improve our rules when we find a counter-example.
  2. Manually specify the full exact name explicitly. So that users can specify the asset name without the need to understand the different searching systems.
hdhoang commented 1 year ago

sccache has an extra pattern to disambiguate for x86_64 musl, eg https://github.com/mozilla/sccache/releases/tag/v0.3.0

sccache-dist-v0.3.0-x86_64-unknown-linux-musl.tar.gz
sccache-dist-v0.3.0-x86_64-unknown-linux-musl.tar.gz.sha256

sccache-v0.3.0-x86_64-unknown-linux-musl.tar.gz
sccache-v0.3.0-x86_64-unknown-linux-musl.tar.gz.sha256

where the version is embedded in the middle, and a companion asset has a different name

k9s 0.29 shipped a companion .sbom file. We should filter that extension out.