Skip to content

all-packages.nix: Alias self to res, deprecating self#51401

Merged
Ericson2314 merged 1 commit intoNixOS:masterfrom
roberth:all-packages-res
Dec 3, 2018
Merged

all-packages.nix: Alias self to res, deprecating self#51401
Ericson2314 merged 1 commit intoNixOS:masterfrom
roberth:all-packages-res

Conversation

@roberth
Copy link
Member

@roberth roberth commented Dec 2, 2018 •
edited
Loading

For historical reasons, self in all-packages.nix was ill-named. This removes its usages from all-packages.nix and provides a deprecation message for those who use a patched Nixpkgs.

Some packages seem to depend on the peculiarities of res, as can be seen by making res into an alias of pkgs (normally self). The super variable doesn't have all attributes that are needed.

Therefore the simple fix is not guaranteed to work and as such,
usages of res need to be changed to pkgs or super, case by case.

Motivation for this change

#34881

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@roberth
all-packages.nix: Alias self to res, deprecating self
22aac3b 
For historical reasons, self was ill-named. This removes its usages
from all-packages.nix and provides a deprecation message for those
who use a patched Nixpkgs.

Some packages seem to depend on the peculiarities of res, as can
be seen by making res into an alias of pkgs (normally "self").
The super variable doesn't have all that is needed.

Therefore the simple fix is not guaranteed to work and as such,
usages of res need to be changed to pkgs or super, case by case.
@matthewbauer
Copy link
Member

matthewbauer commented Dec 2, 2018

Since we use rec here i think we could just get rid of res.

@GrahamcOfBorg GrahamcOfBorg 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 Dec 2, 2018
@roberth
Copy link
Member Author

roberth commented Dec 2, 2018

@matthewbauer I don't see a rec keyword. Did I miss something?

@matthewbauer
Copy link
Member

matthewbauer commented Dec 2, 2018

Oh maybe not then! with pkgs; here does a similar thing. It's used inconsistently though.

@roberth
Copy link
Member Author

roberth commented Dec 2, 2018

So we have with self, as it's usually named. That's like some class-based object oriented languages.
I don't think we should do that here, but that's a separate issue.

@Ericson2314
Copy link
Member

Ericson2314 commented Dec 3, 2018

Can we just go straight to pkgs: super:? I can't imagine why the self/res to pkgs switch would cause problems in any of those cases.

@Ericson2314 Ericson2314 mentioned this pull request Dec 3, 2018
Open
@roberth
Copy link
Member Author

roberth commented Dec 3, 2018

@Ericson2314 When switching, some hashes change, so it's not guaranteed to work.
Even though I don't expect things to break, it's still good to separate the concerns.
This pr only fixes the naming. I suppose a follow-up pr/prs can improve the way these packages are structured.

@Ericson2314 Ericson2314 merged commit 805d32d into NixOS:master Dec 3, 2018
@Ericson2314
Copy link
Member

Ericson2314 commented Dec 3, 2018

Fair enough. If you want to make the next one removing all the res for self, and ofborg can spit out the changed hashes, I'll happily triage them.

@matthewbauer
Copy link
Member

matthewbauer commented Dec 4, 2018

The warning here breaks tarball:

https://hydra.nixos.org/build/85466401/nixlog/1/tail

roberth added a commit to roberth/nixpkgs that referenced this pull request Dec 4, 2018
@roberth
all-packages.nix: Fix reference to self warning
6fcf12e 
This reference was added to master while the deprecation PR NixOS#51401 was open.
@roberth roberth mentioned this pull request Dec 4, 2018
Merged
10 tasks
NeQuissimus pushed a commit that referenced this pull request Dec 5, 2018
@roberth @NeQuissimus
all-packages.nix: Fix reference to self warning
296b333 
This reference was added to master while the deprecation PR #51401 was open.
@roberth roberth deleted the all-packages-res branch December 6, 2018 10:34
@roberth roberth mentioned this pull request Jan 24, 2019
Closed
10 tasks
@edolstra
Copy link
Member

edolstra commented Feb 3, 2019

IMHO, res is even vaguer than self. It should be final or something like that.

@Ericson2314
Copy link
Member

Ericson2314 commented Feb 4, 2019

It's now been removed!

@roberth roberth mentioned this pull request Aug 8, 2021
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ericson2314 Ericson2314 Ericson2314 approved these changes

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@nbp nbp Awaiting requested review from nbp

Assignees

No one assigned

Labels

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@roberth @matthewbauer @Ericson2314 @edolstra @GrahamcOfBorg