NixOS / nix

Nix, the purely functional package manager
https://nixos.org/
GNU Lesser General Public License v2.1
12.27k stars 1.48k forks source link

`--update-input <input>` more restrictive than actual input names #6891

Open Radvendii opened 2 years ago

Radvendii commented 2 years ago

Describe the bug There are several characters disallowed from the input names passed into --update-input that are fine to use in actual input names, making it impossible to selectively update those inputs.

For instance, \ and .. Also, / is interpreted as specifying a transitive input, but it could also be part of the input name itself.

Steps To Reproduce

  1. git clone git@github.com:radvendii/update-input-bug
  2. ./produce_errors.sh

Expected behavior

There should be some way to update specific inputs with any characters in them. At best, it wouldn't require any escaping, but at the very least i should be able to escape special characters like /.

A much worse solution would be to limit the range of characters allowed in input names.

nix-env --version output

Additional context

  1. --update-input gets defined here https://github.com/NixOS/nix/blob/master/src/libcmd/installables.cc#L75
  2. which calls parseInputPath defined here https://github.com/NixOS/nix/blob/master/src/libexpr/flake/lockfile.cc#L240
  3. flakeIdRegex is defined (more or less) here https://github.com/NixOS/nix/blob/master/src/libutil/url-parts.hh#L42
Radvendii commented 2 years ago

I'm not sure how to resolve this, seeing as we presumably do need a syntax for updating transitive dependencies. Intuitively I would expect --update-input '"foo/bar"' to work, similar to how " escapes special characters in nix attr names. I think it would be better for the special syntax to be for the transitive inputs, but I can't think of a good syntax for that.

I also don't know why flakeIdRegex is being used there. Is it just a miscategorization? Someone thought flake input names had to be valid flake IDs, but they don't?

Radvendii commented 2 years ago

It seems that this bug was introduced in the commit that introduced --override-input. We should make sure whatever solution we come up with applies well in that case too.

Come to think of it, that may actually be the case where referring to transitive inputs is important.

edolstra commented 2 years ago

A much worse solution would be to limit the range of characters allowed in input names.

This is actually the intended behavior. Flake input names should have a limited character set, exactly because allowing arbitrary characters leads to problem in other places like the CLI.

Looks like we should test input names against the regex in parseFlakeInputs() and maybe some other places.

Radvendii commented 2 years ago
  1. This would break existing flakes. I know that it's experimental, but this seems like kind of a big breakage.
  2. This severely limits the set of separator characters that can be used, and requires awkward solutions like character substitutions. In particular, it often makes sense to define things like
    inputs = {
     "nixpkgs/master".url = "github:nixos/nixpkgs/master";
     "nixpkgs/another-branch".url = "github:nixos/nixpkgs/another-branch";
    };
  3. Are there any places where this is an inherent limitation, or just where it's been imposed by some check?
lheckemann commented 2 years ago

@Radvendii it's an inherent limitation as far as "follows" is concerned: for instance, here hydra's flake.nix expresses that the nixpkgs input should be the same as the nixpkgs input of the nix input:

https://github.com/NixOS/hydra/blob/2b1c1e65d5fbbe25625a31ee93cb14c9a9edf969/flake.nix#L7

/ is used to delimit input names here.

Radvendii commented 2 years ago

Hmm... so maybe disallowing / is necessary. flakeIdRegex limits the names far more than that, though.

Prillan commented 2 years ago

This also affects nix flake lock --override-input which prevents one from overriding e.g. haskell-nix/ghc-8.6.5-iohk directly https://github.com/input-output-hk/haskell.nix/blob/a374b4b2acb637ebff951b2853abe57bb738714b/flake.nix#L37

I agree that the names should be limited, but sometimes the inputs are outside for your control.

A workaround in this specific example is to use inputs.haskell-nix.inputs."ghc-8.6.5-iohk".url = ... instead.