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

Support cross-compile install #5614

Merged
merged 2 commits into from
Jun 29, 2018
Merged

Support cross-compile install #5614

merged 2 commits into from
Jun 29, 2018

Conversation

infinity0
Copy link
Contributor

@infinity0 infinity0 commented Jun 7, 2018

Amazingly this Just Works, tested cross-compiling aho-corasick from amd64 to armhf on Debian.

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@matklad
Copy link
Member

matklad commented Jun 7, 2018

So looks like we have somewhat conflicting requests:

If we support --target argument, we probably support .cargo/config as well.

@infinity0 could you explain your use-case a little bit? cargo install installs binaries into the host's bin directory, so binaries installed by cargo install are always run by host and it seems to me that cross-compilation for install, in general, does not make sense. There are specific cases where it makes sense though, like #5441, but I am curious about yours!

@infinity0
Copy link
Contributor Author

@matklad I'm giving --root to do a staged install, the end results are being distributed onto other computers of the target architecture.

@matklad
Copy link
Member

matklad commented Jun 7, 2018

@infinity0 do you distribute only the binaries, or .crates.toml file as well?

If you are interested only in binaries, the currently-unstable cargo build --target $target --release --outupt-dir $root/bin option might a better fit here: cargo install's intended use-case is installing binaries on the local machine.

@infinity0
Copy link
Contributor Author

I'm currently not distributing the .crates.toml but in Debian a staged install for end user machines is supposed to have no behavioral differences to a local install, whether that's done on a machine for the same or different host. If I use cargo install for a native build then a cross build needs to run pretty much the same process.

I'm not sure what the intended differences between cargo install and --output-dir are, can you clarify?

@matklad
Copy link
Member

matklad commented Jun 7, 2018

I'm not sure what the intended differences between cargo install and --output-dir are, can you clarify?

Sure! cargo install --root foo would basically do two things:

  • put the binary into ./foo/bin/
  • write a cargo-specific metadata file, .crates.toml, to ./foo/.crates.toml

cargo build --output-dir foo compiles the requested Cargo target (--bin or --lib) and puts it to the foo directory.

I.e., install writes some custom cargo-specific metadata, and --output-dir is just a usual build with additional "I'd love to fetch final artifacts from here".

Neither is a perfect fit for creating a binary distribution of the application. install's intended use-case is to install binaries for a current user in a Cargo-specific location (that's why it writes cargo-specific metadata into root/.crates.toml). --output-dir's intended use-case is to handle over compiled binaries to some other automation, which will package them or integrate into other build system or just about anything else.

@infinity0
Copy link
Contributor Author

infinity0 commented Jun 7, 2018

When you say "cargo-specific" do you mean "specific to how cargo has its own registered set of installed standalone binaries" as opposed to "specific to cargo as a dependency manager"? If I understood that correctly then it does sound like --output-dir is more suitable for us than install. But I also worry about the future if cargo extends support for standardised locations to install e.g. manpages, that --output-dir would become a second-class citizen to install. How likely is it that it will be stabilised and maintained as good as install is?

@matklad
Copy link
Member

matklad commented Jun 7, 2018

When you say "cargo-specific" do you mean "specific to how cargo has its own registered set of installed standalone binaries"

This one. By default, cargo install installs into Cargo-specific location, ~/.cargo/bin, and pays no attention to the conventions of the OS. It is rather different from make install, which tries to put stuff into proper directories like /usr/bin.

But I also worry about the future

Yep, the future is very hazy here! I am not on top of the things in this area, but looks like https://github.com/rust-lang-nursery/cli-wg/issues/8 is the current discussion.

I think that the future directions would look like this:

  • --output-dir would certainly become stable and would be well-supported. It is useful in various circumstances, not only packaging, and is easy to support because it actually does very little.

  • cargo install might grow some notion of "post install scripts" to install manpages and such, but this will be geared towards installing into ./cargo/bin enclave, and not towards following OS conventions (because there are too many OSes to support them all in Cargo).

  • An external tool (stager?) might be written for packaging rust applicaitons. This tool would probably have some integration with cargo, so that running something like cargo dist --deb --rpm --msi would automatically use the tool to produce various packages with an app.

So, for know, I think the best way is to use --output-dir (we probably could stabilize it right now, seems low-risk), or even to just cargo build and copy the binary from ./target. This would be stable. In contrast, using cargo install might break one day, if we add support for man-pages or something. When the proper solution for packaging emerges (which likelly would be different from both bulid/install), switch to that.

@matklad
Copy link
Member

matklad commented Jun 8, 2018

Fascinating! Regardless of what I think about the purposes of cargo install, people do use it for packaging: https://github.com/snapcore/snapcraft/blob/4cbe8c1abea0d72b9defa2db7f79264b073c5491/snapcraft/plugins/rust.py#L100-L113.

@infinity0
Copy link
Contributor Author

Traditionally packagers at least on UNIX/FHS-based systems rely on make install to put stuff in the right place with DESTDIR to do a staged install, and most other buildsystems like cmake have some sort of equivalent. cargo install "looks like" it's doing this but it's not, but it looks very close to fool most people including me :).

I can understand the need to distinguish these though so am happy to use --output-dir when that gets released as an unstable flag.

@alexcrichton
Copy link
Member

I think I'd personally be inclined to take this route rather than #5606 after reading up on this. I do think that for the disto use case cargo install is the way to go as it's somewhat future-proof with what we'd like to do with install scripts.

@alexcrichton
Copy link
Member

Note that if we do indeed decide to merge this we'd probably want to revert #5606 which I've basically accidentally merged at this point

@infinity0
Copy link
Contributor Author

infinity0 commented Jun 28, 2018

FWIW there are actually not that many different possible installation layouts, and everything I've seen can be done using the first few variables prescribed by GNU Make. Only the first few are essential, stuff further below is optional.

Cargo could define its own installation layout for ~/.cargo based on the above variables. Then, to support system-wide installation, cargo install could either support overriding individual variables via the CLI, or just support a new flag --system to install into a "system layout" defined by the distro (e.g. Debian). Under this override, it could install .crates.toml into $(datadir)/cargo (going by the GNU standard I just linked), which on Debian and most other FHS systems would be /usr/share/cargo. If it needs to, it can read .crates.toml from both this system directory and ~/.cargo.

What I just described is a pretty standard structure across package managers.

It is also quite common to use ~/.local as a user-specific prefix (instead of /usr/local) which is defined by systemd but also in use by various other distros. That would be an alternative to ~/.cargo if you don't want to continually make up new directory layouts under it.

@infinity0
Copy link
Contributor Author

infinity0 commented Jun 28, 2018

Under this override, it could install .crates.toml into $(datadir)/cargo [..]

Correction, this would not be suitable - since the file gets rewritten every time something new is installed, it would have to maintain a file in $(localstatedir)/cargo (/var/lib/cargo usually) and update it in a post-install hook. Alternatively, don't use a single file .crates.toml but generate a new static file for each binary, and keep these files in a directory under $(datadir)/cargo (/usr/share/cargo usually).

@alexcrichton
Copy link
Member

@infinity0 can you include a revert of #5606 in this PR now as well since it'll be required if we merge? Additionally can you add a test that the --target flag works?

@matklad
Copy link
Member

matklad commented Jun 28, 2018

To me, both #5606 and this PR seem valid (separately, of course :) ). I don't know the right solution here, but I agree that we should revert #5606 at least because it will break current packagers relying on this behavior, and that we should add --target and explicitly document that you can use install to install to a different machine.

The reasoning behind this my opinion is really fuzzy though, and relies on interaction with yet to be implemented features. I think cargo install can be used for three purposes:

  • install rust-specific dev tools, like cargo install wasm-bindgen-cli. For this use-case we currently want cargo install will ignore the target triple specified in a project directory #5606. However, hopefully custom workflows would eliminate the need for this use-case, and instead you would add something like [workflow] wasm-bindgen = "0.1" and make this project local.

  • creating canonical distributable artifacts. Here, I think we miss the ability to cargo install static and dynamic libraries. I can envision someone writing a dynamic library with C API in rust with the intention of distributing it just like any over libfoo.so. For this use-case, we'll need some command to produce this canonical artifact, and, preferably, this should be the same command that is used for binaries (and similar desire for install scripts applies to, for example, distribute the header file for the library).

  • a low friction way to distribute generally useful CLI tools (think ripgrep) to Rust users specifically, and here CLI group are doing something: https://github.com/rust-lang-nursery/cli-wg/issues/8

All in all, I fear that this in general perhaps is "needs design", but I don't want to block anything because I don't know a single person who is on top of all the things above.

@alexcrichton
Copy link
Member

Excellent points @matklad! With those use cases in mind though I don't think it's clear that --target and [build.target] should be rejected though (in my mind at least). In that sense I'd personally be ok landing something like this

@matklad
Copy link
Member

matklad commented Jun 28, 2018

In that sense I'd personally be ok landing something like this

Yeah, my current understanding is that we should merge it (+ a revert) as well, I am just not confident that my current understanding is sufficiently informed :-)

@infinity0 infinity0 force-pushed the master branch 2 times, most recently from f2e8b01 to 86ac059 Compare June 29, 2018 01:48
@infinity0
Copy link
Contributor Author

OK I've updated the PR, reverted #5606 as requested and added a test for the new --target behaviour, including using a foreign arch when cross-compile tests are enabled.

@alexcrichton
Copy link
Member

@bors: r+

Ok thanks @infinity0!

@bors
Copy link
Contributor

bors commented Jun 29, 2018

📌 Commit 0774e97 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jun 29, 2018

⌛ Testing commit 0774e97 with merge 7139192...

bors added a commit that referenced this pull request Jun 29, 2018
Support cross-compile install

Amazingly this Just Works, tested cross-compiling aho-corasick from amd64 to armhf on Debian.
@bors
Copy link
Contributor

bors commented Jun 29, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 7139192 to master...

@bors bors merged commit 0774e97 into rust-lang:master Jun 29, 2018
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 14, 2018
Version 1.29.0 (2018-09-13)
==========================

Compiler
--------
- [Bumped minimum LLVM version to 5.0.][51899]
- [Added `powerpc64le-unknown-linux-musl` target.][51619]
- [Added `aarch64-unknown-hermit` and `x86_64-unknown-hermit` targets.][52861]

Libraries
---------
- [`Once::call_once` now no longer requires `Once` to be `'static`.][52239]
- [`BuildHasherDefault` now implements `PartialEq` and `Eq`.][52402]
- [`Box<CStr>`, `Box<OsStr>`, and `Box<Path>` now implement `Clone`.][51912]
- [Implemented `PartialEq<&str>` for `OsString` and `PartialEq<OsString>`
  for `&str`.][51178]
- [`Cell<T>` now allows `T` to be unsized.][50494]
- [`SocketAddr` is now stable on Redox.][52656]

Stabilized APIs
---------------
- [`Arc::downcast`]
- [`Iterator::flatten`]
- [`Rc::downcast`]

Cargo
-----
- [Cargo can silently fix some bad lockfiles ][cargo/5831] You can use
  `--locked` to disable this behaviour.
- [`cargo-install` will now allow you to cross compile an install
  using `--target`][cargo/5614]
- [Added the `cargo-fix` subcommand to automatically move project code from
  2015 edition to 2018.][cargo/5723]

Misc
----
- [`rustdoc` now has the `--cap-lints` option which demotes all lints above
  the specified level to that level.][52354] For example `--cap-lints warn`
  will demote `deny` and `forbid` lints to `warn`.
- [`rustc` and `rustdoc` will now have the exit code of `1` if compilation
  fails, and `101` if there is a panic.][52197]
- [A preview of clippy has been made available through rustup.][51122]
  You can install the preview with `rustup component add clippy-preview`

Compatibility Notes
-------------------
- [`str::{slice_unchecked, slice_unchecked_mut}` are now deprecated.][51807]
  Use `str::get_unchecked(begin..end)` instead.
- [`std::env::home_dir` is now deprecated for its unintuitive behaviour.][51656]
  Consider using the `home_dir` function from
  https://crates.io/crates/dirs instead.
- [`rustc` will no longer silently ignore invalid data in target spec.][52330]

[52861]: rust-lang/rust#52861
[52656]: rust-lang/rust#52656
[52239]: rust-lang/rust#52239
[52330]: rust-lang/rust#52330
[52354]: rust-lang/rust#52354
[52402]: rust-lang/rust#52402
[52103]: rust-lang/rust#52103
[52197]: rust-lang/rust#52197
[51807]: rust-lang/rust#51807
[51899]: rust-lang/rust#51899
[51912]: rust-lang/rust#51912
[51511]: rust-lang/rust#51511
[51619]: rust-lang/rust#51619
[51656]: rust-lang/rust#51656
[51178]: rust-lang/rust#51178
[51122]: rust-lang/rust#51122
[50494]: rust-lang/rust#50494
[cargo/5614]: rust-lang/cargo#5614
[cargo/5723]: rust-lang/cargo#5723
[cargo/5831]: rust-lang/cargo#5831
[`Arc::downcast`]: https://doc.rust-lang.org/std/sync/struct.Arc.html#method.downcast
[`Iterator::flatten`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.flatten
[`Rc::downcast`]: https://doc.rust-lang.org/std/rc/struct.Rc.html#method.downcast
@ehuss ehuss added this to the 1.29.0 milestone Feb 6, 2022
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 this pull request may close these issues.

6 participants