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

treewide: deprecate toplevel {build,host,target}Platform #209816

Merged
merged 2 commits into from
Jan 9, 2023
Merged

treewide: deprecate toplevel {build,host,target}Platform #209816

merged 2 commits into from
Jan 9, 2023

Conversation

Artturin
Copy link
Member

@Artturin Artturin commented Jan 9, 2023 •
edited
Loading

deprecation was attempted in e51f736 #46059

but had to be reverted in 2a6e4ae #46517

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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.05 Release Notes (or backporting 22.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@Artturin Artturin force-pushed the deprecate-top-level-platform-aliases branch 2 times, most recently from 97f4343 to f489615 Compare January 9, 2023 03:22
@Artturin Artturin changed the title top-level: Deprecate top-level {build,host,target}Platform top-level: move top-level {build,host,target}Platform to aliases Jan 9, 2023
@Artturin
top-level: move top-level {build,host,target}Platform to aliases
ecab3ed 
deprecation was attempted in e51f736

but had to be reverted in 2a6e4ae
@Artturin Artturin force-pushed the deprecate-top-level-platform-aliases branch from f489615 to ecab3ed Compare January 9, 2023 03:25
@Artturin Artturin changed the title top-level: move top-level {build,host,target}Platform to aliases treewide: remove toplevel {build,host,target}Platform Jan 9, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Jan 9, 2023
@github-actions github-actions bot added 6.topic: haskell 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jan 9, 2023
@ofborg ofborg bot added 10.rebuild-linux: 1-10 and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Jan 9, 2023
@Artturin Artturin force-pushed the deprecate-top-level-platform-aliases branch from 1e0941f to 96f7a94 Compare January 9, 2023 04:06
@Artturin Artturin mentioned this pull request Jan 9, 2023
Open
3 tasks
@trofi
Copy link
Contributor

trofi commented Jan 9, 2023 •
edited
Loading

2 seemingly minor changes in packages. Should be OK:

$ ./maintainers/scripts/rebuild-amount.sh --print HEAD~2
Estimating rebuild amount by counting changed Hydra jobs (parallel=unset).
      2 x86_64-linux

nixos-install-tools.x86_64-linux                                                                                 /nix/store/kb47yl7zbj569dvxmnwrwaribznix5nb-nixos-install-tools-23.05pre-git
tests.nixos-functions.nixos-test.x86_64-linux                                                                    /nix/store/hvsjj6843vj6dl4l218g0b2liid8ck3d-nixos-system-nixos-test

ghost
ghost approved these changes Jan 9, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Do it!

@xaverdh
Copy link
Contributor

xaverdh commented Jan 9, 2023

The nixpkgs.tarball job succeeds for me locally on the pr, lets hope it works on hydra!

@Artturin Artturin marked this pull request as ready for review January 9, 2023 13:51
@Ericson2314
Copy link
Member

Thanks for doing this, @Artturin!

@Artturin Artturin force-pushed the deprecate-top-level-platform-aliases branch from 96f7a94 to 2eeb34c Compare January 9, 2023 19:13
@Artturin
Copy link
Member Author

Artturin commented Jan 9, 2023

2 seemingly minor changes in packages. Should be OK:

$ ./maintainers/scripts/rebuild-amount.sh --print HEAD~2
Estimating rebuild amount by counting changed Hydra jobs (parallel=unset).
      2 x86_64-linux

nixos-install-tools.x86_64-linux                                                                                 /nix/store/kb47yl7zbj569dvxmnwrwaribznix5nb-nixos-install-tools-23.05pre-git
tests.nixos-functions.nixos-test.x86_64-linux                                                                    /nix/store/hvsjj6843vj6dl4l218g0b2liid8ck3d-nixos-system-nixos-test

thats because nixos-install-tools depends on the nixpkgs revision

nixpkgs/nixos/modules/installer/tools/tools.nix

Line 58 in b326f62

nixpkgsRevision = config.system.nixos.revision;

@trofi
Copy link
Contributor

trofi commented Jan 9, 2023

nixos-install-tools.x86_64-linux /nix/store

thats because nixos-install-tools depends on the nixpkgs revision

nixpkgs/nixos/modules/installer/tools/tools.nix

Line 58 in b326f62

nixpkgsRevision = config.system.nixos.revision;

TIL! Thank you!

@Artturin Artturin merged commit 27c1ec2 into NixOS:master Jan 9, 2023
@Artturin Artturin deleted the deprecate-top-level-platform-aliases branch January 9, 2023 20:15
@Artturin Artturin changed the title treewide: remove toplevel {build,host,target}Platform treewide: deprecate toplevel {build,host,target}Platform Apr 26, 2023
@Artturin Artturin mentioned this pull request Apr 26, 2023
Closed
@Artturin
Copy link
Member Author

The attrs in splice.nix weren't removed #350299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maralorn maralorn maralorn left review comments

@SuperSandro2000 SuperSandro2000 SuperSandro2000 approved these changes

@trofi trofi trofi approved these changes

@l0b0 l0b0 l0b0 approved these changes

@mmahut mmahut Awaiting requested review from mmahut

@RaghavSood RaghavSood Awaiting requested review from RaghavSood

@adisbladis adisbladis Awaiting requested review from adisbladis

@cdepillabout cdepillabout Awaiting requested review from cdepillabout

@sternenseemann sternenseemann Awaiting requested review from sternenseemann

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

@nbp nbp Awaiting requested review from nbp

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

Assignees
No one assigned
Labels
6.topic: emacs Text editor 6.topic: haskell 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python 6.topic: rust 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 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Artturin @trofi @xaverdh @Ericson2314 @l0b0 @maralorn @SuperSandro2000