Empty DUNE_PROFILE should be ignored

[dra27]

dra@thor:~/dune$ DUNE_PROFILE= dune printenv --verbose 2>&1 | fgrep 'profile =' -A2 -B2
 { name = "default"
 ; kind = "default"
 ; profile = User_defined ""
 ; merlin = true
 ; for_host = None

This isn't useful, and empty environment variables cannot (reliably) be specified on Windows. My pseudo-religious opinion on this is that an empty environment variable should always be treated as unset in cross-platform development.

Related to #4632 - at the moment an opam package must specify -p --profile=release if it wants to guarantee that it actually gets the release profile with -p (since opam doesn't have a syntax to unset ernvironment variables).

[dra27]

It's a small change which I'm happy to make, given the "nod"!

[nojb]

xref #3959

[ghost]

Why doesn't opam clear the environment in the sandbox?

[dra27]

opam (mostly) doesn't scrub the environment (it's an extremely difficult thing to do reliably - cf. Visual Studio configuration, etc.) - in the absence of package variables, it's actually relied on (😱) for some things (you can feed configuration to a package this way, but it's obviously a horribly hacky thing to do).

opam 2.1 will scrub newly introduced OPAM* environment variables (ocaml/opam#4660) and for 2.2 we'll aim to scrub all OPAM* environment variables, having assessed its impact (ocaml/opam#4661) but I think it's extremely unlikely that opam will ever scrub any other environment variables.

[ghost]

I don't buy the Visual Studio example (why would the build of packages care about this?) but I can believe it is tricky in general.

So back to Dune, it seems that we should plain ignore the DUNE_PROFILE variable when we pass -p (or more generally --release). -p is meant for building packages via a package manager and it definitely doesn't sounds right to impact such builds via environment variables. The only variables that seems safe to take into account are the ones that don't modify what Dune produces. For instance the ones related to the shared cache.

[dra27]

I was referring to the Visual Studio Command Prompts which are a vast host of batch files which set the environment variables to use the compilers.

[ghost]

Oh, that makes sense

[dra27]

My religious fervour remains, but this issue is essentially a duplicate of #3959, so closing until such time as it bites more significantly 🙂