-
Notifications
You must be signed in to change notification settings - Fork 464
Description
DUNE_PROFILE being set in the environment can break the installation of opam packages:
dra@thor:~$ DUNE_PROFILE=dev opam install uri.2.2.1
The following actions will be performed:
∗ install uri 2.2.1
<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
⬇ retrieved uri.2.2.1 (cached)
[ERROR] The compilation of uri.2.2.1 failed at "dune build -p uri -j 7".
#=== ERROR while compiling uri.2.2.1 ==========================================#
# context 2.1.0~beta4 | linux/x86_64 | ocaml-base-compiler.4.11.2 | file:///home/dra/opam-repository
# path ~/.opam/dev-4.11/.opam-switch/build/uri.2.2.1
# command ~/.opam/opam-init/hooks/sandbox.sh build dune build -p uri -j 7
# exit-code 1
# env-file ~/.opam/log/uri-3421537-40d977.env
# output-file ~/.opam/log/uri-3421537-40d977.out
### output ###
# [...]
# - [@sexp_drop_default.equal] if the type supports [%equal]
# - [@sexp_drop_default.sexp] if you want to compare the sexp representations
#
# File "lib/uri_sexp.ml", line 21, characters 18-31:
# 21 | scheme: string option [@default None] [@sexp_drop_default];
# ^^^^^^^^^^^^^
# Error (warning 22): [@sexp_drop_default] is deprecated: please use one of:
# - [@sexp_drop_default f] and give an explicit equality function ([f = Poly.(=)] corresponds to the old behavior)
# - [@sexp_drop_default.compare] if the type supports [%compare]
# - [@sexp_drop_default.equal] if the type supports [%equal]
# - [@sexp_drop_default.sexp] if you want to compare the sexp representations
#
<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
┌─ The following actions failed
│ λ build uri 2.2.1
└─
╶─ No changes have been performed
--profile got changed as part of the 2.0 bootstrap, so it can be specified with -p (so that the bootstrap can do -p --profile dune-bootstrap. This is fine for the command line, but DUNE_PROFILE should definitely be ignored in this case.
Cmdliner doesn't make the provenance of a term available. There are a few possible fixes:
- Don't pass
DUNE_PROFILEto Cmdliner at all and manually fill it in (this is what opam does for pretty much all of its environment variables - the downside is that the man-page no longer automatically documents the variable). This is a fairly simple change. - Put a sentinel value on the start of
DUNE_PROFILEbefore evaluating the term, so that Dune can tell that the environment variable was used. The more serious downside is that it affects the evaluation of the term for all the commands so it's a bigger diff. - Only allow
dune-releaseas the argument for--profilewhen-pis given. Also fairly simple; might break some unexpected usage of dune. - Pre-process
Sys.argvto identify the presence of-pand anything beginning--profile(this isn't as crazy as it sounds, but it's quite crazy - FWIW we have to pre-process Sys.argv in opam, too!). If-pis present (before any--) and no term begins--profile, the the--profileargument could be omitted in the term. - Restore
--profileto the pre-2.0 behaviour of conflicting with-pand introduce a different argument for the bootstrap to communicate the bootstrap profile.
Technically speaking I broke this in #2854 (although I think the change was actually Jeremie's 😉) - I'm very happy to do whichever of these fixes we settle on! The options above are given in increasing order of preference (so I think 4 or 5 are probably best - 4 is slightly dirtier internally, but doesn't require a special bootstrap flag which is always visible in the CLI).