neon-bindings / rfcs

RFCs for changes to Neon
Apache License 2.0
14 stars 9 forks source link

RFC: cargo wrapper #4

Closed dherman closed 4 years ago

dherman commented 6 years ago

This RFC proposes we replace the neon CLI with a lightweight wrapper for cargo, called vargo. This also involves a change to the directory structure of Neon projects.

(Original PR was #1.)

Rendered

dherman commented 6 years ago

@nixpulvis I agree with you about cargo install vargo instead of npm install -g vargo. I think now that it's clearly supposed to be thought of as a Rust abstraction instead of a JS abstraction, it makes more sense to install it with cargo. I updated the RFC to reflect that.

matklad commented 6 years ago

cc @japaric: you've said that you've talked to @alexcrichton at rustfest about making wrappers like xargo unnecessary, so you may have something interesting to say about this RFC :)

nixpulvis commented 6 years ago

My understanding is that the implementation (and name vargo) are just temporary, awaiting more features of cargo itself. Whereas the directory structure should ideally remain consistent with a new cargo based implementation.

nixpulvis commented 6 years ago

I do worry that people will be confused by the CLI having a different name than the project itself.

nixpulvis commented 6 years ago

Having a neon executable that points people at vargo and explains the rational given that it's a thin wrapper we hope to be rid of one day might be a good idea, though maybe I'm just overthinking this.

japaric commented 6 years ago

@matklad I was referring only to Xargo. We are going to eventually land Xargo's "you can build core / std from source" feature in Cargo since it's been a long wanted feature (there's a few years old RFC about this) and the feature does nothing magic like other Cargo wrappers do (e.g. the Cross wrapper invokes Cargo within a Docker container -- that's never going to be merged into Cargo) -- it just calls Cargo twice.

What would help to ease the creation of Cargo wrappers, or maybe even make them obsolete, would be some sort of plugin system for Cargo (wrappers could become plugins) -- I think Nick was referring to this during the meeting. Plugins would be able to add custom behavior to different phases of the Cargo build process (e.g. before compilation, before linking, etc.) or maybe even tweak some phases of the build process (like a custom rustc driver, probably). Alex, Brian and I talked about this idea at some point but that's about it. Someone needs to come up with a design and write an RFC for something like it.

dherman commented 6 years ago

@nixpulvis I have some of the same worry myself. I suppose another possibility is to keep the name neon for the command but otherwise have its design be just like this RFC.

dherman commented 6 years ago

@japaric I've had one conversation with @aturon about this but I'd love to help participate in continuing that discussion, to figure out just what kinds of extension points cargo needs.

dherman commented 6 years ago

@nixpulvis I reverted the name back to neon and the design still feels like it holds up. I think I had convinced myself that the mental model couldn’t be understood without a name that sounds like cargo but I think I was wrong.

dherman commented 6 years ago

I got some awesome feedback from @hone and @wycats today. I think the merging of directories was a mistake but it was still solving a legitimate issue: that you should be able to run all your rust commands from the root directory, without having to cd into the rust directory.

But it looks like Cargo workspaces can solve this! We should be able to put a root Cargo.toml in the project root that declares a workspace and then you should be able to run neon from there. I’ll dig into the workspace docs some more and do another iteration of the RFC.

I am thinking I like the name crate/ better than native/ for the Rust subdirectory though. It seems a bit more conventional for Rust and easier to guess at its meaning. We can afford to change the name because this workspace approach is a backwards incompatible change to the directory structure anyway, albeit a less major one.

matklad commented 6 years ago

I am thinking I like the name crate/ better than native/ for the Rust subdirectory though.

May I suggest rust/ instead? I don't really like crate because it's somewhat ambiguous: it can mean either a single compilation unit (crate from the Rust language reference) or it can mean a Cargo package (typically, a single library crate + a bunch of supporting bin/ test/ example/ crates).

dherman commented 4 years ago

This was an interesting exploration, but lately we've been more interested in exploring ways we can eventually eliminate the CLI entirely and allow people to use cargo build and npm i directly. For neon new we'd like to see if we could use https://github.com/ashleygwilliams/cargo-generate or https://github.com/cli/cli with template repos. There's more to explore before we know if that will work, but it doesn't seem worth making major overhauls of the CLI right now.

matklad commented 4 years ago

Shamless plug: https://github.com/matklad/cargo-xtask is a horrible interesting hack to get something like a custom wrapper without investing into an actual wrapper.