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

git-cliff: instead of finding git-cliff. It tries to install git-cliff-completions and git-cliff-mangen #387

Closed
NobodyXu opened this issue Sep 15, 2022 · 22 comments · Fixed by #413
Labels
Blocked: upstream Fix or feature is needed to be implemented upstream (in a dependency) Report: bug Something isn't working

Comments

@NobodyXu
Copy link
Member

git-cliff: instead of finding git-cliff. It tries to install git-cliff-completions and git-cliff-mangen

This should be a bug of cargo-binstall, I would investigate on that.
P.S. cargo-binstall v0.13.1 should support installing git-cliff out-of-the-box.

Originally posted by @NobodyXu in #385 (comment)

@NobodyXu
Copy link
Member Author

Its Cargo.toml contains:

[[bin]]
name = "git-cliff-completions"
path = "src/bin/completions.rs"

[[bin]]
name = "git-cliff-mangen"
path = "src/bin/mangen.rs"

It also has src/main.rs.

@NobodyXu
Copy link
Member Author

I've tried using --manifest-path, which uses Manifest::from_path_with_metadata in load_manifest_path, but that still doesn't work.

I suspect that this is an upstream bug.

@NobodyXu
Copy link
Member Author

NobodyXu commented Sep 15, 2022

The code in question is this line:

            if package.autobins && self.bin.is_empty() {

It disables auto discovery of bins if self.bins is not empty and skips checking for src/main.rs.

@NobodyXu
Copy link
Member Author

I have submit a PR to fix this: https://gitlab.com/crates.rs/cargo_toml/-/merge_requests/12

@NobodyXu NobodyXu added Report: bug Something isn't working Blocked: upstream Fix or feature is needed to be implemented upstream (in a dependency) labels Sep 15, 2022
@NobodyXu
Copy link
Member Author

cargo_toml v0.11.7 is released.

Unfortunately, we cannot upgrade to it yet.
embed-resource v1.7.3 depends on toml =0.5.8, but v0.11.7 depends on 0.5.9

I think the best way to resolve this is to use toml_edit in cargo_toml.

@NobodyXu
Copy link
Member Author

I think the best way to resolve this is to use toml_edit in cargo_toml.

I just realized that cargo_toml re-export toml::Value, so replacing toml with toml_edit really isn't an option.

However, what I can do is relax toml requirement to 0.5 so that embed-resource can be satisfied.

@NobodyXu
Copy link
Member Author

@azzamsa This bug is fixed and released in v0.13.2

@azzamsa
Copy link
Contributor

azzamsa commented Sep 24, 2022

Amazing, thanks for all the hardwork 🎉

@NobodyXu
Copy link
Member Author

NobodyXu commented Sep 24, 2022

I have submit a PR to fix this: https://gitlab.com/crates.rs/cargo_toml/-/merge_requests/12

Just realized that this PR I made is buggy when installing cargo-edit gives me warning "bin: add not found"

https://gitlab.com/crates.rs/cargo_toml/-/merge_requests/15

@NobodyXu
Copy link
Member Author

@azzamsa v0.13.2 has yanked due to #416, we have now released v0.13.3 that fixed this issue.

@azzamsa
Copy link
Contributor

azzamsa commented Sep 25, 2022

@NobodyXu Am I missing something?
It still fallback to cargo install

❯ cargo binstall --no-confirm --no-symlinks git-cliff --pkg-url="https://github.com/orhun/git-cliff/releases/download/v{ version }/{ name }-{ version }-{ target }.tar.gz" --pkg-fmt tgz --bin-dir "{ name }-{ version }/{ bin }" --install-path ~/.local/bin
13:52:41 [INFO] Resolving package: 'git-cliff'
13:52:49 [WARN] Error while downloading and extracting from fetcher github.com: bin file /home/azzamsa/.local/bin/cargo-binstallEr45nw/bin-git-cliff-x86_64-unknown-linux-gnu-GhCrateMeta/git-cliff-0.9.2/git-cliff-completions not found
13:52:52 [WARN] Error while downloading and extracting from fetcher github.com: bin file /home/azzamsa/.local/bin/cargo-binstallEr45nw/bin-git-cliff-x86_64-unknown-linux-musl-GhCrateMeta/git-cliff-0.9.2/git-cliff-completions not found
13:52:52 [WARN] The package will be installed from source (with cargo)
    Updating crates.io index
error: binary `git-cliff` already exists in destination
binary `git-cliff-completions` already exists in destination
binary `git-cliff-mangen` already exists in destination
Add --force to overwrite
13:52:53 [ERROR] Cargo errored! ExitStatus(unix_wait_status(25856))
13:52:53 [ERROR] Fatal error:
binstall::subprocess

  × subprocess Command { std: "cargo" "install" "git-cliff" "--version" "0.9.2" "--target" "x86_64-unknown-linux-gnu",
  │ kill_on_drop: false } errored with exit status: 101

@NobodyXu
Copy link
Member Author

It's because the pre-built binary tarball does not contain git-cliff-{completions, mangen}.
This issue is actually the same as #357 .

I will submit a PR soon for this.

@NobodyXu
Copy link
Member Author

#438 fixed installation of sccache, but git-cliff still fails because of this

@NobodyXu
Copy link
Member Author

@azzamsa I've created a new issue #439

@NobodyXu
Copy link
Member Author

@azzamsa cargo-binstall v0.14.0 has released.

@azzamsa
Copy link
Contributor

azzamsa commented Sep 28, 2022

@NobodyXu Am I missing some params?

❯ cargo binstall --no-confirm --no-symlinks git-cliff \
  --pkg-url="https://github.com/orhun/git-cliff/releases/download/v{ version }/{ name }-{ version }-{ target }.tar.gz" \
  --pkg-fmt tgz --bin-dir "{ name }-{ version }/{ bin }" --install-path ~/.local/bin

07:50:08 [INFO] Resolving package: 'git-cliff'
07:50:13 [WARN] Error while downloading and extracting from fetcher github.com: bin file /home/azzamsa/.local/bin/cargo-binstallA062gP/bin-git-cliff-x86_64-unknown-linux-gnu-GhCrateMeta/git-cliff-0.9.2/git-cliff-completions not found
07:50:14 [WARN] Error while downloading and extracting from fetcher github.com: bin file /home/azzamsa/.local/bin/cargo-binstallA062gP/bin-git-cliff-x86_64-unknown-linux-musl-GhCrateMeta/git-cliff-0.9.2/git-cliff-completions not found
07:50:14 [WARN] The package will be installed from source (with cargo)

@NobodyXu
Copy link
Member Author

@azzamsa Nope, this is an upstream issue as I commented here.

@NobodyXu
Copy link
Member Author

@azzamsa You don't need any --pkg-url or --pkg-fmt as the auto discovery mechanism of cargo-binstall is now smart enough to find it out-of-the-box.

@NobodyXu
Copy link
Member Author

NobodyXu commented Sep 28, 2022

@azzamsa To fix the issue, I recommend to submit a PR to git-cliff that makes git-cliff-completions and git-cliff-mangen optional by adding field required-features for them.

@orhun
Copy link

orhun commented Oct 8, 2022

I'm not sure how adding required-features makes the both binaries optional and makes them possible to install via cargo-binstall. Isn't that option only about the features that should be enabled for the given bin targets?

@NobodyXu
Copy link
Member Author

NobodyXu commented Oct 8, 2022

cargo-binstall considers binaries with required-features optional.

The pre-built artifacts provided by git-cliff only contains bin git-cliff but not git-cliff-completions or git-cliff-mangen.
If they are marked as optional by adding required-features, then cargo-binstall can install using the pre-built artifacts provided by git-cliff.

@passcod
Copy link
Member

passcod commented Oct 8, 2022

Binstall, unlike a more traditional/proper package manager, mostly relies on guessing rather than an authoritative manifest for packaging.

We could probably add a binstall metadata option to override the packaged bins list, to make it clearer / more explicit.

As we don't have that yet, and binstall's behaviour is to consider binaries with required-features optional (in that they may reasonably not be part of the default build, therefore not part of a packaging), as a workaround you can do something like add an empty feature and use required-features with that feature on the bins you don't package.

...or just include these bins in your tarballs~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked: upstream Fix or feature is needed to be implemented upstream (in a dependency) Report: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants