xapi-project / xs-opam

Opam repository for OCaml libraries to build Citrix Hypervisor toolstack components
https://xapi-project.github.io/
MIT License
15 stars 24 forks source link

Make sure we're passing the right number of jobs to dune everywhere #162

Closed gaborigloi closed 1 month ago

gaborigloi commented 6 years ago

As the dune manuals says here: https://dune.readthedocs.io/en/latest/usage.html#invocation-from-opam , we should pass the number of jobs from OPAM to dune, both for the build and test instructions:

build: [["dune" "build" "-p" name "-j" jobs]]
build-test: [["dune" "runtest" "-p" name "-j" jobs]]

We don't do this for all of our packages, so we should update our opam files where necessary. This will ensure that dune will use the concurrency option provided by opam, and that we can centrally control the number of jobs when building xs-opam by passing the -j parameter to OPAM - dune uses all the cores by default.

psafont commented 5 years ago

in packages/xs:

$ rg "\[.*(\"dune\"(?!.*\"-j\"| \{.*\})).*\]" --pcre2
fd-send-recv.2.0.1/opam
20:  ["dune" "build" "@doc" "-p" name] {with-doc}

crc.2.1.0/opam
9:build: [[ "dune" "build" "-p" name ]]

xapi-rrd.1.6.0/opam
12:  ["dune" "runtest" "-p" name] {with-test}

in xs-extra:

xen-api-sdk.master/opam
14:build: [[ "dune" "build" "-p" name ]]

xapi-types.master/opam
7:build: [[ "dune" "build" "-p" name ]]

xapi-forkexecd.master/opam
11:  ["dune" "runtest" "-p" name] {with-test}

xe.master/opam
7:build: [[ "dune" "build" "-p" name ]]

xapi-database.master/opam
7:build: [[ "dune" "build" "-p" name ]]

xen-api-client-lwt.master/opam
14:build: [[ "dune" "build" "-p" name ]]

xapi-nbd.master/opam
9:  ["dune" "runtest"] {with-test}

xapi-client.master/opam
7:build: [[ "dune" "build" "-p" name ]]

xen-api-client-async.master/opam
14:build: [[ "dune" "build" "-p" name ]]

xapi-consts.master/opam
7:build: [[ "dune" "build" "-p" name ]]

xen-api-client.master/opam
16:  ["dune" "build" "-p" name]

xapi-cli-protocol.master/opam
7:build: [[ "dune" "build" "-p" name ]]

xapi.master/opam
8:  ["dune" "build" "-p" name]

rrd-transport.master/opam
9:  ["dune" "runtest" "-p" name] {with-test}

xapi-datamodel.master/opam
7:build: [[ "dune" "build" "-p" name ]]
lindig commented 5 years ago

The dune documentation still recommends providing -j but I strongly believe that dune uses an optimal number of processes by itself without -j. Hence, I believe this is not critical at all.

build: [
  ["dune" "subst"] {pinned}
  ["dune" "build" "-p" name "-j" jobs]
]
psafont commented 5 years ago

I strongly believe that dune uses an optimal number of processes by itself

That's correct, not sure why the docs recommend it: https://github.com/ocaml/dune/pull/726

lindig commented 5 years ago

I now think that if you restrict the number of processes for Opam, this information needs to be passed on to Dune. It does not matter if you want to maximise concurrency.

gaborigloi commented 5 years ago

Yes I think the point of this is that you can control the number of processes from opam when installing multiple things in parallel.

lindig commented 5 years ago

Discussion here: https://discuss.ocaml.org/t/opam-and-dune-j-parameter/4299 The interaction of -j at the Opam and Dune level does not look well defined.

psafont commented 1 year ago

There are no packages remaining in xs-extra, for xs the same packages are present

psafont commented 1 month ago

This is a mostly-solved problem now that we're autogenerating most opam metadata with dune, I could only find two instances of this, in small libraries:

pci.v1.0.4/opam
21:  ["dune" "runtest" "-p" name] {with-test}

crc.2.2.0/opam
19:build: ["dune" "build" "-p" name]