Skip to content

treewide: stdenv.is -> stdenv.hostPlatform.is#356363

Merged
JohnRTitor merged 2 commits intoNixOS:masterfrom
JohnRTitor:stdenv-is-fix
Nov 17, 2024
Merged

treewide: stdenv.is -> stdenv.hostPlatform.is#356363
JohnRTitor merged 2 commits intoNixOS:masterfrom
JohnRTitor:stdenv-is-fix

Conversation

@JohnRTitor
Copy link
Member

@JohnRTitor JohnRTitor commented Nov 16, 2024 •
edited
Loading

Commands ran:

fd --type f "\.nix" | xargs sd --fixed-strings "stdenv.is" "stdenv.hostPlatform.is"
fd --type f "\.nix" | xargs sd --fixed-strings "stdenv'.is" "stdenv'.hostPlatform.is"
fd --type f "\.nix" | xargs sd --fixed-strings "clangStdenv.is" "clangStdenv.hostPlatform.is"
fd --type f "\.nix" | xargs sd --fixed-strings "gccStdenv.is" "gccStdenv.hostPlatform.is"
fd --type f "\.nix" | xargs sd --fixed-strings "stdenvNoCC.is" "stdenvNoCC.hostPlatform.is"
fd --type f "\.nix" | xargs sd --fixed-strings "inherit (stdenv) is" "inherit (stdenv.hostPlatform) is"
fd --type f "\.nix" | xargs sd  "buildStdenv.is" "buildStdenv.hostPlatform.is"
fd --type f "\.nix" | xargs sd  "effectiveStdenv.is" "effectiveStdenv.hostPlatform.is"
fd --type f "\.nix" | xargs sd  "originalStdenv.is" "originalStdenv.hostPlatform.is"

Following #341407

stdenv.isLinux -> stdenv.hostPlatform.isLinux, is pretty much easy expansion of alias. This should cause no rebuilds.

However some packages have stdenv.isLinux in doCheck, meaning package tests have to be run when building in linux. So instead of hostPlatform there use buildPlatform there. This could cause some rebuilds.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: jitsi VoIP and instant messaging application with video conferencing capabilities labels Nov 16, 2024
Artturin
Artturin reviewed Nov 16, 2024
View reviewed changes
pkgs/by-name/ar/arti/package.nix Outdated
Copy link
Member

@Artturin Artturin Nov 16, 2024

Choose a reason for hiding this comment

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

These sneaked back in 5136e37 lol

@Artturin
Copy link
Member

Artturin commented Nov 16, 2024

Just do the commands in #341407 instead of only isLinux

@emilazy
Copy link
Member

emilazy commented Nov 16, 2024

I am unsure about the second commit here. Is it going to work with cross? Don’t we want a canExecute thing instead?

@Artturin
Copy link
Member

Artturin commented Nov 16, 2024

I am unsure about the second commit here. Is it going to work with cross? Don’t we want a canExecute thing instead?

nixpkgs/pkgs/stdenv/generic/make-derivation.nix

Line 249 in cac8a72

doCheck' = doCheck && stdenv.buildPlatform.canExecute stdenv.hostPlatform;
canExecute is added by stdenv on top

However not sure if it's added when doing overrideAttrs like done in lua-modules

@JohnRTitor
Copy link
Member Author

JohnRTitor commented Nov 16, 2024 •
edited
Loading

I am unsure about the second commit here. Is it going to work with cross?

It should work I suppose, the checks needs to be enabled on the build machine, not on the host machine where it would run.

OKAY I fundamentally misunderstood I believe, let's change it back to hostPlatform

@JohnRTitor JohnRTitor changed the title treewide: stdenv.isLinux -> stdenv.hostPlatform.isLinux; stdenv.isLinux -> stdenv.buildPlatform.isLinux for doCheck treewide: stdenv.is -> stdenv.hostPlatform.is Nov 16, 2024
@github-actions github-actions bot added 6.topic: qt/kde Object-oriented framework for GUI creation 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: TeX Issues regarding texlive and TeX in general labels Nov 16, 2024
@emilazy
Copy link
Member

emilazy commented Nov 16, 2024

Formatter is unhappy.

@JohnRTitor
Copy link
Member Author

JohnRTitor commented Nov 16, 2024

Not related to this PR, the errors are occuring as the original files are not formatted. We are only touching a line, formatting them here is out of scope

@emilazy
Copy link
Member

emilazy commented Nov 16, 2024

CI only complains if files that were formatted before the PR become unformatted, so I don’t think that’s true.

@jopejoe1
Copy link
Member

jopejoe1 commented Nov 16, 2024

It complains about pkgs/applications/radio/sdrangel/default.nix, pkgs/by-name/er/erlang-language-platform/package.nix and pkgs/by-name/im/imhex/package.nix not being formated.

@JohnRTitor
Copy link
Member Author

JohnRTitor commented Nov 16, 2024

Yeah, it seems like nixfmt will throw errors if line char limits have been exceeded and use an alternate style.

Done, this should make the CI happy, I have still decided to make the commit seperately though, for easier skimming through the diff. The PR can be squash merged if needed.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 16, 2024
JohnRTitor
JohnRTitor commented Nov 17, 2024
View reviewed changes
Copy link
Member Author

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

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

Eval successfully passed but a merge conflict is shown. Intend to merge after rebasing, as while we race for the eval to complete again likely another conflict will arise

@JohnRTitor JohnRTitor merged commit e138313 into NixOS:master Nov 17, 2024
@JohnRTitor JohnRTitor deleted the stdenv-is-fix branch November 17, 2024 10:41
@JohnRTitor JohnRTitor mentioned this pull request Nov 17, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Artturin Artturin Artturin left review comments

@Scrumplex Scrumplex Awaiting requested review from Scrumplex

@emilazy emilazy Awaiting requested review from emilazy

@Aleksanaa Aleksanaa Awaiting requested review from Aleksanaa

@figsoda figsoda Awaiting requested review from figsoda

@K900 K900 Awaiting requested review from K900

@NickCao NickCao Awaiting requested review from NickCao

@SuperSandro2000 SuperSandro2000 Awaiting requested review from SuperSandro2000

@ttuegel ttuegel Awaiting requested review from ttuegel

@winterqt winterqt Awaiting requested review from winterqt

@zowoq zowoq Awaiting requested review from zowoq

Assignees

No one assigned

Labels

6.topic: jitsi VoIP and instant messaging application with video conferencing capabilities 6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: qt/kde Object-oriented framework for GUI creation 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: TeX Issues regarding texlive and TeX in general 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@JohnRTitor @Artturin @emilazy @jopejoe1