ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.63k stars 401 forks source link

"dune top -p <pkg>" tries to pull the dependencies of unrelated vendored executables #7586

Open kit-ty-kate opened 1 year ago

kit-ty-kate commented 1 year ago

Expected Behavior

For dune top -p <pkg> to not pull unrelated dependencies

Actual Behavior

dune top -p <pkg> pulls unrelated dependencies

Reproduction

$ cat dune-project
(lang dune 2.0)
(package (name a))
$ cat dune
(vendored_dirs vendored)
$ cat a/dune
(library
 (public_name a))
$ cat vendored/dune-project
(lang dune 2.0)
$ cat vendored/dune
(executable
 (name test)
 (libraries base))

If base is not installed on your system, dune top -p a will return the expected:

#directory "/tmp/test/_build/default/a/.a.objs/byte";;
#load "/tmp/test/_build/default/a/a.cma";;

However if base is installed, the same command will return:

#directory "/home/kit_ty_kate/.opam/default/lib/base";;
#directory "/home/kit_ty_kate/.opam/default/lib/base/base_internalhash_types";;
#directory "/home/kit_ty_kate/.opam/default/lib/base/caml";;
#directory "/home/kit_ty_kate/.opam/default/lib/base/shadow_stdlib";;
#directory "/home/kit_ty_kate/.opam/default/lib/sexplib0";;
#directory "/tmp/test/_build/default/a/.a.objs/byte";;
#load "/tmp/test/_build/default/a/a.cma";;
#load "/home/kit_ty_kate/.opam/default/lib/base/base_internalhash_types/base_internalhash_types.cma";;
#load "/home/kit_ty_kate/.opam/default/lib/base/caml/caml.cma";;
#load "/home/kit_ty_kate/.opam/default/lib/sexplib0/sexplib0.cma";;
#load "/home/kit_ty_kate/.opam/default/lib/base/shadow_stdlib/shadow_stdlib.cma";;
#load "/home/kit_ty_kate/.opam/default/lib/base/base.cma";;

Specifications

anmonteiro commented 1 year ago

I think this is the same as #4814 and https://github.com/ocaml/dune/pull/7419, i.e. -p isn't actually taken into account for these commands.

anmonteiro commented 1 year ago

Actually I think this is a bit more complicated than that: I think this is actually an instance of https://github.com/ocaml/dune/issues/6830, where executables in vendored dirs are built because they might need to be executed.

kit-ty-kate commented 1 year ago

That sounds about right. dune top -p has an effect and works as expected, except when it is in presence of a vendored executable.

where executables in vendored dirs are built because they might need to be executed.

In this case though the executable is not built. Here is the build log after dune clean && dune top -p a with the test case above:

$ (cd _build/default && /home/kit_ty_kate/.opam/default/bin/ocamlc.opt -w -40 -w -49 -nopervasives -nostdlib -g -bin-annot -I a/.a.objs/byte -no-alias-deps -o a/.a.objs/byte/a.cmo -c -impl a/a.ml-gen)
$ (cd _build/default && /home/kit_ty_kate/.opam/default/bin/ocamlc.opt -w -40 -g -a -o a/a.cma a/.a.objs/byte/a.cmo)
rgrinberg commented 1 year ago

However if base is installed, the same command will return:

I think this is because of:

                match libs with
                | Error () -> (acc, pps)
                | Ok libs ->
                  List.fold_left libs ~init:(acc, pps) ~f:(fun (acc, pps) lib ->

In other words, if we are unable to find or load a dependency for whatever reason, we keep going.