Check for parameterised libraries from the compiler config#12393
Check for parameterised libraries from the compiler config#12393rgrinberg merged 1 commit intoocaml:mainfrom
Conversation
272e435 to
bc5330b
Compare
|
I think this isn't directly related to #12236, this PR checks for a specific feature while the latter checks for a compiler branch. |
art-w
left a comment
There was a problem hiding this comment.
Looks good! We are hoping that the parameterized feature gets upstream to OCaml one day, so this check is more future proof instead of relying on detecting the OxCaml compiler :)
|
When OxCaml features are upstreamed, are the config names ever changed? That would be my only worry with this. |
| || Stdune.String.is_suffix ~suffix:ox version | ||
| ;; | ||
|
|
||
| let supports_parametrized_library version = |
There was a problem hiding this comment.
Why did you change the spelling of parametrized?
There was a problem hiding this comment.
I guess this is the spelling they have upstream in oxcaml? In that case, this is the spelling we should use.
There was a problem hiding this comment.
Indeed, parameterised_modules is the spelling in OxCaml.
shonfeder
left a comment
There was a problem hiding this comment.
LGTM! Seems like a nice clean improvement.
IIUC, there are no objections to this PR and it can be merged as soon as the merge conflicts are resolved.
Signed-off-by: Sudha Parimala <sudharg247@gmail.com>
bc5330b to
12434bb
Compare
* 'main' of github.com:/ocaml/dune: (946 commits) refactor(boot): some helpers for command line flags (ocaml#12375) refactor(boot): move file exists check to one place (ocaml#12395) Check for parameterised libraries from the compiler config (ocaml#12393) Update test/blackbox-tests/test-cases/ocaml-config-ox.t Update test/blackbox-tests/test-cases/oxcaml/ocaml-config-ox-true.t Add changes Turn the %{ocaml-config:ox} test into one file, and add a +ve test case Clarify variable vars in ocaml_config.ml Expose the ocamlc -config parameter "ox" doc: changing heading levels for sub-sections (ocaml#12389) chore(deps): bump actions/checkout from 4 to 5 (ocaml#12380) refactor(boot): remove pointless [assert false] (ocaml#12378) refactor(boot): mangling simplification (ocaml#12377) refactor(boot): move ocaml-config bindings (ocaml#12376) refactor(boot): simplify blake3 flag handling (ocaml#12379) refactor(boot): use [Module_name.t] in ocamldep parsing (ocaml#12373) refactor(boot): replace concat + filter_map with concat_map (ocaml#12374) chore: move tests to dune-init move test to virtual-libraries chore: move test to directory-targets ...
Changes the check for parameterised libraries to the flag emitted by the OxCaml compiler, rather than relying on parsing the compiler version string. This way, we rely on the implementation and not the compiler version, as this feature could get upstreamed to ocaml/ocaml. The OxCaml compiler config (
ocamlc -config) emitsparameterised_modules: true, which is being checked to detect whether parameterised libraries are available.