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.

Sign up for GitHub

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

Jump to bottom

rustc: correct propagation of NIX_BUILD_CORES #38869

Merged
merged 1 commit into from Apr 15, 2018
Merged

rustc: correct propagation of NIX_BUILD_CORES #38869

merged 1 commit into from Apr 15, 2018

Conversation

coreyoconnor
Copy link
Contributor

@coreyoconnor coreyoconnor commented Apr 12, 2018 •
edited

Also attempted to override the BOOTSTRAP_ARGS via makeFlags but did not have any success.

Motivation for this change
  1. compilation of rustc bootstrap will default to as many cores as available. This may exceed NIX_BUILD_CORES requested
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Mic92 referenced this pull request Apr 12, 2018
@dezgeg
rustc: Disable failing test on aarch64
1abd235 
https://hydra.nixos.org/build/72562211/nixlog/1
rust-lang/rust#49807
@GrahamcOfBorg
Copy link

Failure on x86_64-darwin (full log)

Attempted: rustc

Partial log (click to expand)

   Compiling parking_lot v0.5.3
   Compiling env_logger v0.4.3
   Compiling flate2 v1.0.1
   Compiling rls-span v0.4.0
   Compiling rustc_data_structures v0.0.0 (file:///private/tmp/nix-build-rustc-1.25.0.drv-0/rustc-1.25.0-src/src/librustc_data_structures)
   Compiling rls-data v0.15.0
   Compiling syntax_pos v0.0.0 (file:///private/tmp/nix-build-rustc-1.25.0.drv-0/rustc-1.25.0-src/src/libsyntax_pos)
   Compiling rustc_errors v0.0.0 (file:///private/tmp/nix-build-rustc-1.25.0.drv-0/rustc-1.25.0-src/src/librustc_errors)
building of '/nix/store/0xbchrdwp5is09d5pbg1pnx9j1hwlqfb-rustc-1.25.0.drv' timed out after 1800 seconds
�[31;1merror:�[0m build of '/nix/store/0xbchrdwp5is09d5pbg1pnx9j1hwlqfb-rustc-1.25.0.drv' failed

LnL7
LnL7 reviewed Apr 12, 2018
View reviewed changes
pkgs/development/compilers/rust/rustc.nix
# The BOOTSTRAP_ARGS used by this Makefile must include all flags to pass
# to the bootstrap builder.
postConfigure = ''
substituteInPlace Makefile --replace 'BOOTSTRAP_ARGS :=' 'BOOTSTRAP_ARGS := --jobs $(NIX_BUILD_CORES)'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for the llvm build?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not familiar with the rust build chain, but AFAICT:

This will impact the number of cores used by the LLVM build. However, this change targets the args passed to the rustc bootstrap builder. Which has a "jobs" arg:

The docs do not explicitly state "only LLVM" but that seems to be the only build actually impacted. shrug

Specifically this modifies

Which is copied by the configure script to form the top level Makefile.

Though, now that I look at flags.rs closer: Maybe a "JOBS" env variable would work as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was asking about llvm specifically because I noticed that it does indeed use more resources than it's supposed to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood.

I did try this change using a simple JOBS env variable. No such luck. Patching Makefile.in is the only version I can get to work as expected.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: rustc

Partial log (click to expand)

shrinking RPATHs of ELF executables and libraries in /nix/store/yyaf8lp2mvqls2flyb8nspg5s1mlpam3-rustc-1.25.0-man
gzipping man pages under /nix/store/yyaf8lp2mvqls2flyb8nspg5s1mlpam3-rustc-1.25.0-man/share/man/
strip is /nix/store/j75dgadrff2d1fyc4fczmcgqkid2imdx-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/yyaf8lp2mvqls2flyb8nspg5s1mlpam3-rustc-1.25.0-man
checking for references to /build in /nix/store/yyaf8lp2mvqls2flyb8nspg5s1mlpam3-rustc-1.25.0-man...
shrinking RPATHs of ELF executables and libraries in /nix/store/hyxpgwsibyaa3g81gfzk8mqpszcca0i9-rustc-1.25.0-doc
strip is /nix/store/j75dgadrff2d1fyc4fczmcgqkid2imdx-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/hyxpgwsibyaa3g81gfzk8mqpszcca0i9-rustc-1.25.0-doc
checking for references to /build in /nix/store/hyxpgwsibyaa3g81gfzk8mqpszcca0i9-rustc-1.25.0-doc...
/nix/store/s92vr5q33vry5caipwi2lsqz6zdf9g7c-rustc-1.25.0

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Attempted: rustc

Partial log (click to expand)

   Compiling syntax_pos v0.0.0 (file:///build/rustc-1.25.0-src/src/libsyntax_pos)
   Compiling rls-span v0.4.0
   Compiling rls-data v0.15.0
   Compiling rustc_errors v0.0.0 (file:///build/rustc-1.25.0-src/src/librustc_errors)
   Compiling backtrace v0.3.5
   Compiling proc_macro v0.0.0 (file:///build/rustc-1.25.0-src/src/libproc_macro)
   Compiling rustc_const_math v0.0.0 (file:///build/rustc-1.25.0-src/src/librustc_const_math)
   Compiling syntax_ext v0.0.0 (file:///build/rustc-1.25.0-src/src/libsyntax_ext)
building of '/nix/store/faqa7mm1i55kf6yvi5xqy1lz4139sm23-rustc-1.25.0.drv' timed out after 3600 seconds
�[31;1merror:�[0m build of '/nix/store/faqa7mm1i55kf6yvi5xqy1lz4139sm23-rustc-1.25.0.drv' failed

@LnL7 LnL7 merged commit a071e9f into NixOS:master Apr 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LnL7 LnL7 LnL7 approved these changes

@Mic92 Mic92 Awaiting requested review from Mic92 Mic92 is a code owner

Assignees
No one assigned
Labels
6.topic: rust 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@coreyoconnor @GrahamcOfBorg @LnL7