don't control concurrency from INSIDE_DUNE

[Alizter]

INSIDE_DUNE sets the concurrency of dune to 1. We use this behaviour when running our tests. However, running dune inside of dune, as is common in package management, means that the child process also observes INSIDE_DUNE and therefore affects the concurrency when we haven't explicitly passed -j, as is the case for OxCaml:

• Nesting of INSIDE_DUNE and build concurrency #12737

This PR introduces a new environment variable DUNE_JOBS whose sole job is to configure the concurrency from the environment.

Our second change is to decouple the concurrency control from INSIDE_DUNE. This means that INSIDE_DUNE no longer sets the concurrency to 1. This means that we have to explicitly set DUNE_JOBS in our cram tests. We don't however set the value for the expect tests.

Setting DUNE_JOBS for our cram tests has an unfortunate side-effect, and is was the cause of most of the noise in this PR. It triggers a warning message given by an RPC client when a dune's config differs from the default values. I've therefore disabled this warning when INSIDE_DUNE since it doesn't have any utility for us. Also, depending on the pid contents of the build lock in a racy way is not sound.

• update docs
  • mention new env var in cli docs and man pages
  • document concurrency properly
• changelog
• Find a way to silence or perhaps get rid of the warning. (It's not a very good warning to begin with)

[Alizter]

FTR oxcaml still won't build with the correct concurrency with this patch because they are specifically vendoring Dune 3.20 and using that to build. We therefore have to patch their opam build to not do that. If we are going to do that anyway, then we might as well unset INSIDE_DUNE. @Sudha247 may have some updates on this.

[Alizter]

Another issue with this PR is that comparing against Common.Builder.default value is used as a heuristic for displaying the RPC forwarding message. This means that INSIDE_DUNE no longer sets this as 1 and every test differs from the default of Auto. Therefore triggering the message in each RPC forwarding situation.

[Leonidas-from-XIV]

I do wonder why the CI doesn't want to run the jobs that run the test-suite…

[Alizter]

@Leonidas-from-XIV The nix tests are a dependency of the rest of the tests. That way we can catch errors early and not bother using up all the allocated machines. The fact that the environment is Nix doesn't mean much here, its just the regular test suite.

[Sudha247]

Yes, I can confirm that this doesn't help the OxCaml build by itself. I believe we will need to update the Dune version used to bootstrap the compiler to actually see the difference.

[Sudha247]

This looks reasonable to me. I'll run tests with this branch on the build of the OxCaml compiler and report back.

[Sudha247]

I've run some tests for this with OxCaml compiler build and it does appear to enable using concurrency with nested Dune calls which it previously wasn't.

To reproduce:

1. Install Dune from this branch.

opam pin add dune git+https://github.com/Alizter/dune.git#push-ysnonzkmwzym

2. Use this branch Patched OPAM repository in dune-workspace: https://github.com/Sudha247/oxcaml-opam-repository/blob/inside-dune-test/packages/ocaml-variants/ocaml-variants.5.2.0%2Box/opam

(repository
 (name oxcaml)
 (url git+file:///home/user/path/to/oxcaml-opam-repository))

(lock_dir
 (repositories :standard oxcaml))

3. Build the project with dune build, I built hello-oxcaml. This will trigger a compiler build.

To compare some numbers on time taken to build hello-oxcaml:

Build	Time
Upstream dune pkg	32:00.89
opam switch creation	14:21.62
dune pkg with this patch	16:57.90
Manually unsetting INSIDE_DUNE	14:04.00

Given the performance of the patch, I believe it does enable concurrent builds in the case of nested Dune. I think we can go ahead and merge this PR, provided everything else is ok.