Skip to content

DUNE_PROFILE should be ignored when -p specified #4632

@dra27

Description

@dra27

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:

  1. Don't pass DUNE_PROFILE to 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.
  2. Put a sentinel value on the start of DUNE_PROFILE before 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.
  3. Only allow dune-release as the argument for --profile when -p is given. Also fairly simple; might break some unexpected usage of dune.
  4. Pre-process Sys.argv to identify the presence of -p and 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 -p is present (before any --) and no term begins --profile, the the --profile argument could be omitted in the term.
  5. Restore --profile to the pre-2.0 behaviour of conflicting with -p and 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).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions