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

dune describe external-lib-deps: printing out more information #6839

Merged
merged 3 commits into from
Feb 6, 2023

Conversation

moyodiallo
Copy link
Collaborator

Printing out more information, the package in which an external library belongs to, the libraries, executables and tests at this point with their respective external_lib_deps.

It was possible with the command dune external-lib-deps which was removed since dune.3.0, to use @install @runtest aliasis. To get it back, we ended up to not be able to use those (dune describe external-lib-deps), this is why knowing the package of an external library could help.

The goal is to have much information for ocurrent/opam-dune-lint#46.

moyodiallo added a commit to moyodiallo/opam-dune-lint that referenced this pull request Jan 12, 2023
In this stage, opam-dune-lint can only works with this dune PR
"ocaml/dune#6839".

Not ready yet, works is needed at this stage.
@moyodiallo moyodiallo force-pushed the describe-external-libs branch from 020f6fa to bac7454 Compare January 12, 2023 15:35
@rgrinberg rgrinberg requested review from esope and emillon January 27, 2023 02:49
@emillon emillon modified the milestone: 3.7.0 Jan 31, 2023
@emillon
Copy link
Collaborator

emillon commented Jan 31, 2023

@moyodiallo 3.7 is due soon - do you want it to include these changes? if so can you rebase, mark this as ready and fix the tests? thanks!
(otherwise postponing to 3.8 is fine too)

@moyodiallo
Copy link
Collaborator Author

I wanted to add the information about the package of an executable that can be installed by the rule (install ...). Like in this description : ocurrent/opam-dune-lint#46 (comment).

I need some suggestion/help how to do that properly, because I want to avoid an extra analysis of dune files.

@emillon

@emillon
Copy link
Collaborator

emillon commented Jan 31, 2023

OK, I see. Thanks for the pointer.

First, about the scope of this PR - can we first try to land what is already working, and then extend the behavior to separate (install) in a subsequent PR (that might not make it in 3.7)?

As for the extension to (install): it's not straightforward to do that in the general case. Maybe the following can work:

  • extend dune describe workspace to also include info about (install) stanza. Concretely that would be pairs of (source, destination) paths
  • in opam-dune-lint, you'd be able to cross reference the sources of installed files with the infos about executables in dune describe workspace - these have some info about their dependencies
    Would that be enough for your use case?

@moyodiallo moyodiallo marked this pull request as ready for review January 31, 2023 16:56
@moyodiallo moyodiallo force-pushed the describe-external-libs branch from bac7454 to 8986036 Compare January 31, 2023 16:57
@moyodiallo
Copy link
Collaborator Author

First, about the scope of this PR - can we first try to land what is already working, and then extend the behavior to
separate (install) in a subsequent PR (that might not make it in 3.7)?

Yes sound good.

As for the extension to (install): it's not straightforward to do that in the general case. Maybe the following can work:

  • extend dune describe workspace to also include info about (install) stanza. Concretely that would be pairs of (source, destination) paths

Thanks, good proposition, if we can have that without using dune describe workspace because we don't want dune to fail resolving library dependencies. A new command like dune describe installs would be nice.

@emillon emillon added this to the 3.7.0 milestone Jan 31, 2023
bin/describe.ml Outdated Show resolved Hide resolved
bin/describe.ml Outdated Show resolved Hide resolved
The command print out more information, the package in which an external
library belongs to, the libraries, executables and tests at this point
with their respective external_lib_deps.

It was possible with the command `dune external-lib-deps` which was
removed, to use `@install` `@runtest` aliasis. And we ended up with the
new command to not be able to use those, this is why knowing the package
of an external library could help.

The goal is to have much information for `opam-dune-lint`.

Signed-off-by: Alpha DIALLO <moyodiallo@gmail.com>
Signed-off-by: Alpha DIALLO <moyodiallo@gmail.com>
@moyodiallo moyodiallo force-pushed the describe-external-libs branch from f1ac2c5 to d13411b Compare February 2, 2023 15:54
@moyodiallo moyodiallo requested review from emillon and removed request for esope February 3, 2023 10:26
@emillon emillon merged commit 14daaf6 into ocaml:main Feb 6, 2023
@emillon
Copy link
Collaborator

emillon commented Feb 6, 2023

Thanks!

@esope
Copy link
Collaborator

esope commented Feb 10, 2023

Apologies for not reviewing this PR. I was not able to find enough time.

moyodiallo added a commit to moyodiallo/opam-dune-lint that referenced this pull request Sep 22, 2023
In this stage, opam-dune-lint can only works with this dune PR
"ocaml/dune#6839".

Not ready yet, works is needed at this stage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants