opam doesn't remember what packages failed to upgrade

[pmetzger]

If you do an opam upgrade and an upgrading package fails to build, the system doesn't record the failure in such a way that it "knows" that it is missing packages that were once present but which failed to upgrade. This means that a transient error can leave packages that used to be installed permanently absent.

I've noticed this a couple of times, most recently this morning when an aggressive setting of --jobs caused an opam upgrade to fail for merlin; I discovered a bit later that merlin was no longer installed.

I'd suggest a mechanism that records what failed to upgrade and, at the very least, it should be printed out at the end, if not recorded in some mechanism as a set of packages the user wants but which are not yet installed.

[dra27]

What's the existing mechanism to restore the previous state? I rigged dune.2.7.1 locally to include "false" as a build step...

dra27@summer:~$ opam upgrade
The following actions will be performed:
  ↗ upgrade   conf-pkg-config         1.2 to 1.3
  ↗ upgrade   dune                    2.5.1 to 2.7.1
  ∗ install   stdlib-shims            0.1.0              [required by ppxlib]
  ↗ upgrade   sexplib0                v0.13.0 to v0.14.0
  ↗ upgrade   ocaml-migrate-parsetree 1.7.3 to 2.0.0
  ↗ upgrade   jane-street-headers     v0.13.0 to v0.14.0
  ↗ upgrade   dune-private-libs       2.5.1 to 2.7.1
  ↻ recompile spawn                   v0.13.0            [uses dune]
  ↻ recompile result                  1.5                [uses dune]
  ↻ recompile re                      1.9.0              [uses dune]
  ↻ recompile ppx_derivers            1.2.1              [uses dune]
  ↻ recompile octavius                1.2.2              [uses dune]
  ↻ recompile ocaml-compiler-libs     v0.12.1            [uses dune]
  ∗ install   csexp                   1.3.1              [required by dune-configurator]
  ↗ upgrade   ppxlib                  0.13.0 to 0.16.0
  ↗ upgrade   dune-configurator       2.5.1 to 2.7.1
  ↗ upgrade   ppx_pipebang            v0.13.0 to v0.14.0
  ↗ upgrade   pcre                    7.4.3 to 7.4.6
  ↗ upgrade   base                    v0.13.2 to v0.14.0
  ∗ install   ppx_fixed_literal       v0.14.0            [required by ppx_jane]
  ↗ upgrade   variantslib             v0.13.0 to v0.14.0
  ↗ upgrade   typerep                 v0.13.0 to v0.14.0
  ↗ upgrade   stdio                   v0.13.0 to v0.14.0
  ↗ upgrade   ppx_stable              v0.13.0 to v0.14.1
  ↗ upgrade   ppx_sexp_conv           v0.13.0 to v0.14.1
  ↗ upgrade   ppx_optional            v0.13.0 to v0.14.0
  ↗ upgrade   ppx_let                 v0.13.0 to v0.14.0
  ↗ upgrade   ppx_js_style            v0.13.0 to v0.14.0
  ↗ upgrade   ppx_here                v0.13.0 to v0.14.0
  ↗ upgrade   ppx_enumerate           v0.13.0 to v0.14.0
  ↗ upgrade   ppx_compare             v0.13.0 to v0.14.0
  ↗ upgrade   ppx_cold                v0.13.0 to v0.14.0
  ↗ upgrade   parsexp                 v0.13.0 to v0.14.0
  ↗ upgrade   fieldslib               v0.13.0 to v0.14.0
  ↗ upgrade   ppx_variants_conv       v0.13.0 to v0.14.1
  ↗ upgrade   ppx_typerep_conv        v0.13.0 to v0.14.1
  ↗ upgrade   ppx_optcomp             v0.13.0 to v0.14.0
  ↗ upgrade   ppx_custom_printf       v0.13.0 to v0.14.0
  ↗ upgrade   ppx_sexp_value          v0.13.0 to v0.14.0
  ↗ upgrade   ppx_sexp_message        v0.13.0 to v0.14.0
  ↗ upgrade   ppx_fail                v0.13.0 to v0.14.0
  ↗ upgrade   ppx_hash                v0.13.0 to v0.14.0
  ↗ upgrade   ppx_assert              v0.13.0 to v0.14.0
  ↗ upgrade   sexplib                 v0.13.0 to v0.14.0
  ↗ upgrade   ppx_fields_conv         v0.13.0 to v0.14.1
  ↗ upgrade   ppx_base                v0.13.0 to v0.14.0
  ↗ upgrade   jst-config              v0.13.0 to v0.14.0
  ↗ upgrade   bin_prot                v0.13.0 to v0.14.0
  ∗ install   ppx_string              v0.14.1            [required by ppx_jane]
  ↗ upgrade   time_now                v0.13.0 to v0.14.0
  ↗ upgrade   ppx_bin_prot            v0.13.0 to v0.14.0
  ↗ upgrade   ppx_module_timer        v0.13.0 to v0.14.0
  ↗ upgrade   ppx_inline_test         v0.13.1 to v0.14.1
  ↗ upgrade   ppx_expect              v0.13.1 to v0.14.0
  ↗ upgrade   ppx_bench               v0.13.0 to v0.14.1
  ↗ upgrade   splittable_random       v0.13.0 to v0.14.0
  ↗ upgrade   base_quickcheck         v0.13.0 to v0.14.0
  ↗ upgrade   ppx_jane                v0.13.0 to v0.14.0
  ↗ upgrade   base_bigstring          v0.13.0 to v0.14.0
  ↗ upgrade   core_kernel             v0.13.1 to v0.14.0
  ∗ install   timezone                v0.14.0            [required by core]
  ↗ upgrade   patience_diff           v0.13.0 to v0.14.0
  ↗ upgrade   core                    v0.13.0 to v0.14.0
  ↗ upgrade   patdiff                 v0.13.0 to v0.14.0
===== ∗ 5   ↻ 6   ↗ 53 =====
Do you want to continue? [Y/n]

answering yes gives...

<><> Gathering sources ><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
...

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
⊘ removed   conf-pkg-config.1.2
∗ installed conf-pkg-config.1.3
[ERROR] The compilation of dune failed at "/home/dra27/.opam/opam-init/hooks/sandbox.sh build false".

#=== ERROR while compiling dune.2.7.1 =========================================#
# context     2.0.4 | linux/x86_64 | ocaml-base-compiler.4.10.0 | file:///home/dra27/opam-repository
# path        ~/.opam/patdiff/.opam-switch/build/dune.2.7.1
# command     ~/.opam/opam-init/hooks/sandbox.sh build false
# exit-code   1
# env-file    ~/.opam/log/dune-12393-034685.env
# output-file ~/.opam/log/dune-12393-034685.out



<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
┌─ The following actions failed
│ λ build dune 2.7.1
└─
┌─ The following changes have been performed (the rest was aborted)
│ ↗ upgrade conf-pkg-config 1.2 to 1.3
└─
# Run eval $(opam env) to update the current shell environment

The former state can be restored with:
    opam switch import "/home/dra27/.opam/patdiff/.opam-switch/backup/state-20200907141653.export"

Copying and pasting the last command has restored the switch (it's not guaranteed to, obviously).

[AltGr]

Have you tried opam install --restore ?

[pmetzger]

Restoring the switch to exactly what it was before isn't really what I want; I presume that's what the opam switch import would do.

I didn't realize --restore existed. I wonder if it should be the default. However, obviously that (more or less) would fix my concern, but only if I remembered to use it every single time I tried install.

[AltGr]

--restore should be advertised by opam on failure, I'll check if that's still the case 🤔

EDIT: the code is here

opam/src/state/opamSwitchState.ml

Lines 1140 to 1146 in 175e294

	if OpamPackage.Set.is_empty
	(new_selections.sel_roots -- new_selections.sel_installed)
	then "" else
	Printf.sprintf
	"Or you can retry to install your package selection with:\n\
	\ %s install --restore\n"
	Sys.executable_name)))

, but might be broken

[AltGr]

The feature should be here 🤔
So either there is a bug where the message is'nt printed as it should, or it should be closed

[pmetzger]

I don't understand what is meant by "the feature should be here", but again: if I run an opam upgrade without running --restore, it is too late to try that afterwards, and I don't have a good way to know what damage was done to my switch.

[dra27]

@AltGr means that the code pointed to should be displaying the recommendation for --restore, so it's a bug if it wasn't. I realise it's a while on, but do you have a reproduction case where the installation fails and this message isn't displayed?

[pmetzger]

I can try but I don't have an easy reproducing case right now.