Update unison.opam #1100
Update unison.opam #1100
Conversation
|
That makes two of us! I'll merge this after 26 0Z December, unless there are contrary comments. |
|
Any reason why the NATIVE variable is no longer used on the build command line? From what I can remember, unison failed to build on machines where no native version of ocaml is installed, and Opam CI checks this. |
|
Apart from this, I don't think this patch would cause breakage when building on unix platforms. For Windows, we'll have to test, but I don't have a Windows machine at hand, so I can't check. What I do usually is wait for a unison release to happen, then I submit the package on the opam repo, and Opam CI is telling me if there is any reasonable platform where the build fails. If something breaks, I'll patch the file on Opam repo, and submit a PR here. For the latest release, Windows broke, so I disabled Windows support temporarilly (I don't think it ever worked, Windows support for Opam is very recent). Thank's to this PR, it may finally work ;) |
This was fixed. There is now automatic fallback to ocamlc if ocamlopt is not found. NATIVE should only be used for manual override now. If this is not desired or if it proves to work unreliably then this part of the change can be reverted. |
That's true. All this work was inspired by the recent Windows support in Opam. I believe the GUI builds will still fail as cairo2 and lablgtk3 are missing some patches (our CI workflow adds those patches during building, that's why it works here). |
Then I would prefer if the change is reverted. Opam installations are typically the cases where the detection of |
Are these patches expected to be merged any time soon? If not, then I would prefer if we could specify that the |
Include NMAKE build option. Do a manual install for Win32 as there is no automatic installation in a non-POSIX environment.
tleedjarv
force-pushed
the
portable-make-opam
branch
from
74b1e44 to
06a21f7
Compare
|
Shouldn't the |
Unfortunately, this looks unlikely. The patches in question are: I don't use Windows myself, so proposed those patches as comments, not PRs, but I doubt this makes any difference. |
No need, the install target no longer builds anything, just copies the files. This was changed precisely to avoid building something with an unwanted conf. |
Alright, but does the set of copied files depend on this flag? |
|
No, the NATIVE flag is pretty much just a choice between ocamlc and ocamlopt. |
|
Ok, so I guess this should work. |
|
This change is now just about installing on Windows. @jhjourdan Are you happy to have this PR merged into unison now, or do you have concerns? My interpretation is that you are ok with the latest force push but I want to make sure I'm not misreading. |
|
Sorry, I wasn't completely clear: I am ok with this PR. |
This is hard to deal with. It seems that unison-gui is not actually incompatible with Windows; it's just that things fail due to upstream bugs. Someone who has installed fixed versions of dependencies should be able to build, and I don't feel right (from a Free Software doctrine viewpoint) marking it broken. It is likely that our detection of dependencies could be improved, to reject versions with known bugs that prevent compilation, but that seems not a good use of time. It might be easier to fork and fix, but that's not clearly reasonable either. It also seems like it should be easier to say "don't try to build foo", sort of --disable-gui, I know there are explicit targets, but that's messy with build/install, and with omitting 1 of 3, without enumerating the others. But this point is entirely separable I think, and I always try to detangle if possible. |
No worries -- it was easy to ask. Thank you for the review. |
The point here is that we are not telling that unison is incompatible with Windows. We are telling that its Opam package is, which, in effect, is true, because of the bugs in the Cairo and Lablgtk packages. Windows users, anyway, are more likely to use the Windows installer. The whole point of Opam is to provide a plug-and-play mechanism to build and install OCaml software. I'm afraid that patching dependencies does not satisfy this requirement. There is a general policy of Opam repository to only include packages that are known to build on platforms they support, so a package without such an annotation will not be accepted. |
|
Then we'd need to have a Big Scary Comment explaining that no, it is not that unison is incompatible, but that dependencies are buggy, a URL to the policy and URLs to the dependency bug reports. Does Opam really not carry patches to defective packages? It seems all packaging systems, more or less, do this as finding bugs in packages that need to be fixed to allow other builds is not strange. |
Some packages do that, indeed. But this needs to be done in the packages of these dependencies. I am not the maintainer of the opam packages for Lablgtk and Cairo (and I don't want to start doing this), these packages are maintained by their respective developers. So I guess the easiest way to make these package include these patch is to make them merged upstream. |
Disclaimer: I don't know what I am doing, as I don't use opam myself.