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

Make OpamConfigCommand.global_allowed_fields fully lazy #5162

Merged
merged 1 commit into from
Jun 27, 2022

Conversation

LasseBlaauwbroek
Copy link
Contributor

@LasseBlaauwbroek LasseBlaauwbroek commented Jun 11, 2022

When it is not lazy, it causes other lazy values to be forced, notably
OpamStd.Env.list. This is unexpected, and a bit of a problem for people using
Opam as a library because they may want to set additional environment variables
before callin Opam.

Additionally, I'd expect this to be faster.

When it is not lazy, it causes other lazy values to be forced, notably
`OpamStd.Env.list`. This is unexpected, and a bit of a problem for people using
Opam as a library because they may want to set additional environment variables
before callin Opam.

Additionally, I'd expect this to be faster.
Copy link
Member

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

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

Looks good to me but I’ll leave the merge decision to someone with more experience with this specific part of the code

@rjbou rjbou added this to PR in progress in Opam 2.2.0 via automation Jun 27, 2022
@rjbou rjbou added this to the 2.2.0~alpha milestone Jun 27, 2022
@rjbou
Copy link
Collaborator

rjbou commented Jun 27, 2022

good for me!
What are environment variables that you were looking for?

@LasseBlaauwbroek
Copy link
Contributor Author

I have an OCaml program that uses Opam as a library to install a bunch of Coq packages and performs some benchmarks. Some of these Coq packages have Pip dependencies that are not explicitly listed in the Opam package. So I'm installing a separate Python virtualenv for them and adding the virtualenv to PATH. (Yes, I know it is a hack :-( )

@rjbou rjbou merged commit 954e4c5 into ocaml:master Jun 27, 2022
Opam 2.2.0 automation moved this from PR in progress to Done Jun 27, 2022
@LasseBlaauwbroek LasseBlaauwbroek deleted the lazy-global-allowed branch June 27, 2022 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Opam 2.2.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants