Skip to content

Fix interaction between appendOverlays and otherPackageSets#136267

Merged
roberth merged 1 commit intoNixOS:masterfrom
hercules-ci:fix-pkgs-appendOverlays-otherPackageSets-interaction
Aug 31, 2021
Merged

Fix interaction between appendOverlays and otherPackageSets#136267
roberth merged 1 commit intoNixOS:masterfrom
hercules-ci:fix-pkgs-appendOverlays-otherPackageSets-interaction

Conversation

@roberth
Copy link
Member

@roberth roberth commented Aug 31, 2021

The comment

a dirty hack that should be removed

has led me to believe that nixpkgsFun isn't the right solution,
but bypassing it is worse, because it creates a second, inner
overriding mechanism that doesn't pass its changes to the old,
outer overriding mechanism.

Before this change:

nix-repl> ((import <nixpkgs> {}).appendOverlays([(f: s: { foobarbaz = "ok"; })])).foobarbaz
"ok"

nix-repl> ((import <nixpkgs> {}).appendOverlays([(f: s: { foobarbaz = "ok"; })])).pkgsCross.aarch64-multiplatform.foobarbaz
error: attribute 'foobarbaz' missing

       at «string»:1:1:

            1| ((import <nixpkgs> {}).appendOverlays([(f: s: { foobarbaz = "ok"; })])).pkgsCross.aarch64-multiplatform.foobarbaz
             | ^
            2|

After this change:

nix-repl> ((import ./. {}).appendOverlays([(f: s: { foobarbaz = "ok"; })])).pkgsCross.aarch64-multiplatform.foobarbaz
"ok"

Thanks to @samueldr for discovering this problem.

The bug was introduced with the introduction of appendOverlays itself: #47430

Motivation for this change
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 via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages 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/)
  • 21.11 Release Notes (or backporting 21.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.

The comment

    a dirty hack that should be removed

has led me to believe that nixpkgsFun isn't the right solution,
but bypassing it is worse, because it creates a second, inner
overriding mechanism that doesn't pass its changes to the old,
outer overriding mechanism.

Before this change:

    nix-repl> ((import <nixpkgs> {}).appendOverlays([(f: s: { foobarbaz = "ok"; })])).foobarbaz
    "ok"

    nix-repl> ((import <nixpkgs> {}).appendOverlays([(f: s: { foobarbaz = "ok"; })])).pkgsCross.aarch64-multiplatform.foobarbaz
    error: attribute 'foobarbaz' missing

           at «string»:1:1:

                1| ((import <nixpkgs> {}).appendOverlays([(f: s: { foobarbaz = "ok"; })])).pkgsCross.aarch64-multiplatform.foobarbaz
                 | ^
                2|

After this change:

    nix-repl> ((import ./. {}).appendOverlays([(f: s: { foobarbaz = "ok"; })])).pkgsCross.aarch64-multiplatform.foobarbaz
    "ok"

Thanks to samueldr for discovering this problem.
@roberth roberth added 0.kind: bug Something is broken 6.topic: developer experience nixpkgs development workflow 6.topic: hygiene Cleaning up and removing cruft labels Aug 31, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.kind: bug Something is broken 6.topic: developer experience nixpkgs development workflow 6.topic: hygiene Cleaning up and removing cruft 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants