Skip to content

Deduplicate & clean up Nix shell #139297

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

Merged
merged 1 commit into from
Apr 19, 2025
Merged

Deduplicate & clean up Nix shell #139297

merged 1 commit into from
Apr 19, 2025

Conversation

RossSmyth
Copy link
Contributor

  1. Deduplicate the flake and shell files
  2. Remove flake-utils
  3. Remove with statements
    They are considered bad practice nowadays because the slow down the evalulator and have weird shadowing rules.
  4. Use env
    :3
  5. use callPackage
    It's the recommended way for derivations like these.
  6. Use packages in the shell
    There is no reason to use buildInputs for a mkShell (except in funny cases that will not be seen here), so the packages attr is recommended now days.

r? WaffleLapkin

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 3, 2025
Copy link
Member

@WaffleLapkin WaffleLapkin left a comment

Choose a reason for hiding this comment

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

Why did you move a bunch of stuff from shell.nix to x/default.nix? Doesn't this break ./x.py invocations?

@RossSmyth
Copy link
Contributor Author

RossSmyth commented Apr 4, 2025

Why did you move a bunch of stuff from shell.nix to x/default.nix? Doesn't this break ./x.py invocations?

Do you actually invoke x.py? I thought that is what the x wrapper was for. The wrapper works fine with the moving of the dependencies to what actually depends on them.

I moved them because my assumption is that one would use the x wrapper and not be invoking cmake and such directly in the shell.

The other packages in the shell, git, nix, and glibc I think are not really needed either but they are mostly harmless so I just left them.

@WaffleLapkin
Copy link
Member

@RossSmyth while I personally always use the x wrapper, I do know that some people still prefer to use x.py. It'd be nice to still support this.

@WaffleLapkin WaffleLapkin added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 7, 2025
@RossSmyth RossSmyth requested a review from WaffleLapkin April 8, 2025 00:57
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 8, 2025
@WaffleLapkin
Copy link
Member

@RossSmyth I've tried this commit locally and it doesn't appear to work:

: ~/p/rust-q (+ pqm c2cc empty); ./x.py check
downloading https://static.rust-lang.org/dist/2025-02-18/rust-std-beta-x86_64-unknown-linux-gnu.tar.xz
###################################################################################################################################### 100.0%
downloading https://static.rust-lang.org/dist/2025-02-18/rustc-beta-x86_64-unknown-linux-gnu.tar.xz
###################################################################################################################################### 100.0%
downloading https://static.rust-lang.org/dist/2025-02-18/cargo-beta-x86_64-unknown-linux-gnu.tar.xz
###################################################################################################################################### 100.0%
extracting /chonky/wffl/build/rust-q/cache/2025-02-18/rust-std-beta-x86_64-unknown-linux-gnu.tar.xz
extracting /chonky/wffl/build/rust-q/cache/2025-02-18/rustc-beta-x86_64-unknown-linux-gnu.tar.xz
extracting /chonky/wffl/build/rust-q/cache/2025-02-18/cargo-beta-x86_64-unknown-linux-gnu.tar.xz
INFO: You seem to be using Nix.
attempting to patch /chonky/wffl/build/rust-q/x86_64-unknown-linux-gnu/stage0/bin/cargo
attempting to patch /chonky/wffl/build/rust-q/x86_64-unknown-linux-gnu/stage0/bin/rustc
attempting to patch /chonky/wffl/build/rust-q/x86_64-unknown-linux-gnu/stage0/bin/rustdoc
attempting to patch /chonky/wffl/build/rust-q/x86_64-unknown-linux-gnu/stage0/libexec/rust-analyzer-proc-macro-srv
attempting to patch /chonky/wffl/build/rust-q/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/bin/rust-lld
attempting to patch /chonky/wffl/build/rust-q/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/bin/gcc-ld/ld.lld
attempting to patch /chonky/wffl/build/rust-q/x86_64-unknown-linux-gnu/stage0/lib/libLLVM.so.19.1-rust-1.86.0-beta
attempting to patch /chonky/wffl/build/rust-q/x86_64-unknown-linux-gnu/stage0/lib/librustc_driver-9370499289e43e75.so
warning: entry 48 in symbol table refers to a non-existent section, skipping
Building bootstrap
   Compiling proc-macro2 v1.0.89
   Compiling unicode-ident v1.0.13
   Compiling memchr v2.7.4
   Compiling typenum v1.17.0
   Compiling version_check v0.9.5
   Compiling libc v0.2.167
   Compiling shlex v1.3.0
   Compiling regex-syntax v0.8.5
   Compiling serde v1.0.215
   Compiling crossbeam-utils v0.8.20
   Compiling rustix v0.38.40
   Compiling bitflags v2.6.0
   Compiling linux-raw-sys v0.4.14
   Compiling pkg-config v0.3.31
   Compiling cfg-if v1.0.0
   Compiling clap_lex v0.7.2
   Compiling anstyle v1.0.10
   Compiling heck v0.5.0
   Compiling cc v1.2.17
   Compiling clap_builder v4.5.20
   Compiling semver v1.0.23
   Compiling serde_json v1.0.132
   Compiling log v0.4.22
   Compiling generic-array v0.14.7
   Compiling same-file v1.0.6
   Compiling itoa v1.0.11
   Compiling bootstrap v0.0.0 (/home/wffl/prog/rust-q/src/bootstrap)
   Compiling walkdir v2.5.0
   Compiling ryu v1.0.18
   Compiling cpufeatures v0.2.15
   Compiling termcolor v1.4.1
   Compiling aho-corasick v1.1.3
   Compiling object v0.36.5
   Compiling crossbeam-epoch v0.9.18
   Compiling home v0.5.9
   Compiling quote v1.0.37
   Compiling syn v2.0.87
   Compiling crossbeam-deque v0.8.5
   Compiling filetime v0.2.25
   Compiling cmake v0.1.54
   Compiling lzma-sys v0.1.20
   Compiling regex-automata v0.4.9
   Compiling block-buffer v0.10.4
   Compiling crypto-common v0.1.6
   Compiling digest v0.10.7
   Compiling sha2 v0.10.8
   Compiling xattr v1.3.1
   Compiling fd-lock v4.0.2
   Compiling tar v0.4.43
   Compiling toml v0.5.11
   Compiling clap_derive v4.5.18
   Compiling serde_derive v1.0.215
   Compiling bstr v1.10.0
   Compiling globset v0.4.15
   Compiling opener v0.5.2
   Compiling ignore v0.4.23
   Compiling clap v4.5.20
   Compiling clap_complete v4.5.37
   Compiling xz2 v0.1.7
   Compiling build_helper v0.1.0 (/home/wffl/prog/rust-q/src/build_helper)
    Finished `dev` profile [unoptimized] target(s) in 15.60s
Warning: rust.description is deprecated. Use build.description instead.
There have been changes to x.py since you last updated:
  [INFO] Rustdoc now respects the value of rust.lto.
    - PR Link https://github.com/rust-lang/rust/pull/135832
  [INFO] The llvm.ccache option has moved to build.ccache. llvm.ccache is now deprecated.
    - PR Link https://github.com/rust-lang/rust/pull/136941
  [INFO] It is now possible to configure `jemalloc` for each target
    - PR Link https://github.com/rust-lang/rust/pull/137170
  [INFO] Added `build.test-stage = 2` to 'tools' profile defaults
    - PR Link https://github.com/rust-lang/rust/pull/137215
  [INFO] `rust.channel` now supports "auto-detect" to load the channel from `src/ci/channel`
    - PR Link https://github.com/rust-lang/rust/pull/137220
  [INFO] The rust.description option has moved to build.description and rust.description is now deprecated.
    - PR Link https://github.com/rust-lang/rust/pull/137723
  [INFO] There is now a new `gcc` config section that can be used to download GCC from CI using `gcc.download-ci-gcc = true`
    - PR Link https://github.com/rust-lang/rust/pull/138051
  [WARNING] Removed `src/tools/rls` tool as it was deprecated long time ago.
    - PR Link https://github.com/rust-lang/rust/pull/126856
  [INFO] Added new option `build.exclude` which works the same way as `--exclude` flag on `x`.
    - PR Link https://github.com/rust-lang/rust/pull/137147
  [WARNING] The default configuration filename has changed from `config.toml` to `bootstrap.toml`. `config.toml` is deprecated but remains supported for backward compatibility.
    - PR Link https://github.com/rust-lang/rust/pull/137081
  [INFO] You can now use `change-id = "ignore"` to suppress `change-id ` warnings in the console.
    - PR Link https://github.com/rust-lang/rust/pull/138986
NOTE: to silence this warning, update `bootstrap.toml` to use `change-id = 138986` or change-id = "ignore" instead
In file included from /nix/store/2agih0y5ns3sgbviw2xhivdgg59b41g9-gcc-14-20241116/include/c++/14-20241116/ext/string_conversions.h:43,
                 from /nix/store/2agih0y5ns3sgbviw2xhivdgg59b41g9-gcc-14-20241116/include/c++/14-20241116/bits/basic_string.h:4154,
                 from /nix/store/2agih0y5ns3sgbviw2xhivdgg59b41g9-gcc-14-20241116/include/c++/14-20241116/string:54,
                 from /nix/store/2agih0y5ns3sgbviw2xhivdgg59b41g9-gcc-14-20241116/include/c++/14-20241116/bits/locale_classes.h:40,
                 from /nix/store/2agih0y5ns3sgbviw2xhivdgg59b41g9-gcc-14-20241116/include/c++/14-20241116/bits/ios_base.h:41,
                 from /nix/store/2agih0y5ns3sgbviw2xhivdgg59b41g9-gcc-14-20241116/include/c++/14-20241116/ios:44,
                 from /nix/store/2agih0y5ns3sgbviw2xhivdgg59b41g9-gcc-14-20241116/include/c++/14-20241116/ostream:40,
                 from /nix/store/2agih0y5ns3sgbviw2xhivdgg59b41g9-gcc-14-20241116/include/c++/14-20241116/iostream:41,
                 from /home/wffl/prog/rust-q/src/tools/libcxx-version/main.cpp:8:
/nix/store/2agih0y5ns3sgbviw2xhivdgg59b41g9-gcc-14-20241116/include/c++/14-20241116/cstdlib:79:15: fatal error: stdlib.h: No such file or directory
   79 | #include_next <stdlib.h>
      |               ^~~~~~~~~~
compilation terminated.
Command has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 0:00:25

@WaffleLapkin WaffleLapkin added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 9, 2025
And clean it up a little.
@RossSmyth
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 17, 2025
Copy link
Member

@WaffleLapkin WaffleLapkin left a comment

Choose a reason for hiding this comment

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

Thanks!

@WaffleLapkin
Copy link
Member

bors r plus

@bors
Copy link
Collaborator

bors commented Apr 18, 2025

📌 Commit 5a38550 has been approved by WaffleLapkin

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 18, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 18, 2025
Deduplicate & clean up Nix shell

1. Deduplicate the flake and shell files
2. Remove flake-utils
3. Remove `with` statements
They are considered bad practice nowadays because the slow down the evalulator and have weird shadowing rules.
4. Use `env`
:3
5. use `callPackage`
It's the recommended way for derivations like these.
6. Use `packages` in the shell
There is no reason to use `buildInputs` for a mkShell (except in funny cases that will not be seen here), so the  `packages` attr is recommended now days.

r? WaffleLapkin
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#137454 (not lint break with label and unsafe block)
 - rust-lang#138934 (support config extensions)
 - rust-lang#139297 (Deduplicate & clean up Nix shell)
 - rust-lang#139834 (Don't canonicalize crate paths)
 - rust-lang#139868 (Move `pal::env` into `std::sys::env_consts`)
 - rust-lang#139978 (Add citool command for generating a test dashboard)
 - rust-lang#140000 (skip llvm-config in autodiff check builds, when its unavailable)
 - rust-lang#140007 (Disable has_thread_local on i686-win7-windows-msvc)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 18, 2025
Deduplicate & clean up Nix shell

1. Deduplicate the flake and shell files
2. Remove flake-utils
3. Remove `with` statements
They are considered bad practice nowadays because the slow down the evalulator and have weird shadowing rules.
4. Use `env`
:3
5. use `callPackage`
It's the recommended way for derivations like these.
6. Use `packages` in the shell
There is no reason to use `buildInputs` for a mkShell (except in funny cases that will not be seen here), so the  `packages` attr is recommended now days.

r? WaffleLapkin
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#137454 (not lint break with label and unsafe block)
 - rust-lang#138934 (support config extensions)
 - rust-lang#139297 (Deduplicate & clean up Nix shell)
 - rust-lang#139834 (Don't canonicalize crate paths)
 - rust-lang#139978 (Add citool command for generating a test dashboard)
 - rust-lang#140000 (skip llvm-config in autodiff check builds, when its unavailable)
 - rust-lang#140007 (Disable has_thread_local on i686-win7-windows-msvc)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Apr 19, 2025
Deduplicate & clean up Nix shell

1. Deduplicate the flake and shell files
2. Remove flake-utils
3. Remove `with` statements
They are considered bad practice nowadays because the slow down the evalulator and have weird shadowing rules.
4. Use `env`
:3
5. use `callPackage`
It's the recommended way for derivations like these.
6. Use `packages` in the shell
There is no reason to use `buildInputs` for a mkShell (except in funny cases that will not be seen here), so the  `packages` attr is recommended now days.

r? WaffleLapkin
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang#137454 (not lint break with label and unsafe block)
 - rust-lang#139297 (Deduplicate & clean up Nix shell)
 - rust-lang#139535 (Implement `Default` for raw pointers)
 - rust-lang#139753 (Make `#[naked]` an unsafe attribute)
 - rust-lang#139922 (fix incorrect type in cstr `to_string_lossy()` docs)
 - rust-lang#139978 (Add citool command for generating a test dashboard)
 - rust-lang#140007 (Disable has_thread_local on i686-win7-windows-msvc)
 - rust-lang#140016 (std: Use fstatat() on illumos)
 - rust-lang#140025 (Re-remove `AdtFlags::IS_ANONYMOUS`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#137454 (not lint break with label and unsafe block)
 - rust-lang#138934 (support config extensions)
 - rust-lang#139297 (Deduplicate & clean up Nix shell)
 - rust-lang#139834 (Don't canonicalize crate paths)
 - rust-lang#139978 (Add citool command for generating a test dashboard)
 - rust-lang#140000 (skip llvm-config in autodiff check builds, when its unavailable)
 - rust-lang#140007 (Disable has_thread_local on i686-win7-windows-msvc)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#137454 (not lint break with label and unsafe block)
 - rust-lang#138934 (support config extensions)
 - rust-lang#139297 (Deduplicate & clean up Nix shell)
 - rust-lang#139834 (Don't canonicalize crate paths)
 - rust-lang#139978 (Add citool command for generating a test dashboard)
 - rust-lang#140000 (skip llvm-config in autodiff check builds, when its unavailable)
 - rust-lang#140007 (Disable has_thread_local on i686-win7-windows-msvc)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2025
…enton

Rollup of 8 pull requests

Successful merges:

 - rust-lang#137454 (not lint break with label and unsafe block)
 - rust-lang#139297 (Deduplicate & clean up Nix shell)
 - rust-lang#139535 (Implement `Default` for raw pointers)
 - rust-lang#139919 (Make rustdoc JSON Span column 1-based, just like line numbers)
 - rust-lang#139922 (fix incorrect type in cstr `to_string_lossy()` docs)
 - rust-lang#140007 (Disable has_thread_local on i686-win7-windows-msvc)
 - rust-lang#140016 (std: Use fstatat() on illumos)
 - rust-lang#140025 (Re-remove `AdtFlags::IS_ANONYMOUS`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 103be60 into rust-lang:master Apr 19, 2025
@rustbot rustbot added this to the 1.88.0 milestone Apr 19, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 19, 2025
Rollup merge of rust-lang#139297 - RossSmyth:NixClean, r=WaffleLapkin

Deduplicate & clean up Nix shell

1. Deduplicate the flake and shell files
2. Remove flake-utils
3. Remove `with` statements
They are considered bad practice nowadays because the slow down the evalulator and have weird shadowing rules.
4. Use `env`
:3
5. use `callPackage`
It's the recommended way for derivations like these.
6. Use `packages` in the shell
There is no reason to use `buildInputs` for a mkShell (except in funny cases that will not be seen here), so the  `packages` attr is recommended now days.

r? WaffleLapkin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants