install --dry-run accidentally updates .opam-switch/install/<pkg-name>.changes

[na4zagin3]

opam install --dry-run accidentally wipes out .opam-switch/install/<pkg-name>.changes when OPAM dry-runs package rebuild of any packages, including depending and depended ones.

Minimum reproducible example

This example directly builds num.1.4 and num.1.3 because they are available at opam-repository and their size and dependencies are small. The issue should be reproducible with any packages.

Prepare a switch.

$ opam switch new . 4.12.0

Install some version.

$ opam install --yes num.1.4
The following actions will be performed:
  - install ocamlfind 1.9.3 [required by num]
  - install num       1.4
===== 2 to install =====
Do you want to continue? [Y/n] y

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-> retrieved num.1.4  (cached)
-> retrieved ocamlfind.1.9.3  (cached)
-> installed ocamlfind.1.9.3
-> installed num.1.4
Done.

OPAM correctly detected the installed files as added files.

$ cat $(opam var prefix)/.opam-switch/install/num.changes | head
opam-version: "2.0"
added: [
  "lib/num" {"D"}
  "lib/num-top" {"D"}
  "lib/num-top/META" {"F:S123T1651169248.48"}
  "lib/num-top/num_top.cma" {"F:S2058T1651169248.39"}
  "lib/num-top/num_top.cmi" {"F:S187T1651169248.37"}
  "lib/num-top/num_top_printers.cmi" {"F:S1076T1651169248.36"}
  "lib/num/META" {"F:S647T1651169248.48"}
  "lib/ocaml/arith_flags.cmx" {"F:S208T1651169248.48"}

Dry-run downgrade to an older version.

$ opam install --yes --dry-run num.1.3
The following actions will be simulated:
  - downgrade num 1.4 to 1.3

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-> removed   num.1.4
Installing num.1.3.
-> installed num.1.3
Done.

Surprisingly, OPAM wiped out num.changes.

$ cat $(opam var prefix)/.opam-switch/install/num.changes | head
opam-version: "2.0"

Then, try installing the older version.

$ opam install --yes num.1.3
The following actions will be performed:
  - downgrade num 1.4 to 1.3

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-> retrieved num.1.3  (cached)
-> removed   num.1.4
-> installed num.1.3
Done.

The installed files are recognized as contents-changed.

$ cat $(opam var prefix)/.opam-switch/install/num.changes | head
opam-version: "2.0"
contents-changed: [
  "lib/num-top/META" {"F:S123T1651169414.75"}
  "lib/num-top/num_top.cma" {"F:S2058T1651169414.64"}
  "lib/num-top/num_top.cmi" {"F:S187T1651169414.63"}
  "lib/num-top/num_top_printers.cmi" {"F:S1076T1651169414.61"}
  "lib/num/META" {"F:S647T1651169414.74"}
  "lib/ocaml/arith_flags.cmx" {"F:S208T1651169414.75"}
  "lib/ocaml/arith_status.cmi" {"F:S1368T1651169414.74"}
  "lib/ocaml/arith_status.cmti" {"F:S11562T1651169414.74"}

Notes

This problem was found during investigation of CI failures at na4zagin3/satyrographos-repo#434.
This may be the root cause of #4419.

Environment

# opam config report
# opam-version         2.1.2 
# self-upgrade         no
# system               arch=x86_64 os=linux os-distribution=gentoo os-version=2.7
# solver               builtin-mccs+glpk
# install-criteria     -removed,-count[avoid-version,changed],-count[version-lag,request],-count[version-lag,changed],-count[missing-depexts,changed],-changed
# upgrade-criteria     -removed,-count[avoid-version,changed],-count[version-lag,solution],-count[missing-depexts,changed],-new
# jobs                 3
# repositories         2 (http), 2 (local), 5 (version-controlled) (default repo at 83c88d3b)
# pinned               0
# current-switch       /home/mrty/Documents/Development/satysfi/test-repo
# ocaml:native         true
# ocaml:native-tools   true
# ocaml:native-dynlink true
# ocaml:stubsdir       /home/mrty/Documents/Development/satysfi/test-repo/_opam/lib/ocaml/stublibs:/home/mrty/Documents/Development/satysfi/test-repo/_opam/lib/ocaml
# ocaml:preinstalled   false
# ocaml:compiler       4.12.0

[na4zagin3]

Here is the output of strace. Apparently, OPAM writes to .changes file regardless of whether --dry-run option is used.

chdir("/home/mrty/Documents/Development/satysfi/test-repo") = 0
newfstatat(AT_FDCWD, "/home/mrty/Documents/Development/satysfi/test-repo/_opam/.opam-switch/install", {st_mode=S_IFDIR|0755, st_size=542, ...}, 0) = 0
openat(AT_FDCWD, "/home/mrty/Documents/Development/satysfi/test-repo/_opam/.opam-switch/install/num.changes", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC, 0666) = 5
lseek(5, 0, SEEK_CUR)                   = 0   
fcntl(5, F_SETLKW, {l_type=F_WRLCK, l_whence=SEEK_CUR, l_start=0, l_len=0}) = 0
write(5, "opam-version: \"2.0\"\n", 20) = 20
close(5)                                = 0   
write(1, "-> installed \33[01mnum\33[0m.1.3\n", 30) = 30

[na4zagin3]

I just found this bug was not limited to install --dry-run but also happens with install.

+ find /home/runner/work/satyrographos-repo/satyrographos-repo/_opam/.opam-switch/install -iname 'satysfi-*.changes' -exec grep -e '^contents-changed:' '{}' +
...

# OK

...

+ opam install snapshot-stable-0-0-5

<><> Synchronising pinned packages ><><><><><><><><><><><><><><><><><><><><><><>
[NOTE] Ignoring uncommitted changes in /home/runner/work/satyrographos-repo/satyrographos-repo (`--working-dir' not active).
[snapshot-stable-0-0-5.dev] synchronised (no changes)

The following actions will be performed:
  ↻ recompile satysfi-matrix        0.0.1+dev2019.10.15 [upstream or system changes]
  ↻ recompile satysfi-karnaugh      0.0.1               [upstream or system changes]
  ↻ recompile satysfi-make-markdown 0.1.0               [upstream or system changes]
  ∗ install   satysfi-assert-eq-doc 0.1.2               [required by snapshot-stable-0-0-5]
  ↻ recompile satysfi-matrix-doc    0.0.1+dev2019.10.15 [uses satysfi-matrix]
  ↻ recompile satysfi-karnaugh-doc  0.0.1               [uses satysfi-karnaugh]
  ∗ install   snapshot-stable-0-0-5 dev*
===== ∗ 2   ↻ 5 =====

...

+ find /home/runner/work/satyrographos-repo/satyrographos-repo/_opam/.opam-switch/install -iname 'satysfi-*.changes' -exec grep -e '^contents-changed:' '{}' +
/home/runner/work/satyrographos-repo/satyrographos-repo/_opam/.opam-switch/install/satysfi-karnaugh-doc.changes:contents-changed:
/home/runner/work/satyrographos-repo/satyrographos-repo/_opam/.opam-switch/install/satysfi-assert-eq-doc.changes:contents-changed:
/home/runner/work/satyrographos-repo/satyrographos-repo/_opam/.opam-switch/install/satysfi-matrix-doc.changes:contents-changed:

# Corruption detected here!

https://github.com/na4zagin3/satyrographos-repo/runs/6240760811?check_suite_focus=true

[na4zagin3]

The files are wiped out by this part:

opam/src/client/opamAction.ml

Lines 1066 to 1074 in e36650b

	| Left config, changes ->
	let changes_f = OpamPath.Switch.changes root t.switch nv.name in
	OpamFile.Changes.write changes_f changes;
	OpamConsole.msg "%s installed %s.%s\n"
	(if not (OpamConsole.utf8 ()) then "->"
	else OpamActionGraph.
	(action_color (`Install ()) (action_strings (`Install ()))))
	(OpamConsole.colorise `bold name)
	(OpamPackage.version_to_string nv);

OpamFile.Changes.write is called even with option --dry-run.

[rjbou]

Thanks!