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

[RFC 0011] Per project config #11

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@Profpatsch

This comment has been minimized.

Show comment
Hide comment
@Profpatsch

Profpatsch Apr 5, 2017

Member

-1, what speaks against using plain shell scripts?

devshell:

#!/bin/sh

devshellDir=$(dirname $(readlink -f "$0"))
baseDir=$(dirname $devshellDir)

export GOPATH=$devshellDir:$baseDir/leaps

nix-shell "$devshellDir/default.nix" -A env -I nixpkgs="$baseDir/nixpkgs" "$@"
Member

Profpatsch commented Apr 5, 2017

-1, what speaks against using plain shell scripts?

devshell:

#!/bin/sh

devshellDir=$(dirname $(readlink -f "$0"))
baseDir=$(dirname $devshellDir)

export GOPATH=$devshellDir:$baseDir/leaps

nix-shell "$devshellDir/default.nix" -A env -I nixpkgs="$baseDir/nixpkgs" "$@"
@shlevy

This comment has been minimized.

Show comment
Hide comment
@shlevy

shlevy Apr 5, 2017

Member

So I need a new script for nix-shell and nix-build, and what about tools like nix-buffer or nixops that call out to nix-build under the hood?

Member

shlevy commented Apr 5, 2017

So I need a new script for nix-shell and nix-build, and what about tools like nix-buffer or nixops that call out to nix-build under the hood?

@Mic92

This comment has been minimized.

Show comment
Hide comment
@Mic92

Mic92 Apr 6, 2017

So this is basically what direnv with the nix integration currently does.

Mic92 commented Apr 6, 2017

So this is basically what direnv with the nix integration currently does.

@shlevy

This comment has been minimized.

Show comment
Hide comment
@shlevy

shlevy Apr 6, 2017

Member

I don't think so, does it? This is about setting options for nix itself.

Member

shlevy commented Apr 6, 2017

I don't think so, does it? This is about setting options for nix itself.

@Mic92

This comment has been minimized.

Show comment
Hide comment
@Mic92

Mic92 Apr 6, 2017

Yes, you are correct, I did not read carefully enough.

Mic92 commented Apr 6, 2017

Yes, you are correct, I did not read carefully enough.

@Profpatsch

This comment has been minimized.

Show comment
Hide comment
@Profpatsch

Profpatsch Apr 6, 2017

Member

and what about tools like nix-buffer or nixops that call out to nix-build under the hood?

Normally these should have a way to forward relevant options if necessary. nixops does.

So this is basically what direnv with the nix integration currently does.

Exactly this, so I don’t see why we need to increase the nix codebase with this functionality, apart from making nix invocation suddenly depend on additional files (that may be out of the user’s direct control).

Combined with builtins.exec I suspect happy fun times ahead!

Member

Profpatsch commented Apr 6, 2017

and what about tools like nix-buffer or nixops that call out to nix-build under the hood?

Normally these should have a way to forward relevant options if necessary. nixops does.

So this is basically what direnv with the nix integration currently does.

Exactly this, so I don’t see why we need to increase the nix codebase with this functionality, apart from making nix invocation suddenly depend on additional files (that may be out of the user’s direct control).

Combined with builtins.exec I suspect happy fun times ahead!

@shlevy

This comment has been minimized.

Show comment
Hide comment
@shlevy

shlevy Apr 6, 2017

Member

I don't see how scripts actually solve this. For my project that uses a custom hydra, I need a separate script for nix-instantiate, nix-build, nix-store, and nix-shell at least, plus I need to modify nix-buffer to be able to pass an option flag based on the current directory, plus I need to tell all users that they need to use the scripts instead of the tools they're used to when working with nix...

Member

shlevy commented Apr 6, 2017

I don't see how scripts actually solve this. For my project that uses a custom hydra, I need a separate script for nix-instantiate, nix-build, nix-store, and nix-shell at least, plus I need to modify nix-buffer to be able to pass an option flag based on the current directory, plus I need to tell all users that they need to use the scripts instead of the tools they're used to when working with nix...

@shlevy

This comment has been minimized.

Show comment
Hide comment
@shlevy

shlevy Apr 6, 2017

Member

And again, this is completely orthogonal to direnv.

Member

shlevy commented Apr 6, 2017

And again, this is completely orthogonal to direnv.

@Profpatsch

This comment has been minimized.

Show comment
Hide comment
@Profpatsch

Profpatsch Apr 6, 2017

Member

or my project that uses a custom hydra, I need a separate script for nix-instantiate, nix-build, nix-store, and nix-shell at least, plus I need to modify nix-buffer to be able to pass an option flag based on the current directory

I strongly suspect there’s a better solution to your specific problem.

plus I need to tell all users that they need to use the scripts instead of the tools they're used to when working with nix

That’s valid, yes, but I think it should be solved in a way similar to npm run instead.

As a general gripe with this RFC (and RFCs in general): Not having at least three practical examples on how the proposed feature would look and play out makes me very suspicious it has been thought out very well.

Member

Profpatsch commented Apr 6, 2017

or my project that uses a custom hydra, I need a separate script for nix-instantiate, nix-build, nix-store, and nix-shell at least, plus I need to modify nix-buffer to be able to pass an option flag based on the current directory

I strongly suspect there’s a better solution to your specific problem.

plus I need to tell all users that they need to use the scripts instead of the tools they're used to when working with nix

That’s valid, yes, but I think it should be solved in a way similar to npm run instead.

As a general gripe with this RFC (and RFCs in general): Not having at least three practical examples on how the proposed feature would look and play out makes me very suspicious it has been thought out very well.

@shlevy

This comment has been minimized.

Show comment
Hide comment
@shlevy

shlevy Apr 6, 2017

Member

I strongly suspect there’s a better solution to your specific problem.

What is it? I've been dealing with this problem and ones similar to it for over 5 years of working on nix based projects and this is the best I've seen.

That’s valid, yes, but I think it should be solved in a way similar to npm run instead.

Can you expand on that?

Not having at least three practical examples on how the proposed feature would look and play out makes me very suspicious it has been thought out very well.

There's no section in the RFC for practical examples, but that doesn't mean I don't have any... I could provide several if you think it's relevant. If you disagree with the utility of this feature, fine, but it is well thought out and does solve a real need I've had multiple times, even if it's not the ideal solution.

Member

shlevy commented Apr 6, 2017

I strongly suspect there’s a better solution to your specific problem.

What is it? I've been dealing with this problem and ones similar to it for over 5 years of working on nix based projects and this is the best I've seen.

That’s valid, yes, but I think it should be solved in a way similar to npm run instead.

Can you expand on that?

Not having at least three practical examples on how the proposed feature would look and play out makes me very suspicious it has been thought out very well.

There's no section in the RFC for practical examples, but that doesn't mean I don't have any... I could provide several if you think it's relevant. If you disagree with the utility of this feature, fine, but it is well thought out and does solve a real need I've had multiple times, even if it's not the ideal solution.

@shlevy

This comment has been minimized.

Show comment
Hide comment
@shlevy

shlevy Apr 6, 2017

Member

Also, in case this wasn't clear, if this is merged in it never has to affect you, just don't set trusted-project-roots

Member

shlevy commented Apr 6, 2017

Also, in case this wasn't clear, if this is merged in it never has to affect you, just don't set trusted-project-roots

@shlevy

This comment has been minimized.

Show comment
Hide comment
@shlevy

shlevy Apr 6, 2017

Member

Some real cases this would have improved:

  1. A project that needed build-use-sandbox off because of impurities (that were eventually fixed, but in the mean time)
  2. At least three different projects with custom but slow binary-caches whose inclusion in the global list would significantly impact builds on other projects
  3. A project that wanted to enforce restrict-eval-clean evaluations
  4. Two projects that wanted to use nix plugins via importNative
Member

shlevy commented Apr 6, 2017

Some real cases this would have improved:

  1. A project that needed build-use-sandbox off because of impurities (that were eventually fixed, but in the mean time)
  2. At least three different projects with custom but slow binary-caches whose inclusion in the global list would significantly impact builds on other projects
  3. A project that wanted to enforce restrict-eval-clean evaluations
  4. Two projects that wanted to use nix plugins via importNative
@Profpatsch

This comment has been minimized.

Show comment
Hide comment
@Profpatsch

Profpatsch Apr 7, 2017

Member

That’s valid, yes, but I think it should be solved in a way similar to npm run instead.
Can you expand on that?

I haven’t thought it through exactly, but in my experience the "script" field in package.json of npm is very nice. It is an attrset that executes the command given in the context of the loaded dependencies, with npm run <script key>
Thinking about it, the file that would contain the "scripts" field might also contain nix options. Instead of putting it in nix, I’d rather have a separate tool, though.

There's no section in the RFC for practical examples

I’d argue there should be one for an RFC to be complete. Means there is a mockup to be developed against. The examples should act as acceptance tests.

Member

Profpatsch commented Apr 7, 2017

That’s valid, yes, but I think it should be solved in a way similar to npm run instead.
Can you expand on that?

I haven’t thought it through exactly, but in my experience the "script" field in package.json of npm is very nice. It is an attrset that executes the command given in the context of the loaded dependencies, with npm run <script key>
Thinking about it, the file that would contain the "scripts" field might also contain nix options. Instead of putting it in nix, I’d rather have a separate tool, though.

There's no section in the RFC for practical examples

I’d argue there should be one for an RFC to be complete. Means there is a mockup to be developed against. The examples should act as acceptance tests.

@shlevy

This comment has been minimized.

Show comment
Hide comment
@shlevy

shlevy Apr 7, 2017

Member

I’d argue there should be one for an RFC to be complete

Open a PR 😄

Member

shlevy commented Apr 7, 2017

I’d argue there should be one for an RFC to be complete

Open a PR 😄

[unresolved]: #unresolved-questions
Is the path nix is operating on determined by the current working
directory or the path of the nix expression(s) it's evaluating?

This comment has been minimized.

@dezgeg

dezgeg Apr 8, 2017

If the latter were implemented, how would this feature interact with import? Consider that when building a NixOS configuration, the "entrypoint" is not your configuration.nix but rather $NIXPKGS/nixos/default.nix. So if imports wouldn't trigger reading nix.conf.locals, then this feature wouldn't work with NixOS configurations without forking nixpkgs into the project direrctory.

On the other hand, the thought of nix config options changing in the middle of the evaluation after an import sounds like a massive headache.

@dezgeg

dezgeg Apr 8, 2017

If the latter were implemented, how would this feature interact with import? Consider that when building a NixOS configuration, the "entrypoint" is not your configuration.nix but rather $NIXPKGS/nixos/default.nix. So if imports wouldn't trigger reading nix.conf.locals, then this feature wouldn't work with NixOS configurations without forking nixpkgs into the project direrctory.

On the other hand, the thought of nix config options changing in the middle of the evaluation after an import sounds like a massive headache.

This comment has been minimized.

@shlevy

shlevy Apr 8, 2017

Member

Right, yes. I'm pretty sure current working directory is the only sane choice.

@shlevy

shlevy Apr 8, 2017

Member

Right, yes. I'm pretty sure current working directory is the only sane choice.

@zimbatm

This comment has been minimized.

Show comment
Hide comment
@zimbatm

zimbatm Apr 9, 2017

Member
Member

zimbatm commented Apr 9, 2017

@shlevy

This comment has been minimized.

Show comment
Hide comment
@shlevy

shlevy Apr 9, 2017

Member

@zimbatm What's wrong with builtins.fetchTarball? Or something like

let nixPath =
  [
    { path = "https://github.com/NixOS/nixpkgs-channels/archive/nixos-unstable.tar.gz";
      prefix = "nixpkgs";
    }
  ]; in (import (builtins.findFile nixPath "nixpkgs") {}).hello
Member

shlevy commented Apr 9, 2017

@zimbatm What's wrong with builtins.fetchTarball? Or something like

let nixPath =
  [
    { path = "https://github.com/NixOS/nixpkgs-channels/archive/nixos-unstable.tar.gz";
      prefix = "nixpkgs";
    }
  ]; in (import (builtins.findFile nixPath "nixpkgs") {}).hello
@edolstra

This comment has been minimized.

Show comment
Hide comment
@edolstra

edolstra Apr 10, 2017

Member

A few thoughts:

  • Making Nix behaviour dependent on what directory you're in seems inelegant. However, making it dependent on the directory of the arguments seems worse because then you have to consider cases like nix-build /path1 /path2.

  • This breaks compositionality of Nix expression trees. Building foo.nix no longer necessarily produces the same result as building import /path/to/foo.nix from another directory.

  • I understand the goal of easily supporting per-project settings, but having to set a trusted-project-roots variable in nix.conf makes this still a pretty manual process. For example, ideally you could tell users to do git checkout foo; cd foo; nix-build, but instead they have to manually edit nix.conf first. Maybe something like Emacs's .dir-locals.el is worth considering, i.e. apply all "safe" options automatically (not sure if we have any), then ask for permission once for unsafe options.

  • It would be better to get rid of unsafe options (e.g. turn certain exec use cases into safe builtin functions). Options are bad, so options to modify the processing of options are really bad...

Member

edolstra commented Apr 10, 2017

A few thoughts:

  • Making Nix behaviour dependent on what directory you're in seems inelegant. However, making it dependent on the directory of the arguments seems worse because then you have to consider cases like nix-build /path1 /path2.

  • This breaks compositionality of Nix expression trees. Building foo.nix no longer necessarily produces the same result as building import /path/to/foo.nix from another directory.

  • I understand the goal of easily supporting per-project settings, but having to set a trusted-project-roots variable in nix.conf makes this still a pretty manual process. For example, ideally you could tell users to do git checkout foo; cd foo; nix-build, but instead they have to manually edit nix.conf first. Maybe something like Emacs's .dir-locals.el is worth considering, i.e. apply all "safe" options automatically (not sure if we have any), then ask for permission once for unsafe options.

  • It would be better to get rid of unsafe options (e.g. turn certain exec use cases into safe builtin functions). Options are bad, so options to modify the processing of options are really bad...

@edolstra edolstra changed the title from [RFC 0011] Add per-project-config RFC to [RFC 0011] Per-project-config Apr 10, 2017

@edolstra edolstra changed the title from [RFC 0011] Per-project-config to [RFC 0011] Per project config Apr 10, 2017

@shlevy

This comment has been minimized.

Show comment
Hide comment
@shlevy

shlevy Apr 10, 2017

Member

Maybe something like Emacs's .dir-locals.el is worth considering, i.e. apply all "safe" options automatically (not sure if we have any), then ask for permission once for unsafe options.

Not a bad idea. I wonder the best way to do this.

It would be better to get rid of unsafe options (e.g. turn certain exec use cases into safe builtin functions). Options are bad, so options to modify the processing of options are really bad...

If we have #9 and we're OK with putting a lot of stuff upstream into nix, that may be fine

Alternatively, perhaps we can let users specify a list/directory of blessed plugins, rather than turning on all unsafe native eval?

Member

shlevy commented Apr 10, 2017

Maybe something like Emacs's .dir-locals.el is worth considering, i.e. apply all "safe" options automatically (not sure if we have any), then ask for permission once for unsafe options.

Not a bad idea. I wonder the best way to do this.

It would be better to get rid of unsafe options (e.g. turn certain exec use cases into safe builtin functions). Options are bad, so options to modify the processing of options are really bad...

If we have #9 and we're OK with putting a lot of stuff upstream into nix, that may be fine

Alternatively, perhaps we can let users specify a list/directory of blessed plugins, rather than turning on all unsafe native eval?

@0xABAB

This comment has been minimized.

Show comment
Hide comment
@0xABAB

0xABAB Jun 30, 2017

I agree with most of @edolstra's comments.

0xABAB commented Jun 30, 2017

I agree with most of @edolstra's comments.

@zimbatm

This comment has been minimized.

Show comment
Hide comment
@zimbatm

zimbatm Jul 2, 2017

Member

@shlevy how is that channel loaded with nix-shell? Ideally I can run nix-shell -p curl --channel .nixpkgs or something short like that.

One option that hasn't been discussed yet would be to introduce pragmas to the language. That's what languages generally use to change behaviour that isn't configurable at runtime. For illustration's sake:

default.nix

## build-use-sandbox=false
{ pkgs ? import <nixpkgs> {} }:
# ...

The pragmas would only be loaded from the entry-point script. The user would have to be a trusted user for the pragmas to work.

Member

zimbatm commented Jul 2, 2017

@shlevy how is that channel loaded with nix-shell? Ideally I can run nix-shell -p curl --channel .nixpkgs or something short like that.

One option that hasn't been discussed yet would be to introduce pragmas to the language. That's what languages generally use to change behaviour that isn't configurable at runtime. For illustration's sake:

default.nix

## build-use-sandbox=false
{ pkgs ? import <nixpkgs> {} }:
# ...

The pragmas would only be loaded from the entry-point script. The user would have to be a trusted user for the pragmas to work.

@zimbatm

This comment has been minimized.

Show comment
Hide comment
@zimbatm

zimbatm Jul 2, 2017

Member

If we could get rid of the security features somehow then that would be much easier to implement. I think that all the security features revolve around the fact that a user could pollute the /nix/store with untrusted results. That could be solved by not letting untrusted users write in /nix/store, only in a unionfs-mount of their personal $HOME/.nix/store.

Member

zimbatm commented Jul 2, 2017

If we could get rid of the security features somehow then that would be much easier to implement. I think that all the security features revolve around the fact that a user could pollute the /nix/store with untrusted results. That could be solved by not letting untrusted users write in /nix/store, only in a unionfs-mount of their personal $HOME/.nix/store.

@0xABAB

This comment has been minimized.

Show comment
Hide comment
@0xABAB

0xABAB Jul 2, 2017

@zimbatm In my previous comment, which I redacted based on some feedback, I also mentioned an overlay solution (by which I was thinking of a unionfs-mount as well, overlayfs perhaps). Removing security from the discussion seems desirable. It could even be designed such that this overlay feature could then be disabled.

0xABAB commented Jul 2, 2017

@zimbatm In my previous comment, which I redacted based on some feedback, I also mentioned an overlay solution (by which I was thinking of a unionfs-mount as well, overlayfs perhaps). Removing security from the discussion seems desirable. It could even be designed such that this overlay feature could then be disabled.

@shlevy shlevy closed this Jan 19, 2018

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