Skip to content

top-level, lib: Remove platform attribute of platforms#110544

Merged
Ericson2314 merged 2 commits intoNixOS:masterfrom
Ericson2314:no-platform
Jan 23, 2021
Merged

top-level, lib: Remove platform attribute of platforms#110544
Ericson2314 merged 2 commits intoNixOS:masterfrom
Ericson2314:no-platform

Conversation

@Ericson2314
Copy link
Copy Markdown
Member

@Ericson2314 Ericson2314 commented Jan 22, 2021

Motivation for this change

First commit: take II of #107214 which for some reason became a mass rebuild.

Second commit: Simplify top level.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Ericson2314 Ericson2314 requested a review from nbp as a code owner January 22, 2021 21:45
@Ericson2314 Ericson2314 requested a review from ttuegel as a code owner January 23, 2021 14:10
@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: qt/kde Object-oriented framework for GUI creation 6.topic: stdenv Standard environment 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` labels Jan 23, 2021
@Ericson2314 Ericson2314 changed the title top-level: Simplify impure and pure fallback top-level, lib: Remove platform attribute of platforms Jan 23, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. labels Jan 23, 2021
Second attempt of 8929989; see that
commit for details.

This reverts commit 0bc275e.
This is now possible, since the `platform` attribute has been removed in
PR NixOS#107214. I've been waiting to do a cleanup like this for a long time!
@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. and removed 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. labels Jan 23, 2021
@ofborg ofborg bot removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. labels Jan 23, 2021
@Ericson2314 Ericson2314 merged commit 12881a7 into NixOS:master Jan 23, 2021
@Ericson2314 Ericson2314 deleted the no-platform branch January 23, 2021 21:32
@r-burns
Copy link
Copy Markdown
Contributor

r-burns commented Jan 24, 2021

I think this broke the tarball build on darwin which is blocking unstable

@Ericson2314
Copy link
Copy Markdown
Member Author

I'll investigate.

@Ericson2314 Ericson2314 mentioned this pull request Jan 24, 2021
10 tasks
Ericson2314 added a commit that referenced this pull request Jan 25, 2021
This was not working after #110544 as caught by @r-burns in
#110544 (comment).
Thankfully it isn't used anymore and I believe wasn't documented either.
(I at least did not remember it existed.)

system.build.installBootLoader = builder;
system.boot.loader.id = "raspberrypi";
system.boot.loader.kernelFile = platform.kernelTarget;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

linux-kernel does not exist in this scope. platform comes from inherit (pkgs.stdenv.hostPlatform) platform; and is now unused.

This does not evaluate.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sorry, I will fix. How should I test this?

Copy link
Copy Markdown
Member

@ajs124 ajs124 Jan 25, 2021

Choose a reason for hiding this comment

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

Good question. nix-build -E '(import ./. {}).nixos { boot.loader.raspberryPi.enable = true; boot.loader.grub.enable = false; fileSystems."/".device = "e"; }' should work.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

See #110803

Ericson2314 added a commit that referenced this pull request Jan 25, 2021
Thanks @ajs124 in
#110544 (comment) for
catching this. According to:

    git grep 'inherit.*Platform.*platform'
    git grep ' linux-kernel'

We now don't have any remaining problems of this sort, thankfully.
@samueldr
Copy link
Copy Markdown
Member

samueldr commented Feb 2, 2021

Were those values intended to be entirely internal to Nixpkgs?

Should a compatibility shim be put in place to prevent external breakage?

Right now, I'm in a situation where I cannot just push the fix to Mobile NixOS because there are other breakage with cross-compilation that needs to be addressed. Applying the patch means I need an out-of-tree workaround to work with a known-good Nixpkgs. Not applying it means Hydra can't show the progress with regards to regressions.

As a broader topic, not really fit for here, what's the expected stability of Nixpkgs APIs? Should things just disappear like that? Should there always be a transition period where both are possible?

Sorry, I don't want to only rant here. I know the change is entirely made with good intentions in mind, and with Nixpkgs in mind.

I want to get the ball rolling about thinking how to make all the platform things less problematic to use external to Nixpkgs. I think there were other changes with 20.09->21.05, that'll bite a few individuals who stick to stable I'm sure.

These kind of breakage also make bisecting across different repos a bit more cumbersome.

samueldr added a commit to samueldr-wip/mobile-nixos-wip that referenced this pull request Feb 2, 2021
Deals with NixOS/nixpkgs#110544

This is a breaking change; the kernel builder cannot be used with a
Nixpkgs from before this change, and vice-versa.
samueldr added a commit to samueldr-wip/mobile-nixos-wip that referenced this pull request Feb 2, 2021
Deals with NixOS/nixpkgs#110544

This is a breaking change; the kernel builder cannot be used with a
Nixpkgs from before this change, and vice-versa.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: qt/kde Object-oriented framework for GUI creation 6.topic: stdenv Standard environment 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 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

Development

Successfully merging this pull request may close these issues.

4 participants