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

Set OCAMLTOP_INCLUDE_PATH (as in ocaml.5.3.*) #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dra27
Copy link
Owner

@dra27 dra27 commented Jan 28, 2025

Extension of the setting of OCAMLTOP_INCLUDE_PATH which was trialled with ocaml#26594.

4.08.0 is the first version of the compiler supporting this environment variable. Updating the remaining packages mitigates the issue seen in dbuenzli/topkg#142 and reported in ocaml#25819 (comment).

Opened here for review, as the revdeps testing impact is obviously quite large...

[OCAML_TOPLEVEL_PATH = "%{toplevel}%"]
]
x-env-path-rewrite: [
[CAML_LD_LIBRARY_PATH (";" {os = "win32"} ":" {os != "win32"}) "target"]
[OCAMLTOP_INCLUDE_PATH (";" {os = "win32"} ":" {os != "win32"}) "target"]

Choose a reason for hiding this comment

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

Any reason why you don't do this for OCAML_TOPLEVEL_PATH ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's not a PATH-like variable - it's only ever set using =

Choose a reason for hiding this comment

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

Ah, I see :–)

@dra27
Copy link
Owner Author

dra27 commented Jan 28, 2025

Unfortunately this does also trigger a rebuild of the ocaml package

@dra27
Copy link
Owner Author

dra27 commented Jan 28, 2025

cc @mseri, @raphael-proust, @shonfeder

@dbuenzli
Copy link

That's not really different from a dune update :–)

@raphael-proust
Copy link

looks ok to me (although I don't have time to try to reproduce the conditions in which this is an issue)

OCAML_TOPLEVEL_PATH is left for now as ocamlfind's stub still uses it.
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