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

Allow custom crate install commands (cargo local-install) #608

Closed
thomasheartman opened this issue Nov 7, 2021 · 9 comments
Closed

Allow custom crate install commands (cargo local-install) #608

thomasheartman opened this issue Nov 7, 2021 · 9 comments
Assignees
Milestone

Comments

@thomasheartman
Copy link

thomasheartman commented Nov 7, 2021

Hi 🙋🏾

This is actually more of a question, but it might make sense to turn it into a feature request if the answer is no.

The question is: can I use cargo local-install instead of cargo install with the install_crate command?

(cargo-local-install (cargo-local-install on crates.io) solves some issues related to having a global cargo directory and crate versioning. )

Background

I was working on setting up Bevy for WASM compilation (using cargo-make) recently and ran into an issue where it wouldn't compile. As outlined in this issue thread, it's due to a version mismatch that's happening. The solution is specifying an exact version of the crate, so that was easy enough to solve.

However, the person who opened the thread suggested using cargo local-install to isolate the version used from other potentially conflicting versions of the crate.

I don't personally need this right now (the regular cargo install works just fine for me), but I thought I'd go and look the docs to see how or if it could be done. Reading through the docs, I couldn't find anything that seemed like it would solve it. I also looked through the code and think I found the function responsible for crate installation (in crate_installer.rs, right?), and it doesn't look like it supports changing the cargo subcommand used for installing crates.

And that's how I ended up here. It's mostly out of curiosity, but I could imagine that this is a use case that you'd want to support, so I thought I'd bring it up. I also couldn't find any issues that seemed to relate to this, but sorry if I missed something.

Feature Description

When using install_crate, I'd like to be able to customize the subcommand that is used for installation. In particular, I'd like to be able to use cargo local-install instead of cargo install.

Describe The Solution You'd Like

I think there's a few ways to go about this.

  1. Allow the user to specify whether an install is local or global, for instance via a boolean local attribute. The attribute would default to false to maintain existing behavior. Example:
    install_crate = {crate_name = "wasm-bindgen-cli", local=true}
  2. Allow the user to specify what cargo subcommand to use when installing the create via a string install_subcommand or install_command attribute. The attribute would default to install to maintain existing behavior. Example:
    install_crate = {crate_name = "wasm-bindgen-cli", install_command="local-install"}

The first option might be simpler, but the second option is more future-proof and would be easier to extend with other commands in the future. There might also be other options that are better suited, but I don't know the tool that well yet.


Anyway, thanks for listening. Let me know if this is interesting at all, if it's already possible (and I just couldn't find it in the docs), or if you've got any other questions and comments.

Cheers! 😄

Edit:

Added link to cargo-local-install on crates.io

@sagiegurari
Copy link
Owner

this is very cool idea. i never heard of local-install so very interesting.
naturally option 2 is more relevant.
i do have install_script so users can write their own logic but its more complex to do that.
would you like to submit a PR or prefer me to do it?

@thomasheartman
Copy link
Author

Yeah, I hadn't heard about local-install before either, but it seems like it would be relevant. I also realize I forgot to link to the crate in the OP, but it can be found on crates.io, where there's also a description of what it does and why.

I saw the install_script option, but because I've barely started using makefiles, it was a bit intimidating and I wasn't sure how I'd have to wire it up to make it work properly. So if it was an option on the install_crate command, I think that would make it more accessible.

As for the PR, I don't think I'll have time to work on this for at least a month and a bit, but could possibly look into it after that. Of course, if you (or anyone else) would like to work on it before then, please go ahead ☺️ Does that sound reasonable?

@sagiegurari sagiegurari added this to the 0.35.7 milestone Nov 8, 2021
@sagiegurari
Copy link
Owner

@thomasheartman I've implemented it in the dev branch 0.35.7
you can try it out.

@thomasheartman
Copy link
Author

@sagiegurari Oh, sweet! 😄 I'll try and take it for a spin tomorrow!

When the user specifies a custom install command, will cargo-make ensure that the required command is installed (will it trigger a cargo install cargo-local-install, for instance), or will it fail?

@sagiegurari
Copy link
Owner

no it won't, but you can have a task depending on another that will make that happen.
i felt it is extra complexity that can better be manual

@thomasheartman
Copy link
Author

thomasheartman commented Nov 9, 2021

@sagiegurari Thanks! Yeah, it seems to work as expected 🎉

Regarding the manual installation of the custom install command: you're right that it would definitely provide extra complexity to the implementation. I do think it would provide a better user experience, though.

Obviously, you know your users much better than I do, and also know how the code works better than I do, so it's up to you in the end.

That said, I think I would be very impressed if it 'just worked' when you specify a custom install command. You could try to cargo install cargo-<install-command> (and use --help as the test arg) unless the user specifies something else?

At the same time, it's quite possible that the custom install arg doesn't implement --help, so you'd need a way to override it and also to override the install command if it's not just cargo install cargo-<install_command>, and at this point, there's a lot to keep track of, all of a sudden.

So actually, after closer consideration, maybe you're right. Maybe the user should be responsible for installing their own custom command. But in that case, I think this should be very clear from the documentation and that the examples should definitely include that as well 💁🏾‍♂️

What do you think?

Edit:

Oh, and what's the right way to use install crate for cargo extensions?

I used this:

[tasks.cargo-local-install]
install_crate = {crate_name = "cargo-local-install", test_arg="--help"}

But that won't work right, will it? Will it know that it's cargo local-install --help it should run?

@sagiegurari
Copy link
Owner

  • that would work actually
[tasks.cargo-local-install]
install_crate = {crate_name = "cargo-local-install", test_arg="--help"}

cargo-make would know to attempt to call that binary with the arg to see it exists or not and only if it doesn't, it will install it.

  • as to 'just make it work', i think it just works for 99.99% of the time.
    people can just do
[tasks.magic]
command = "cargo"
args = ["somecrate", "whatever"]

and it will first ensure that somecrate is installed without defining install crate attribute.
your specific use case, i think, is important (which is why i implemented it) but i do not want to over eng stuff.
ya, i can do cargo install as you suggested, but... what if i need to use rustup instead?
cargo-make supports rustup as well, so maybe i need to use that? how would i specify it?
that would make install crate depend on another set of install crate info.
a better way (i think) is that, for this specific use case, you can just do depend on the task to another task.
this way you can define install_crate attribute which depends on another install_crate attribute.
the depended one, you can setup rustup or cargo install easily.

@thomasheartman
Copy link
Author

Sorry for the late reply; it's been a bit crazy lately 🙇🏼

cargo-make would know to attempt to call that binary with the arg to see it exists or not and only if it doesn't, it will install it.

Ah, that's nice.

And yeah, I agree with you on the other things 😄

@sagiegurari
Copy link
Owner

@thomasheartman this is no officially released. thanks a lot for the idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants