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

haskellPackages.developPackage: Add a defaulted provideShellEnv argument #40582

Merged
merged 1 commit into from May 18, 2018
Merged

haskellPackages.developPackage: Add a defaulted provideShellEnv argument #40582

merged 1 commit into from May 18, 2018

Conversation

jwiegley
Copy link
Contributor

@jwiegley jwiegley commented May 16, 2018 •
edited

Motivation for this change

This will allow for greater flexbility when building custom scripting and tooling, by getting access to the core derivation constructed by developPackage, even while evaluating an invocation of nix-shell.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
  • Fits CONTRIBUTING.md.

@jwiegley jwiegley requested a review from peti as a code owner May 16, 2018 05:04
@jwiegley
Copy link
Contributor Author

Pinging @mightybyte @shlevy @peti

shlevy
shlevy requested changes May 17, 2018
View reviewed changes
Copy link
Member

@shlevy shlevy left a comment

Choose a reason for hiding this comment

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

The idea seems fine to me, but I'd prefer either an argument name or some extra documentation that let me understand what the argument means without reading the code

@jwiegley
Copy link
Contributor Author

@shlevy Any naming suggestions? I had thought "provide derivation" was pretty direct.

@shlevy
Copy link
Member

shlevy commented May 18, 2018

Maybe reverse the meaning and say useShellEnv?

@jwiegley
Copy link
Contributor Author

jwiegley commented May 18, 2018 •
edited

Well, it's not quite using it, but how about returnShellEnv?

Note that the only reason we've run into this difficulty is that, unlike mkDerivation, which would require us to write something like this:

let drv = stdenv.mkDerivation ...; in

if stdenv.lib.inNixShell then drv.env else drv

developPackage is trying to be fancy and eliminate that if for us.

The "right" change, in my opinion, is to remove this fancy logic and mirror mkDerivation, which is more consistent and easily understood. However, that ship has sailed, and we'd break a lot of people's configurations in confusing ways if we changed it now.

@shlevy
Copy link
Member

shlevy commented May 18, 2018

Yeah, agreed on all counts. returnShellEnv seems good to me.

@jwiegley
haskellPackages.developPackage: Add a defaulted provideDrv argument
d33322d 
The purpose of this argument is to allow you to get at the derivation (rather
than it's env) even when in the evaluation context of a nix-shell invocation.
@jwiegley jwiegley changed the title haskellPackages.developPackage: Add a defaulted provideDrv argument haskellPackages.developPackage: Add a defaulted provideShellEnv argument May 18, 2018
@shlevy shlevy merged commit d33322d into NixOS:master May 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shlevy shlevy shlevy requested changes

@peti peti Awaiting requested review from peti

Assignees
No one assigned
Labels
6.topic: haskell 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
3 participants
@jwiegley @shlevy @GrahamcOfBorg