Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
slyshykO opened this issue Sep 22, 2022 · 12 comments · May be fixed by #164
Open

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

slyshykO opened this issue Sep 22, 2022 · 12 comments · May be fixed by #164
Labels
bug Something isn't working config TOML configuration, config-related CLI options
Milestone

Comments

@slyshykO
Copy link

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.
@slyshykO slyshykO changed the title Can't sync starship Can't sync starship (multiple name matches found) Sep 22, 2022
@MitchellBerend MitchellBerend added bug Something isn't working config TOML configuration, config-related CLI options labels Sep 22, 2022
@MitchellBerend
Copy link
Collaborator

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
Copy link
Owner

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 🤔

@chshersh chshersh added this to the v0.3.0: Auto milestone Sep 22, 2022
@MitchellBerend
Copy link
Collaborator

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

@slyshykO
Copy link
Author

slyshykO commented Sep 22, 2022

can we start with wildmatch ?

@MitchellBerend
Copy link
Collaborator

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
Copy link
Collaborator

MitchellBerend commented Sep 23, 2022

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
Copy link
Owner

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
Copy link
Collaborator

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
Copy link
Owner

chshersh commented Oct 6, 2022

@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
Copy link
Collaborator

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
Copy link
Owner

chshersh commented Oct 7, 2022

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
Copy link
Contributor

hdhoang commented Oct 26, 2022

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.

hdhoang added a commit to hdhoang/tool-sync that referenced this issue Feb 18, 2024
@hdhoang hdhoang linked a pull request Feb 18, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working config TOML configuration, config-related CLI options
Projects
None yet
4 participants