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

eclass/{rust, rust-utils, cargo}: add support for rust multi-target #9388

Open
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@gibix

gibix commented Jul 30, 2018

  • add rust.eclass (with rust-utils.class) to support rust multi-target with multibuild
  • modify cargo.class to support rust multi-target
  • change dev-lang/rust slot system

This will allows projects like rustfmt, clippy, bindgen that need runtime linking with the proper rust version to work correctly. Beyond this while rust is getting older as project we will see more projects that will require a specific rust version for compilation.

This PR replaces the need for rustup in gentoo for toolchain handling and components.

requires:

see first discussion on gentoo-rust

@gibix gibix changed the title from Rust eclass to eclass/{rust, rust-utils, cargo}: add support for rust multi-target Jul 30, 2018

@gentoo-bot

This comment has been minimized.

gentoo-bot commented Jul 30, 2018

Pull Request assignment

Areas affected: ebuilds, eclasses, profiles
Packages affected: dev-lang/rust, dev-util/rustfmt

dev-lang/rust: @gentoo/rust
dev-util/rustfmt: @gentoo/rust

No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and ping us to reset the assignment.

If you do not receive any reply to this pull request, please open or link a bug to attract the attention of maintainers.

In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

@gibix gibix force-pushed the gibix:rust-eclass branch 8 times, most recently from ea66c5c to 2eeb917 Jul 30, 2018

gibix added some commits Jul 31, 2018

rust-utils.eclass: add rust utils
A utility eclass providing functions to query rust implementations.

This eclass does not set any metadata variables nor export any phase
functions. It can be inherited safely.

CREDITS: has been hardly-inspired by python*.class

Signed-off-by: gibix <gibix@riseup.net>
rust.eclass: add rust eclass for multitarget
A common eclass providing helper functions to build and install
packages multiple rust implementations.

CREDITS: has been hardly-inspired by python*.class

Signed-off-by: gibix <gibix@riseup.net>
profiles: add RUST_TARGETS
Setup RUST_TARGETS USE_EXPAND and setup default value and description

Signed-off-by: gibix <gibix@riseup.net>

@gibix gibix force-pushed the gibix:rust-eclass branch from 2eeb917 to 376eb6a Jul 31, 2018

@gibix gibix referenced this pull request Jul 31, 2018

Closed

rust eclass for multi-target #362

@gibix

This comment has been minimized.

gibix commented Aug 3, 2018

@cardoe

What's the big gain we're getting from having to mark supported rust versions in an eclass? I have plenty of binaries that I've built under older Rust's and they continue to work fine. Given the forward compatible nature I don't see it breaking code either. You mention rustfmt, clippy, and bindgen. I will agree that the two prior matter but now Rust bundles those two components with a release. The push that @djc has made for "fat" rust ebuilds that include the necessary utilities would seem to make more sense here. Or this has been solved previously with slot dependencies and rebuilding packages as necessary. clippy is a special snowflake since it works with a nightly as well.

As far as bindgen goes, it doesn't have any inherent dependency on a rustc version. It even takes arguments to allow you to target a specific version. At work we often don't use the absolute latest version of rustc but due use the latest bindgen and we make sure we supply the version we're using and all is well.

Overall I don't like the addition of RUST_TARGETS and the dependency on features in other projects that haven't had merges.

@@ -13,13 +13,13 @@ if [[ ${PV} = *beta* ]]; then
MY_P="rustc-beta"
SLOT="beta/${PV}"
SRC="${BETA_SNAPSHOT}/rustc-beta-src.tar.gz"
KEYWORDS="amd64 x86"
KEYWORDS="~amd64 ~x86"

This comment has been minimized.

@cardoe

cardoe Aug 4, 2018

Contributor

Don't change keywords on existing builds

else
ABI_VER="$(get_version_component_range 1-2)"
SLOT="stable/${ABI_VER}"
MY_P="rustc-${PV}"
SRC="${MY_P}-src.tar.gz"
KEYWORDS="amd64 ~arm64 x86"
KEYWORDS="~amd64 ~arm64 ~x86"

This comment has been minimized.

@cardoe

cardoe Aug 4, 2018

Contributor

If you want to make changes to an existing ebuild rev bump it and change the keyword there.

${PYTHON_DEPS}
|| (
>=sys-devel/gcc-4.7
>=sys-devel/clang-3.5

This comment has been minimized.

@cardoe

cardoe Aug 4, 2018

Contributor

The dependency has come up much higher than 3.5 in 1.26 now days.

@lu-zero

This comment has been minimized.

Member

lu-zero commented Aug 6, 2018

No, "fat" rustc would work only for the software sync-released with the compiler.

The libsyntax consumers aren't in sync with rustc releases and nothing prevents new of them to appear and stay outside of the nursery.

Probably would be better to clarify that most rust code won't require the whole setup described here.

dev-lang/rust: make rust testing
rust-1.25 should be considered with the same keywords of others rust
implementation
@djc

This comment has been minimized.

Contributor

djc commented Aug 6, 2018

So as I asked on the mailing list; what libsyntax consumers are we talking about, anyway?

@gibix gibix force-pushed the gibix:rust-eclass branch from 376eb6a to a568af0 Aug 6, 2018

@cardoe

This comment has been minimized.

Contributor

cardoe commented Aug 6, 2018

@lu-zero I too am curious what uses libsyntax and isn't released with the compiler? rustfmt and clippy now days are. They're still in preview so they might have changes separate from the compiler. clippy has always had a tight coupling to the compiler which is why they're now advising folks to not even cargo +nightly install clippy but instead use rustup. Similarly for rustfmt the recommended way is to switch to using rustup. Anything installable by rustup is considered "shipped with the compiler".

bindgen isn't https://crates.io/crates/bindgen dependent on libsyntax and it supports a range of rustc versions (it explicitly has a getopt flag for it).

@lu-zero

This comment has been minimized.

Member

lu-zero commented Aug 7, 2018

Bindgen used to have a quite brittle dependency to rustfmt, now it is less brittle and luckily optional.
Hopefully we could ignore it for now.

I have to doublecheck the status of doxydize and the many rust->${language} bindings generators to see if it would make sense to distribute them already. I'm not aware of additional widespread code-analysis tools at the moment, but they would have the same issue.

If we restrict the scope to the rustup-provided optional software we have: clippy, rls and rustfmt.

Having those 3 available as stand-alone ebuilds spares you up to 3x the rustc compile time if we assume those 3 are release-synchronous with the actual compiler.

What is exactly the problem in making easier to, hopefully, skip some long rebuilds?

@djc

This comment has been minimized.

Contributor

djc commented Aug 7, 2018

Of course skipping rebuilds is nice, but you're acting as if there are no downsides to the approach proposed here, which I disagree with. As I've argued before, there are two in particular:

  • Added complexity: someone has to maintain all this eclass code going forward and hook every version bump up right with the eclass logic. In particular, since I have been doing most of the version bumps and the larger bits of maintenance, it might fall down to me to take care of that.
  • Alignment with upstream/other platforms: I regularly use Rust code across my Gentoo box and a macOS machine. It is useful to me if the Rust ebuilds in Portage are carefully and clearly aligned with the versions I get from rustup; I also imagine it's useful for less experienced Rust developers on Gentoo if the ebuilds in Portage align with documentation they're reading targeted at a rustup world.

Finally, I don't see a big advantage to skipping those rebuilds. These extra tools are extremely short builds compared with the main rustc build. Currently the dev-lang/rust build system builds the tools anyway, so you're paying for that cost even if you don't install them. On every minor version bump to rust, you have to rebuild the tools anyway. There's some added cost if you want to change the set of tools installed, but I just don't think that's a common enough scenario to optimize for.

@gibix

This comment has been minimized.

gibix commented Aug 7, 2018

Just a note about the complexity on of maintenance we are talking about editing two lines in rust-utils.eclass. Particularly _RUST_ALL_IMPLS and rust_package_dep on every major release

@gibix gibix force-pushed the gibix:rust-eclass branch from a568af0 to df065e0 Aug 7, 2018

gibix added some commits Aug 7, 2018

dev-util/cargo: make cargo testing
cargo keywords should be aligned with rust version
dev-lang/rust: change rust slot system
Allow rust to slot on major version (26,27) in order to support a wider
range of multiple versions installed simultaneously.

Signed-off-by: gibix <gibix@riseup.net>

gibix added some commits Jul 31, 2018

cargo.eclass: add multi-target support to cargo
Start to use rust eclass into cargo eclass in order to support multi-target
binaries installation.

If an ebuild is compiled with multiple rust targets the binaries will be
installed into /usr/lib/${rust-target}/bin. Is not the perfect solution to
everyone to have some binaries outside $PATH, but is more clear to have a
single version of the binary handled by eselect into /usr/bin instead of
multiple versions. This because if the binary needs to be linked to rustc
library could be stand to a conflict.

Signed-off-by: gibix <gibix@riseup.net>
cargo.eclass: add cargo features as use flag
Add support to cargo features as portage use flag

Signed-off-by: gibix <gibix@riseup.net>

@gibix gibix force-pushed the gibix:rust-eclass branch from df065e0 to 7f55c9c Aug 7, 2018

@gentoo-repo-qa-bot

This comment has been minimized.

Collaborator

gentoo-repo-qa-bot commented Aug 7, 2018

Pull request CI report

Report generated at: 2018-08-07 15:37 UTC
Newest commit scanned: 7f55c9c
Status: broken

New issues caused by PR:
https://qa-reports.gentoo.org/output/gentoo-ci/30c8d68/output.html#dev-lang/rust

@lu-zero

This comment has been minimized.

Member

lu-zero commented Aug 8, 2018

Of course skipping rebuilds is nice, but you're acting as if there are no downsides to the approach proposed here, which I disagree with. As I've argued before, there are two in particular:

Added complexity: someone has to maintain all this eclass code going forward and hook every version bump up right with the eclass logic. In particular, since I have been doing most of the version bumps and the larger bits of maintenance, it might fall down to me to take care of that.

Could you tell me how that would be different from the alternative work-wise? To me it seems pretty much equivalent.

Alignment with upstream/other platforms: I regularly use Rust code across my Gentoo box and a macOS machine. It is useful to me if the Rust ebuilds in Portage are carefully and clearly aligned with the versions I get from rustup; I also imagine it's useful for less experienced Rust developers on Gentoo if the ebuilds in Portage align with documentation they're reading targeted at a rustup world.

How having a way to do emerge $tool would make it less consistent exactly?
it is not different from using rustup component add $tool

If you are targeting rustup, you fare much better by using it directly though...

Finally, I don't see a big advantage to skipping those rebuilds. These extra tools are extremely short builds compared with the main rustc build. Currently the dev-lang/rust build system builds the tools anyway, so you're paying for that cost even if you don't install them. On every minor version bump to rust, you have to rebuild the tools anyway. There's some added cost if you want to change the set of tools installed, but I just don't think that's a common enough scenario to optimize for.

I just tested on the current nightly, always building those tools would add about 1/6 of the build time, not negligible but also not extremely terrible.

rustc

real	30m10.160s
user	348m45.914s
sys	15m2.941s

rustfmt

real	1m23.610s
user	4m50.329s
sys	0m12.835s

rls

real	2m7.269s
user	15m20.884s
sys	0m45.378s

clippy

real	1m33.454s
user	4m6.891s
sys	0m11.480s
@djc

This comment has been minimized.

Contributor

djc commented Aug 8, 2018

Could you tell me how that would be different from the alternative work-wise? To me it seems pretty much equivalent.

So here are the reasons as I can see them:

  • Bumping separate ebuilds is more painful. You have to have a repository for each tool to get the list of crates, use cargo ebuild to generate that, then copy/paste it into another ebuild.
  • We closely couple the component builds to the rust version anyway, so in most cases you'll build them in the same session anyway.
  • Upstream doesn't actually support distribution of the separate source components, and you can see this in that cargo "0.29.0" actually reports "1.28.0" as its --version.

So, in my view, it's (a) more work, (b) for little gain, (c) and if the feces hit the ventilating device, we (I) get to keep the pieces.

@lu-zero

This comment has been minimized.

Member

lu-zero commented Aug 8, 2018

Does not look like so:

cargo run --bin cargo-clippy -- --version
    Finished dev [unoptimized + debuginfo] target(s) in 0.07s
     Running `target/debug/cargo-clippy --version`
0.0.212
cargo run -- --version
    Finished dev [unoptimized + debuginfo] target(s) in 0.26s
     Running `target/debug/rls --version`
rls-preview 0.130.4-nightly (c51e3ff 2018-08-07)
cargo run --bin rustfmt -- --version
    Finished dev [unoptimized + debuginfo] target(s) in 0.12s
     Running `target/debug/rustfmt --version`
rustfmt 0.99.2-nightly (6a8a06ab 2018-08-07)

The assumption that those components are release-synchronized is not correct as well.

@djc

This comment has been minimized.

Contributor

djc commented Aug 8, 2018

All of those are preview components. I think it's likely they will also end up with the rustc version numbers before they get out of preview.

In general, we can keep chewing on this, but I'm pretty firm in my belief that upstream intends to release all of these tools together as a single package. That doesn't make it impossible to split them out again for our build system, but it does require a whole bunch of effort and continually swimming against the stream if there are issues. I'm not willing to spend that effort. If you are, feel free to put the separate components into the tree and keep them up to date.

@lu-zero

This comment has been minimized.

Member

lu-zero commented Aug 9, 2018

I care about the current status, IF rustfmt/rls/clippy end up part of rustc I have no problems in fitting them in.

Right now that's not the case and having the ability to package programs with strong dependencies on specific rustc version is good in itself.

@djc

This comment has been minimized.

Contributor

djc commented Aug 9, 2018

Fine, go ahead and write and maintain separate ebuilds for these other components.

I still don't think that warrants all this RUST_TARGETS stuff from the PR.

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