Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opam-2.1 install -y fails #4682

Closed
avsm opened this issue May 27, 2021 · 4 comments · Fixed by #4683 or #4691
Closed

opam-2.1 install -y fails #4682

avsm opened this issue May 27, 2021 · 4 comments · Fixed by #4683 or #4691

Comments

@avsm
Copy link
Member

avsm commented May 27, 2021

Since the change to CONFIRMLEVEL instead of OPAMDEPEXTYES setting, we changed the default in docker-base-images to use it ocurrent/docker-base-images#111

However, now passing -y on the install line causes the opam install to fail:

e.g.

The following actions will be performed:
  - install seq                     base    [required by fmt, re]
  - install mirage-no-xen           1       [required by mirage-crypto-pk]
  - install conf-capnproto          1       [required by capnp-rpc-net]
  - install ocamlbuild              0.14.0  [required by astring]
  - install conf-pkg-config         2       [required by mirage-crypto]
  - install ocamlfind               1.9.1   [required by astring]
  - install conf-gmp                3       [required by zarith]
  - install mirage-no-solo5         1       [required by mirage-crypto-pk]
  - install cmdliner                1.0.4   [required by capnp-rpc-unix]
  - install dune                    2.8.5   [required by capnp-rpc-unix]
  - install topkg                   1.0.3   [required by astring]
  - install num                     1.4     [required by sexplib]
  - install base-bytes              base    [required by extunix]
  - install zarith                  1.12    [required by asn1-combinators]
  - install conf-gmp-powm-sec       3       [required by mirage-crypto-pk]
  - install stdlib-shims            0.3.0   [required by asn1-combinators]
  - install stdint                  0.7.0   [required by capnp, capnp-rpc]
  - install sexplib0                v0.14.0 [required by base]
  - install result                  1.5     [required by capnp]
  - install re                      1.9.0   [required by prometheus]
  - install ppx_derivers            1.2.1   [required by ppxlib]
  - install ocaml-syntax-shims      1.0.0   [required by lwt, angstrom]
  - install ocaml-migrate-parsetree 2.1.0   [required by ppxlib]
  - install ocaml-compiler-libs     v0.12.3 [required by ppxlib]
  - install mmap                    1.1.0   [required by lwt]
  - install mirage-clock            3.1.0   [required by tls-mirage]
  - install gmap                    0.3.0   [required by x509]
  - install duration                0.1.3   [required by mirage-crypto-rng]
  - install csexp                   1.5.1   [required by dune-configurator]
  - install cppo                    1.6.7   [required by ocplib-endian]
  - install bigarray-compat         1.0.0   [required by asn1-combinators, mirage-crypto]
  - install base64                  3.5.0   [required by capnp-rpc-unix]
  - install mtime                   1.2.0   [required by mirage-crypto-rng]
  - install astring                 0.8.5   [required by capnp-rpc-unix]
  - install asetmap                 0.8.1   [required by capnp-rpc-net]
  - install stringext               1.6.0   [required by uri]
  - install res                     5.0.1   [required by capnp]
  - install fmt                     0.8.9   [required by capnp-rpc-unix]
  - install rresult                 0.6.0   [required by tls]
  - install ptime                   0.8.5   [required by capnp-rpc-net]
  - install ppxlib                  0.22.0  [required by extunix]
  - install dune-configurator       2.8.5   [required by extunix]
  - install ocplib-endian           1.1     [required by capnp]
  - install cstruct                 6.0.0   [required by cstruct-lwt]
  - install bigstringaf             0.7.0   [required by angstrom]
  - install domain-name             0.3.0   [required by tls]
  - install extunix                 0.3.2   [required by capnp-rpc-unix]
  - install base                    v0.14.1 [required by capnp]
  - install lwt                     5.4.0   [required by capnp-rpc-unix]
  - install eqaf                    0.7     [required by mirage-crypto]
  - install asn1-combinators        0.2.5   [required by capnp-rpc-net]
  - install angstrom                0.15.0  [required by uri]
  - install stdio                   v0.14.0 [required by capnp]
  - install ppx_sexp_conv           v0.14.3 [required by tls]
  - install parsexp                 v0.14.0 [required by sexplib]
  - install prometheus              1.0     [required by capnp-rpc-net]
  - install mirage-flow             2.0.1   [required by capnp-rpc-net]
  - install mirage-device           2.0.0   [required by mirage-kv]
  - install logs                    0.7.0   [required by capnp-rpc-unix]
  - install cstruct-lwt             6.0.0   [required by capnp-rpc-unix]
  - install mirage-crypto           0.10.1  [required by capnp-rpc-net]
  - install uri                     4.2.0   [required by capnp-rpc-net]
  - install capnp                   3.4.0   [required by capnp-rpc-net]
  - install sexplib                 v0.14.0 [required by tls]
  - install mirage-kv               3.0.1   [required by tls-mirage]
  - install capnp-rpc               1.1     [required by capnp-rpc-net]
  - install pbkdf                   1.1.0   [required by x509]
  - install mirage-crypto-rng       0.10.1  [required by capnp-rpc-unix]
  - install hkdf                    1.0.4   [required by tls]
  - install ppx_cstruct             6.0.0   [required by tls]
  - install cstruct-sexp            6.0.0   [required by tls]
  - install capnp-rpc-lwt           1.1     [required by capnp-rpc-net]
  - install mirage-crypto-pk        0.10.1  [required by tls, tls-mirage]
  - install mirage-crypto-ec        0.10.1  [required by tls]
  - install x509                    0.13.0  [required by capnp-rpc-net]
  - install tls                     0.13.1  [required by capnp-rpc-net]
  - install tls-mirage              0.13.1  [required by capnp-rpc-net]
  - install capnp-rpc-net           1.1     [required by capnp-rpc-unix]
  - install capnp-rpc-unix          1.1
===== 79 to install =====

The following system packages will first need to be installed:
    capnproto libcapnp-dev libgmp-dev pkg-config

<><> Handling external dependencies <><><><><><><><><><><><><><><><><><><><><><>
Let opam run your package manager to install the required system packages?
(answer 'n' for other options) [Y/n] y
+ /usr/bin/sudo "apt-get" "install" "capnproto" "libcapnp-dev" "libgmp-dev" "pkg-config"
- Reading package lists...
- Building dependency tree...
- Reading state information...
- The following additional packages will be installed:
-   libcapnp-0.7.0 libglib2.0-0 libglib2.0-data libgmpxx4ldbl libicu63 libxml2
-   shared-mime-info xdg-user-dirs
- Suggested packages:
-   gmp-doc libgmp10-doc libmpfr-dev
- The following NEW packages will be installed:
-   capnproto libcapnp-0.7.0 libcapnp-dev libglib2.0-0 libglib2.0-data
-   libgmp-dev libgmpxx4ldbl libicu63 libxml2 pkg-config shared-mime-info
-   xdg-user-dirs
- 0 upgraded, 12 newly installed, 0 to remove and 0 not upgraded.
- Need to get 15.6 MB of archives.
- After this operation, 71.1 MB of additional disk space will be used.
- Do you want to continue? [Y/n] Abort.
You can retry with '--assume-depexts' to skip this check, or run 'opam option depext=false' to permanently disable handling of system packages altogether.
[ERROR] System package install failed with exit code 1 at command:

Removing the -y allows installation to continue normally.

@dra27
Copy link
Member

dra27 commented May 27, 2021

There's a subtle break with the OPAMCONFIRMLEVEL handling. OPAMCONFIRMLEVEL=unsafe-yes opam install -y ... gives confirm-level yes instead of unsafe-yes. We have the handling the env vars right and the handling of the params but not quite the interaction of the two: if OPAMCONFIRMLEVEL is given then --yes and --no on the CLI should be ignored (because you effective gave --confirm-level, which wins). Same goes for OPAMYES and --no.

@rjbou rjbou linked a pull request May 27, 2021 that will close this issue
@rjbou
Copy link
Collaborator

rjbou commented May 27, 2021

In fact, opam default handling of environment variables vs cli is: if a flag is given it is taken prior to the corrsponding environment variable. OPAMDEPEXTYES was a specific environment variable, that does not behave exactly the same way than OPAMCONFIRMLEVEL. The first is specific to depext handling, but the second is more global and set the always yes for opam and depexts. The correct fix/adptation would be to set OPAMCONFIRMLEVEL to unsafe-yes and remove all -y ; or to change (or append) all -y with --confirm-level=unsafe-yes.
As OPAMDEPEXTYES was introduced in beta, it is ok to break its behaviour (the prerelase rule).

That said, it could be ok to make a specific case for OPAMCONFIRMLEVEL, but for that we will introduce some "breakage" on some "consistency" rule : all opam environment variables are overwritten when the flag is given except for OPAMX then it will behave as [specific explanation], etc.

@dra27
Copy link
Member

dra27 commented May 27, 2021

When the CLI is scanned, opam determines if --confirm-level was given and at this point should apply the default for OPAMCONFIRMLEVEL. Then it should see if --yes was given and apply the default with OPAMYES. Finally it looks at --no and applies the default with OPAMNO.

Prior to #4683, opam is looking at --confirm-level, --yes and --no first to determine just answer and only then looking at any environment variables to work out a default if none of --confirm-level, --yes and --no were given, which I don't think is consistent.

@rjbou
Copy link
Collaborator

rjbou commented May 27, 2021

Yes, it is more consistent to keep each environment variable tied with the flags!

kit-ty-kate added a commit to kit-ty-kate/docker-base-images that referenced this issue May 27, 2021
kit-ty-kate added a commit to kit-ty-kate/docker-base-images that referenced this issue May 27, 2021
@rjbou rjbou linked a pull request Jun 2, 2021 that will close this issue
kit-ty-kate added a commit to kit-ty-kate/docker-base-images that referenced this issue Jun 2, 2021
kit-ty-kate added a commit to kit-ty-kate/docker-base-images that referenced this issue Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants