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

Cycle after resolution of compiler upgrade (2.1.0~beta2) #4357

Closed
AltGr opened this issue Sep 14, 2020 · 4 comments · Fixed by #4358
Closed

Cycle after resolution of compiler upgrade (2.1.0~beta2) #4357

AltGr opened this issue Sep 14, 2020 · 4 comments · Fixed by #4358
Projects
Milestone

Comments

@AltGr
Copy link
Member

AltGr commented Sep 14, 2020

I believe this is a bug in the function that alters the action graph in orde to push uninstall to the latest possible moment (OpamCudf.ActionGraph.explicit).; logging the case here so we can reproduce later.

repo revision: 3c21183

$ opam sw cr /tmp/testsw ocaml-variants.4.10.0+flambda --fake
$ opam install --sw /tmp/testsw ocaml-base-compiler.4.11.1 --fake --update-invariant
The following actions will be faked:
  ⊘ remove    ocaml-variants      4.10.0+flambda   [conflicts with ocaml-base-compiler]
  ∗ install   ocaml-base-compiler 4.11.1
  ↻ recompile ocaml-config        1                [uses ocaml-base-compiler]
  ↗ upgrade   ocaml               4.10.0 to 4.11.1 [uses ocaml-base-compiler]
===== ∗ 1   ↻ 1   ↗ 1   ⊘ 1 =====
Do you want to continue? [Y/n] y

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
[ERROR] Cycles found during dependency resolution:
          - ⊘ ocaml-config.1 -> ⊘ ocaml-variants.4.10.0+flambda -> λ ocaml-base-compiler.4.11.1 -> ⊘
        ocaml.4.10.0

Base action graph

base-actions

Expanded action graph

explicit-actions

Presumably, the edge from build ocaml-base-compiler.4.11.1 to remove ocaml.4.10.0 was wrongly added by this function.

@AltGr
Copy link
Member Author

AltGr commented Sep 14, 2020

Fun fact: this doesn't happen without --fake, due to another bug: package removals that "do nothing" are not properly detected because the .opam-switch/install/<pkg>.changes file is still created but empty, and the detection just polls from existence. (EDIT: actually, upon re-reading, the code seems correct... Probably the --fake flags made all .changes files be empty, which triggered the bug; but the behaviour without --fake was correct)

The algorithm to delay package removals handles the case of no-op removals, in order to optimise the delay: if a package removall/reinstall does nothing, it's not required to do all the cascading removals before reinstalling it. Supposing bar depends on conf-foo, both are installed and conf-foo, which installs nothing, needs a reinstall: this would allow us to recompile and install conf-foo and start building bar without removing anything, hence with no ill effect in case bar compilation fails. Without this optimisation, the reinstallation of conf-foo would require the removal of its dependent bar, before the compilation of bar can be started. In case of failure, you end up with bar removed.

If fixing this optimisation, we should double-check that package configuration files and setenv: fields are handled properly when skipping the removal/reinstall though; or that there are none.

AltGr added a commit to OCamlPro/opam that referenced this issue Sep 14, 2020
Fixes ocaml#4357, but not the fact that the optim is generally not applied.
@AltGr
Copy link
Member Author

AltGr commented Sep 14, 2020

Corrected action graph after #4358

fixed-actions

@AltGr
Copy link
Member Author

AltGr commented Sep 15, 2020

Ideally, we should compile the new compiler before doing any removals, but the current fix keeps a conservative approach and just avoids the issue.

@AltGr
Copy link
Member Author

AltGr commented Sep 15, 2020

The question is more general though: if we have a conflict between A and B, do we allow B to be built while A is still installed ? I am pretty sure that would break some packages, even if that's exactly what we do for different versions of the same package...

@rjbou rjbou added this to the 2.1.0~beta3 milestone Sep 15, 2020
@rjbou rjbou added this to To do in Opam 2.1.x via automation Sep 15, 2020
@rjbou rjbou moved this from To do to In Progress in Opam 2.1.x Sep 15, 2020
Opam 2.1.x automation moved this from In Progress to Done Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Opam 2.1.x
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants