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

Errors while parsing file on Cygwin #2320

Closed
antoine1fr opened this issue Oct 1, 2015 · 18 comments
Closed

Errors while parsing file on Cygwin #2320

antoine1fr opened this issue Oct 1, 2015 · 18 comments

Comments

@antoine1fr
Copy link
Contributor

I'm using Opam 1.2.2 on Cygwin and I get the following warnings when doing an update:

[WARNING] Errors while parsing 2009-0.1.1 OPAM file, skipping.
[WARNING] Errors while parsing 4.9.30-0.1.1 OPAM file, skipping.
[WARNING] Errors while parsing shell.1 OPAM file, skipping.

Is it something to be afraid of? Here is the full listing:

$ opam update -vvv

=-=- Updating package repositories =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
[default: http] Command started
+ wget "--content-disposition" "--no-check-certificate" "-t" "3" "https://opam.ocaml.org/urls.txt" (CWD=/tmp/opam-3044-42bb52)
- --2015-10-01 10:50:40--  https://opam.ocaml.org/urls.txt
- Resolving opam.ocaml.org (opam.ocaml.org)... 192.237.250.17, 2001:4801:7823:76:bb74:1f8e:ff10:627
- Connecting to opam.ocaml.org (opam.ocaml.org)|192.237.250.17|:443... connected.
- HTTP request sent, awaiting response... 200 OK
- Length: 1562056 (1.5M) [text/plain]
- Saving to: ‘urls.txt’
-
-      0K .......... .......... .......... .......... ..........  3%  202K 7s
-     50K .......... .......... .........
- . .......... ..........  6%  447K 5s
-    100K .......... .......... .......... .......... ..........  9%  441K 4s
-    150K .......... .......... .......... .......... .......... 13%  489K 4s
-    200K .......... .......... .......... .......... .......... 16%  423K 4s
-    250K .......... .......... .......... .......... .......... 19%  385K 3s
-    300K .......... .......... .......... .......... .......... 22%  453K 3s
-    350K .......... .......... .......... .......... .......... 26% 3.07M 3s
-    400K .......... .......... .......... .......... .......... 29%  404K 3s
-    450K .......... .......... .......... .......... .......... 32%  450K 2s
-    500K .......... .......... .......... .......... .......... 36%  388K 2s
-    550K .......... .......... .......... .......... .......... 39% 1.96M 2s
-    600K .......
- ... .......... .......... .......... .......... 42%  402K 2s
-    650K .......... .......... .......... .......... .......... 45%  600K 2s
-    700K .......... .......... .......... .......... .......... 49% 1.20M 2s
-    750K .......... .......... .......... .......... .......... 52%  439K 2s
-    800K .......... .......... .......... .......... .......... 55%  544K 1s
-    850K .......... .......... .......... .......... .......... 58%  478K 1s
-    900K .......... .......... .......... .......... .......... 62%  417K 1s
-    950K .......... .......... .......... .......... .......... 65% 1.68M 1s
-   1000K .......... .......... .......... .......... .......... 68%  399K 1s
-   1050K .......... .......... .......... .......... .......... 72%  683K 1s
-   1100K .......... .......... .......... .......... .......... 75%  939K 1s
-   1150K .......... .......... .......... .......... .........
- . 78%  745K 1s
-   1200K .......... .......... .......... .......... .......... 81%  897K 1s
-   1250K .......... .......... .......... .......... .......... 85%  337K 0s
-   1300K .......... .......... .......... .......... .......... 88% 5.66M 0s
-   1350K .......... .......... .......... .......... .......... 91% 3.22M 0s
-   1400K .......... .......... .......... .......... .......... 95%  397K 0s
-   1450K .......... .......... .......... .......... .......... 98% 4.41M 0s
-   1500K .......... .......... .....                           100%  611K=2.8s
-
- 2015-10-01 10:50:44 (550 KB/s) - ‘urls.txt’ saved [1562056/1562056]
-
[default: http] Command started
+ mv "/tmp/opam-3044-42bb52/urls.txt" "/home/IEUser/.opam/repo/default/urls.txt.1" (CWD=/tmp/opam-3044-42bb52)
+ mv "/home/IEUser/.opam/repo/default/urls.txt.1" "/home/IEUser/.opam/repo/default/urls.txt"
[default] synchronized from https://opam.ocaml.org
[WARNING] Errors while parsing 2009-0.1.1 OPAM file, skipping.
[WARNING] Errors while parsing 4.9.30-0.1.1 OPAM file, skipping.
[WARNING] Errors while parsing shell.1 OPAM file, skipping.
@haochenx
Copy link

haochenx commented Oct 7, 2015

I have the same problem here on Cygwin64. My opam version is also 1.2.2

@haochenx
Copy link

haochenx commented Oct 7, 2015

The same error still exists in 1.3.0-dev (I compiled and installed from master, which is ff0fa0f)

I think I located the problem to be the occurrences of %{...}% in OPAM files. It's probably that in-string config substitution is not working well on cygwin. Is there anyway to get more debugging information?

@AltGr
Copy link
Member

AltGr commented Oct 9, 2015

Thanks for reporting. --debug should print more information on what is going on, but what I can guess from the above is that some package directory names aren't cut properly ("2009-0.1.1" doesn't look like a package name).
Looking at the repository, it correspond to package tidy, which has version 0:2009-0.1.1. i can see how the colon in the version would break there!

So for you, nothing to worry about unless you want to install one of these packages ("coq:shell" and "tidy") ; but thanks for reporting, so we can fix it!

@dbuenzli
Copy link
Contributor

dbuenzli commented Oct 9, 2015

Looking at the repository, it correspond to package tidy, which has version 0:2009-0.1.1. i can see how the colon in the version would break there!

Having colons in names breaks windows IIRC, it should be forbidden. @dra27 can you confirm ?

@dra27
Copy link
Member

dra27 commented Oct 9, 2015

Yes - see also ocaml/opam-repository#4673. It would good to have it fixed by policy so that colons are simply prohibited (along with ? < > \ * | ", if those are even presently permitted) - the major PITA is that it prevents checking out of opam-repository on Windows as well!

@AltGr
Copy link
Member

AltGr commented Oct 9, 2015

@dra27 does it ? The reports above only mention spurious warnings..

@dbuenzli
Copy link
Contributor

dbuenzli commented Oct 9, 2015

@dra27 does it ? The reports above only mention spurious warnings..

I don't see warnings there, it seems that the packages with : simply can't be checked out (and then when you try to commit it marks them as deleted). The set @dra27 mentions sounds safe to forbid. See here if you want to have a little tour of windows path particularities.

@dra27
Copy link
Member

dra27 commented Oct 9, 2015

I should have said git checking out opam-repository:

[master] C:\MetaStack\Projects\Working\opam-repository>git fetch upstream
remote: Counting objects: 1930, done.
Receiving objects: 100% (1930/1930), 345.82 KiB | 269.00 KiB/s, done.
emote: Total 1930 (delta 774), reused 774 (delta 774), pack-reused 1156
Resolving deltas: 100% (985/985), completed with 181 local objects.
From https://github.com/ocaml/opam-repository
   da6115d..6cecf26  master     -> upstream/master

[master] C:\MetaStack\Projects\Working\opam-repository>git merge upstream/master
Updating 040855e..6cecf26
fatal: cannot create directory at 'packages/coq:shell': Invalid argument

Note that that error is fatal - at the moment it is impossible to checkout opam-repository in Git for Windows (I think GitHub Desktop works, but only because it skips those files rather than aborting).

It seems easier simply to ban the characters than try to alter OPAM to allow them in names, but somehow avoid them in both the git repository and .opam. The full horror of Windows file naming is very well-summarised in the link @dbuenzli posted, but I don't expect we'd hit the special names in everyday package naming!

@haochenx
Copy link

haochenx commented Oct 9, 2015

Well, colons work on cygwin though. It seems that cygwin converts colons in filenames to a special character (it seems to be a unicode char in the private use area) in the windows file system. So I don't see that cygwin cannot handle colons well being the problem. The interesting thing is that, by looking at the debugging log, it seems that OPAM is replacing the last colon of the opam file path to slash, thus causing it not able to find the file. Here's some lines from the opam update command:

see here for the full log

00:17.064  SYSTEM                          error: File /cygdrive/c/Users/Haochen/.opam/repo/default/packages/tidy/tidy.0/2009-0.1.1/opam does not exist
[WARNING] Errors while parsing 2009-0.1.1 OPAM file, skipping.
00:17.064  SYSTEM                          error: File /cygdrive/c/Users/Haochen/.opam/repo/default/packages/tidy/tidy.1/4.9.30-0.1.1/opam does not exist
[WARNING] Errors while parsing 4.9.30-0.1.1 OPAM file, skipping.
00:17.315  SYSTEM                          error: File /cygdrive/c/Users/Haochen/.opam/repo/coq-released/packages/coq:color/coq/color.1.0.0/opam does not exist
[WARNING] Errors while parsing color.1.0.0 OPAM file, skipping.
00:17.315  SYSTEM                          error: File /cygdrive/c/Users/Haochen/.opam/repo/coq-released/packages/coq:color/coq/color.1.1.0/opam does not exist
[WARNING] Errors while parsing color.1.1.0 OPAM file, skipping.

and here's how the coq-released repo is structured.

@AltGr
Copy link
Member

AltGr commented Oct 10, 2015

gasps in horror seeing @dbuenzli's link

Well this will be a problem for this coq repository: https://github.com/coq/opam-coq-archive/tree/master/extra-dev/packages
(but if it's only a policy on the repo, we should just let them know about it... :/ )

-- see http://lists.ocaml.org/pipermail/opam-devel/2014-October/000772.html for the discussion

on the other hand, putting this policy in place for opam-repository seems like a good move.

I still think we can find a way to fix this on cygwin though: it's probably a Filename.basename or similar that doesn't return the expected part of the path and triggers the issue.

@dra27
Copy link
Member

dra27 commented Nov 7, 2015

Note that a possible fix to this would be a change of policy of the name and version fields in opam files - so the directory name would be packages/coq-shell/coq-shell.1/* but opam would contain

name: "coq:shell"
version: "1"

(where arguably the version remains optional as the file is rooted in directory foo/foo.version)
So we define the name and version fields as principal. Another option might be to allow a very restricted alteration in the name field - the only differences must be characters that we know are prohibited on Windows?

dra27 added a commit to dra27/opam-repository that referenced this issue Nov 7, 2015
Windows doesn't support : in file or directory names.

See ocaml/opam#2320 (comment)
dra27 added a commit to dra27/opam-repository that referenced this issue Nov 7, 2015
Windows doesn't support : in filenames. This package has an unusual
version numbering scheme - though it's already forced to use a
different scheme in its own repository as Mercurial tags can't contain
colons either!

See ocaml/opam#2320 (comment)
dra27 added a commit to dra27/opam-repository that referenced this issue Nov 7, 2015
Windows doesn't support : in file or directory names.

See ocaml/opam#2320 (comment)
dra27 added a commit to dra27/opam-repository that referenced this issue Nov 7, 2015
Windows doesn't support : in filenames. This package has an unusual
version numbering scheme - though it's already forced to use a
different scheme in its own repository as Mercurial tags can't contain
colons either!

See ocaml/opam#2320 (comment)
@dbuenzli
Copy link
Contributor

dbuenzli commented Nov 7, 2015

xref coq/opam#16 and /cc @clarus.

Not very fond of having different name: value and file system name. AFAIK there isn't anything special about colons in the coq system and it's only related to opam packages. That unfortunate convention was suggested to Guillaume in this discussion. So I think it's better to simply change the convention.

@dra27
Copy link
Member

dra27 commented Nov 7, 2015

My thought was whether some of the other restricted characters (in particular * and ?) may come back to bite us later - but I'm personally all for simply prohibiting that entire set!

@AltGr
Copy link
Member

AltGr commented Nov 9, 2015

opam 1.2 accepts package names made of the following characters:

"#$%&'()*+,-0123456789:;?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`abcdefghijklmnopqrstuvwxyz{|}~

(which is quite permissive; basically, only characters that would clash with the version constraint are forbidden).
Besides, there is not even this restriction on versions.

I think the best move is to choose a more restricted set or characters, and add a lint error -- so that it doesn't cause failures with names not following the new convention right away.

@AltGr
Copy link
Member

AltGr commented Nov 9, 2015

(also, package names are case-sensitive...)

@dsheets
Copy link
Member

dsheets commented Nov 12, 2015

My 2 cents:

  1. Package names should be case sensitive but having packages with the same name which differ only in case should be an error on lint, repo scan, etc.
  2. Only ~+-0123456789ABCDEFGHIJKLMNOPQRSTUVWYXZ_abcdefghijklmnopqrstuvwxyz should be allowed in package names:
$ ls packages/ | grep -v -E "^[-+~0123456789ABCDEFGHIJKLMNOPQRSTUVWYXZ_abcdefghijklmnopqrstuvwxyz]+$"
coq:shell

It might be nice to disallow + as well due to common confusion with that character and URL query string encoding which causes it to be interpreted as a space. The only package using a name with + is ocaml+twt which could easily be renamed simply twt. The version alphabet will probably always require ~, though, so I'm not sure if this allowed character difference is worth it.

I believe these changes would make the namespace as compatible and unambiguous as possible without burdening package creators or repository manipulators with case-equivalence classes or special encoding and decoding to embed names in other identifier syntaxes.

@dra27
Copy link
Member

dra27 commented Nov 21, 2015

Agreed - although we need to extend it to the version field as well, as that's also used in the directory name.
ocaml/opam-repository#5090 is merged - would it be better now to open a separate issue for updating the linting - is that an issue for opam-repository or opam (or both)?

@kit-ty-kate
Copy link
Member

I believe this is fixed in opam 2.2.0 which brought native windows support. If it is not, please do reopen an issue

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

7 participants