Define switch invariants rather than "base packages" #3894
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AltGr
force-pushed
the
switch-invariants
branch
from
f71ca3c
to
a43e37e
Compare
|
Auto-merge with the merged #3960 was completely broken (duplicated definitions), hence the failures. |
AltGr
force-pushed
the
switch-invariants
branch
from
a43e37e
to
e043fa8
Compare
AltGr
force-pushed
the
switch-invariants
branch
from
e043fa8
to
7cda490
Compare
Merged
AltGr
force-pushed
the
switch-invariants
branch
from
7cda490
to
e67b57e
Compare
This was referenced Jan 20, 2020
|
Rebased. I am still working on improving the conflict messages, but it ended up being much more difficult than expected. So I say we don't stall this any longer, knowing that there will be a regression on conflict error messages when the compiler versions are in cause; that'll be improved in another PR asap |
rjbou
reviewed
Mar 10, 2020
rjbou
force-pushed
the
switch-invariants
branch
2 times, most recently
from
ccac087
to
3e9440b
Compare
AltGr
force-pushed
the
switch-invariants
branch
from
3e9440b
to
c5238fe
Compare
in particular, conflict messages. Besides checking for regressions, this is also a useful way to track the evolution of these messages in a clearer way.
they sometimes got split into version subsets for A cflt B and B.1 cflt A
For consistency with the computation of invalidated and available packages.
This is done from base packages when no invariant is defined, and the switch-config version is older than 2.1
- fix handling of the invariant in installability queries - remove the remaining use of "base" packages for cleanup, it was still enforcing that base packages don't change version. A more involved cleanup could be implemented if the solvers can't cope
The hack we used, based on the base packages, no longer works
The approach is to - remove the invariant while resolving - update the configured invariant, keeping the package names that were part of it, but replacing the constraints that no longer match with adjusted ones. Note that this should work OK for allowing compiler up/downgrades, but can lead to an empty invariant if you just switched compiler implementation. Doing the simpler things still seems better in this case than some magic, though, and the user is told how to update.
To make things much simpler than what we had (with lots of code to choose the compilers when creating switches on existing projects), the switch creation is now done in a single pass (compiler + local packages), which means the single call to the solver determines the compiler: this should be much more reliable. Then the "switch invariant" is set according to the packages with the "compiler" flag that got installed. Note that this should also be much more friendly when creating a switch in a directory that contains compiler definitions (this is detected, and the "default compiler" selection isn't used).
Now called 'invalidated' packages Based on, and closes ocaml#3816 and ocaml#3817
…e it as optionnal
aspcud will still eat your memory in some cases, but it won't crash on our submitted file.
it just doesn't scale anymore
AltGr
force-pushed
the
switch-invariants
branch
from
c5238fe
to
1aaea2a
Compare
|
The last test on appveyor apparently failed due to OCaml 4.10, but otherwise all tests seem OK. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTE: this is WIP and should be rebased once #3478 is merged to avoidpatch duplication
Currently, we define switches with a set of "base packages", that
normally correspond to the compiler, and that are strongly locked
(unless
--unlock-baseis specified). This is somehow inherited fromthe older "compiler definitions" that weren't packages.
This PR proposes to switch to a much more flexible notion of "switch
invariant", which can be an arbitrary dependency formula (same format
as
depends:, minus the extended formulas).SWITCH/.opam-switch/switch-configupgrade should be smooth. No global upgrade needed)
(typically 'ocaml-system')
internally, without consequences on the optim criteria)
for migration). They will be automatically updated on changes
This works quite well, but some small parts are still missing on top
of it:
inferred from the arguments on the CLI, or the otherwise known base
packages
--unlock-basewon't work anymore, it needs to affect theinvariant instead of the base packages
opam fixupswitch-exportformat to includeinvariants