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 2.4 changed escaping #1000

Closed
Anton-Latukha opened this issue Nov 2, 2021 · 11 comments
Closed

Nix 2.4 changed escaping #1000

Anton-Latukha opened this issue Nov 2, 2021 · 11 comments

Comments

@Anton-Latukha
Copy link
Collaborator

Anton-Latukha commented Nov 2, 2021

Change is not documented.

https://github.com/haskell-nix/hnix/runs/4081568140?check_suite_focus=true

   Eval comparison tests
    ind-string-14.nix:                                               FAIL (0.02s)
      tests/TestCommon.hs:68:
      tests/eval-compare/ind-string-14.nix
      expected: "\"Escaping of ' followed by ': ''\\nEscaping of $ followed by {: \\${\\nAnd finally to interpret \\\\n etc. as in a string: \\n, \\r, \\t.\\n\"\n"
       but got: "\"Escaping of ' followed by ': ''\\nEscaping of $ followed by {: ${\\nAnd finally to interpret \\\\n etc. as in a string: \\n, \\r, \\t.\\n\"\n"
      Use -p '/ind-string-14.nix/' to rerun this test only
      
      tests/TestCommon.hs:68:
      tests/eval-compare/ind-string-05.nix
      expected: "\"  The \\\\ is not special here.\\n' can be followed by any character except another ', e.g. 'x'.\\nLikewise for $, e.g. $$ or $varName.\\nBut ' followed by ' is special, as is $ followed by {.\\nIf you want them, use anti-quotations: '', \\${.\\n\"\n"
       but got: "\"  The \\\\ is not special here.\\n' can be followed by any character except another ', e.g. 'x'.\\nLikewise for $, e.g. $$ or $varName.\\nBut ' followed by ' is special, as is $ followed by {.\\nIf you want them, use anti-quotations: '', ${.\\n\"\n"
      Use -p '/ind-string-05.nix/' to rerun this test only.
@Anton-Latukha
Copy link
Collaborator Author

We hit #1000.

Recently I was doing a lot of work.
& now suddenly upstream changes breakages piled on.

I would rest & work at my own pace.

Anybody who wants can take this.

@Anton-Latukha
Copy link
Collaborator Author

Anton-Latukha commented Nov 2, 2021

Report is a part of the #996

@Anton-Latukha
Copy link
Collaborator Author

Anton-Latukha commented Nov 3, 2021

It is probably some change regarding Nix.Parser : nixString' :: Parser (NString NExprLoc)

HNix code is complex. But complex in a much better meaning.

It happened between Nix git diff 2.3.15..2.4, if it is not: https://github.com/NixOS/nix/blob/02dff9e5299eccd8199dfbf1b5c20bf6ed9f07bd/src/nix/get-env.sh#L104 and not shellEscape - then idk the place currently.

Also - if it is not in diff or in git logs - it is probably an upstream bug.

@Anton-Latukha Anton-Latukha pinned this issue Nov 8, 2021
@Anton-Latukha
Copy link
Collaborator Author

Anton-Latukha commented Nov 8, 2021

Nope.

It should be just a representation.

But also there are some other changes. Nix did not change the official semantic version of the language, but breakages are there.

It is time to open #610 & sort them out.

@Anton-Latukha
Copy link
Collaborator Author

Seems it still requires the change at least in Parser.

And somewhere else, maybe/probably in prettyString.

So far I not understood what is happening with escaping there.

@Anton-Latukha
Copy link
Collaborator Author

Nope, again.

Understood the parser part - it does escape ''${, but Nix changed the representation of it to \\{$, so... So far I do not have idea where it should be placed.

@soulomoon
Copy link
Collaborator

Nope, again.

Understood the parser part - it does escape ''${, but Nix changed the representation of it to \\{$, so... So far I do not have idea where it should be placed.

Awkward situation here, string escape.

  • hnix: $ -> \$, ${ -> \${
  • nix-instantiate: $ -> $, ${ -> \${
  • hnix-tests: $ -> $, ${ -> ${

@soulomoon
Copy link
Collaborator

soulomoon commented Jan 4, 2022

It is related to nix 4012 issue here

explicitly formatting ${ to \${
else if (*i == '$' && *(i+1) == '{') str << "\\" << *i;

If hnix decide to follow the nix 4012 behavior,
I have gone through the related code in hnix and nix and I am willing to take this and make a pull request for it.

@Anton-Latukha
Copy link
Collaborator Author

Anton-Latukha commented Jan 4, 2022

I would be thankful for it.


P.S. In the nearest time I would also attend to other accumulated reports & ship 1.4 support release & 1.6 major release.

@soulomoon
Copy link
Collaborator

soulomoon commented Jan 5, 2022

I would be thankful for it.

P.S. In the nearest time I would also attend to other accumulated reports & ship 1.4 support release & 1.6 major release.

#1019 have been made.
It seems we have to bumped up the nix version in the submodule to make everything compatible.

The eval-okay-ind-string.exp contains relevant expected test result and the version in the submodule is outdated

P.S. I am still very new to hnix, hope that I am helping.

@Anton-Latukha
Copy link
Collaborator Author

Considering this closed.

If things pop-up, a special report would work better.

And we still need to release this.

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

2 participants