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

Modules opened in the wrong order? #1900

Open
vouillon opened this issue Feb 21, 2025 · 4 comments · Fixed by ocaml/dune#11503
Open

Modules opened in the wrong order? #1900

vouillon opened this issue Feb 21, 2025 · 4 comments · Fixed by ocaml/dune#11503
Assignees
Labels

Comments

@vouillon
Copy link
Member

I get an error when I edit file link.ml even though Dune compiles it just fine.
Image

When compiling the library Wasm_of_ocaml_compiler which contains this file, I also open the library Js_of_ocaml_compiler. Both libraries contain a file generate.ml.

(library
 (name wasm_of_ocaml_compiler)
 (libraries js_of_ocaml_compiler)
 (flags
  (:standard -w -7-37 -safe-string -open Js_of_ocaml_compiler))
[...]

I suspect what happens is that merlin and dune do not perform the opens in the same order.

Adding an explicit open flag, even in the wrong order, seems to work around the issue.

 (flags
  (:standard -w -7-37 -safe-string -open Wasm_of_ocaml_compiler -open Js_of_ocaml_compiler))

I have taken a dump of the configuration.

@voodoos
Copy link
Collaborator

voodoos commented Feb 24, 2025

Thanks for the report Jérôme. That's indeed very suspicious.
I will try to find some time to investigate, meanwhile, do you think you could craft a smaller reproduction ?

@vouillon
Copy link
Member Author

  1. unzip merlin-bug.zip
  2. cd merlin-bug
  3. dune exec -- ./main.exe
  4. Open main.ml in your editor
    Image

voodoos added a commit to voodoos/dune that referenced this issue Feb 25, 2025
voodoos added a commit to voodoos/dune that referenced this issue Feb 25, 2025
voodoos added a commit to voodoos/merlin that referenced this issue Feb 25, 2025
voodoos added a commit to voodoos/merlin that referenced this issue Feb 25, 2025
@voodoos
Copy link
Collaborator

voodoos commented Feb 25, 2025

Thanks a lot, I added your example to the testsuite and proposed a change in Dune. The ordering is indeed wrong in the generated configuration. (but not in Merlin's treatment of the ordering)

@voodoos voodoos self-assigned this Feb 25, 2025
@vouillon
Copy link
Member Author

Thanks !

voodoos added a commit to voodoos/dune that referenced this issue Feb 27, 2025
voodoos added a commit to voodoos/dune that referenced this issue Feb 27, 2025
voodoos added a commit to voodoos/dune that referenced this issue Feb 27, 2025
Leonidas-from-XIV pushed a commit to voodoos/dune that referenced this issue Feb 28, 2025
Leonidas-from-XIV pushed a commit to ocaml/dune that referenced this issue Feb 28, 2025
* Preserve flags ordering in Merlin configuration files.

Fixes ocaml/merlin#1900

Signed-off-by: Ulysse Gérard <[email protected]>

* Promote expected changes in the testsuite

Signed-off-by: Ulysse Gérard <[email protected]>

* Make flag treatment clearer

Signed-off-by: Ulysse Gérard <[email protected]>

* Add changelog entry for #11503

Signed-off-by: Ulysse Gérard <[email protected]>

* Use dedicated filter opt function

Signed-off-by: Ulysse Gérard <[email protected]>

---------

Signed-off-by: Ulysse Gérard <[email protected]>
voodoos added a commit to voodoos/merlin that referenced this issue Feb 28, 2025
voodoos added a commit to voodoos/merlin that referenced this issue Mar 3, 2025
voodoos added a commit that referenced this issue Mar 3, 2025
Co-authored-by: Jerome.Vouillon <[email protected]>
Leonidas-from-XIV pushed a commit to Leonidas-from-XIV/dune that referenced this issue Mar 6, 2025
* Preserve flags ordering in Merlin configuration files.

Fixes ocaml/merlin#1900

Signed-off-by: Ulysse Gérard <[email protected]>

* Promote expected changes in the testsuite

Signed-off-by: Ulysse Gérard <[email protected]>

* Make flag treatment clearer

Signed-off-by: Ulysse Gérard <[email protected]>

* Add changelog entry for ocaml#11503

Signed-off-by: Ulysse Gérard <[email protected]>

* Use dedicated filter opt function

Signed-off-by: Ulysse Gérard <[email protected]>

---------

Signed-off-by: Ulysse Gérard <[email protected]>
maiste added a commit to maiste/opam-repository that referenced this issue Mar 31, 2025
CHANGES:

### Fixed

- Support HaikuOS: don't call `execve` since it's not allowed if other pthreads
  have been created. The fact that Haiku can't call `execve` from other threads
  than the principal thread of a process (a team in haiku jargon), is a
  discrepancy to POSIX and hence there is a [bug about
  it](https://dev.haiku-os.org/ticket/18665). (@Sylvain78, ocaml/dune#10953)
- Fix flag ordering in generated Merlin configurations (ocaml/dune#11503, @voodoos, fixes
  ocaml/merlin#1900, reported by @vouillon)

### Added

- Add `(format-dune-file <src> <dst>)` action. It provides a replacement to
  `dune format-dune-file` command.  (ocaml/dune#11166, @nojb)
- Allow the `--prefix` flag when configuring dune with `ocaml configure.ml`.
  This allows to set the prefix just like `$ dune install --prefix`. (ocaml/dune#11172,
  @rgrinberg)
- Allow arguments starting with `+` in preprocessing definitions (starting with
  `(lang dune 3.18)`). (@amonteiro, ocaml/dune#11234)
- Support for opam `(maintenance_intent ...)` in dune-project (ocaml/dune#11274, @art-w)
- Validate opam `maintenance_intent` (ocaml/dune#11308, @art-w)
- Support `not` in package dependencies constraints (ocaml/dune#11404, @art-w, reported
  by @hannesm)

### Changed

- Warn when failing to discover root due to reads failing. The previous
  behavior was to abort. (@KoviRobi, ocaml/dune#11173)
- Use shorter path for inline-tests artifacts. (@hhugo, ocaml/dune#11307)
- Allow dash in `dune init` project name (ocaml/dune#11402, @art-w, reported by @saroupille)
- On Windows, under heavy load, file delete operations can sometimes fail due to
  AV programs, etc. Guard against it by retrying the operation up to 30x with a
  1s waiting gap (ocaml/dune#11437, fixes ocaml/dune#11425, @MSoegtropIMC)
- Cache: we now only store the executable permission bit for files (ocaml/dune#11541,
  fixes ocaml/dune#11533, @ElectreAAS)
- Display negative error codes on Windows in hex which is the more customary
  way to display `NTSTATUS` codes (ocaml/dune#11504, @MisterDA)
maiste added a commit to maiste/opam-repository that referenced this issue Apr 3, 2025
CHANGES:

### Fixed

- Support HaikuOS: don't call `execve` since it's not allowed if other pthreads
  have been created. The fact that Haiku can't call `execve` from other threads
  than the principal thread of a process (a team in haiku jargon), is a
  discrepancy to POSIX and hence there is a [bug about
  it](https://dev.haiku-os.org/ticket/18665). (@Sylvain78, ocaml/dune#10953)
- Fix flag ordering in generated Merlin configurations (ocaml/dune#11503, @voodoos, fixes
  ocaml/merlin#1900, reported by @vouillon)

### Added

- Add `(format-dune-file <src> <dst>)` action. It provides a replacement to
  `dune format-dune-file` command.  (ocaml/dune#11166, @nojb)
- Allow the `--prefix` flag when configuring dune with `ocaml configure.ml`.
  This allows to set the prefix just like `$ dune install --prefix`. (ocaml/dune#11172,
  @rgrinberg)
- Allow arguments starting with `+` in preprocessing definitions (starting with
  `(lang dune 3.18)`). (@amonteiro, ocaml/dune#11234)
- Support for opam `(maintenance_intent ...)` in dune-project (ocaml/dune#11274, @art-w)
- Validate opam `maintenance_intent` (ocaml/dune#11308, @art-w)
- Support `not` in package dependencies constraints (ocaml/dune#11404, @art-w, reported
  by @hannesm)

### Changed

- Warn when failing to discover root due to reads failing. The previous
  behavior was to abort. (@KoviRobi, ocaml/dune#11173)
- Use shorter path for inline-tests artifacts. (@hhugo, ocaml/dune#11307)
- Allow dash in `dune init` project name (ocaml/dune#11402, @art-w, reported by @saroupille)
- On Windows, under heavy load, file delete operations can sometimes fail due to
  AV programs, etc. Guard against it by retrying the operation up to 30x with a
  1s waiting gap (ocaml/dune#11437, fixes ocaml/dune#11425, @MSoegtropIMC)
- Cache: we now only store the executable permission bit for files (ocaml/dune#11541,
  fixes ocaml/dune#11533, @ElectreAAS)
- Display negative error codes on Windows in hex which is the more customary
  way to display `NTSTATUS` codes (ocaml/dune#11504, @MisterDA)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants