Install to lib/stublibs in legacy mode as well
#40
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Leonidas-from-XIV
force-pushed
the
install-to-pkg-dir
branch
from
54775ff
to
712d66e
Compare
|
This seems a reasonable compromise to me - I want to do some local testing just to be certain that there's nothing strange going on with .cmi shadowing, but otherwise this just needs a separate CI tweak (again sigh) and it should be good to go, thanks. |
The switch will be configured to load dllnums.so from the switch stublibs directory anyway and the compatibility is necessary for the cma and cmxa to be in the stdlib directory, not for the stubs library.
|
I pushed an extra commit to remove the duplicated dllnums.so. As well as helping Dune package management, this I think creates a better a story for how the library is packaged (as testified by the simplified META and .install files) - for all versions of OCaml, we install num as a findlib package, it's just that for < 5.0.0 we also install the core files to the OCaml stdlib directory for compatibility. i.e. the compatibility packaging is a strict superset of the "master/trunk" packaging, rather than being completely different. |
|
(for packaging reasons, it would be helpful if whoever merges this PR squashes it, please) |
Leonidas-from-XIV
added a commit
to Leonidas-from-XIV/opam-repository
that referenced
this pull request
Jul 2, 2024
Num does a bit of an unusual thing for findlib packages as it installs
into (what it considers) the OCaml stdlib.
This works as long as the stdlib is in `%{lib_root}%/ocaml` and thus it
doesn't findlib to add it to the compiler search path. In the new dune
package management `%{lib_root}%/ocaml` will not point to the ocaml
compiler stdlib thus `num` will not be found unless its installation
directory is added to the search path (via findlib or dune or any other
mechanism that evaluates META files).
In OCaml 5.0 this is the default already, `num` is never installed in
the stdlib but always into its package folder.
This patch changes the behavior for OCaml 4.x where before `num` was
only installed into the stdlib folder (and didn't need to be stated as
dependency) but referencing it via findlib did not hurt to install it in
*both* the stdlib and the package folder.
This way if a package depends on num in the stdlib without referencing
num it will find num in the stdlib, but if `num` is referenced via
findlib it will add the package folder to the search path and find
either copy.
This patch has been merged upstream, see the discussion at
ocaml/num#40 for more details. This patch just
backports the change to the latest released `num` version as future
versions of `num` will come with this change. The next num release might
be some time off given `num` is mostly "done" therefore this patch is
meant to bridge the gap and a recommendation from @dra27 who is
maintaining `num` upstream.
avsm
pushed a commit
to avsm/opam-repository
that referenced
this pull request
Sep 5, 2024
Num does a bit of an unusual thing for findlib packages as it installs
into (what it considers) the OCaml stdlib.
This works as long as the stdlib is in `%{lib_root}%/ocaml` and thus it
doesn't findlib to add it to the compiler search path. In the new dune
package management `%{lib_root}%/ocaml` will not point to the ocaml
compiler stdlib thus `num` will not be found unless its installation
directory is added to the search path (via findlib or dune or any other
mechanism that evaluates META files).
In OCaml 5.0 this is the default already, `num` is never installed in
the stdlib but always into its package folder.
This patch changes the behavior for OCaml 4.x where before `num` was
only installed into the stdlib folder (and didn't need to be stated as
dependency) but referencing it via findlib did not hurt to install it in
*both* the stdlib and the package folder.
This way if a package depends on num in the stdlib without referencing
num it will find num in the stdlib, but if `num` is referenced via
findlib it will add the package folder to the search path and find
either copy.
This patch has been merged upstream, see the discussion at
ocaml/num#40 for more details. This patch just
backports the change to the latest released `num` version as future
versions of `num` will come with this change. The next num release might
be some time off given `num` is mostly "done" therefore this patch is
meant to bridge the gap and a recommendation from @dra27 who is
maintaining `num` upstream.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the
Makefileslighty so that in the legacy mode the files will be installed into the package directory as well as into the stdlib.This allows dune to install
numoutside of the compiler and still use num like a regular findlib package (with thecmi,cmaetc in the package folder), while keeping it in the stdlib for packages that do not use findlib nor dune and just addnums.cmain their build.