Implement CLI versioning #4316
Merged
Implement CLI versioning #4316
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
|
I'm now working on actually doing something with the information, and will push more! |
Using Format instead of Printf tidies up the Windows version of the logging, since writing to a buffer and a channel become equivalent. Technically, this now permits colour in log lines! It also opens the door for using semantic tags instead of embedded escape sequences which will allow the code to be further simplified. The only downside is that the @ character now needs escaping in a few messages.
Allow OpamConsole.log to be used before OpamCoreConfig.set has been called. debug_level defaults to 0, so any debugging messages sent to OpamConsole.log before the config has been loaded are dropped. This PR stores them with their timestamp and level and on the first call to OpamConsole.log after OpamCoreConfig.set has been called, these log lines are reinspected and printed if necessary. The only caveat is that the log lines are always evaluated (i.e. OpamConsole.slog with `%a` has no computational benefit).
To be used for CLI options where it's necessary for opam to know whether the value came from the command line, the environment or is the default. Generalises the type used for OPAMSWITCH.
|
Rebased! |
|
Great work, thanks!
Also, while I agree with the clearer |
|
Catching up, and answering @AltGr's questions:
|
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.
Implements the CLI Versioning spec
Fundamentally, this PR allows the user/author to select backwards-compatibility with the opam 2.0 CLI by setting
OPAMCLIto2.0and forwards-compatibility with future releases of the opam client by passing--cli=2.1anywhere on the command line.This is implemented by increasing the pre-processing which is done to
Sys.argvbeforeCmdliner.Term.evalis invoked. Various things to note:opam admas an abbreviation foropam adminso this is now implemented (seeOpamCommands.is_admin_subcommand).OpamConsole.logto useFormat.fprintfinstead ofPrintf.fprintfwhich has been on my TODO-list for a while. This actually improves the Windows story ever-so-slightly and it turns out with only tiny alterations to a couple of messages which included@signs. In future, it will allow the formatting to be applied using tags which will be better in terms of the code and also better for Windows (sinceFormatwill do the heavy lifting instead of my horrendous VT100 parser). I've done it here because I wanted to be able to debug the CLI parsing process, and its hard to setOpamCoreConfig.!r.debug_levelbefore you've parsed the command-line!Sys.argvin order to allowopam --yes plugin argsto mean "automatically build and install plugin and then call it with args". I've extracted this pre-processing and included the--cliprocessing with it.--yesto allow all commands to benefit from it. This means thatopam --yes listnow works, where before it was a syntax error. This is necessary to allow plugins to become built-in commands compatibly. At the moment this has a slight change of semantics in thatopam --yes locknow meansopam lock --yeswhere before the plugin would not have been invoked with--yes. I think this is acceptable and consistent to do universally, rather than tying that handling to CLI version (although we could...).--yesis transformed - that is,opam --yes listis still handled by Cmdliner, it just seesopam list --yesinstead.--cliis completely extracted since it affects the generation of the Cmdliner parser ---cliitself is included in the parser only so that it's included in--help(in particular, this is why parsing the prefix--clis important oropam --cli=2.0 --cl=2.1would be a contradiction).