opam
opam copied to clipboard
Add `opam tree` subcommand
Closes #3775 .
This PR adds a subcommand opam tree
which displays a dependency tree as a Unicode art or ASCII art.
It can also display a reverse-dependency tree (through opam tree --rev-deps
option or opam why
alias), which can be useful to examine how dependency versions get constrained.
TODO
- [x] add
.mli
- [x] ask for feedbacks
- [x] complete doc and man
- [x] add tests
- [x] update
master_changes.md
file
..., it handles only installed packages. It needs to be checked/specified or extended, depending on what we need
All of cargo tree
, npm ls
, and yarn why
, which are mentioned in #3775 and inspired the current implementation, only take account of installed packages. So I guess it is ok to support only the installed packages.
We need to add handling of test/doc/build dependencies, ftm they are dropped
Yes 🚀
Should test/doc/build deps be distinguishable in the tree e.g.
foo 1.0.0
|--- bar 1.0.0 (>= 1.0 & with-test)
'--- ...
? It would be a bit complicated to do this since test and doc are processed in OpamSwitchState.universe
and the information seems to be lost.
Should this feature be at toplevel or a subcommand of opam list? In that case how can it be combined? Module itself is quite long also too.
Personally I'm against opam list --tree
: it would be confusing if opam list --tree
took a completely different set of options than in opam list
.
Also, man opam list
would become too noisy because something like (used with --tree)
/(can't be used with --tree)
would need to appear in every command option.
It seems I have to either get rid of List.fold_left_map
or implement it on OpamStd
.
Good for me (and more consistent about the command itself, its difference with list
) to keep only installed packages.
About dependencies filters, they can be retrieved unfiltered via st.opams
: OpamFile.OPAM.depends (OpamSwitchState.opam st pkg)
, and construct universe with those variables. I began a canevas for that. I've added doc/test/tools
, post
& dev
(copy paste from opam list
). Maybe we should also add build
, and i'm not sure about dev
if it is really needed?
I've also added a test in tests/reftests/tree.test
, you can find the test files syntax in tests/reftests/run.ml
top comment. You can run it with make reftest-tree
.
@rjbou I'm not very sure how to add a new subcommand to master_changes.md
.
## Global CLI
* ◈ Add `tree` subcommand to ...
or
## ◈ Tree
* Add `tree` subcommand to ...
or something else?
The first one: an entry for tree
, the other for why
. We'll need also need to add API changes (addition of modules / changes of signtures, etc. but better once the PR is no more wip to avoid updating it each time.
I think this is now ready for review 🎉
I accidentally edited the API changes in master_changes.md
too, so I'll edit it once more after all the future reviews are resolved.
It would be nice if we could also support JSON output for CI use.
I'm working on more inclusive Opam dependency support on GitHub, which requires that: https://github.com/ocaml/setup-ocaml/pull/555
@smorimoto I've just added a basic support of JSON output. It would look like this: example.txt
What do you think about it?
Could the JSON thing be its own PR instead? This is going to make the review process even more long and there would be more things to argue on.
@cannorin That looks good to me. @kit-ty-kate Um, is that really true? What we need to do is just review one small commit, and the rest is just the opam development team's intention. If the code and intent are ok, it won't take much time to get there.
@smorimoto this is not "one small commit", a json output would be used by programs outside of opam so the format is extremely important to get right as a change in the format would break ever users of it, and that requires extra time
Agreed - please can this PR not acquire new features. PRs against your own fork are a better way to start previewing the next stage while this part gets reviewed.
On second thought, it surely makes sense.
ok, I'll revert it.
@rjbou @kit-ty-kate I've updated master_changes.md
to make it ready to merge.
Thanks. @rjbou wants to review it first too before merging.
@rjbou Thank you so much! All of your changes look good to me, so please force-push it.
Requires https://github.com/ocaml/opam/pull/5269 to be merged first to be able to fix CI on Windows (once the CI issues have been fixed too)
We found the CI issue. We just need to merge https://github.com/ocaml/opam/pull/5281, rebase on master, and it should be good to go.
Now that CI itself is fixed, looks like there is a genuine issue with the tests of the --no-switch
option on Windows:
### opam tree j --no-switch
The following actions are simulated:
-=== install 1 package
+=== install 2 packages
+ - install a 1 [required by j]
- install j 1
j.1
+'-- a.1 (os = win32)
oupsy
updated
Thanks a lot!
Thanks all!
Thanks everyone 🚀 🚀 🚀