-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Unison 2.53.8. #28851
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
base: master
Are you sure you want to change the base?
Unison 2.53.8. #28851
Conversation
|
There is something very weird happening with |
|
@jmid what is the correct syntaxt to be uninstallable on mingw? |
|
I had something similar happen: #28478 (comment) 🤷 My current understanding is that the Rather than
where we utilize the bottom table from here: https://github.com/ocaml/opam-repository/wiki/Depexts-os-distribution---os-family-values |
|
What's the values of the variables for the msvc port? |
dfb5611 to
7165ff4
Compare
|
The following did the trick: Not particularly elegant. |
|
Looks perfectly good to me :) |
The opam CI linter is failing with I don't know how serious this is though. Is it "just" a bad-practice warning or does it mean that it is somehow broken to do so? 🤔 |
|
I don’t know, @rjbou or @kit-ty-kate what do you think? |
mmh, how did you test this? Package variables aren't supposed to be evaluated in the I've tried the following for good measure, just in case there is something wrong with the code: but i'm unable to install it even if |
packages/unison/unison.2.53.8/opam
Outdated
| available: (os != "win32" | host-system-msvc:installed) | ||
| depends: [ | ||
| "ocaml" {>= "4.08"} | ||
| ] | ||
| depopts: [ | ||
| "lablgtk3" {>= "3.1.0"} | ||
| "ocamlfind" | ||
| "host-system-msvc" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like that would work instead, for example:
| available: (os != "win32" | host-system-msvc:installed) | |
| depends: [ | |
| "ocaml" {>= "4.08"} | |
| ] | |
| depopts: [ | |
| "lablgtk3" {>= "3.1.0"} | |
| "ocamlfind" | |
| "host-system-msvc" | |
| depends: [ | |
| "ocaml" {>= "4.08"} | |
| "host-system-msvc" {os = "win32"} | |
| ] | |
| depopts: [ | |
| "lablgtk3" {>= "3.1.0"} | |
| "ocamlfind" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not work. If I specify this, then opam simply uninstalls host-system-msys and installs host-system-msvc (at least on CI)...
7165ff4 to
591613f
Compare
|
Thanks. I did that change, let's see how it goes. |
|
Cc @tleedjarv : again, I don't have a Windows machine at hand, and it looks like Unison refuses to build on the MinGW port. Since the build script explicitly mentions the MSVC port, I restricted Windows support to MSVC. Please tel me if this is ok with you. |
591613f to
2548201
Compare
|
Alright, I tried to disable |
packages/unison/unison.2.53.8/opam
Outdated
| ] | ||
| conflicts: [ | ||
| "lablgtk3" {os = "win32"} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | |
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To address the parse error:
Warning: Could not read file D:\opamroot\repo\default\packages\unison\unison.2.53.8\opam. skipping:
At D:\opamroot\repo\default\packages\unison\unison.2.53.8\opam:29:0-29:1::
Parse error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Where do you get these lints?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Somehow the "opam-ci" link disappeard for some time...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Where do you get these lints?
Click the Windows CI / build ... link
https://github.com/ocaml/opam-repository/actions/runs/19137212003/job/54692236076?pr=28851
- then unfold the
Install packagesstep - it is then written at the top:
<><> Updating package repositories ><><><><><><><><><><><><><><><><><><><><><><>
[default] synchronised from file://D:/a/opam-repository/opam-repository
Warning: Could not read file D:\opamroot\repo\default\packages\unison\unison.2.53.8\opam. skipping:
At D:\opamroot\repo\default\packages\unison\unison.2.53.8\opam:29:0-29:1::
Parse error
Now run 'opam upgrade' to apply any package updates.
- Testing unison-gui.2.53.8
unison-gui.2.53.8 is not installable. Skip.
- Testing unison.2.53.8
unison.2.53.8 is not installable. Skip.
You can further unfold the CI log for each individual package.
That's where I could see unison.2.53.8 building and unison-gui.2.53.8 failing.
This kind of error could also have been caught by running opam lint path/to/opam/file locally first (not pointing any fingers here - there's a reason I started sometimes remembering to do so... 😅 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Somehow the "opam-ci" link disappeard for some time...)
Yes, I've noticed too (both yesterday and today). I've notified @mtelvers
2548201 to
359bde1
Compare
|
I'm not sure I understand the intent of limiting this to only MSVC on Windows. |
Well, apparently not. |
359bde1 to
1f124b1
Compare
Perhaps I am misunderstanding something. On commit 591613f I see both Windows workflows building Looking at the package I know from a self-PR that there doesn't seem to be much preventing it from installing on an existing setup with That being said, it still appears as if |
|
I posted a bug report on Unison for the build failure on MinGW: bcpierce00/unison#1164. Let's see what happens. It may be the case that I have a quick answer... |
|
Appart from that, CI is failing under FreeBSD. The issue is caused by a fialure to build Cairo (the FreeBSD package, it seems). I am even less a FreeBSD user than a Windows user, so I don't want to debug this. Should I leave the FreeBSD issue as is, or should I disable the GUI on FreeBSD? It seems to be more a FreeBSD issue than an Opam/Unison issue. |
|
It is fine to leave FreeBSD as is. These days it is failing more often than succeeding, so I believe it is a CI issue. |
|
Is it possible to get the log files of the opam build from the CI? I can get |
TUI/CLI (so |
|
I'm trying to patch the sources of Unison to see if that fixes the issue with Msys. |
47f7f66 to
519dfcd
Compare
How should I patch the dependencies with an opam-only approach? I can apply a patch to unison, but I don't see how I could apply a patch to e.g., Cairo. |
Right, maybe you can't. |
|
I opened actual PRs for these two issues. Let's see if the maintainers don't react. My problems with patching in the opam package is that 1- they have not been validated and thoroughly tested in other platforms than Windows and 2- I don't know where to host the patches. |
519dfcd to
910a454
Compare
|
Ok, so I replaced the patch with bcpierce00/unison#1166, which should fix the issue on Windows. |
|
That's ready to merge. |
910a454 to
2afebf5
Compare
|
Actually, hold on: @gdt wants to do some cosmetic changes on the patch, and the hashes will change... |
No description provided.