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

OpamFilter: handle converted variables correctly when no_undef_expand is true #4811

Merged
merged 2 commits into from
Sep 13, 2021

Conversation

timbertson
Copy link
Contributor

@timbertson timbertson commented Aug 26, 2021

Currently it returns the unconverted value when no_undef_expand is true, which means that lwt:enable gets rendered as "true"/"false", not "enable" / "disable". This PR applies conversions if there is some value which can be cast to a boolean.

I don't think this affects end users at all, I discovered this behaviour by using OpamFilter.expand_string ~partial:true directly.

@rjbou rjbou added this to PR in progress in Opam 2.2.0 via automation Aug 30, 2021
Opam 2.2.0 automation moved this from PR in progress to PR finalised Aug 30, 2021
@rjbou rjbou requested a review from AltGr August 30, 2021 17:12
@rjbou rjbou moved this from PR finalised to PR to review in Opam 2.2.0 Aug 30, 2021
@rjbou rjbou added this to the 2.2.0~alpha milestone Sep 2, 2021
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.

Well spotted, thanks !

For a bit of context, partial evaluation is used first for all user-variables, but some variables remain to be resolved later because their value depend on the context (e.g. the post flag needs to be manipulated by the solver). I guess this didn't break things because we always end up with a non-partial resolution, but it may have led to extra work.

Opam 2.2.0 automation moved this from PR to review to PR finalised Sep 2, 2021
@timbertson
Copy link
Contributor Author

Yep, that makes sense. In my case I'm (attempting) to use this to half-resolve build + install instructions, for opam2nix.

That is, things like foo:enabled will be resolved and baked in as simple strings because that information is known from the result of a solve, but prefix and other path variables can't be known until build-time. When it comes to actually building, I'm using a more primitive variable substitution to resolve all the remaining path-related variables.

Hopefully that's a reasonable (if uncommon) use of this feature.

@rjbou rjbou merged commit ec3f9ca into ocaml:master Sep 13, 2021
Opam 2.2.0 automation moved this from PR finalised to Done Sep 13, 2021
@rjbou
Copy link
Collaborator

rjbou commented Sep 13, 2021

Thanks!

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