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

Feature request: Disable quick-install for specific crate in Cargo.toml #1721

Closed
NobodyXu opened this issue Jun 9, 2024 · 8 comments · Fixed by #1828
Closed

Feature request: Disable quick-install for specific crate in Cargo.toml #1721

NobodyXu opened this issue Jun 9, 2024 · 8 comments · Fixed by #1828

Comments

@NobodyXu
Copy link
Member

NobodyXu commented Jun 9, 2024

Our project just had a mis-adventure with binstall. It seems that cargo binstall preferred to download binaries from QuickInstall as opposed to our own, and they turned out to be broken.

martinvonz/jj#3844 (comment)
cargo-bins/cargo-quickinstall#250

Is there a way to specify in our Cargo.toml that our binaries should be preferred? We already have some cargo binstall config:
https://github.com/martinvonz/jj/blob/65a988e3d2d6b3ed201ed8c2b4952ef08344e55f/cli/Cargo.toml#L118-L121

Another issue is that we only provide musl binaries, so perhaps binstall prefers QuickInstall because it has gnu binaries.

Originally posted by @ilyagr in #19 (comment)

@ilyagr
Copy link

ilyagr commented Jun 9, 2024

I think this would be great! Thank you for considering it.

Clearly, QuickInstall is wonderful to have and make available to users, especially for projects that aren't aware of cargo binstall, but also for unusual targets. The only problem is that we'd like to make the installation of binaries from sources other than https://github.com/martinvonz/jj/releases to be opt-in, so that users know they installed unofficial binaries when they file a bug report. A setting in Cargo.toml would be perfect.

One possibility I was thinking of is that, if the install of official binaries fails, binstall could tell the user "You can try cargo binstall --allow-unofficial-binaries --allow-install-from-source jj-cli to install from other sources" (instead of installing from source). But a simple disabling of quickinstall would also work.

@NobodyXu
Copy link
Member Author

NobodyXu commented Jun 9, 2024

That can be added after Cargo.toml configuration support is done

@ilyagr
Copy link

ilyagr commented Jun 13, 2024

I'm not sure when/if I would get to it, but would you accept a PR for this or do you prefer to do these kinds of features yourself (because it's easier or for any other reason)?

@NobodyXu
Copy link
Member Author

Yes, I welcome any PR and contribution from cummunity!
FYI binstalk-{type, manifest} contain the manifest
And the binstalk is where you need to put the per-crate quick-install disable logic in

@senekor
Copy link
Contributor

senekor commented Jun 13, 2024

I have another idea, created a PR directly because the diff is tiny. Just close it if that's no good. We can just change the order of preference such that official binaries are always preferred over quickinstall ones.

#1741

@ilyagr
Copy link

ilyagr commented Jun 16, 2024

Just to say this explicitly: while #1741 is very helpful, and decreases the urgency as far as jj is concerned, I believe this feature still desirable in some form. I think you think so too as you haven't closed this.

If a project distributes official binaries via binstall, and even if the user is on a platform there are no official binaries for,
they should on one hand be able to get them from QuickInstall, but on the other hand there has to be a way to make sure binstall warns them that they are getting unofficial binaries. So, I think binstall should fail by default in this case, but tell the user how to override it and allow QuickInstall installation.

@NobodyXu
Copy link
Member Author

Yeah I agree, probably time to focus on cargo-binstall in my free time

@ilyagr
Copy link

ilyagr commented Jun 16, 2024 via email

NobodyXu added a commit that referenced this issue Jul 24, 2024
NobodyXu added a commit that referenced this issue Jul 26, 2024
github-merge-queue bot pushed a commit that referenced this issue Jul 26, 2024
* Update doc in SUPPORT.md for disabled-strategies

For #1828 and #1721

Signed-off-by: Jiahao XU <[email protected]>

* Update SUPPORT.md

Signed-off-by: Jiahao XU <[email protected]>

---------

Signed-off-by: Jiahao XU <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants