feat(oxcaml): add support for OxCaml testing suite

[maiste]

This PR introduces some support to OxCaml testing inside of Dune. To do so, it provides three different things:

• It adds a variable, %{oxcaml_supported} to enable test in such case.
• It updates the OCcaml_toolchain.t to support a is_oxcaml_supported and is_parametrized_library_supported functions. It allows building extra functionalities related to OxCaml and separate the compiler variant (needed for the variable) and the functionality (needed for a specific functionality).
• It introduces the test suite for OxCaml and only activates it when detecting the correct compiler (OxCaml one) using the new Dune variable.
• It adds a GitHub Actions to build this test suite with an OxCaml compiler.

The action is pinned to a specific commit to prevent it from breaking because of a change in the JaneStreet repository.

[art-w]

Very nice! Only some small nitpicks to clean up the PR :)

[maiste]

@art-w I have rebased, you should be able to check again.
@rgrinberg what do you think? Would it be OK to include this part? We will open further PRs for the various functionalities.

[maiste]

@rgrinberg I encounter the problem where if I develop a new feature and guard it with the next release version, I can't use it inside of Dune until this version is released. Should I add the guard to the current version deployed version (3.19) or do you think I should look at the solution we discussed to have dune use the latest version internally?

[rgrinberg]

Should I add the guard to the current version deployed version (3.19) or do you think I should look at the solution we discussed to have dune use the latest version internally?

Thinking about the problem again, I think this isn't going to work because the issue is our CI. We are building the dune workspace with the version of dune coming from opam (is that right? this needs confirmation). We should just not do that and always use the bootstrapped version of dune. If we do that, no hacks will be necessary and you'll be able to upgrade dune itself to 3.20 as soon as you introduce it.

[maiste]

@rgrinberg with the help of @Alizter (thanks again 🙏), I have managed to introduce the extension as you suggested. This PR is ready for another round of review.

[rgrinberg]

Shouldn't you be checking this against the oxcaml extension? I think you will need to extend Since to be more generic to support that.

[maiste]

As it is nor possible to check against the extension here (because of dependency cycle problems) the code for checking this was moved to the macro evaluation. It also helps to limit the spreading of OxCaml extension to the rest of the code. It is modelled the same way as what is done for rocq (coq). Hence, this is not necessary to add extra checks here.

[rgrinberg]

I have no idea how a cycle comes into play here. It seems like it should be trivial to move the definition of the extension to avoid it.

the code for checking this was moved to the macro evaluation

I don't really understand this.

It also helps to limit the spreading of OxCaml extension to the rest of the code

At the cost of making the implementation inconsistent and buggy wrt the rest of dune.

It is modelled the same way as what is done for rocq (coq). Hence, this is not necessary to add extra checks here.

This is not the example we should follow. The coq extension is maintained exclusively by the coq folks and we don't control their quality.

[maiste]

The problem I encountered is: to use extensions, I need to deal with the Dune_project module because this is where the information about extensions are stored. However Dune_project already depends on pform so I can't do anything there without creating a cycle. The idea was to move the check to where all of this would be instantiated, more particularly when we are expanding the values. At this point of the code, we know everything is instantiated.

You made me realized it doesn't need to be a macro as I can also get the Dune_project.t from the Var expander.

[maiste]

@rgrinberg I have moved it back to the variable and port the check directly inside the expander. Would it be better this way?

[rgrinberg]

I think your test should demonstrate usage of this variable and the error message that you get when oxcaml isn't enabled.

[maiste]

535e83b demonstrates the features with and without the extension.

[rgrinberg]

I have no idea how a cycle comes into play here. It seems like it should be trivial to move the definition of the extension to avoid it.

the code for checking this was moved to the macro evaluation

I don't really understand this.

It also helps to limit the spreading of OxCaml extension to the rest of the code

At the cost of making the implementation inconsistent and buggy wrt the rest of dune.

It is modelled the same way as what is done for rocq (coq). Hence, this is not necessary to add extra checks here.

This is not the example we should follow. The coq extension is maintained exclusively by the coq folks and we don't control their quality.

[art-w]

Looks good! Thanks :)

[rgrinberg]

Some background: before dune 3.0, all percent forms were dynamically resolved during evaluation. This was a very painful situations because it meant that users could write junk in a section of their dune file, never evaluate the said section, and be unaware that their dune file is invalid.

In 3.0, we decided to change our frontend to resolve percent forms during parsing. This way, any invalid percent form would trigger an error when the dune file is first loaded. This indeed was a much better experience, but we left some escape hatches for backwards compatibility, and we never made it general enough to work with language extensions.

I pushed a commit that moves the validation of this percent form to the parser. This way, even if you don't hit the branch that evaluates the form, you should still get the error that the dune file is invalid. There might be some polish lacking, but this is the general direction to take.

[maiste]

Indeed, I was lacking the context and thought you wanted to have the expander fail faster. Thank you for the commit and the explanation, it is clearer to me.

I will extract your commit and split this PR in two to make smaller and more targeted PRs.

[maiste]

I have extracted the commit to another branch. The other one must be merged first. Then, I'll rebase this one on main to add the specific OxCaml functionalities.