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.

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

all-packages: Just refer to self, not super, or res #55097

Merged
merged 2 commits into from Feb 2, 2019
Merged

all-packages: Just refer to self, not super, or res #55097

merged 2 commits into from Feb 2, 2019

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Feb 2, 2019 •
edited

Motivation for this change

Includes a change to PostgreSQL to use self.callPackage which is the last step needed for this.

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.

@Ericson2314
postgresql: No need to refer to super
c85a8a1 
self.callPackage works fine for now. If/when that changes, we have a
repo-wide thing to deal with so no reason to do anything now.
@Ericson2314
all-packages: Just refer to self, not super, or res
755e824 
This ends a years-long process to removoe pointless fixed points in this
file!
@Ericson2314 Ericson2314 merged commit e375f6d into NixOS:master Feb 2, 2019
danbst added a commit to danbst/nixpkgs that referenced this pull request Feb 3, 2019
@danbst
postgresql test: fix
df44d45 
Commit NixOS#55097 didn't modify all usages of postgresql/default.nix.

Also, replaced "random" pg with pg11. Random pg was always pg10.
@danbst danbst mentioned this pull request Feb 3, 2019
Merged
@danbst
Copy link
Contributor

danbst commented Feb 3, 2019

the grand idea was to make pg default.nix an overlay. For example, packages are also "overlay" https://github.com/nixos/nixpkgs/blob/755e824291623e9610fb1a5b8656dab5901e236c/pkgs/servers/sql/postgresql/packages.nix

Not sure that was a great idea...

danbst added a commit that referenced this pull request Feb 3, 2019
@danbst
postgresql test: fix (#55106)
bd0bb9e 
Commit #55097 didn't modify all usages of postgresql/default.nix.

Also, replaced "random" pg with pg11. Random pg was always pg10.
@roberth
Copy link
Member

roberth commented Feb 3, 2019

Partially reverted in 055ac8e. Migration message can be removed on master after release-19.03 is branched out, but no earlier.

@Ericson2314
Copy link
Member Author

@danbst, it can still be used as an overlay with a tiny wrapper function that throws away super.

@roberth OK. But I'd like to define res as pkgs as soon as possible, and deprecate super too. When was super even added?

@roberth
Copy link
Member

roberth commented Feb 5, 2019

Yes, res should go. super was already there but had a different name. Aren't we supposed to keep super around for the @nbp vulnerability patching proposal?

@Ericson2314 Ericson2314 deleted the remove-res branch February 5, 2019 06:34
@Ericson2314
Copy link
Member Author

Ericson2314 commented Feb 5, 2019 •
edited

@roberth I think we can add it back if/when that happens. (I perhaps am biased because I think the rigorous separation of build-time and run-time deps from cross + disallowedRequisites can be used to achieve the same thing better, making self vs super distinction just about overriding.)

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

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@roberth roberth Awaiting requested review from roberth

@danbst danbst Awaiting requested review from danbst

@nbp nbp Awaiting requested review from nbp

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice thoughtpolice is a code owner

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Ericson2314 @danbst @roberth @GrahamcOfBorg