Ease "lockfile" usage on Unix by removing host-arch-* and host-system-* from Unix compiler installs #20
Conversation
| @@ -14,3 +14,6 @@ license: "CC0-1.0+" | |||
| homepage: "https://opam.ocaml.org" | |||
| bug-reports: "https://github.com/ocaml/opam-repository/issues" | |||
| conflict-class: "ocaml-host-arch" | |||
| # Temporary, while these packages are not being installed by the compilers on | |||
| # non-Windows builds. | |||
| available: os = "win32" | arch = "arm32" | |||
There was a problem hiding this comment.
| available: os = "win32" | arch = "arm32" | |
| available: os = "win32" & arch = "arm32" |
???
Isn't it the prupose of this change that the package only appears on windows setups?
Or do we still want the packages available if they are requested specifically?
There was a problem hiding this comment.
Currently in opam-repository, when you create a 4.13+ switch, you get the appropriate host-arch- package - which is either a specially-determined one for Windows (which does not depend on the arch variable) or, for Unix, you get the package corresponding to the arch global variable. The available constraint here is designed to leave the selection of the host-arch- package to the compiler package when os = "win32" (which is important - for x86_32 and x86_64 that doesn't necessarily correspond with the arch variable) while stopping a user from managing to manually install, say, host-arch-s390x.1 on macOS, just because that's confusing!
In particular, it means that depending on, say, host-arch-arm64 can still be used to indicate that a package requires arm64 support on Unix. What you can't do with this PR anymore is conflict host-arch-arm64 on Unix.
There was a problem hiding this comment.
Oh ok, that makes more sense. Thanks.
dra27
force-pushed
the
rm-non-win32-host-pkgs
branch
from
1924559 to
3853123
Compare
|
Rebased |
dra27
force-pushed
the
rm-non-win32-host-pkgs
branch
from
3853123 to
8b1b896
Compare
|
Previous changes applied to the 5.2.1 and 5.3.0~beta2 packages and an extra commit applying the same principles to ocaml-system (two force-pushes separated, so the Compare link above should allow just the new changes to be reviewed, despite the rebase) |
|
So am I understanding correctly that this will not affect the new windows conf- packages since these host- packages are installed on windows in any case? |
|
correct |
|
looks good to me but I also have to admit that I don't have a high degree of confidence that I understand all the intricacies |
ocaml-base-compiler and the +options versions only installing a host-arch-* package on Windows to limit damage to inadequate locking mechanisms.
If ocaml-base-compiler and +options aren't installing a host-arch- package, ensure that _dependencies_ at least can never cause an unexpected host-arch- package to be selected. This obviously does not address conflicts.
dra27
force-pushed
the
rm-non-win32-host-pkgs
branch
from
8b1b896 to
d93aa7c
Compare
|
@dra27 mentioned that we mainly want to run linting checks on this PR, and otherwise minimize the amount of time a PR is open on the opam-repository. Using @punchagan's work on the (still nascent) opam-ci-check, we can run the linting checks locally to validate whether the packages have the expected linting results before we bother the opam-ci. I've done it for this changeset, and am reporting the results in hopes that they are helpful: # I installed the same version of opam-ci-check currently running on the opam repo
$ opam pin opam-ci-check -y https://github.com/ocurrent/opam-repo-ci.git#live
# I was in the opam repo on this PR branch:
$ git status | head -n 2
On branch rm-non-win32-host-pkgs
Your branch is up to date with 'dra27/rm-non-win32-host-pkgs'.
# I got just the package versions updated in this PR, and saved them for future reference
$ git diff --name-only HEAD dra27/master | sed 's:packages/.*/\(.*\)/opam:\1:' | paste -sd, - > updated-packags.txt
$ cat updated-packags.txt
host-arch-arm32.1,host-arch-arm64.1,host-arch-ppc64.1,host-arch-riscv64.1,host-arch-s390x.1,host-arch-unknown.1,host-arch-x86_32.1,host-arch-x86_64.1,host-system-mingw.1,host-system-msvc.1,host-system-other.1,ocaml-base-compiler.4.13.0,ocaml-base-compiler.4.13.1,ocaml-base-compiler.4.14.0,ocaml-base-compiler.4.14.1,ocaml-base-compiler.4.14.2,ocaml-base-compiler.5.0.0,ocaml-base-compiler.5.1.0,ocaml-base-compiler.5.1.1,ocaml-base-compiler.5.2.0,ocaml-base-compiler.5.2.1,ocaml-compiler.5.3.0~alpha1,ocaml-compiler.5.3.0~beta1,ocaml-compiler.5.3.0~beta2,ocaml-secondary-compiler.4.14.2,ocaml-system.4.13.0,ocaml-system.4.13.1,ocaml-system.4.14.0,ocaml-system.4.14.1,ocaml-system.4.14.2,ocaml-system.5.0.0,ocaml-system.5.1.0,ocaml-system.5.1.1,ocaml-system.5.2.0,ocaml-system.5.2.1,ocaml-variants.4.13.0+options,ocaml-variants.4.13.1+options,ocaml-variants.4.14.0+options,ocaml-variants.4.14.1+options,ocaml-variants.4.14.2+options,ocaml-variants.5.0.0+options,ocaml-variants.5.1.0+options,ocaml-variants.5.1.1+effect-syntax,ocaml-variants.5.1.1+options,ocaml-variants.5.2.0+options,ocaml-variants.5.2.0+statmemprof,ocaml-variants.5.2.1+options
# Then I fed these packages into the linter and saved the results
# (Note that this took quit longer than I expected, indicating we have lots of
# of room to improve perf -- perf has not been a focus so far.)
$ opam-ci-check lint -r . --changed-packages=$(cat updated-packags.txt) 2> lint-results.txt
# Then I checked out the master branch
$ git checkout upstream/master
Previous HEAD position was 2ac5b4411d Merge pull request #26998 from Julow/release-ocamlformat-0.27.0
HEAD is now at d34cae5550 Merge pull request #27075 from kit-ty-kate/liquidsoap-ocaml-5.3
# And ran the same lint checks, again saving the results
$ opam-ci-check lint -r . --changed-packages=$(cat updated-packags.txt) 2> lint-master-results.txt
# Finally confirming that the linter results are the exact same on this branch and on master:
$ diff lint-results.txt lint-master-results.txt
# Those results are as follows:
$ cat lint-results.txt
Warning in ocaml-base-compiler.4.13.0: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-base-compiler.4.13.1: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-base-compiler.4.14.0: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-base-compiler.4.14.1: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-base-compiler.4.14.2: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-base-compiler.5.0.0: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-base-compiler.5.1.0: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-base-compiler.5.1.1: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-base-compiler.5.2.0: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-base-compiler.5.2.1: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-compiler.5.3.0~alpha1: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-compiler.5.3.0~beta1: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-compiler.5.3.0~beta2: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-secondary-compiler.4.14.2: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-variants.4.13.0+options: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-variants.4.13.1+options: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-variants.4.14.0+options: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-variants.4.14.1+options: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-variants.4.14.2+options: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-variants.5.0.0+options: The package has a dune-project file but no explicit dependency on dune was found.
Error in ocaml-variants.5.0.0+options: warning 41: Some packages are mentioned in package scripts or features, but there is no dependency or depopt toward them: "ocaml-option-fp"
Warning in ocaml-variants.5.1.0+options: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-variants.5.1.1+effect-syntax: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-variants.5.1.1+options: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-variants.5.2.0+options: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-variants.5.2.0+statmemprof: The package has a dune-project file but no explicit dependency on dune was found.
Warning in ocaml-variants.5.2.1+options: The package has a dune-project file but no explicit dependency on dune was found.For greater assurance, I think it is still prudent to let the lint check pass in opam-repo-ci, but we should see the exact same results in opam-repo-ci. (As an aside, assuming these results are not actually indicative of problems in compiler packages, we should probably special case these packages in the linter logic). |
That crept in with ocaml#25861 and is fixed in 545ec34
It's not a special case - the existing check is incorrect. The build steps for the package don't mention Dune, so the warning shouldn't trigger. I don't know if there are (m)any packages which require Dune but don't actually mention it as a build command (e.g. by invoking |
This won't be breaking Unix locking, because it's the same, but for consistency with making the host-arch-* packages only be installed by Windows for now, do the same for host-system-*
A host-arch- and host-system- package is only installed on Windows for ocaml-system. This is slightly different from the other packages in that host-system-other and host-arch-other _are_ installed even on Windows if detection fails.
Removes lint warning 41 - no frame pointers in 5.0.0
dra27
force-pushed
the
rm-non-win32-host-pkgs
branch
from
545ec34 to
80425a2
Compare
repository-local copy of dra27#20
|
This has been merged upstream in ocaml#27302 and can now be closed |
The host-arch- and host-system- packages were added as part of the updates to opam-repository to support native Windows. This support is very much required for Windows OCaml, where the choice between mingw-w64 / MSVC (the system) and i686 / x86_64 is a more critical feature than it is on Linux / macOS / etc.
This has caused a problem for slightly broken, but nonetheless useful, uses of
opam lockwhere a lockfile is generated on one system and then applied to another system (e.g. in CI). In particular, this means that a lockfile generated on an x86_64 Linux system can never be installed on macOS arm64 hardware, because host-arch-x86_64 would be included in the lockfile, where an arm64 installation requires host-arch-arm64. Note that even before this change, there are various easy-enough-to-hit instances where using lockfiles this way is completely broken, but the host-arch- packages unintentionally made this limitation more widespread!There are plans to work on a different lockfile mechanism to cater for this properly, but in the meantime this PR backs out the change for Unix systems. That is, Windows still installs the host-arch- and host-system- packages, because they are needed, but Linux, macOS, etc. no longer install a host-arch- package or host-system-other.