dune icon indicating copy to clipboard operation
dune copied to clipboard

pkg: `ocamlbuild -where` returns a sandbox path when ocamlbuild is built by dune

Open gridbugs opened this issue 1 year ago • 5 comments

E.g.

$ _build/_private/default/.pkg/ocamlbuild/target/bin/ocamlbuild -where
/home/s/tmp/bonsai/_build/.sandbox/7158a9da761d39477e414a6f2dd9d3fc/_private/default/.pkg/ocamlbuild/target/lib/ocamlbuild

This creates problems when packages use ocamlbuild -where to find the file ocamlbuild.cmo which causes these build failures when trying to compile ocaml.org:

Warning: Won't be able to compile a native plugin
Failure: Cannot find "ocamlbuild.cmo" in ocamlbuild -where directory.
pkg.ml: [ERROR] cmd ['ocamlbuild' '-use-ocamlfind' '-classic-display' '-j' '4' '-tag' 'debug'
     '-build-dir' '_build' 'opam' 'pkg/META' 'CHANGES.md' 'LICENSE.md'
     'README.md' 'src/ptime.a' 'src/ptime.cmxs' 'src/ptime.cmxa'
     'src/ptime.cma' 'src/ptime.cmx' 'src/ptime.cmi' 'src/ptime.mli'
     'src/ptime_top.a' 'src/ptime_top.cmxs' 'src/ptime_top.cmxa'
     'src/ptime_top.cma' 'src/ptime_top.cmx' 'src/ptime_top_init.ml'
     'src-clock/ptime_clock.a' 'src-clock/ptime_clock.cmxs'
     'src-clock/ptime_clock.cmxa' 'src-clock/ptime_clock.cma'
     'src-clock/ptime_clock.cmx' 'src-clock/ptime_clock.cmi'
     'src-clock/ptime_clock.mli' 'src-clock/dllptime_clock_stubs.so'
     'src-clock/libptime_clock_stubs.a' 'src-clock/runtime.js'
     'doc/index.mld' 'test/min_clock.ml']: exited with 2
-> required by _build/_private/default/.pkg/ptime/target/cookie
-> required by - package crunch
Warning: Won't be able to compile a native plugin
Failure: Cannot find "ocamlbuild.cmo" in ocamlbuild -where directory.
pkg.ml: [ERROR] cmd ['ocamlbuild' '-use-ocamlfind' '-classic-display' '-j' '4' '-tag' 'debug'
     '-build-dir' '_build' 'opam' 'pkg/META' 'CHANGES.md' 'LICENSE.md'
     'README.md' 'src/mtime.a' 'src/mtime.cmxs' 'src/mtime.cmxa'
     'src/mtime.cma' 'src/mtime.cmx' 'src/mtime.cmi' 'src/mtime.mli'
     'src/mtime_top.a' 'src/mtime_top.cmxs' 'src/mtime_top.cmxa'
     'src/mtime_top.cma' 'src/mtime_top.cmx' 'src/mtime_top_init.ml'
     'src-clock/mtime_clock.a' 'src-clock/mtime_clock.cmxs'
     'src-clock/mtime_clock.cmxa' 'src-clock/mtime_clock.cma'
     'src-clock/mtime_clock.cmx' 'src-clock/mtime_clock.cmi'
     'src-clock/mtime_clock.mli' 'src-clock/dllmtime_clock_stubs.so'
     'src-clock/libmtime_clock_stubs.a' 'src-clock/runtime.js'
     'doc/index.mld' 'test/min_clock.ml']: exited with 2
-> required by _build/_private/default/.pkg/mtime/target/cookie
-> required by - package mirage-crypto-rng-lwt

A temporary hack to work around this problem is to use a regexp to remove the sandbox from paths in the generated file ocamlbuild_config.ml before compilation: https://github.com/gridbugs/ocamlbuild/commit/39c27f04d7640b437ebe9a733c6ebc193d3233ed

gridbugs avatar Mar 20 '24 06:03 gridbugs

This feels like a similar issue to https://github.com/ocaml/dune/issues/10291 in that it involves a generated file containing paths to files within the build sandbox.

gridbugs avatar Mar 20 '24 06:03 gridbugs

That's right. We should forbid tools from recording any absolute paths. Or at least allow us a way to override them with an environment variable.

rgrinberg avatar Mar 20 '24 14:03 rgrinberg

I can't think of a way to fix this that doesn't involve modifying ocamlbuild. Any package that persists an opam variable containing a path during its build or install step will be broken in dune's sandbox as all such paths are translated into their corresponding sandboxed path. Fortunately this only seems to affect the ocamlbuild package.

My plan is to modify ocamlbuild so that it only persists paths relative toOPAM_SWITCH_PREFIX rather than absolute paths.

gridbugs avatar Mar 26 '24 07:03 gridbugs

Modifying ocamlbuild sounds like the right way to go. It will allow us to sandbox the build to guarantee its correctness, and speed up subsequent build with caching.

My plan is to modify ocamlbuild so that it only persists paths relative to OPAM_SWITCH_PREFIX rather than absolute paths.

My advice is to follow the same approach you are following for ocamlfind.It would be ideal if there's a small library or module one could vendor to allow relocatability in a consistent way.

rgrinberg avatar Mar 26 '24 09:03 rgrinberg

Digging deeper, I'm hesitant to modify ocamlbuild to solve this problem. Ocamlbuild already has compile-time configuration for relocating its installation to a specific destination known at compile-time. The build commands in its opam package are:

build: [
  [
    make
    "-f"
    "configure.make"
    "all"
    "OCAMLBUILD_PREFIX=%{prefix}%"
    "OCAMLBUILD_BINDIR=%{bin}%"
    "OCAMLBUILD_LIBDIR=%{lib}%"
    "OCAMLBUILD_MANDIR=%{man}%"
    "OCAML_NATIVE=%{ocaml:native}%"
    "OCAML_NATIVE_TOOLS=%{ocaml:native}%"
  ]
  [make "check-if-preinstalled" "all" "opam-install"]
]

This was written under the assumption that prefix, bin, lib, etc are paths within the destination directory structure where ocamlbuild will be installed, which is a reasonable assumption as that is how opam behaves. The only reason this is a problem for dune is it sets those variables to be within the build sandbox. I'm starting to think that we should rethink the decision to translate variables into the sandbox as it deviates from opam's behaviour and will likely lead to more packages that work in opam but not in dune. When a new package is published on opam, it's unreasonable for us to expect that it be tested in dune's build sandbox.

In the short term I propose using an package overlay for ocamlbuild that preprocesses the generated ocamlbuild_config.ml file to translate paths out of the sandbox. Short-term we already need to use a modified version of the ocamlbuild package because it contains directory symlinks which dune cannot handle due to a recently-introduced bug (https://github.com/ocaml/dune/issues/9873).

Long term I want to explore alternative ways to effect sandboxing that is consistent with the way that opam builds packages.

See also: https://github.com/ocaml/dune/issues/10316

gridbugs avatar Mar 28 '24 01:03 gridbugs