Skip to content

toplevel: stdenv.lib -> lib#108934

Merged
Profpatsch merged 1 commit intoNixOS:masterfrom
Profpatsch:toplevel-stdenv-lib
Jan 10, 2021
Merged

toplevel: stdenv.lib -> lib#108934
Profpatsch merged 1 commit intoNixOS:masterfrom
Profpatsch:toplevel-stdenv-lib

Conversation

@Profpatsch
Copy link
Member

@Profpatsch Profpatsch commented Jan 10, 2021

The library does not depend on stdenv, that stdenv exposes lib is
an artifact of the ancient origins of nixpkgs.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

alyssais
alyssais approved these changes Jan 10, 2021
View reviewed changes
Copy link
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

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

LGTM as long as OfBorg identifies it as rebuild-0.

siraben
siraben approved these changes Jan 10, 2021
View reviewed changes
Copy link
Member

@siraben siraben left a comment

Choose a reason for hiding this comment

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

LGTM

@Atemu
Copy link
Member

Atemu commented Jan 10, 2021

Should such a change be done treewide whenever you get the chance to?

@FRidh
Copy link
Member

FRidh commented Jan 10, 2021

Are we in any way fixing anything by this or will this just cause work for people that have expressions relying on this?

@Profpatsch
Copy link
Member Author

Profpatsch commented Jan 10, 2021

or will this just cause work for people that have expressions relying on this?

How do you mean expressions relying on this?

@FRidh
Copy link
Member

FRidh commented Jan 10, 2021

How do you mean expressions relying on this?

Nevermind, I thought the change in stdenv was included here. This here in Nixpkgs is good.

@Atemu
Copy link
Member

Atemu commented Jan 10, 2021

Doesn't eval. Fix: Profpatsch#3

With that patch, nixpkgs-review says it doesn't change anything.

@siraben
Copy link
Member

siraben commented Jan 10, 2021

Should such a change be done treewide whenever you get the chance to?

The end goal is to completely remove stdenv.lib. When we perform it treewide it would make sense to split it into several smaller PRs.

@alyssais
Copy link
Member

alyssais commented Jan 10, 2021

The end goal is to completely remove stdenv.lib. When we perform it treewide it would make sense to split it into several smaller PRs.

I’m not sure we necessarily want to completely remove it, because there’ll be lots of external code that uses it. But we don’t want to be using it within Nixpkgs.

@Profpatsch
Copy link
Member Author

Profpatsch commented Jan 10, 2021

yeah, just not use it, maybe add a warning eventually, but that’s far out.

If we give people less examples, they won’t copy it around so much.

@siraben siraben mentioned this pull request Jan 10, 2021
Closed
@ofborg ofborg bot added 6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: python Python is a high-level, general-purpose programming language. labels Jan 10, 2021
@Profpatsch
Copy link
Member Author

Profpatsch commented Jan 10, 2021

@GrahamcOfBorg build dotnetPackages emscriptenPackages haxePackages luaPackages pythonPackages

@Profpatsch
toplevel: stdenv.lib -> lib
86b122c 
The library does not depend on stdenv, that `stdenv` exposes `lib` is
an artifact of the ancient origins of nixpkgs.
@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 Jan 10, 2021
@Profpatsch Profpatsch merged commit 2497553 into NixOS:master Jan 10, 2021
@Profpatsch Profpatsch deleted the toplevel-stdenv-lib branch January 10, 2021 20:52
@bobby285271 bobby285271 mentioned this pull request Sep 19, 2021
Merged
jmercouris added a commit to atlas-engineer/nyxt that referenced this pull request Feb 2, 2022
@jmercouris
shell.nix: update to remove stdenv.lib -> lib
aa77727 
NixOS/nixpkgs#108934
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alyssais alyssais alyssais approved these changes

@siraben siraben siraben approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

+1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: python Python is a high-level, general-purpose programming language. 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

@Profpatsch @Atemu @FRidh @siraben @alyssais