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

rfc: make cargo install extensible #2376

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
@ashleygwilliams
Member

ashleygwilliams commented Mar 27, 2018

Rendered

The goal of this RFC is to introduce the ability for an end user to be able to extend the cargo install command arbitrarily to include instructions that should be executed occur after cargo install <project>.


This RFC was a collaborative effort between @joshtriplett @spacekookie and @ashleygwilliams.


A note about cargo as a distribution tool

We are in no way suggesting that cargo install should be the definitive or exclusive distribution mechanism, or that it should supplant other distribution mechanisms such as platform package management tools; we are only proposing to improve the experience for the class of applications that already depend on cargo as a distribution platform today, notably small CLI applications and development tools for the Rust ecosystem.

Additionally, this proposal will make cargo install more useful for the larger set of rust applications that leverage the larger, often preferred, platform-specific distribution workflows, which typically want to install into a temporary directory and package the result. This point is discussed further in the “Rationale and Alternatives” section.

Fundamentally, the aim of this RFC is aligned directly with Rust’s value of “productivity”. It aims to make both building and using small developer CLI tools simpler and easier which has a direct positive effect on the ability of all to improve their workflow.

@ashleygwilliams

This comment has been minimized.

Member

ashleygwilliams commented Mar 27, 2018

for context i would really like this (if considered to be viable) to be blocked on #2196 because i believe the pattern that this RFC introduces builds the affordances in a way that mitigates much of what i believe could be seen as a downside of this RFC

@est31

This comment has been minimized.

Contributor

est31 commented Mar 27, 2018

Currently, cargo install tracks what files it installs, and supports cargo uninstall; this extension mechanism does not hook into that. Potentially, install.rs could emit a list of installed files and let cargo install install them, producing a log of those files for later uninstallation. However, we do not intend for cargo install to become a full-featured package management mechanism; rather, we expect cargo install to work analogously to make install. While the occasional package provides a make uninstall, few developers expect such a mechanism.

I guess I'm one of those who think that clean uninstallation is an important feature. I hope the statement that we are few is wrong.

I avoid make install like the plague and if I actually have to use it, if just invoking some binary in some build dir is not enough, I create a separate root fs and let it install inside there. This is a major hassle that I do just to have the simple pleasure of not getting my file system messed up with random stuff where I don't even know where it came from.

Currently, Cargo solves this in a beautiful way: it just puts a binary into a directory. It is simple, yes, not very extensible, yes. But it allows for clean and simple uninstallation. Updating is equivalent to "forcing" the installation.

With cargo install I actually started installing things into a per-user context because I trusted cargo: it just puts a binary into a directory. But if cargo install becomes extendible, it might put stuff anywhere in my home directory. And I'm not talking about malicious uses, but people who want to "do good" by instaling manpages or desktop shortcuts or file associations or similar.

If the RFC is merged in the current form, I'd probably stop using cargo install, and will be going back to patterns I've learned from C/C++: cargo build'ing and invoking binaries by their path inside the build directory.

Instead I think we should focus on artifact generation: allowing post-build scripts to create .msi files or .snap files or .deb files and allowing users to install them on their own, or maybe even on the user's behalf.

@liigo

This comment has been minimized.

Contributor

liigo commented Mar 28, 2018

Maybe a more general way is post-build.rs, which can be used to invoke bindgen for example (not only for cargo install).

@joshtriplett

This comment has been minimized.

Member

joshtriplett commented Mar 28, 2018

@est31

Instead I think we should focus on artifact generation: allowing post-build scripts to create .msi files or .snap files or .deb files and allowing users to install them on their own, or maybe even on the user's behalf.

I'm all for supporting that as well. I'd like to make packaging easier. (That also includes policy-compliant packaging that could go into distributions, not just creating a .deb from thin air without a source package.)

What I'm looking for here is a way to let crates provide additional files, not just binaries. Generating manpages, generating completions, and putting them in the appropriate places.

We'd be open to talking about how exactly that should work. For instance, we certainly could have install.rs output a list of files to install and where they should go (including paths to files in the output directory that it just generated), and then cargo install could do the actual installation, and track the files for later uninstallation. That has downsides, in that it's a lot less flexible and puts more policy into Cargo, but it would support cargo uninstall.

Another possibility would be to just install those files into a temporary directory under the output directory, and cargo install would install everything under that temporary directory to the directory specified as its --root.

We'd like to do the simplest thing that could work, here.

@joshtriplett

This comment has been minimized.

Member

joshtriplett commented Mar 28, 2018

@liigo We talked about having a post-build as well, but that doesn't solve the problem we're trying to solve. The issue is installing these files, not just generating them.

- Making `cargo install` more capable could encourage people to use it as a primary distribution mechanism for a broader class of applications, rather than just for simple command-line tools. On the other hand, this same mechanism can also serve as the basis for distribution packaging, which typically wants to install into a temporary directory and package the result.
- It makes the emergence of conventions significantly more difficult as the option to reuse, share, or automate this task has significantly affordance than integrating it into the familiar `cargo` step.
- Currently, `cargo install` tracks what files it installs, and supports `cargo uninstall`; this extension mechanism does not hook into that. Potentially, `install.rs` could emit a list of installed files and let `cargo install` install them, producing a log of those files for later uninstallation. However, we do not intend for `cargo install` to become a full-featured package management mechanism; rather, we expect `cargo install` to work analogously to `make install`. While the occasional package provides a `make uninstall`, few developers expect such a mechanism.
- Currently, `cargo install` tracks what files it installs, and supports `cargo uninstall`; this extension mechanism does not hook into that. Potentially, `install.rs` could emit a list of installed files and let cargo install install them, producing a log of those files for later uninstallation. However, we do not intend for `cargo install` to become a full-featured package management mechanism; rather, we expect `cargo install` to work analogously to `make install`. While the occasional package provides a `make uninstall`, few developers expect such a mechanism.

This comment has been minimized.

@fbstj

fbstj Mar 28, 2018

duplicate of previous point?

This comment has been minimized.

@ashleygwilliams

ashleygwilliams Mar 28, 2018

Member

whoops! excellent call. i'll fix that!

@est31

This comment has been minimized.

Contributor

est31 commented Mar 28, 2018

@joshtriplett

For instance, we certainly could have install.rs output a list of files to install and where they should go (including paths to files in the output directory that it just generated), and then cargo install could do the actual installation, and track the files for later uninstallation.

That would work as well. in fact we could have post-build.rs generate those files. In the traditional make install set up, the installation step is only copying stuff to the appropriate places, while the previous steps did the remainder of the work.

Your proposal (calling it "microcargo") sounds fine @joshtriplett but I'd like to give an alternative "monolithic Cargo" proposal to just be able to compare the two:

You could have post-build.rs output whether installation of the program is supported and if yes, outputs a list of the files together with a semantic information what they actually are: executables, manpages, completion files, etc... all need to be put into different places and some would require an additional set up step.

Cargo would then store that list and cargo install would just use that list as a recipe for installation. It would then decide based on input parameters whether to install on a global, or on a per-user basis, and depending on that it could then also tell users to set up a ~/.bash_completion or decide which of the completion files to install based on the used shell. It would then track all the files it installed and support uninstallation. Of course this would require to put some logic into Cargo, but after all, Cargo is not a generic make replacement but a tool specifically built for the Rust ecosystem. It does many opinionated choices already.

So with the "microcargo" approach you have install.rs parse all the input parameters and environment info (per-user or global, name of the shell, distro directory layout, which files the user wants to be installed) and output a list of desired destinations. The advantage is flexibility. The disadvantage is that writing a install.rs becomes far more complicated... I've seen so many issues with make install. Often this is not the main attention of the developer and so often it sucks in one way or another. With the monolithic approach, you would have a central place where improvement can happen. However, this downside of the minicargo approach would be fixed by the metabuild RFC.

So summarizing, compared to the "shellout" proposal of the current RFC, I think "minicargo" and the monolithic approaches are both better, with me preffering the minicargo approach.

@Centril

I'm generally in favor of a move in this direction, but I have some questions and concerns =)

## Summary
The goal of this RFC is to introduce the ability for an end user to be able to extend the `cargo install` command arbitrarily to include instructions that should be executed occur after `cargo install <project>`.

This comment has been minimized.

@Centril

Centril Mar 28, 2018

Contributor

"to be able" seems redundant here?
I propose the following wording:

This RFC gives end users the ability to extend the cargo install command arbitrarily, allowing them to include instructions executed after cargo install <project>.

This comment has been minimized.

@ashleygwilliams

ashleygwilliams Mar 28, 2018

Member

Sure thing. Happy to update.

### New Named Concepts
- `install.rs`: a file that contains a set of instructions to occur after `cargo install` is run.
- `metainstall`: a key in `Cargo.toml` that specifies crate dependencies in an ordered list. Each crate dependency listed must be a library crate that provides a `metainstall` function. The `metainstall` function accepts no arguments, produces no return value, and should panic on failure.

This comment has been minimized.

@Centril

Centril Mar 28, 2018

Contributor

What is the rationale for a metainstall function accepting no arguments, .. ?

This should be described precisely in the reference-level explanation. =)

This comment has been minimized.

@ashleygwilliams

ashleygwilliams Mar 28, 2018

Member

@Centril this is in #2196

metainstall = ["cli-install"]
[dependencies]
cli-install = "0.1.0"

This comment has been minimized.

@Centril

Centril Mar 28, 2018

Contributor

Why is this a regular dependency and not a build or "meta-install" dependency?
Will the CLI app's code use cli-install as a library? It feels like there should be a separation...?

This comment has been minimized.

@ashleygwilliams

ashleygwilliams Mar 28, 2018

Member

This is probably best discussed on the metabuild repo where this syntax is being determined. #2196. Further discussing it here is not relevant, but discussion on that issue would be welcomed.

This comment has been minimized.

@Centril

Centril Mar 28, 2018

Contributor

Oh I see; missed that =)

### New Named Concepts
- `install.rs`: a file that contains a set of instructions to occur after `cargo install` is run.

This comment has been minimized.

@Centril

Centril Mar 28, 2018

Contributor

To @est31's point on uninstallation, should uninstall.rs be required if there is a install.rs?
Of course, a crate can cheat and have uninstall.rs which is not the inverse of install.rs, but it is better than nothing?

This comment has been minimized.

@ashleygwilliams

ashleygwilliams Mar 28, 2018

Member

Yeah we discussed this issue at length. We're not sure if requiring the uninstall.rs is better or worse as I would think, personally, that it gives the appearance of a guarantee that we cannot make, i.e. that it's not the inverse of install.rs by I'm open to being convinced otherwise.

This comment has been minimized.

@fbstj

fbstj Mar 28, 2018

were any thoughts made for making/mandating that install.rs be reversible/isomorphic by design to allow the undoing automatically?

This comment has been minimized.

@est31

est31 Mar 28, 2018

Contributor

I agree with @ashleygwilliams here: mandating (or even supporting) uninstall.rs is no guarantee for uninstall.rs actually uninstalling all of the artifacts. We should go either with the minicargo or monolithic approaches IMO. As for isomorphism @fbstj do you have any precedent of an isomorphic scripting language or similar that you can share? Also, we would have to make sure that all of the information that enters the program will stay the same, otherwise even if we have an isomorphic subset of Rust, we won't be able to guarantee equal outcome.

This comment has been minimized.

@Centril

Centril Mar 28, 2018

Contributor

@fbstj Is that actually possible tho? Unless we have some snapshot of how things were before, install.rs being possibly non-deterministic should not allow you to find an inverse function / program. Of course you could require that the main of an install.rs file be a const fn which would get you determinism, but even then - we'd have to rule out many good programs to make reversibility efficient or decidable, no? And you'd need to extend the type system to rule out such good programs.

@ashleygwilliams Given that install.rs and indeed build.rs could do all sorts of things to your computer, including invoking UB, I think that guarantees were never something that we could make on this front, and that it's all inherently trust based. I think that if we clearly communicate that this is all best-effort, then it is better to require uninstall.rs scripts and build great helper crates around this.

@est31

As for isomorphism @fbstj do you have any precedent of an isomorphic scripting language or similar that you can share?

That would be quite the restrictive language, see: https://stackoverflow.com/questions/13404208/in-pure-functional-languages-is-there-an-algorithm-to-get-the-inverse-function

This comment has been minimized.

@est31

est31 Mar 28, 2018

Contributor

guarantees were never something that we could make on this front, and that it's all inherently trust based.

Right now we still have the option to sandbox install.rs and friends to one directory where they can put stuff in, asking cargo to do the actual installation for them so that uninstallation is possible.

But even if we decide to not sandbox, this argument sounds too much like: "you need to already trust them so making it easier for them to not introduce bugs is not important".

## Reference-level explanation
This is the technical portion of the RFC. Explain the design in sufficient detail that:

This comment has been minimized.

@Centril

Centril Mar 28, 2018

Contributor

This part seems left over from the RFC template?

This comment has been minimized.

@ashleygwilliams

ashleygwilliams Mar 28, 2018

Member

Yup. Can fix.

### Corner Cases
- Installing an application should not run the `install.rs` files of any of it’s dependencies, only of the application itself.
- When re-installing a binary package this might run a different `install.rs` than the one that was initially run. This might lead to inconsistent/ undefined behavior.

This comment has been minimized.

@Centril

Centril Mar 28, 2018

Contributor

The use of the phrase "undefined behavior" should probably be reserved for when referring to UB as in your code not having any semantics. If that is what is intended here, then that is concerning.

This comment has been minimized.

@ashleygwilliams

ashleygwilliams Mar 28, 2018

Member

I don't think that's what is intended here, I think it was meant more as "potentially unexpected behavior"/"surprising behavior". I'm happy to update the language. Thanks for the catch!

- Tools only support specific popular platforms because being cross-platform is deemed “too difficult”.
- Making `cargo install` more capable could encourage people to use it as a primary distribution mechanism for a broader class of applications, rather than just for simple command-line tools. On the other hand, this same mechanism can also serve as the basis for distribution packaging, which typically wants to install into a temporary directory and package the result.
- It makes the emergence of conventions significantly more difficult as the option to reuse, share, or automate this task has significantly affordance than integrating it into the familiar `cargo` step.
- Currently, `cargo install` tracks what files it installs, and supports `cargo uninstall`; this extension mechanism does not hook into that. Potentially, `install.rs` could emit a list of installed files and let `cargo install` install them, producing a log of those files for later uninstallation. However, we do not intend for `cargo install` to become a full-featured package management mechanism; rather, we expect `cargo install` to work analogously to `make install`. While the occasional package provides a `make uninstall`, few developers expect such a mechanism.

This comment has been minimized.

@Centril

Centril Mar 28, 2018

Contributor

Do we want to emulate make install tho? I've never found make to be a reliable program.
I also question the assertion on expectations. What is the evidence for it?

This comment has been minimized.

@ashleygwilliams

ashleygwilliams Mar 28, 2018

Member

I think this is fair. The relation to make was in a convo with @joshtriplett @alexcrichton and @withoutboats - I also share your concern a bit re this, but I found that the affordances this gives us for both cargo install distribution AS WELL as platform specific package managers outweighed it in my opinion. I'm not sure if there's any data/anecdata to be found but I would be happy to try to find some!

This comment has been minimized.

@aidanhs

aidanhs Mar 29, 2018

Member

If we did want anecdata, I wonder if cross compilers and distro packagers would be a good source of information on make install behaviour as they're groups of people likely to spend a high percentage of their time compiling things.

My own experience/expectation from when I was doing a lot of cross-compilation with emscripten is that things supporting make install a) permit some way to set the installation 'prefix' (be it --prefix with autotools and autotools-mimicking things like the rust configure script or the cmake variable or something else) and nothing gets installed outside of that prefix (e.g. completions end up in $PREFIX/etc/bash_completion.d) and b) make uninstall is a bit flaky.

This comment has been minimized.

@Centril

Centril Mar 29, 2018

Contributor

Small note: I don't think cross compiler and distro packagers are representative of the set "developers". ;) Even so, any and all anecdata would be nice to have even if unrepresentative.

This comment has been minimized.

@aidanhs

aidanhs Mar 29, 2018

Member

Ah I take your point, perception is important. I was just thinking about the most efficient way to get feedback on large numbers of executed make install commands.

@joshtriplett

This comment has been minimized.

Member

joshtriplett commented Mar 29, 2018

Something that seems worth mentioning, explicitly: some responses have expressed the sentiment that the right answer is to work towards distribution packaging and similar, rather than making cargo install better. It seems worth pointing out, as someone who does care about distribution packaging as well, that making cargo install better will make it much easier to do distribution packaging. As mentioned in the RFC, we'd like to be able to cargo install to a directory and then package the result, and we'd like to make that include things like completions, manpages, and other associated files.

@ashleygwilliams

This comment has been minimized.

Member

ashleygwilliams commented Mar 29, 2018

So, as was mostly expected, the primary question brought up by the comments so far is the issue of uninstall. The current RFC presents the opinion that we should make no effort to offer an uninstall.rs, however it is an open question as to when an install.rs what we can do to mitigate the absence of an uninstall.rs. Several suggestions have been made, but I think that ensuring that cargo emits any files systems changes when an install.rs is run is a reasonable strategy to take.

Would that appease any of those who are concerned about uninstall? If not, let's try to aggregate other suggestions about mitigating the uninstall issue and discuss which if any would be reasonable to implement alongside this, with an eye that there is still the goal of not packing cargo with so many features that it begins to resemble a platform-specific package manager. (which i think we all agree is something we DON'T want to happen.)

@est31

This comment has been minimized.

Contributor

est31 commented Mar 29, 2018

ensuring that cargo emits any files systems changes when an install.rs is run is a reasonable strategy to take.

I'm not sure I understand your proposal fully. Could you explain it? Suppose I did cargo install foo. With which command would I uninstall it? And how would it look implementation wise?

@Manishearth

This comment has been minimized.

Member

Manishearth commented Mar 29, 2018

I think she's talking of a general class of solutions (correct me if I'm wrong), but one such proposal would be that install.rs emits cargo:installed-file:/path/to/manpage, and cargo keeps these strings around. On uninstall it removes these files and then removes the binary.

(This doesn't cover all cases of uninstall, but as the RFC mentions this isn't intended to be a generalized distribution solution)

@joshtriplett

This comment has been minimized.

Member

joshtriplett commented Mar 29, 2018

@Manishearth Right, the idea would be that install.rs either prints a list of all filesystem changes it wants to make but doesn't make them directly, or alternatively that install.rs only installs things to a subdirectory of the output directory and then cargo install installs those things. Either of those would allow cargo install to optionally remember what it installed for later uninstallation.

@ashleygwilliams

This comment has been minimized.

Member

ashleygwilliams commented Mar 29, 2018

@est31 yup, what both @Manishearth and @joshtriplett expansion of what i said is correct. i'm curious if that's enough to address the issue- and if not, i'm curious what strategies would best address the issue without growing cargo into a thing similar to a full fledged platform-specific package manager, which, i believe, we all agree that we don't want.

if we can agree on something i am very happy to update the RFC to include this strategy explicitly.

@Centril

This comment has been minimized.

Contributor

Centril commented Mar 29, 2018

This strategy seems reasonable, and mostly assuages my concerns. There's of course the issue of regedit and such things on Windows where you are dealing with non-files. But I think that falls under not having a "full fledged platform-specific package manager"?

Personally I prefer the variant where cargo moves the actual files and the install.rs creates them and points to where they should be.

I think an argument in favor of this RFC is that we can always decide to permit an (optional) uninstall.rs file later on (if we wish / there's sufficient demand for it). And so we can start with a conservative initial feature.

@Screwtapello

This comment has been minimized.

Screwtapello commented Mar 29, 2018

One of the problems that crops up with other languages' package managers (hi, Python!) is that the tool chooses one particular convention for where to install files and uses it on every platform in the name of consistency. Usually this convention is highly-POSIX-flavoured, making things weird and awkward to use on Windows.

I realise that ship has kind of already sailed for Cargo, since it installs binaries into $CARGO_HOME/bin, but is there a risk that adding more "install" support to Cargo will make it even more awkward on Windows, or might even make some tools unnecessarily Windows-incompatible if they try to install a file named "aux" or something?

@ashleygwilliams

This comment has been minimized.

Member

ashleygwilliams commented Mar 29, 2018

@Screwtapello so, i share this concern but i think this RFC alleviates it instead of exacerbating it. we specifically are picking to do a install.rs instead of a post-build type situation so that authors can detect platform and perform platform specific functions. there's no way to necessarily guarantee that authors will do it, but the motivation for this RFC, at least partially, is the fact that authors WANT a better cross-platform story.

@aidanhs

This comment has been minimized.

Member

aidanhs commented Mar 29, 2018

I didn't see any mention of passing arguments to cargo install - is this a deliberate choice or just unspecified? If unspecified I'd like to propose allowing it (to avoid a world of ad-hoc environment variable usage).

@ashleygwilliams

This comment has been minimized.

Member

ashleygwilliams commented Mar 29, 2018

@aidanhs it was not opinionated! i think that would be fine, and likely uncontroversial? but i'm curious what others think (assuming no one objects i can specifically add that to the RFC)

@est31

This comment has been minimized.

Contributor

est31 commented Mar 29, 2018

the idea would be that install.rs either prints a list of all filesystem changes it wants to make but doesn't make them directly, or alternatively that install.rs only installs things to a subdirectory of the output directory and then cargo install installs those things.

@ashleygwilliams

i'm curious if that's enough to address the issue

Yes, both variants as laid out by @joshtriplett would be enough (see quote above). I'd personally prefer the variant where install.rs is not doing the changes directly and would like install.rs to be sandboxed. It could have unrestricted read access but write access restricted to one directory inside the target/ hierarchy to ensure that install.rs is actually using cargo to install stuff.

Especially, it seems to me that the intention is not for install.rs to be as flexible as possible, as this would mean support for any kind of installation of applications (cargo replacing an OS package manager), but rather to allow the minimum of features for CLI applications to install their manpages and completion files.

@ehuss

This comment has been minimized.

ehuss commented Mar 31, 2018

I realise that ship has kind of already sailed for Cargo, since it installs binaries into $CARGO_HOME/bin

@Screwtapello This may not be true in the future if something like rust-lang/cargo#5183 is merged.

@alexreg

This comment has been minimized.

alexreg commented Mar 31, 2018

Installing to a staging area, and from there copying to either ~/.cargo or /usr/local would seem sensible (depending on whether the install is global or user-local). Then the uninstall actions can be easily generated just by walking the staging area directory tree. Possibly custom install/uninstall commands could also be added for things like registry edits on Windows. Alternatively, the staging area could be a permanent one, with symlinks into one of the two aforementioned dirs. this is how Mac’s Homebrew does it.

@casey

This comment has been minimized.

casey commented Mar 31, 2018

I think that I'm against this RFC. Instead of allowing crate authors to put arbitrary steps inside of install.rs, I would prefer that we extend cargo and Cargo.toml to support additional crate artifacts types.

To make it concrete, I'll use man pages as an example. With this RFC, crate authors would write an install.rs script which would generate and move man pages into the correct location. I'll refer to this as the imperative approach.

An alternate approach would be to allow crate authors to put something like man-dir = "man" into their Cargo.toml, and let Cargo put any man pages it finds there in the appropriate place. I'll refer to this as the declarative approach.

I think that the declarative approach is superior for a number of reasons.

  • Under the imperative approach, all crate authors must write their own install.rs scripts. Although they can certainly draw from each other's scripts, this is a great deal of duplicated effort. Additionally, installation is complicated by different distro and platform quirks and conventions, so many install.rs scripts will likely have bugs. With the declarative approach, this code lives in one place. This allows for less duplicated effort. And, when bugs are found, they can be fixed for everyone in one place.

  • Different distros have different ideas about where things should go. For example, man pages will go somewhere entirely different on Ubuntu, homebrew, macports, nix, or windows. With the declarative approach, they can write a single adaptor script that parses Cargo.toml, which will work with all crates without further effort. With an imperative approach each distro will have to examine and adapt the install.rs script to identify man pages and put them into the correct location.

  • With the declarative approach, we can add additional support for special handling of different kinds of crate artifacts. For example, we could support a command line flag to only install binaries and not man pages, or vice versa. It would be very hard to support this using the imperative approach, since all install.rs files would need to support this flag. Additionally, with the declarative approach, different crate artifact post-processing steps can be added as needed, for example converting to CRLF line endings on windows.

So, to sum up, I think we should extend cargo with the ability to understand and install crate artifacts on a type-by-type basis, so that they can be handled consistently and appropriately across platforms, and not left to individual crate authors.

@alexreg

This comment has been minimized.

alexreg commented Mar 31, 2018

@casey I think that's fair for a large number of cases, but what about certain Rust projects like tectonic, for which this RFC would be hugely useful in its present form?

@casey

This comment has been minimized.

casey commented Mar 31, 2018

@alexreg, what kinds of artifacts does tectonic need to install? I'm wondering if there might be a better way to support them.

@alexreg

This comment has been minimized.

alexreg commented Mar 31, 2018

@alexreg, what kinds of artifacts does tectonic need to install? I'm wondering if there might be a better way to support them.

Probably best we check with the Tectonic people, like @pkgw, since I'm not 100% sure at this point.

@kornelski

This comment has been minimized.

Contributor

kornelski commented Apr 9, 2018

@RalfJung OK, so that's a bug. But is that a justification to completely give up on ability to uninstall packages?

@alexreg

This comment has been minimized.

alexreg commented Apr 9, 2018

@kornelski Lots of package managers allow modification of system-wide directories, but deny it by convention. One can even deny permissions for the program to modify stuff outside of a certain dir, using things like chroot. You overstate the issue here.

@kornelski

This comment has been minimized.

Contributor

kornelski commented Apr 9, 2018

Right, package managers allow modification of system files, because they have ability to revert/uninstall them later. Modification of system files is not bad per se, but it requires care and taking responsibility for the files, which this RFC doesn't do.

@alexreg

This comment has been minimized.

alexreg commented Apr 9, 2018

@kornelski I think I agree with you that this RFC isn't yet complete, but I do think it just needs a bit of expansion to be in a workable form. As has been stated before, build.rs scripts can already modify anything on your system, but don't, by convention/guidelines. I think guidelines are probably the best way to stop build.rs and install.rs doing stupid system-wide things, as in other package managers (e.g. Homebrew), but if we could use chroot to help with the filesystem aspect, that would be good at least. Support for things like cron/launchd could be done via "notes" at first (that's what some package managers do still) – i.e. simple instructions on how to set it up, but the install not actually carrying it out itself.

@est31

This comment has been minimized.

Contributor

est31 commented Apr 9, 2018

I think guidelines are probably the best way to stop build.rs and install.rs doing stupid system-wide things

install.rs's very purpose is to be unhygienic. To change something. On a per-user level or a system wide level. Saying "build.rs should not change stuff outside the build directory or temporary directories" is a very clear rule. But saying "this way of changing the system is bad but that way of changing it is good" is much harder to formulate and might mean different things to different people.

Can someone tell me about a use case that is important enough that we give up sandboxing or guaranteed uninstallability? The original use cases (completion files, manpages) would all be covered by sandboxed solutions or even declarative ones.

@joshtriplett

This comment has been minimized.

Member

joshtriplett commented Apr 9, 2018

@est31 I think we've already established in this thread that we should find an approach to install.rs that builds things in a build directory and emits information that cargo install can use to install additional files (and optionally remember to uninstall).

@alexreg

This comment has been minimized.

alexreg commented Apr 10, 2018

install.rs's very purpose is to be unhygienic. To change something. On a per-user level or a system wide level. Saying "build.rs should not change stuff outside the build directory or temporary directories" is a very clear rule. But saying "this way of changing the system is bad but that way of changing it is good" is much harder to formulate and might mean different things to different people.

Can someone tell me about a use case that is important enough that we give up sandboxing or guaranteed uninstallability? The original use cases (completion files, manpages) would all be covered by sandboxed solutions or even declarative ones.

Absolute nonsense! We've already been over this issue of what an install.rs should do, how it differs morally from a build.rs script (not a great deal), and collectively countered all your arguments, as I see it. It can be made perfectly clear by guidelines what an install.rs script should do; this is no serious issue.

@est31

This comment has been minimized.

Contributor

est31 commented Apr 10, 2018

I think we've already established in this thread that we should find an approach to install.rs that builds things in a build directory and emits information that cargo install can use to install additional files (and optionally remember to uninstall).

Nice :). Wondering what is being discussed now. @kornelski brought up some very good points about why allowing install.rs to do anything is a bad idea. If we do have sandboxing (do we agree on this being a must have as well?), we'd have guaranteed safe uninstall (at least we could add it to cargo). @kornelski would sandboxing appease you?

@alexreg

This comment has been minimized.

alexreg commented Apr 10, 2018

Full sanboxing is impossible as far as I know, and thus most package manager (all the ones which aren't fully declarative, which is to say all of the ones I've used!) have to rely on conventions or moderation to some degree. Partial sandboxing using built-in features like chroot (or the Windows equivalent, if there is one) is desirable though.

@joshtriplett

This comment has been minimized.

Member

joshtriplett commented Apr 10, 2018

We don't have sandboxing for build.rs, and any sandboxing mechanism would necessarily be platform-specific. I would suggest that we leave sandboxing as an open question, and reconsider it after some experimentation with install.rs and what kinds of things people want to use it for.

Right now, if someone wanted to, they could have build.rs go install arbitrary files on the system, and yet they don't. I think if we make it clear what install.rs is supposed to do (install files to a temporary location and print some output), we'll get similar results.

@alexreg

This comment has been minimized.

alexreg commented Apr 10, 2018

@joshtriplett Yeah, sounds fair. Sandboxing is a somewhat complex problem, and can come later if necessary.

@est31

This comment has been minimized.

Contributor

est31 commented Apr 10, 2018

reconsider it after some experimentation with install.rs and what kinds of things people want to use it for.

I could live with this being converted to an experimental RFC, or alternatively a nightly only feature that turns off sandboxing. But introducing sandboxing after the fact is quite a breaking change...

I think if we make it clear what install.rs is supposed to do (install files to a temporary location and print some output), we'll get similar results.

I admit, the model is a bit limited. You don't need more however if you just want to add shell completion files. However, those people who will run into limitations of that model, will inevitably just ignore it.

We have the choice what to do with these use cases:

  • allow them by not sandboxing, turning install.rs into a mess similar to make install or
  • disallow them and tell them that install.rs is not ready yet for these use cases.

We can either err on safety, or we can err on flexibility. There is no between. And you probably know what I think that should be done =).

@Centril

This comment has been minimized.

Contributor

Centril commented Apr 10, 2018

I also agree that some experimentation first would be a good idea so we can see what people use it for and so on.

@alexreg

This comment has been minimized.

alexreg commented Apr 10, 2018

Things like shell completion, launchd/cron jobs, etc. can be handled either by writing directives to stdout, like previously mentioned (and thus retaining sandboxing), or writing manual instructions of shell commands to run to a NOTES file. The latter could be a solution until we flesh out the former, at least.

@lnicola

This comment has been minimized.

lnicola commented Apr 10, 2018

I think most build systems are converging on a model of installing the files to a given directory. Distribution-specific tools can then take it from there, and maybe re-organize things a little. Others dump everything in the build directory, and let the packager pick the destination files.

I wouldn't say that make install is a mess, however:

  • it doesn't track what it installed, so it can leave a mess behind
  • people do sudo make install and get root-owned build files
  • setting the installation path/prefix isn't always obvious

cargo install can avoid these issues, assuming the crate author is not malicious. Even in that case, the risk is not greater than for build scripts. E.g. I saw a build script that compiled C code to the cargo repository instead of OUT_DIR, and cargo clean didn't know about it.

@kornelski

This comment has been minimized.

Contributor

kornelski commented Apr 10, 2018

The idea of telling install.rs scripts to install files to a chroot-like directory sounds good from perspective of keeping the OS clean, but I'm not sure how script authors would know what subdirectory names to use. In this solution the "API" would be the directory layout, and that still is very system-dependent. Does writing to $OUT_DIR/bin put stuff in C:\Program Files? What paths are mapped to roaming profile directory?

@kornelski

This comment has been minimized.

Contributor

kornelski commented Apr 10, 2018

Would it be possible to limit this RFC only to the "metainstall" feature, and not provide "bare" install.rs scripts, at least initially?

The worries I have are mainly about simplistic install.rs scripts that would just blindly copy-and-forget files to system directories. However, it would be fine if a de-facto standard metainstall crate emerged that worked like a well-behaved package manager.

@alexreg

This comment has been minimized.

alexreg commented Apr 10, 2018

@kornelski We’ve covered most of these points thoroughly already. I don’t see why this discussion is continuing...

@jan-hudec

This comment has been minimized.

jan-hudec commented Apr 10, 2018

@kornelski,

The idea of telling install.rs scripts to install files to a chroot-like directory sounds good from perspective of keeping the OS clean, but I'm not sure how script authors would know what subdirectory names to use. In this solution the "API" would be the directory layout, and that still is very system-dependent. Does writing to $OUT_DIR/bin put stuff in C:\Program Files? What paths are mapped to roaming profile directory?

There are libraries that provide those mappings based on XDG-basedir-spec, XDG-user-dirs, FHS, known folders and standard directories. Cargo install should use one to provide some standard. Quick search on crates.io suggests directories as good candidate.

And then on top of this we need to find out where to install the shell completion files, user systemd units, desktop files etc., which will call for another layer on top of that. There needs to be some guidance provided on that, but it does mean that cargo install will have to provide an option to install to arbitrary path, because cargo can't know all the shell and tools that may need to be integrated with.

@alexreg

This comment has been minimized.

alexreg commented Apr 11, 2018

There are libraries that provide those mappings based on XDG-basedir-spec, XDG-user-dirs, FHS, known folders and standard directories. Cargo install should use one to provide some standard. Quick search on crates.io suggests directories as good candidate.

Yep, sounds sensible. The directories library looks quite nice as well.

And then on top of this we need to find out where to install the shell completion files, user systemd units, desktop files etc., which will call for another layer on top of that. There needs to be some guidance provided on that, but it does mean that cargo install will have to provide an option to install to arbitrary path, because cargo can't know all the shell and tools that may need to be integrated with.

For specific things like this, I suggest (like I mentioned above) not including them initially, but rather instructing the user how to add them via notes. As we progress, we can provide support for special directives that install.rs script output via stdout, and cargo install processes and tracks itself.

@aidanhs

This comment has been minimized.

Member

aidanhs commented Apr 11, 2018

Just look at how many crates still depend on rustc_serialize.

@est31 isn't rustc_serialize an official(ish) solution replaced by the community with a better one? In this RFC comment thread the design is moving to prevent alternative approaches. Regarding "everyone gets the improvements" - this is the large vs small stdlib argument and we won't resolve that here 😄


I can that consensus is forming behind the general thrust of the sandboxing approach, so I'll just jot down some final 'contrary' thoughts:

  • Minor: the discussion seems unix-centric - isn't regedit is a fairly important part of CLI app install on Windows (e.g. to put it on your path / to identify where your app is installed to find assets)? But files on disk is step 1 (and devs can perform 'post-install' steps in their binary) so maybe it's ok.
  • One point of discussion is "who sets conventions - ecosystem or cargo". The sandboxing thing (to me) ends up implying "cargo decides conventions" (since you're only empowered to make decisions on things cargo says you can). I'm ok with collectively selecting that as the approach, just wanted to point out that these are linked.

I personally just want curl ... | sh rewritten in Rust, so I'll wait for a cargo 'run-script' RFC that will allow something like cargo run-script cratename install.

@aidanhs

This comment has been minimized.

Member

aidanhs commented Apr 11, 2018

I could live with this being converted to an experimental RFC, or alternatively a nightly only feature that turns off sandboxing. But introducing sandboxing after the fact is quite a breaking change...

To echo others: robust sandboxing that supports Linux kernel 2.6.18, Windows 7 and OSX is hard (basically requiring a SUID helper binary on Linux). 'Good enough' sandboxing (assuming we agree on what that means) is still tough. Let's bear in mind that the goal of this RFC is modest - "make CLI apps easier to install". I'm sure we can come up with a way to make it so easy to do things the 'right way' that most authors never even think of anything else, or just explicitly document that anything you do that isn't creating a file in a specific directory may break in the future.

I like @RalfJung's take:

I like to think that the reason make uninstall is broken is not that software authors want to make the lives of people that uninstall their software miserable, I'd like to think that they actually would prefer uninstallation to work so if we tell them "that's easy, just use the wrappers in this lib for installation and you are all set", they'll just use it. I hope I am not being too naive here :D

@alexreg

This comment has been minimized.

alexreg commented Apr 11, 2018

Minor: the discussion seems unix-centric - isn't regedit is a fairly important part of CLI app install on Windows (e.g. to put it on your path / to identify where your app is installed to find assets)? But files on disk is step 1 (and devs can perform 'post-install' steps in their binary) so maybe it's ok.

Well, my above suggestion was to use some convention based (Unix-like) FHS-like approach (@jan-hudec expanded on this with good ideas). For registry edits, I suggested we just have install.rs scripts write directive to stdout, which get interpreted by cargo. They'd be ignored on all platforms but Windows (possibly with warnings output, since ideally you'd want install.rs script authors to check the OS before emitting them).

One point of discussion is "who sets conventions - ecosystem or cargo". The sandboxing thing (to me) ends up implying "cargo decides conventions" (since you're only empowered to make decisions on things cargo says you can). I'm ok with collectively selecting that as the approach, just wanted to point out that these are linked.

Makes sense to me.

@est31

This comment has been minimized.

Contributor

est31 commented Apr 12, 2018

One point of discussion is "who sets conventions - ecosystem or cargo". The sandboxing thing (to me) ends up implying "cargo decides conventions" (since you're only empowered to make decisions on things cargo says you can). I'm ok with collectively selecting that as the approach, just wanted to point out that these are linked.
[...]
I personally just want curl ... | sh rewritten in Rust

I'd say that the approach is already "cargo decides conventions".

Make is basically a glorified shell... for the execution of targets, make actually invokes a shell, the syntax is similar, etc etc.
CMake is no different. It has some special syntax built in but every library did its own thing and reimplemented the same stuff over and over and it is still an ugly mess.

Cargo's traditional approach is radically different. Instead of being a generic tool that allows everything, it has a ton of stuff hardcoded that the code has no say over.

  • it only supports Rust
  • non-git non-path deps are hardcoded to the crates.io domain (at least unless the user changes .cargo/config)
  • it has the concepts of build, run, and test

When I came to Rust, it was surprising to me to see all of this stuff hardcoded. But then I saw how all of this hardcoding is benefitting the Rust ecosystem.
You don't have non-rust stuff to worry about, you don't have everything spread out on several hosts with possibly several policies, you know instantly how to compile something (just do cargo build).

So I think we should follow that approach and be opinionated for the user's sake :).

To echo others: robust sandboxing that supports Linux kernel 2.6.18, Windows 7 and OSX is hard (basically requiring a SUID helper binary on Linux). 'Good enough' sandboxing (assuming we agree on what that means) is still tough.

It doesn't have to be watertight. I think something that causes big headaches for people who ignore the convention (or who don't know... most times it is negligence :p ) is enough. I don't want to go into detail about implementation but I think the two options on the table are gaol and interpreting stuff with miri.

@alexreg

This comment has been minimized.

alexreg commented May 29, 2018

Any progress on this lately?

@alexreg

This comment has been minimized.

alexreg commented Jul 20, 2018

I suppose this hasn't been high on people's priority lists lately, given the lack of news. Any chance of reviving things?

Some questions I had:

  • Is the plan now to make cargo closer to a lightweight package manager, per above discussions?
  • Should we retain separate "repositories" of Cargo binaries & their support files that switch depending in the toolchain, in a pyenv-like manner?
  • What's the interaction with Cargo components going to look like in the future? It seems like components could just be "official" versions of Cargo packages, if we extend the cargo install system to support a more general concept of "packages".
@derekdreery

This comment has been minimized.

derekdreery commented Jul 25, 2018

It makes the emergence of conventions significantly more difficult as the option to reuse, share, or automate this task has significantly affordance than integrating it into the familiar cargo step.

I'm struggling to parse this. Can you tell me what you mean?

cargo install could optionally have a flag to disable the running of the install.rs file.

What's the use case for this?

@nrc nrc referenced this pull request Nov 8, 2018

Open

Cargo pre-planning: workflow #14

@DrSensor

This comment has been minimized.

DrSensor commented Nov 8, 2018

One of my use cases of this feature is to enforce all of the collaborators to set up git-hooks scripts to enforce some policy like commit message and others. I usually use husky to enforce all collaborators to follow conventionalcommits

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