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
69 stars 16 forks source link

Possible bug with latest tag in tools.toml #65

Closed MitchellBerend closed 1 year ago

MitchellBerend commented 1 year ago

There is a pr (#62) that adds a generate command that will generate the following default .tools.toml

# This file was automatically generated by tool-sync
#
#store_directory = "$HOME/.local/bin"
#
#    [bat]
#        owner     = "sharkdp"
#        repo      = "bat"
#        exe_name  = "bat"
#        tag       = "latest"
#    [exa]
#        owner     = "ogham"
#        repo      = "exa"
#        exe_name  = "exa"
#        tag       = "latest"
#    [fd]
#        owner     = "sharkdp"
#        repo      = "fd"
#        exe_name  = "fd"
#        tag       = "latest"
#    [ripgrep]
#        owner     = "BurntSushi"
#        repo      = "ripgrep"
#        exe_name  = "rg"
#        tag       = "latest"

If we run this uncommented file I get the following output

$ cargo run -- --config test-tools.toml sync 
   Compiling tool-sync v0.1.0 (/home/mitchell/rust/tool-sync)
    Finished dev [unoptimized + debuginfo] target(s) in 3.18s
     Running `target/debug/tool --config test-tools.toml sync`
⛔  bat     latest   [error] https://api.github.com/repos/sharkdp/bat/releases/tags/latest: status code 404
⛔  exa     latest   [error] https://api.github.com/repos/ogham/exa/releases/tags/latest: status code 404
⛔  fd      latest   [error] https://api.github.com/repos/sharkdp/fd/releases/tags/latest: status code 404
⛔  ripgrep latest   [error] https://api.github.com/repos/BurntSushi/ripgrep/releases/tags/latest: status code 404

My question, is this intended behavior and should the documentation reflect this or is this actually a bug?

I would like @FrancisMurillo's opinion on this as the original implementer as well. As I see it now the tag attribute can only be used to 'pin' a version and if its not pinned it has to be omitted. But should an explicit latest tag not also be allowed?

chshersh commented 1 year ago

@MitchellBerend Thanks for raising this question!

I think that the current behaviour is valid. In my vision, the tag field should exactly match the repository tag. So if there's no tag named latest, the command should fail.

The error message could be improved into saying something like:

There's no tag 'latest'

or even with the suggestion when possible in some cases:

There's no tag 'v13.0.0'. Perhaps you meant '13.0.0'?

There's a long road to this quality of error messages. Maybe there's a simpler way to achieve this 🤔 But at least the current behaviour seems valid and expected to me IMHO.

Documenting this behaviour would be a great idea 👍🏻 Maybe adding an FAQ question in the end of README, explaining potential pitfalls and non-obvious behaviours?


I can imagine a potential feature request where users can specify a separate version field in TOML with more flexible semantics (specifying both tag and version should be an error):

version = "latest"  # uses exactly the latest version
version = "13.0.0"  # matches both tags 'v13.0.0' and '13.0.0' so you don't need to know the naming scheme of a particular repository
version = "13"      # should automatically download the latest minor version matching '13.*.*'
MitchellBerend commented 1 year ago

Okay clear, I will incorporate this in #62 as well since I think this decision should be clear in the config as well.

FrancisMurillo commented 1 year ago

I think an optional explicit latest tag (tag = "lastest", or version = "latest") is valid and would be easy to do. Right now though, I see it really more optional since downloading the latest release is the default behavior (security and features) and only really add the tag option when it does not work on their or teammate's machine.

MitchellBerend commented 1 year ago

As there is consensus about this issue I am closing this. Thanks for the discussion.

The default behavior is:

chshersh commented 1 year ago

Thanks everyone for their input! As a followup, I've created the following two feature suggestions: