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

pkg: Set OCAMLFIND_DESTDIR for install actions #10267

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

gridbugs
Copy link
Collaborator

@gridbugs gridbugs commented Mar 14, 2024

If a package uses ocamlfind to install its files then ocamlfind will consult its config file to determine where in the filesystem the installed files ought to go. In dune, each package is built in an isolated sandbox, and so by default ocamlfind won't install files to the correct location. The OCAMLFIND_DESTDIR environment variable can be used to override ocamlfind's default behaviour for determining where to install files.

This change sets the OCAMLFIND_DESTDIR variable to the "target/lib" directory within a package's build sandbox when executing a package's install command. Build commands are unaffected by this change.

Fixes #10252.

Necessary for fixing #8931, but we also need to merge ocaml/ocamlfind#72 (relocatable ocamlfind) to fix that issue.

In addition to the simple test included in this change, I've also manually tested this change by building zarith using a patched version of ocamlfind (patched with ocaml/ocamlfind#72).

@Leonidas-from-XIV
Copy link
Collaborator

I wonder whether it wouldn't make more sense to set OPAM_SWITCH_PREFIX instead of OCAMLFIND_DESTDIR? That way in theory more than just ocamlfind will know where to install to.

(The relocatable patch should probably make the findlib.conf file entirely optional, defaulting to $PREFIX/lib in case it is not found and if OPAM_SWITCH_PREFIX is set it should probably use that as $PREFIX)

@gridbugs
Copy link
Collaborator Author

I just noticed that when I use dune with this patch to build zarith it still installs its files to the wrong location. My little test library that installs with ocamlfind installs as expected but zarith does not. I'll do more testing with zarith next week.

@rgrinberg
Copy link
Member

Did you confirm that zarith uses ocamlfind install? I recall it had a weird makefile that worked without ocamlfind

@gridbugs
Copy link
Collaborator Author

The problem with zarith turns out to be that it explicitly passes -destdir to ocamlfind install and it gets the destdir by running ocamlfind printconf destdir during its build command (it's part of the configure script). We might be able to fix this by setting OCAMLFIND_DESTDIR during the build command as well as during the install command.

@gridbugs
Copy link
Collaborator Author

I've applied review feedback and fixed the failing test. @Leonidas-from-XIV regarding setting OPAM_SWITCH_PREFIX, I think we should set that in addition to OCAMLFIND_DESTDIR. There's an issue for this here: #10294

@rgrinberg
Copy link
Member

We already set OPAM_SWITCH_PREFIX. The issue is that it's not relocated and some packages do not respect it

@gridbugs gridbugs force-pushed the set-ocamlfind-destdir branch 2 times, most recently from 91c034e to 2f8c3a9 Compare March 26, 2024 00:51
@gridbugs
Copy link
Collaborator Author

We already set OPAM_SWITCH_PREFIX. The issue is that it's not relocated and some packages do not respect it

Makes sense.

I realized the test still had some stuff specific to my machine in it. That's now fixed. Does anything else need to be changed about this PR?

If a package uses ocamlfind to install its files then ocamlfind will
consult its config file to determine where in the filesystem the
installed files ought to go. In dune, each package is built in an
isolated sandbox, and so by default ocamlfind won't install files to the
correct location. The OCAMLFIND_DESTDIR environment variable can be used
to override ocamlfind's default behaviour for determining where to
install files.

This change sets the OCAMLFIND_DESTDIR variable to the "target/lib"
directory within a package's build sandbox when executing a package's
build and install commands.

Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
@gridbugs gridbugs merged commit 9265e2c into ocaml:main Mar 28, 2024
23 of 25 checks passed
@gridbugs gridbugs deleted the set-ocamlfind-destdir branch March 28, 2024 01:01
pmwhite pushed a commit to pmwhite/dune that referenced this pull request Apr 2, 2024
If a package uses ocamlfind to install its files then ocamlfind will
consult its config file to determine where in the filesystem the
installed files ought to go. In dune, each package is built in an
isolated sandbox, and so by default ocamlfind won't install files to the
correct location. The OCAMLFIND_DESTDIR environment variable can be used
to override ocamlfind's default behaviour for determining where to
install files.

This change sets the OCAMLFIND_DESTDIR variable to the "target/lib"
directory within a package's build sandbox when executing a package's
build and install commands.

Signed-off-by: Stephen Sherratt <stephen@sherra.tt>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pkg: support building packages with ocamlfind
3 participants