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

.opam-switch/environment file format is ambiguous and can lead to errors #4325

Closed
lefessan opened this issue Aug 20, 2020 · 3 comments · Fixed by #4326
Closed

.opam-switch/environment file format is ambiguous and can lead to errors #4325

lefessan opened this issue Aug 20, 2020 · 3 comments · Fixed by #4326
Milestone

Comments

@lefessan
Copy link
Contributor

Currently, the .opam-switch/environment file is wrongly parsed by opam when the value of a variable is empty:

If my file is

CPP     =               Updated\ by\ package\ ocaml-base-compiler

CPP will wrongly be set to Updated by package ocaml-base-compiler. This will prevent creating another switch after eval $(opam env) because ocaml will fail to compile with a wrong CPP var.

An immediate fix should be to stop writing these "Updated by package $name" comments. A long-term fix should be to use a more standard format.

@lefessan lefessan changed the title .opam-switch/environment file format is ambiguous and can lead to bugs .opam-switch/environment file format is ambiguous and can lead to errors Aug 20, 2020
@dra27
Copy link
Member

dra27 commented Aug 20, 2020

Which version of opam?

@dra27
Copy link
Member

dra27 commented Aug 20, 2020

This rang a bell, and some digging reveals that I hit this years ago and fixed it in dra27@246b518, however that commit seems to have got lost in my myriad rebasing during the opam 2.0 cycle. My solution in that was to encode "" as @ and "@" as \@. The lexer was updated to recognise the single token @ as meaning "" - no handling is needed for \@ since that's dealt with automatically by the line lexer already.

@dra27
Copy link
Member

dra27 commented Aug 20, 2020

I agree that the format could be changed; the comment is useful though.

@rjbou rjbou linked a pull request Aug 20, 2020 that will close this issue
@avsm avsm added this to the 2.1.0 milestone Aug 23, 2020
@rjbou rjbou modified the milestones: 2.1.0, 2.1.0~beta Aug 26, 2020
@rjbou rjbou closed this as completed Aug 26, 2020
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

Successfully merging a pull request may close this issue.

4 participants