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.

Sign up for GitHub

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

Jump to bottom

ppxs are built in the target context in cross-compilation settings #4156

Open
TheLortex opened this issue Jan 25, 2021 · 3 comments · Fixed by #7415 or ocaml/opam-repository#23814
Open

ppxs are built in the target context in cross-compilation settings #4156

TheLortex opened this issue Jan 25, 2021 · 3 comments · Fixed by #7415 or ocaml/opam-repository#23814
Labels
acknowledged bug cross-compilation
Milestone
3.8.0

Comments

@TheLortex
Copy link
Collaborator

When using dune-workspace features to enable cross-compilation, ppx libraries should be built in the host context.

As dune tries to build them in the target context, it fails because some libraries such as ocaml-compiler-libs may not be available.

I added a failing test in this PR: #4155

It leads to the following error:

$ dune build --debug-dependency-path
File "lib/dune", line 3, characters 18-24:
3 |  (preprocess (pps fooppx)))
                      ^^^^^^
Error: Library "fooppx" in _build/cross-environment/ppx is hidden
(unsatisfied 'enabled_if').
-> required by lib/lib.pp.ml (context cross-environment)
-> required by alias lib/all (context cross-environment)
-> required by alias default (context cross-environment)
Hint: try:
  dune external-lib-deps --missing --debug-dependency-path @@default
[1]
@EduardoRFS
Copy link
Contributor

EduardoRFS commented Feb 7, 2021 •
edited
Loading

This script allows you to have a full ocaml env including compiler-libs.optcomp when cross compiling. Hackish, but it also generates a ocamlc.opt

https://github.com/EduardoRFS/reason-mobile/blob/master/patches/ocaml/files/make.cross.sh

@TheLortex
Copy link
Collaborator Author

Thanks @EduardoRFS, however the problem isn't about setting up a cross-compilation environment, but using it in projects that make use of ppx's.

I've tried to look into the issue, my findings are:

  • if libraries that are hidden in the cross-context are stubbed out, cross-compilation still works because dune build and uses ppx's from the host context. So the bug is that ppx's resolution is an over-approximation of what's actually needed (it requires dependencies to exist both in the host and target context, instead of only the host context).
  • In Preprocessing.ppx_driver_and_flags, library resolution happens in the target context, which is wrong. I have a simple patch that can fix that, by finding the correct scope from the host's super context.
  • In Lib.Resolve.pp_deps, preprocessors resolution also happen in the target context. Here I'm not sure how to fix that, because there's currently no way from a given Lib.DB.t to explore the host's DB.

@rgrinberg rgrinberg added this to the 3.8.0 milestone Mar 27, 2023
@rgrinberg rgrinberg linked a pull request Mar 27, 2023 that will close this issue
fix(x-compilation): find host ppx dependencies in the host context #7415
Merged
@emillon emillon mentioned this issue Apr 18, 2023
Closed
emillon added a commit to emillon/opam-repository that referenced this issue Apr 18, 2023
@emillon
[new release] dune (15 packages) (3.8.0~alpha1)
8f04d31 
CHANGES:

- When a rule's action is interrupted, delete any leftover directory targets.
  This is consistent with how we treat file targets. (@rgrinberg, 7564)

- Fix plugin loading with findlib. The functionality was broken in 3.7.0.
  (ocaml/dune#7556, @anmonteiro)

- Introduce a `public_headers` field on libraries. This field is like
  `install_c_headers`, but it allows to choose the extension and choose the
  paths for the installed headers. (ocaml/dune#7512, @rgrinberg)

- Load the host context `findlib.conf` when cross-compiling (ocaml/dune#7428, fixes
  ocaml/dune#1701, @rgrinberg, @anmonteiro)

- Resolve `ppx_runtime_libraries` in the target context when cross compiling
  (ocaml/dune#7450, fixes ocaml/dune#2794, @anmonteiro)

- Use `$PKG_CONFIG`, when set, to find the `pkg-config` binary  (ocaml/dune#7469, fixes
  ocaml/dune#2572, @anmonteiro)

- Preliminary support for Coq compiled intefaces (`.vos` files) enabled via
  `(mode vos)` in `coq.theory` stanzas. This can be used in combination with
  `dune coq top` to obtain fast re-building of dependencies (with no checking
  of proofs) prior to stepping into a file. (ocaml/dune#7406, @rlepigre)

- Fix dune crashing on MacOS in watch mode whenever `$PATH` contains `$PWD`
  (ocaml/dune#7441, fixes ocaml/dune#6907, @rgrinberg)

- Fix `dune install` when cross compiling (ocaml/dune#7410, fixes ocaml/dune#6191, @anmonteiro,
  @rizo)

- Find `pps` dependencies in the host context when cross-compiling,  (ocaml/dune#7410,
  fixes ocaml/dune#4156, @anmonteiro)

- Dune in watch mode no longer builds concurrent rules in serial (ocaml/dune#7395
  @rgrinberg, @jchavarri)

- `dune coq top` now correctly respects the project root when called from a
  subdirectory. However, absolute filenames passed to `dune coq top` are no
  longer supported (due to being buggy) (ocaml/dune#7357, fixes ocaml/dune#7344, @rlepigre and
  @Alizter)

- Added a `--no-build` option to `dune coq top` for avoiding rebuilds (ocaml/dune#7380,
  fixes ocaml/dune#7355, @Alizter)

- RPC: Ignore SIGPIPE when clients suddenly disconnect (ocaml/dune#7299, ocaml/dune#7319, fixes
  ocaml/dune#6879, @rgrinberg)

- Always clean up the UI on exit. (ocaml/dune#7271, fixes ocaml/dune#7142 @rgrinberg)

- Bootstrap: remove reliance on shell. Previously, we'd use the shell to get
  the number of processors. (ocaml/dune#7274, @rgrinberg)

- Bootstrap: correctly detect the number of processors by allowing `nproc` to be
  looked up in `$PATH` (ocaml/dune#7272, @Alizter)

- Speed up file copying on macos by using `clonefile` when available
  (@rgrinberg, ocaml/dune#7210)

- Adds support for loading plugins in toplevels (ocaml/dune#6082, fixes ocaml/dune#6081,
  @ivg, @richardlford)

- Support commands that output 8-bit and 24-bit colors in the terminal (ocaml/dune#7188,
  @Alizter)

- Speed up rule generation for libraries and executables with many modules
  (ocaml/dune#7187, @jchavarri)

- Add `--watch-exclusions` to Dune build options (ocaml/dune#7216, @jonahbeckford)

- Do not re-render UI on every frame if the UI doesn't change (ocaml/dune#7186, fix
  ocaml/dune#7184, @rgrinberg)

- Make coq_db creation in scope lazy (@ejgallego, ocaml/dune#7133)

- Non-user proccesses such as version control or config checking are now run
  silently. (ocaml/dune#6994, fixes ocaml/dune#4066, @Alizter)

- Add the `--display-separate-messages` flag to separate the error messages
  produced by commands with a blank line. (ocaml/dune#6823, fixes ocaml/dune#6158, @esope)

- Accept the Ordered Set Language for the `modes` field in `library` stanzas
  (ocaml/dune#6611, @anmonteiro).

- dune install now respects --display quiet mode (ocaml/dune#7116, fixes ocaml/dune#4573, fixes
  ocaml/dune#7106, @Alizter)

- Stub shared libraries (dllXXX_stubs.so) in Dune-installed libraries could not
  be used as dependencies of libraries in the workspace (eg when compiling to
  bytecode and/or Javascript).  This is now fixed. (ocaml/dune#7151, @nojb)

- Allow the main module of a library with `(stdlib ...)` to depend on other
  libraries (ocaml/dune#7154, @anmonteiro).

- Bytecode executables built for JSOO are linked with `-noautolink` and no
  longer depend on the shared stubs of their dependent libraries (ocaml/dune#7156, @nojb)

- Added a new user action `(concurrent )` which is like `(progn )` but runs the
  actions concurrently. (ocaml/dune#6933, @Alizter)

- Allow `(stdlib ...)` to be used with `(wrapped false)` in library stanzas
  (ocaml/dune#7139, @anmonteiro).

- Allow parallel execution of inline tests partitions (ocaml/dune#7012, @hhugo)

- Support `(link_flags ...)` in `(cinaps ...)` stanza. (ocaml/dune#7423, fixes ocaml/dune#7416,
  @nojb)

- Allow `(package ...)` in any position within `(rule ...)` stanza (ocaml/dune#7445,
  @Leonidas-from-XIV)

- Always include `opam` files in the generated `.install` file. Previously, it
  would not be included whenever `(generate_opam_files true)` was set and the
  `.install` file wasn't yet generated. (ocaml/dune#7547, @rgrinberg)
@emillon emillon mentioned this issue May 23, 2023
Merged
@emillon
Copy link
Collaborator

emillon commented Jun 5, 2023

Reopening now that this has been reverted in #7887.

@emillon emillon reopened this Jun 5, 2023
@emillon emillon reopened this Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
acknowledged bug cross-compilation
Projects
None yet
Milestone
3.8.0
5 participants
@rgrinberg @emillon @anmonteiro @TheLortex @EduardoRFS