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

Cli version - environment variable edition #4606

Merged
merged 22 commits into from
Apr 6, 2021

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented Mar 17, 2021

New opam environment variable handling. They are defined per lib, and defined and loaded only at opam client start (no more on library loading). There is no more direct access, each environment variable have its associated field in its lib config, or can be accessed throw OpamXxxConfig.Env.<var> function.
When defined in OpamArg, the cli version is required, to handle cli versioning of environment variable in a slightly different way than options: if not defined in current cli, a warning is printed and it is ignored.

@rjbou rjbou added PR: QUEUED Pending pull request, waiting for other work to be merged or closed PR: WIP Not for merge at this stage labels Mar 17, 2021
@rjbou rjbou added this to the 2.1.0~rc milestone Mar 17, 2021
@rjbou
Copy link
Collaborator Author

rjbou commented Mar 17, 2021

Remaining:

  • preinit env vars in commands like OPAMSELFUPGRADE
  • preinit env vars cli main OPAMYES, OPAMCLI
  • admin command env vars (OPAMNOAGGREGATE)
  • add missing env vars doc
  • specify env vars deprecation (except build doc/test) other PRs
  • specify new en vars
  • specialise env var cli warning messages?
  • add test
  • externelise Mk fromOpamArg
  • Some renaming

@rjbou rjbou force-pushed the cli-version-envvar-edition branch from 2f804f7 to d0b6ffb Compare March 19, 2021 15:26
@rjbou rjbou removed PR: QUEUED Pending pull request, waiting for other work to be merged or closed PR: WIP Not for merge at this stage labels Mar 19, 2021
@rjbou rjbou force-pushed the cli-version-envvar-edition branch from 0bb77b3 to df63cce Compare March 26, 2021 16:40
Copy link
Member

@AltGr AltGr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty great! And finally we can have the documentation of the environment variables besides them :)
Two small nitpicks:

  • remember to update the doc index.html when you add modules
  • in [WARNING] OPAMNODEPEXTS was added in version 2.1 of the opam CLI, but version 2.0 has been requested, which is older. it is not completely clear wether the variable was used nonetheless or was ignored. Maybe OPAMNODEPEXTS was ignored because CLI v. 2.0 was requested and it was introduced in v.2.1 ?

@rjbou rjbou force-pushed the cli-version-envvar-edition branch from 64f09fb to 2a5d412 Compare April 6, 2021 09:14
@rjbou rjbou force-pushed the cli-version-envvar-edition branch from 2a5d412 to d4013c6 Compare April 6, 2021 10:43
@rjbou rjbou force-pushed the cli-version-envvar-edition branch from bb9a7d6 to 4818821 Compare April 6, 2021 11:29
@rjbou rjbou linked an issue Apr 6, 2021 that may be closed by this pull request
@rjbou rjbou merged commit 3cd0829 into ocaml:master Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI versioning should warn about ignored environment variables
2 participants