Nixpkgs: Nixpkgs Contributors Guide refers to unquoted variables

[0xABAB]

Issue description

Nixpkgs Contributors Guide Version 17.03.1437.a2c7482319 refers to $out without double quotes in many locations in example code. (Same for other variables.) However, there is no stated semantics that $out can not ever contain a space. There are two ways to resolve this; one is to document that $out will never be able to contain a space (or any of the other characters that cause word splitting). Another is to fix the examples to say "$out" instead.

Because this is done wrong in the documentation, many packages have also quoted $out incorrectly.

[Ralith]

Since nix (at least in principle) supports operating under an arbitrary prefix, clarifying that $out should always be quoted seems justified.

[edolstra]

Nix does not support arbitrary prefixes, as the ubiquitous use of $out shows. It would be good to have Nix validate the store directory at runtime, though.

[7c6f434c]

I think the way we pass buildInputs is guaranteed to break down in case
of prefix with spaces. Also, CFLAGS etc.

So NixPkgs doesn't even have a clear plan to support arbitrary prefix.

Maybe the modern versions of Nix with improved string manipulation make
it possible to be more robust, but it doesn't seem to be a current
priority.

[0xABAB]

@edolstra This looks a lot like "It's not a bug, it's a feature".

It is a feature to change the location of the store, or are you also backing out of that?

http://nixos.org/releases/nix/nix-0.12/manual/ contains, just as the current manual(!):

Nix is a purely functional package manager. This means that it treats packages like values in purely functional programming languages such as Haskell — they are built by functions that don’t have side-effects, and they never change after they have been built. Nix stores packages in the Nix store, usually the directory /nix/store,

Note the word "usually", which implies that the location of the store is modifiable. I have never seen any version of Nix mention that this path has to be of a certain format. As such, I am highly disappointed by your reaction, which goes against a long history of telling people that they can modify the location of the nix store, as long as they build from source.

See also https://stackoverflow.com/questions/29236953/can-nix-be-installed-in-a-different-location-other-than-nix for further communication supporting my theory.

In short, unless you can demonstrate that $out was ever subject to any particular grammar, you are just being obnoxious.

@7c6f434c That sounds like "It is hard, so let's all pretend there is no bug".

[7c6f434c]

As such, I am highly disappointed by your reaction, which goes against a long history of telling people that they can modify the location of the nix store, as long as they build from source.

Not it doesn't. It is possible to modify the location of Nix store, but
various symbols cause various amount of gotchas. In most cases people
are OK with paths matching ^[-_a-zA-Z0-9.,@/]+$, which is the simplest
case for shell scripts and for Nix.

@7c6f434c That sounds like "It is hard, so let's all pretend there is no bug".

It is a bug that we don't have an explicit statement about the set of
unsupported characters in the Nix store path, I don't disagree here.

But we have many problems that are more annoying in practice than this
limitation, so documenting it as a drawback of the current approach
seems to be a very natural response. We want to cheaply pass additional
CFLAGS for additional include paths via environment, so just fixing the
quotation for $out throughout NixPkgs is not even close to sufficient.

[0xABAB]

@7c6f434c I think the reasonable way to solve this is to document something like you suggest for now, but at the same time accept patches to remove this limitation.

So, the manual could contain something like "NixOS currently supports <regex> for the nix store path, but we intend to widen this to be any string (although, you probably shouldn't hold your breath)". This way users know what they can do now, and not quoting $out is also resolved.

For example, we could have a review rule where you cannot say "$out is not quoted", but at the same time allow the opening of PRs fixing such problems in reasonable chunks at a time.

[7c6f434c]

For example, we could have a review rule where you cannot say "$out is not quoted", but at the same time allow the opening of PRs fixing such problems in reasonable chunks at a time.

Quoting $out doesn't help much except as a style cleanup; if we want to ever fix the actual problem, we need to have some solution to the CFLAGS question which would also be not horrible speed-wise and maintenance-wise. I am not sure there is a solution that will reach consensus support on both criteria at once (and without such support I predict that the solution will drown in the discussion if the improved correctness is worth the cost).

[0xABAB]

@7c6f434c

I think the way we pass buildInputs is guaranteed to break down in case
of prefix with spaces. Also, CFLAGS etc.

Is this something which happens in multiple places? Or is it a distributed problem? Can you point at an/the exact line where this is happening?

[7c6f434c]

Well, for examples I would start with looking at ordinary expressions that pass additional NIX_CFLAGS_COMPILE (and the wrapper for gcc that uses it).

The wrapper receives it via environment, and it is a list of multiple flags; basically we expect gcc $NIX_CFLAGS_COMPILE "@$" to work (even though there is a more complicated process involved)

[edolstra]

Supporting store prefixes with whitespace in them would be a massive undertaking for approximately zero gain. The problem is not so much Nix as the untold thousands of places in Nixpkgs and in build scripts invoked from Nixpkgs that are not whitespace-safe.

Do you have a concrete use case that needs this?

[0xABAB]

No, there is no concrete use case for this, but I don't see the problem with just saying something like "Yes, it would be nice to have, and we would accept patches that move it in this direction". I am not saying you'd have to write all the patches, but given that you have a patch review system, you'd have to review them, yes.

I don't think this is such a huge issue as you are trying to make of it; sure, if you do it by hand it's going to be a lot of work, but it seems something for which a script can be developed.

Where do you see an undecidable problem, which would make it impossible to script this?

[volth]

The only 'risky' character $out can contain is ? (question mark)
https://github.com/NixOS/nix/blob/6e01ecd112dce8d8bbe46c839f982892a3ffb589/src/libstore/store-api.cc#L87

[Mic92]

Limiting the character set in nix store creates a less hostile environment for shell scripts and also for third-party build systems and package manager, where we do not have control over. I consider this a safety feature to cope with fundamental design flaws of shell scripting languages.

[dezgeg]

And when not only considering shell scripts (where it's "merely" a bit of extra effort) there are tools where it's totally impossible to have funky characters in filenames (just a few examples):

• Makefile functions (like sort, filter-out, etc...) deal with string elements separated with space (since there is no real list datatype in make) and make doesn't do any sort of string quoting.
• You can't escape a : in the PATH environment variable, so colons are out.
• You often can't escape things in config files where you need to write out paths, like /etc/passwd or ssh_config. So a newline in a such path means you are hosed.

[edolstra]

@volth Also + (the use of package names such as gtk+ can cause expensive globbing against the Nix store). I should have restricted store names to the POSIX portable set, but it's a bit too late for that...

[volth]

On the other hand, some more chars could be allowed, for example ~ (tilda) with the same restriction as . - not on the first position

[0xABAB]

Thanks for supplying some data about potential problematic areas. It is false that it is impossible as @dezgeg says in the case of a Makefile to package a Makefile based library or program.

In all other cases it's not impossible either, but one might not want to modify the rest of the system. In the case of e.g. the PATH environment variable, I have done exactly the kind of escaping that's supposedly impossible in an application context. If one can avoid doing that, that's better, obviously, but I don't like it when people use the word impossible, because not so many things are in fact impossible.

I am not convinced that there would be a problem with specifying a grammar of what is allowed and if that set coincides with the language generated by a POSIX portable set that would also be fine. From a usability point of view the character '+' seems useful to retain. Just because the system would break if we would just make it a hard requirement doesn't mean that we cannot make a specification change such that future code is written better and we keep track of how many places we still need to tackle such that at some point we can make it a hard requirement for nixpkgs as publically hosted. If you want to be really nice to people with their own packages, you can still have another code path for those people.

Overall, it appears that some of the people in this thread are of the opinion that most people who write build systems don't know what they are doing and we cannot possibly make everyone on this planet write build systems that do work correctly in all cases. Additionally, it seems you have the belief that it is not possible to build code that detects flaws in build systems and automatically patches them. On what do you base this belief that humans do not make the exact same mistakes in building a build system?

[Mic92]

If your real use case was to have a relocatable nix store as you brought up in a unrelated discussion (please don't do that, that was rude) there is an easier solution. You can use nix-user-chroot or proot to move the nix store to arbitrary locations. It is also more efficient, since it does not require to recompile everything, because of the changing prefix.

[0xABAB]

@Mic92 Being rude was not the intention of my comment there. If one has a relocatable store (+ an overlay), then we could just have an extra nix daemon running on demand without any special permissions. I just was proposing a more general feature, and now especially that you say that the hard part of that has already been done, it seems even more of a good idea to me.