Introduce 'default-invariant' besides 'default-compiler' #4607
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
default-invariant
branch
from
ad65ce5 to
95c0a03
Compare
|
Ok, so apparently this works... but the perf is not acceptable at the moment :/ |
|
Ok, tests added, and now reasonably performant |
AltGr
force-pushed
the
default-invariant
branch
from
80b78b3 to
179a58d
Compare
rjbou
force-pushed
the
default-invariant
branch
from
179a58d to
08f5950
Compare
dra27
reviewed
Mar 24, 2021
Comment on lines
+156
to
+159
| if config_files = [] then | ||
| OpamConsole.msg "No configuration file found, using built-in defaults.\n" | ||
| else | ||
| OpamConsole.msg "Configuring from %sbuilt-in defaults.\n" others; |
Comment on lines
+23
to
+24
| OpamFormula.Atom (OpamPackage.Name.of_string "ocaml-system", | ||
| OpamFormula.Empty); |
There was a problem hiding this comment.
What happens if the user has an old system compiler (e.g. they happen to be on a really old Ubuntu)? Does it quietly ignore it and move on to ocaml-base-compiler, or should we duplicate the >= "4.05" constraint here too?
There was a problem hiding this comment.
The former: this is in the tests here: https://github.com/ocaml/opam/pull/4607/files#diff-b3f6abdb5ab3eb6c5612e441c0709542022442b9cc6f47b5eae75bc5aa85799cR36-R54
| (fun st -> | ||
| (), | ||
| OpamSwitchCommand.install_compiler st | ||
| ~additional_installs:default_compiler) |
There was a problem hiding this comment.
This causes an interactive prompt in OpamSwitchCommand.install_compiler
There was a problem hiding this comment.
Ah, well spotted, indeed that's unfortunate.
compared to 2.0, this just adds the new 'default-invariant', and keeps similar semantics for 'default-compiler', which is only used for 'opam init' (but must now be compatible with the invariant). previously the 2.1 branch completely replaced the older meaning of 'default-compiler', using it as 'default-invariant' instead, but the order of priority was lost.
This can happen with a bad URL, or if the network is down. I state you end up in is pretty confusing for users: opam considers it's initialised, but you get no packages and no switch, and it's unclear what to do.
it was completely silent, which could be confusing.
AltGr
force-pushed
the
default-invariant
branch
from
e9437f0 to
e8a4d16
Compare
and disable the confirmation in all cases, it doesn't make much sense anymore on 2.1.
to make it follow the documented behaviour: `Without arguments, an invariant is chosen automatically.` previous version just emptied it.
AltGr
force-pushed
the
default-invariant
branch
from
e8a4d16 to
23f1e86
Compare
Merged
|
Hopefully that fixes the testsuite on AppVeyor. The problem was twofold:
|
|
ow thanks ! |
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.
(todo: add tests)
compared to 2.0, this just adds the new 'default-invariant', and keeps
similar semantics for 'default-compiler', which is only used for 'opam
init' (but must now be compatible with the invariant).
previously the 2.1 branch completely replaced the older meaning of
'default-compiler', using it as 'default-invariant' instead, but the order
of priority was lost.