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

nixos/{gnome,pantheon,cinnamon,budgie}: Standardize notExcluded #360817

Merged
merged 5 commits into from
Dec 4, 2024
Merged

nixos/{gnome,pantheon,cinnamon,budgie}: Standardize notExcluded #360817

merged 5 commits into from
Dec 4, 2024

Conversation

jtojnar
Copy link
Member

@jtojnar jtojnar commented Dec 1, 2024

Follow up to #247036

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: pantheon The Pantheon desktop environment 6.topic: cinnamon Desktop environment 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 1, 2024
@jtojnar jtojnar requested a review from bobby285271 December 1, 2024 19:54
@bobby285271 bobby285271 self-assigned this Dec 2, 2024
@bobby285271
Copy link
Member

bobby285271 commented Dec 2, 2024 •
edited
Loading

The nixosTests.cinnamon is a good way to test regressions I think as we have programs.gnome-terminal.enable = mkDefault (notExcluded pkgs.gnome-terminal); in the module and we have the Open GNOME Terminal subtest. But the test does not eval for me yet (and looks like it is no longer "notExcluded") :-)

@ofborg test cinnamon

@jtojnar jtojnar force-pushed the not-excluded-lib branch 2 times, most recently from 8f6ffa9 to b8c2114 Compare December 2, 2024 22:07
@bobby285271
Copy link
Member

and looks like it is no longer "notExcluded"

Ugh, to be clear, nixosTests.cinnamon now fails with machine # -bash: line 1: gnome-terminal: command not found. notExcluded really means "not included in excludePackages" 😂

@ofborg test cinnamon

jtojnar
jtojnar commented Dec 3, 2024
View reviewed changes
nixos/lib/utils.nix Outdated Show resolved Hide resolved
jtojnar
jtojnar commented Dec 3, 2024
View reviewed changes
nixos/lib/utils.nix Outdated Show resolved Hide resolved
@jtojnar
nixos/lib: Add disablePackageByName
4ceb6ec 
We do this in multiple DE modules and the behaviour was not consistent.
@jtojnar
nixos/budgie: Switch notExcluded to disablePackageByName
821535a 
Previously, this check was inconsistent with `removePackagesByName` – the latter compares the packages using their names.
@jtojnar
nixos/pantheon: Switch notExcluded to disablePackageByName
ccab095 
Previously, this check was inconsistent with `removePackagesByName` – the latter compares the packages using their names.
@jtojnar
nixos/gnome: Switch notExcluded to disablePackageByName
03b15c2 
For consistency with other DEs.
@jtojnar
nixos/cinnamon: Switch notExcluded to disablePackageByName
46cc205 
For consistency with other DEs.
@jtojnar
Copy link
Member Author

jtojnar commented Dec 4, 2024

Fixed the negation and renamed the function to disablePackageByName.

Now nix-build -A nixosTests.gnome -A nixosTests.cinnamon -A nixosTests.pantheon -A nixosTests.budgie pass successfully.

@jtojnar jtojnar merged commit ccf9d87 into NixOS:master Dec 4, 2024
31 checks passed
@jtojnar jtojnar deleted the not-excluded-lib branch December 4, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bobby285271 bobby285271 bobby285271 approved these changes

Assignees

@bobby285271 bobby285271

Labels
6.topic: cinnamon Desktop environment 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: pantheon The Pantheon desktop environment 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
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jtojnar @bobby285271