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

nix-shell should support mixing -p with a filename #4230

Open
lilyball opened this issue Nov 6, 2020 · 5 comments
Open

nix-shell should support mixing -p with a filename #4230

lilyball opened this issue Nov 6, 2020 · 5 comments

Comments

@lilyball
Copy link
Member

lilyball commented Nov 6, 2020

Describe the bug

nix-shell can take a filename to source its expression from, or it can take a -p flag to declare packages to use, but it cannot mix the two. This is really unfortunate because it means I can't use a file that declares a customized/pinned version of nixpkgs and pull individual packages from that with -p. I have to instead construct an expression using -E, which is annoying and not something I want to teach my coworkers how to do. I can't use the -I nixpkgs=somePath option either as I have a file that vends nixpkgs rather than a nixpkgs checkout.

What I'd really like to do is to be able to write

#!/usr/bin/env nix-shell
#!nix-shell pinned.nix -i bash -p bash jq
echo script goes here

But today, if I do this, it tries to interpret pinned.nix as a package.

There's two issues here. The first is that the -p flag declares "all arguments not otherwise associated with a flag are to be interpreted as packages, regardless of their location on the command line", which is rather confusing as it makes nix-shell foo -p bar behave like nix-shell -p foo bar, and the second is that the package form hard-codes import <nixpkgs>.

Suggested resolution

I think two things should be done:

  1. Stop treating nix-shell foo -p bar like nix-shell -p foo bar. I realize this is technically a backwards-incompatible change, although the nix-shell documentation indicates that the usage is -p packages....
  2. If I pass a single filename to nix-shell prior to the -p flag, replace import <nixpkgs> {} with something like (let expr = import $path; in if builtins.isFunction expr then expr {} else expr). Ideally any --arg or --argstr flags would be supported here as well. If multiple filenames are given to nix-shell along with the -p flag then they could either all be imported and merged together, or just throw an error.

A less invasive change would be to define a new flag I can use to replace the import <nixpkgs> {} with, such that nix-shell foo -p bar still retains the current behavior (though as I stated before that behavior is confusing). Or this could be hacked further by replacing the import <nixpkgs> {} with something like (let customPath = builtins.tryEval (toString <nix-shell-pkgs>); expr = import (if customPath.success then customPath.value else <nixpkgs>); in if builtins.isFunction expr then expr {} else expr). This way I could run nix-shell -I nix-shell-pkgs=pinned.nix -p bash jq and it would pull from my custom file instead. I don't see any clear benefit to this over defining a new flag though, and the new flag would be easier to document.

@lilyball lilyball added the bug label Nov 6, 2020
@edolstra edolstra added improvement and removed bug labels Nov 19, 2020
@roberth
Copy link
Member

roberth commented Jan 14, 2021

nix-shell relies on Nixpkgs' runCommandNoCC to do the heavy lifting. If you're also using a file, that's technically another shell derivation, which is not currently supported. Nixpkgs does provide mkShell { inputsFrom = shellExprs; }, so we could switch to that function instead. It doesn't seem to have a "NoCC" variant though, calling mkDerivation directly.

Although it communicates the idea qutie well, the isFunction logic isn't quite right, because --arg arguments should be propagated. This is not something you can write generic Nix code, because apparently auto-calling needs to work even if you don't specify , ... in your parameter lists. In the current implementation we build the -p-based expression as a string, but lately this has lead to odd-looking error messages. Presumably we can solve both issues with C++ code calling the evaluator in the right ways for each of the command line elements.

I agree it would be nice to support replacing import <nixpkgs> {} as well. The import <${{x}}> {} form is inconvenient if your Nixpkgs attrset isn't the root expression in a file, which is also the case with flakes.

@lilyball
Copy link
Member Author

Huh I thought we could write an expression like args@{ ... }: let expr = import $path; in if builtins.isFunction expr then expr args else expr, but experimenting right now it turns out that --arg flags are dropped if they don't exactly match a field, which is really weird and means we can't reimplement auto-calling properly in Nix code.

@stale
Copy link

stale bot commented Jul 14, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Jul 14, 2021
@roberth
Copy link
Member

roberth commented Jul 14, 2021

Still valid. nix shell or whatever it will be renamed to doesn't allow combining shell file and packages either. Preceding comment is also a valid concern about auto-calling.

@stale stale bot removed the stale label Jul 14, 2021
@stale
Copy link

stale bot commented Apr 17, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Apr 17, 2022
@stale stale bot removed the stale label Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants