Skip to content

Allow make promote-menhir from Windows#12308

Merged
dra27 merged 1 commit intoocaml:trunkfrom
dra27:menhir-from-windows
Jun 17, 2023
Merged

Allow make promote-menhir from Windows#12308
dra27 merged 1 commit intoocaml:trunkfrom
dra27:menhir-from-windows

Conversation

@dra27
Copy link
Copy Markdown
Member

@dra27 dra27 commented Jun 16, 2023

On the occasions in the past when I've had to run make promote-menhir I've clearly been lazy and done it from Linux 😉

There were two problems which are fixed here:

  • menhir --suggest-menhirLib returns a CRLF string on Windows, and the \r was causing problems. The best fix is to use $(shell instead of backticks, both because it stops menhir being called twice and also because $(shell ) automatically strips the line endings.
  • Menhir's --ocamlc needs to be passed a Windows path (i.e. with backslashes) because it gets passed to Sys.command. The necessary machinery is added to convert the slashes when running from Windows.

Copy link
Copy Markdown
Member

@gasche gasche left a comment

Choose a reason for hiding this comment

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

Okay, okay. As long as it keeps working on Linux, these small-scale horrors are just fine.

@gasche
Copy link
Copy Markdown
Member

gasche commented Jun 16, 2023

The CI fails on the "extra (debug)" configuration, with the following failure for the test weak-ephe-final/finaliser_handover in bytecode:

[02] file runtime/domain.c; line 1749 ### Assertion failed: caml_gc_phase == Phase_sweep_and_mark_main

This is clearly unrelated to the present PR, but also probably a bug. I wonder who could be interested in looking at this. Maybe @NickBarnes?

@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Jun 17, 2023

Just adding the link to the full log. I think this may also be one @jmid and @shym have been seeing?

@dra27
Copy link
Copy Markdown
Member Author

dra27 commented Jun 17, 2023

Okay, okay. As long as it keeps working on Linux, these small-scale horrors are just fine.

I may start haggling on the definition of “small-scale” at some point! 🙂

@dra27 dra27 merged commit 92c4a31 into ocaml:trunk Jun 17, 2023
@dra27 dra27 deleted the menhir-from-windows branch June 17, 2023 07:40
@jmid
Copy link
Copy Markdown
Member

jmid commented Jun 19, 2023

Just adding the link to the full log. I think this may also be one @jmid and @shym have been seeing?

I don't believe we have seen that particular assertion fail before.
It may however be due to the same underlying issue manifesting itself as different assertion failures...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants