Skip to content

bash: avoid using unspliced dependency#265162

Closed
ghost wants to merge 3 commits intostagingfrom
unknown repository
Closed

bash: avoid using unspliced dependency#265162
ghost wants to merge 3 commits intostagingfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Nov 3, 2023 •
edited by ghost
Loading

Description of changes

stdenv is spliced, but stdenv.cc is not, so we can't use the
latter in dependencies.

Closes

Includes

Fixes #243164

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Adam Joseph added 3 commits November 2, 2023 23:58
lib.systems, test.cross.sanity: add test case for #264989
a7d0caa 
We have several cross-compilation bugs that show up if
hostPlatform!=buildPlatform yet
hostPlatform.config==buildPlatform.config.

These bugs have appeared and disappeared as we've fiddled with the
definition of equality for platform objects.  This commit adds a
clear-cut case where they are *not* equal and never will be, so we
can test it.
gcc.libgcc: compare host and target platforms, rathern than their tri…
8408127 
…ples

The rest of our gcc expression prepends "${targetPlatform.config}-"
to paths and binaries if `hostPlatform!=targetPlatform`.  The
`libgcc.nix` expression was using
'hostPlatform.config!=targetPlatform.config`, which caused it to
look in the wrong place when moving files.  This commit corrects that.
bash: avoid using unspliced dependency
667a1ad 
`stdenv` is spliced, but `stdenv.cc` is not, so we can't use the
latter in dependencies.

Closes #264989
Closes #243164
@github-actions github-actions bot added the 6.topic: lib The Nixpkgs function library label Nov 3, 2023
@ghost ghost mentioned this pull request Nov 3, 2023
Merged
13 tasks
@ghost ghost marked this pull request as ready for review November 3, 2023 07:03
@ghost ghost requested review from Ericson2314 and alyssais as code owners November 3, 2023 07:03
@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. labels Nov 3, 2023
@ofborg ofborg bot requested a review from dtzWill November 3, 2023 09:04
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Nov 3, 2023
@alyssais
Copy link
Member

alyssais commented Nov 3, 2023

stdenv is spliced, but stdenv.cc is not, so we can't use the
latter in dependencies.

I'm a bit confused what you mean by this — are you saying buildPackages.stdenv.cc wasn't getting us what we expected? Should it just have been pkgsBuildBuild.stdenv.cc, since it was in depsBuildBuild?

@ghost
Copy link
Author

ghost commented Nov 3, 2023

I'm a bit confused what you mean by this — are you saying buildPackages.stdenv.cc wasn't getting us what we expected?

Correct.

Try changing buildPackages.stdenv.cc to assert buildPackages.stdenv.cc?__spliced; buildPackages.stdenv.cc and you'll get an assertion.

Should it just have been pkgsBuildBuild.stdenv.cc, since it was in depsBuildBuild?

No; you should not use pkgsFooBar anywhere in depsFooBar (or buildInputs or nativeBuildInputs) because pkgsFooBar are unspliced packages.

See:

which adds a lib.warn when people do this. I hope to merge that right after 23.11 branch-off.

@ghost ghost self-assigned this Nov 4, 2023
@ghost ghost marked this pull request as draft November 4, 2023 01:53
This was referenced Nov 4, 2023
Open
Merged
Artturin
Artturin reviewed Nov 6, 2023
View reviewed changes
pkgs/shells/bash/5.nix
strictDeps = true;
# Note: Bison is needed because the patches above modify parse.y.
depsBuildBuild = [ buildPackages.stdenv.cc ];
depsBuildBuild = [ stdenv ];
Copy link
Member

@Artturin Artturin Nov 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think this will work? how would stdenv become stdenv.cc?
if it works then it shouldn't work.

Copy link
Author

@ghost ghost Nov 6, 2023

Choose a reason for hiding this comment

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

You're right; this works "by accident".

@Artturin Artturin mentioned this pull request Nov 6, 2023
Closed
13 tasks
@ghost ghost mentioned this pull request Nov 6, 2023
Merged
12 tasks
@ghost
Copy link
Author

ghost commented Nov 6, 2023

This only works "by accident".

@ghost ghost closed this Nov 6, 2023
@ghost ghost deleted the pkgsCross-sanity-gnu64-bash-lastcommit branch November 6, 2023 09:25
@Artturin Artturin mentioned this pull request Nov 25, 2023
Open
@Artturin Artturin mentioned this pull request Jan 29, 2024
Open
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alyssais alyssais Awaiting requested review from alyssais

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@dtzWill dtzWill Awaiting requested review from dtzWill

Assignees

No one assigned

Labels

6.topic: lib The Nixpkgs function library 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alyssais @Artturin