-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Antiquotes silently fail #1017
Comments
I wonder just how many evaluation errors we'd catch when this is fixed... |
It is in fact valid Nix syntax, since |
|
😱 😱 🙀 @abbradar Wow, your regex-fu... Much respect. |
@ttuegel I won't be able to parse it myself in I think 15 minutes ~_^. Seems an ugly situation without an obvious solution -- enough to tempt me to add some scream smileys to the row. Any ideas? |
I would say, fix all the occurrences in Nixpkgs and add a linting pass to Travis CI. |
First part done: NixOS/nixpkgs@3fca2ce (at least those that I could catch with the regexp -- it's not ideal). |
The only other thing I can think of is disallowing literals from antiquotes. The only valid reason to use a literal that I can think of is to copy an in-tree path to the store. That would definitely be the "scorched earth" approach, though. |
We can just disallow URLs in them -- but I'm not sure it could be done easily in Nix (I imagine they are converted to just strings in the parser). If paths are parsed differently however we can disallow strings in antiquotes entirely (I don't know a good usecase for EDIT: to clarify, I propose to have this test before any reduction, so |
I'm still not sure I buy the value of having special complex URI syntax over writing two extra quote characters for URLs. |
Also note that @vcunat knew about this issue 😄 #836 (comment) |
IIUC @vcunat was not talking about this issue, but about absence of antiquotes in URLs (http://foo/${bar}). I happen to agree that URI syntax is too much pain with parsing, tooling and issues like this for too little gain.Nikolay. |
Abbradar is correct in what I meant. The non-URL URI literals really do seem to be confusing to most people and I don't know of any use case for them. |
It would be pretty easy to disallow uri syntax inside ${ ... } blocks. I don't see much value in them there. |
just implemented this and found an other bug this way: https://github.com/NixOS/nixpkgs/blob/7475728593a49f09c4b7b959b15513aee38ab4b4/pkgs/games/vessel/default.nix#L18 that your regex foo didn't seem to be catching |
Awesome! Instead of improving the abomination I suppose we'd forbid URIs inside antiquotes and be done with it. Please, open a PR. EDIT: by abomination I meant my regex ~_^ |
this fixes NixOS#1017 evaluating `nix-env -qa` and (modulo some typo problem) this doesn't seem to be used inside nixpkgs. this is a non-backward-compatible change though, 3rd party code could break because of this.
done |
Another instance of this NixOS/nixpkgs#19939 |
I vote for dropping native uri parsing since it does not support the full rfc anyway: Two valid uris that are not recognized: |
I marked this as stale due to inactivity. → More info |
I closed this issue due to inactivity. → More info |
See NixOS/nixpkgs#17595. An antiquote of the form
substitutes the string
LINK:0:1
into the builder, instead of failing because this is not valid Nix syntax.The text was updated successfully, but these errors were encountered: