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

[do not merge] test branch for case/cond #8232

Closed
wants to merge 6 commits into from
Closed

Conversation

Alizter
Copy link
Collaborator

@Alizter Alizter commented Jul 19, 2023

This is a test branch for case/cond so @gridbugs can work on top of. I will add these patches to #8229 in due time.

Alizter added 6 commits July 19, 2023 12:02
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
We add a `(case ...)` constructor to dune_lang for actions which when
expanded will reduce down to the matching branch.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
@gridbugs
Copy link
Collaborator

Thanks for sharing this. I'm thinking about whether it's possible to map package constraints to blang expressions so that conditional package actions can represented with cond.

Take this opam file fragment as a motivating example:

build: [
  ["echo" "unconditionally" "runs"]
  ["echo" "only" "runs" "on" "macos"] {os = "macos"}
  ["echo" "only" "runs" "if" "with-test"] {with-test}
]

The conditions are currently represented in dune with the Package_constraint.t type. To implement conditional actions with cond we would need to translate Package_constraint.ts into blangs.

The first constraint ({os = "macos"}) could be represented with the blang (= %{ocaml-config:system} macosx). Some things we'd need to consider:

  • the name for macos in opam is "macos" but dune (by way of ocamlc -config) calls it "macosx"
  • the translation would need to understand mappings between opam environment variables and dune variables.
  • currently we poll the system to determine the values of opam system environment variables (like os) using logic in src/dune_pkg/sys_poll.ml. This logic will differ from the way that dune determines the values of the "corresponding" variables visible to String_with_vars so probably we would need to add new variables to String_with_vars that correspond exactly with the opam variables.

The second constraint ({with-test}) means that the action should only run when the build context specifies that the :with-test flag should be set. In order to translate it to a blang we would need solver flags from the build context to be available in the blang expander (I did a quick skim and it doesn't seem to currently be available - not sure how much work it would be to add). The other issue is that the with-test variable represents a boolean value - not a string - and the only variables in blang are pforms in String_with_vars.t as far as I can tell, and they can only evaluate to strings. We could use sentinal strings for bools like representing true with the string "true" in which case {with-test} would translate to (= %{with-test} true) though it doesn't feel elegant.

It occurs to me that we already have logic for converting Package_constraint.ts into the equivalent types from the opam libraries (we use this to generate opam files for dune projects) so we could probably just do that and then use the evaluator from opam. If we can't do that for whatever reason, the package constraint language is so simple that I expect it will be much less work to implement an evaluator for it than it would be to translate package constraints into blangs.

@Alizter
Copy link
Collaborator Author

Alizter commented Jul 20, 2023

It is possible to add variables to the expander on the fly. This is how bindings for dependencies are done for example. It therefore shouldn't be a problem to define new variables and expand them in a blang. It's all up to the expander argument. Blang expansion happens in dune_rules in Blang_expand.

@emillon
Copy link
Collaborator

emillon commented Jul 20, 2023

I have one question regarding the fact that these forms are evaluated at parse time: is it possible to inspect the various branches in a static way? concretely I'm wondering if dune describe external-lib-deps can "see" under these forms.

@Alizter
Copy link
Collaborator Author

Alizter commented Jul 20, 2023

Having a look at the implementation of external-lib-deps it appears that it only analyses executables, libraries and tests. So probably not.

I think there might be a misconception with how pforms are expanded here however. Roughly it looks like:

  1. Parsing, here we get stuff like blang and string_with_vars
  2. Expansion
  3. Executation

What I was trying to say yesterday was that case/cond don't survive step 2.

@Alizter
Copy link
Collaborator Author

Alizter commented Aug 2, 2023

This is now in #8324

@Alizter Alizter closed this Aug 2, 2023
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.

3 participants