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

META.pkg is illformed #5767

Open
gares opened this issue May 24, 2022 · 13 comments
Open

META.pkg is illformed #5767

gares opened this issue May 24, 2022 · 13 comments
Labels

Comments

@gares
Copy link
Contributor

gares commented May 24, 2022

Expected Behavior

The file is placed at the toot of the build tree and findlib can parse it

Actual Behavior

Findlib cannot parse it since it lacks the directory stanza. The file is only accepted when symlinked in _build/install/...lib/pkg/META since there it does not need the directory stanza.

My suggestion is to create two meta files, one good for installation, and another one good for using uninstalled. The former shall be "hidden".

@rgrinberg
Copy link
Member

My suggestion is to create two meta files, one good for installation, and another one good for using uninstalled. The former shall be "hidden".

What prevents you from wanting to use the installed META? Is it because you want to load a private library with findlib?

@ejgallego
Copy link
Collaborator

@rgrinberg it is because we've been told that depending on things in _install is not good practice.

However I'm not sure that advice is correct.

@rgrinberg
Copy link
Member

You can depend on things being in _build/install if you depend on (package ..) or the @install alias.

@ejgallego
Copy link
Collaborator

I mean, why the rule for pkg.install can depend on things there but our Coq rules cannot?

@rgrinberg
Copy link
Member

Although we never intended this to be user facing, you most certainly can. And we are not planning to shift the location of this directory around just to spite our users :)

But let me try to convince to why you should think twice before doing it. When we load the rules for _build/install, recall that we need to scan the entire workspace to determine all files that might be installable (install stanzas, public libraries, executables, etc). Doing this work requires us to load certain user rules. While we try to load as little rules as possible, it's not something that we ever guarantee to be stable between versions of dune. This means that you might get cycles in future versions of dune if we decide to force the rules in the directory that's using %{workspace_root}../install/%{context_name}. If that risk is acceptable to you, feel free and depend on things inside _install/%{context_name}

@ejgallego
Copy link
Collaborator

ejgallego commented May 24, 2022

@rgrinberg I'm fine not depending on _install, as long as we agree on a general method to provide runtime deps.

For example, I'd be fine emulating (env (bin ...)) and thus creating a _build/$context/.lib and extending OCAMLPATH properly, what do you think?

That would also be the most useful thing to do I think as would provide convenient support for other people using OCaml plugins.

Maybe I'm just not respecting some API boundaries, but for me, morally having (deps (package foo)) is the same than writing a rule which directly depends on something in _install/$context

@ejgallego
Copy link
Collaborator

We could implement this feature as a new (runtime-libraries ...) field (which could just set a boolean), or we could unconditionally make them available, I don't know.

This provides an alternative to some functionality present in the dune sites plugin, but seems much lighter to me.

@ejgallego ejgallego added the requires-team-discussion This topic requires a team discussion label May 25, 2022
@rgrinberg
Copy link
Member

Sure, I have nothing against in generating rules that will help you with runtime deps. I'd like to hear more about your use case however. How are you using the runtime deps? Are you building something with them? Are you dynlinking?

For example, I'd be fine emulating (env (bin ...)) and thus creating a _build/$context/.lib and extending OCAMLPATH properly, what do you think?

I would like something simpler than this. There are also some related use cases in dune that we might be able to unify.

We could implement this feature as a new (runtime-libraries ...) field (which could just set a boolean), or we could unconditionally make them available, I don't know.

Could you describe this a bit more?

@gares
Copy link
Contributor Author

gares commented May 26, 2022

Coq projects are sometimes made of both .v files .ml files. The latter constitutes a plugin coqc must load in order to compile the .v files.
Recently coq switched from a home baked, broken, loading mechanism to findlib. This, for example, makes it possible for plugins to use ocaml libraries.

Now, dune builds a META file and a cmxs but that seems only to be properly after the fact. During the build process the META file is illformed hence unusable.

Emilio proposes to make the file usable while building the package.

Splitting each Coq projects with a plugin into two packages (an ocaml library and a pure .v one) and use a package dependency seems to the Coq devs a bit too much to ask to our users. There are a few PR out there doing that, and they are too large to be considered reasonable, IMO.

Hope it helps.

@ejgallego
Copy link
Collaborator

Are you dynlinking?

Yes, we are dynlinking, and we use Fl_dynload because we need to be able to pull deps when dynlinking, and also we need to support quite a bit of 3rd party systems that still don't use Dune, so we rely on the dep info on META.

We could make things work by depending on the (installed) cmxs and the META file, but as you mentioned, it'd be better to have more constrained rules similarly to what is done for _build/$context/.bin

@ejgallego ejgallego removed the requires-team-discussion This topic requires a team discussion label Jun 1, 2022
@ejgallego
Copy link
Collaborator

This was discussed and a few points where made:

  • there are two ways to make Fl_dynload work for build (run ...) actions:
    • depend on the required files on _build/install, as Dune already extends OCAMLPATH to point there
    • extend OCAMLPATH to point to a _build/%{context}/.lib directory, and add rules for META and the .cmxs.
  • first option is easy, but loads more rules; second option would allow to make private plugins available as (env (bin ...)) does with binaries
  • there is a question of how this overlaps with Dune sites? Dune sites is morally option 1, but with less granularity than what we want here. I dont' know what more special stuff does the Dune sites setup do.

I guess for now the easiest thing to do , but as Rudi points out, we need to be beware of cycles, user messages may not be the best.

@ejgallego ejgallego added the coq label Oct 1, 2022
emillon added a commit to emillon/opam-repository that referenced this issue Oct 19, 2022
…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.5.0)

CHANGES:

- macOS: Handle unknown fsevents without crashing (ocaml/dune#6217, @rgrinberg)

- Enable file watching on MacOS SDK < 10.13. (ocaml/dune#6218, @rgrinberg)

- Sandbox running cinaps actions starting from cinaps 1.1 (ocaml/dune#6176, @rgrinberg)

- Add a `runtime_deps` field in the `cinaps` stanza to specify runtime
  dependencies for running the cinaps preprocessing action (ocaml/dune#6175, @rgrinberg)

- Shadow alias module `Foo__` when building a library `Foo` (ocaml/dune#6126, @rgrinberg)

- Extend dune describe to include the root path of the workspace and the
  relative path to the build directory. (ocaml/dune#6136, @reubenrowe)

- Allow dune describe workspace to accept directories as arguments.
  The provided directories restrict the worskpace description to those
  directories. (ocaml/dune#6107, fixes ocaml/dune#3893, @esope)

- Add a terminal persistence mode that attempts to clear the terminal history.
  It is enabled by setting terminal persistence to
  `clear-on-rebuild-and-flush-history` (ocaml/dune#6065, @rgrinberg)

- Disallow generating targets in sub direcories in inferred rules. The check to
  forbid this was accidentally done only for manually specified targets (ocaml/dune#6031,
  @rgrinberg)

- Do not ignore rules marked `(promote (until-clean))` when
  `--ignore-promoted-rules` (or `-p`) is passed. (ocaml/dune#6010, fixes ocaml/dune#4401, @emillon)

- Dune no longer considers .aux files as targets during Coq compilation. This
  means that .aux files are no longer cached. (ocaml/dune#6024, fixes ocaml/dune#6004, @Alizter)

- Cinaps actions are now sandboxed by default (ocaml/dune#6062, @rgrinberg)

- Allow rules producing directory targets to be not sandboxed (ocaml/dune#6056,
  @rgrinberg)

- Introduce a `dirs` field in the `install` stanza to install entire
  directories (ocaml/dune#5097, fixes ocaml/dune#5059, @rgrinberg)

- Menhir rules are now sandboxed by default (ocaml/dune#6076, @rgrinberg)

- Allow rules producing directory targets to create symlinks (ocaml/dune#6077, fixes
  ocaml/dune#5945, @rgrinberg)

- Inline tests are now sandboxed by default (ocaml/dune#6079, @rgrinberg)

- Fix build-info version when used with flambda (ocaml/dune#6089, fixes ocaml/dune#6075, @jberdine)

- Add an `(include <file>)` term to the `include_dirs` field for adding
  directories to the include paths sourced from a file. (ocaml/dune#6058, fixes ocaml/dune#3993,
  @gridbugs)

- Support `(extra_objects ...)` field in `(executable ...)` and `(library
  ...)` stanzas (ocaml/dune#6084, fixes ocaml/dune#4129, @gridbugs)

- Fix compilation of Dune under esy on Windows (ocaml/dune#6109, fixes ocaml/dune#6098, @nojb)

- Improve error message when parsing several licenses in `(license)` (ocaml/dune#6114,
  fixes ocaml/dune#6103, @emillon)

- odoc rules now about `ODOC_SYNTAX` and will rerun accordingly (ocaml/dune#6010, fixes
  ocaml/dune#1117, @emillon)

- dune install: copy files in an atomic way (ocaml/dune#6150, @emillon)

- Add `%{coq:...}` macro for accessing data about the configuration about Coq.
  For instance `%{coq:version}` (ocaml/dune#6049, @Alizter)

- update vendored copy of cmdliner to 1.1.1. This improves the built-in
  documentation for command groups such as `dune ocaml`. (ocaml/dune#6038, @emillon,
  ocaml/dune#6169, @shonfeder)

- The test suite for Coq now requires Coq >= 8.16 due to changes in the
  plugin loading mechanism upstream (which now uses `Findlib`).

- Starting with Coq build language 0.6, theories can be built without importing
  Coq's standard library by including `(stdlib no)`.
  (ocaml/dune#6165 ocaml/dune#6164, fixes ocaml/dune#6163, @ejgallego @Alizter @LasseBlaauwbroek)

- on macOS, sign executables produced by artifact substitution (ocaml/dune#6137, ocaml/dune#6231,
  fixes ocaml/dune#5650, fixes ocaml/dune#6226, @emillon)

- Added an (aliases ...) field to the (rules ...) stanza which allows the
  specification of multiple aliases per rule (ocaml/dune#6194, @Alizter)

- The `(coq.theory ...)` stanza will now ensure that for each declared `(plugin
 ...)`, the `META` file for it is built before calling `coqdep`. This enables
 the use of the new `Findlib`-based loading method in Coq 8.16; however as of
 Coq 8.16.0, Coq itself has some bugs preventing this to work yet. (ocaml/dune#6167 ,
 workarounds ocaml/dune#5767, @ejgallego)

- Allow include statement in install stanza (ocaml/dune#6139, fixes ocaml/dune#256, @gridbugs)

- Handle CSI n K code in ANSI escape codes from commands. (ocaml/dune#6214, fixes ocaml/dune#5528,
  @emillon)

- Add a new experimental feature `mode_specific_stubs` that allows the
  specification of different flags and sources for foreign stubs depending on
  the build mode (ocaml/dune#5649, @voodoos)
@Alizter Alizter moved this to Todo in Coq + Dune Nov 1, 2022
@paurkedal
Copy link

paurkedal commented Feb 9, 2024

I have a similar need when running the testsuite for Caqti, which also uses Fl_dynload. I had a look at dune-site, but as far as I can tell it has the same limitation, that the test suite will need to depend on the full @install target. Would it be feasible to have more precise dependencies, like having individual @pkg/install aliases which could be referred from deps stanzas, or even more precisely (deps ...(plugin <dir>/<plugin-name>)...) for the dune-site.plugins-based solution? [Update: @pkg/install is available as the already mentioned (package pkg) dependency specification.]

(My current solution has been to rewrite the META files, prepending a directory directive. With sublibraries that becomes more tricky, unless the libraries are rearranged in the source code so that the META.pkg can be copied into pkg/META.)

@ejgallego
Copy link
Collaborator

In src/coq/coq_rules.ml we just depend on the META install location, that works, and IMHO should be better supported in the stanzas.

When we generate dune files, we use a very ugly hack which is to emit (deps ../../../install/default/lib/coq-core/foo/META) etc....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

No branches or pull requests

4 participants