Binary mode when writing to stdout on Windows

[jonahbeckford]

TLDR: Windows should have the binary mode set, but that fix causes other issues.

Context

I'm using Dune in an unusual way that is exposing this ppxlib bug. Basically, since I want to use PPX-es with Dune from a bytecode-only environment, and because Dune requires a C compiler to build a (staged_pps) or (pps) driver, I have to resort to:

 (preprocess
  (action
   (run %{lib:TheNameOfTheLibrary:ppx.exe} -as-pp %{input-file})))

That in turn runs -as-pp which writes to stdout.

So:

# Works
Y:\source\dksdk-type\tests\_std\lib\DkSDKCoder_PPX\ppx.exe -as-pp .\tests\_std\expression.ml -o a.pp.ml

# Fails
Y:\source\dksdk-type\tests\_std\lib\DkSDKCoder_PPX\ppx.exe -as-pp .\tests\_std\expression.ml
Caml1999M031Fatal error: exception Failure("output_value: not a binary channel")
Raised by primitive operation at Stdlib.output_value in file "stdlib.ml", line 386, characters 26-54
Called from Ppxlib__Utils.Ast_io.write.(fun) in file "src/utils.ml", line 187, characters 8-34
Called from Ppxlib__Driver.process_file in file "src/driver.ml", line 1169, characters 6-159
Called from Ppxlib__Driver.standalone in file "src/driver.ml", line 1541, characters 9-27
Re-raised at Location.report_exception.loop in file "parsing/location.ml", line 938, characters 14-25
Called from Ppxlib__Driver.standalone in file "src/driver.ml", line 1544, characters 4-59
Called from Dune__exe___ppx in file ".ppx/6241d9fed22cd957d799b2636a4b50f0/_ppx.ml-gen", line 1, characters 9-36

Initial Attempt. OUTDATED and FIXED.

This now works. metapp fails if the ocamlfind version number of ppxlib is not exactly MAJOR.MINOR.PATCH

I tried to fix it myself in https://github.com/ocaml-ppx/ppxlib/pull/465/files but was unsuccessful. I now get:

File "metapp/dune", line 3, characters 14-30:
3 |   (preprocess (pps metapp.ppx))
                  ^^^^^^^^^^^^^^^^
(cd _build/default && .ppx/e7c203cacf1cd3bb02e6242329effa31/ppx.exe --cookie "library-name=\"metapp\"" -o metapp/metapp.pp.mli --intf metapp/metapp.mli -corrected-suffix .ppx-corrected -diff-cmd - -dump-ast)
Fatal error: exception Dynlink.Error (Dynlink.Library's_module_initializers_failed "File \"version_info/metapp_version_info.ml\", line 9, characters 9-15: Assertion failed")
Raised at Metapp_version_info.ppxlib_version in file "version_info/metapp_version_info.ml", line 9, characters 9-21
Called from Dynlink.Native.run.(fun) in file "otherlibs/dynlink/native/dynlink.ml", line 85, characters 12-29
Called from Dynlink.Native.run.(fun) in file "otherlibs/dynlink/native/dynlink.ml", line 85, characters 12-29
Re-raised at Dynlink.Native.run.(fun) in file "otherlibs/dynlink/native/dynlink.ml", line 87, characters 10-149
Called from Stdlib__List.iter in file "list.ml", line 110, characters 12-15
Called from Dynlink_common.Make.load.(fun) in file "otherlibs/dynlink/dynlink_common.ml", line 337, characters 13-44
Called from Stdlib__List.iter in file "list.ml", line 110, characters 12-15
Called from Dynlink_common.Make.load in file "otherlibs/dynlink/dynlink_common.ml", line 335, characters 8-240
Re-raised at Dynlink_common.Make.load in file "otherlibs/dynlink/dynlink_common.ml", line 345, characters 8-17
Called from Dynlink_common.Make.loadfile in file "otherlibs/dynlink/dynlink_common.ml" (inlined), line 347, characters 26-45
Called from Metapp_ppx.Options.handle.add_object_file in file "ppx/metapp_ppx.ml", line 44, characters 10-38
Called from Stdlib__List.filter_map.aux in file "list.ml", line 258, characters 14-17
Called from Metapp_ppx.Options.handle in file "ppx/metapp_ppx.ml", line 50, characters 30-106
Called from Stdlib__Option.bind in file "option.ml" (inlined), line 22, characters 53-56
Called from Metapp_ppx.transform.Metaopt.map.(fun) in file "ppx/metapp_ppx.ml", line 365, characters 12-69
Called from Ppxlib_ast__Ast_helper_lite.protect_ref.(fun) in file "ast/ast_helper_lite.ml", line 44, characters 10-14
Re-raised at Ppxlib_ast__Ast_helper_lite.protect_ref.(fun) in file "ast/ast_helper_lite.ml", line 50, characters 8-15
Called from Stdlib__List.map in file "list.ml", line 92, characters 20-23
Called from Stdlib__List.map in file "list.ml", line 92, characters 32-39
Called from Metapp_ppx.transform in file "ppx/metapp_ppx.ml", line 382, characters 10-26
Called from Ppxlib__Driver.Transform.register.(fun) in file "src/driver.ml", line 190, characters 61-72
Called from Ppxlib__Driver.apply_transforms.(fun) in file "src/driver.ml", line 574, characters 20-28
Called from Stdlib__List.fold_left in file "list.ml", line 121, characters 24-34
Called from Ppxlib__Driver.apply_transforms in file "src/driver.ml", line 551, characters 6-1023
Called from Ppxlib__Driver.map_signature_gen in file "src/driver.ml", line 785, characters 4-336
Called from Ppxlib__Driver.process_ast in file "src/driver.ml", line 1066, characters 10-127
Called from Ppxlib__Driver.process_file in file "src/driver.ml", line 1120, characters 15-111
Called from Ppxlib__Driver.standalone in file "src/driver.ml", line 1541, characters 9-27
Re-raised at Location.report_exception.loop in file "parsing/location.ml", line 938, characters 14-25
Called from Ppxlib__Driver.standalone in file "src/driver.ml", line 1544, characters 4-59
Called from Dune__exe___ppx in file ".ppx/e7c203cacf1cd3bb02e6242329effa31/_ppx.ml-gen", line 1, characters 9-36

Second Attempt. MOVED TO MERLIN

Edit: Now at https://github.com/ocaml/merlin/issues/1724

It gets past the initial problem so https://github.com/ocaml-ppx/ppxlib/pull/465 looks fine. Now there is a new problem:

[Error - 4:50:25 PM] Request textDocument/codeAction failed.
  Message: uncaught exception
  Code: -32603 
[object Object]
detached: /-----------------------------------------------------------------------
| Internal error: Uncaught exception.
| Invalid_argument("Marshal.from_bytes")
| Raised at Stdlib.invalid_arg in file "stdlib.ml", line 30, characters 20-45
| Called from Stdlib__Marshal.from_string in file "marshal.ml" (inlined), line 67, characters 2-46
| Called from Merlin_kernel__Pparse.decode_potential_ast in file "src/ocaml/driver/pparse.ml", line 200, characters 14-47
| Called from Merlin_kernel__Mpipeline.process in file "src/kernel/mpipeline.ml", line 253, characters 10-120
| Called from CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 31, characters 17-27
| Re-raised at CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 36, characters 4-11
| Called from Merlin_kernel__Mpipeline.timed_lazy in file "src/kernel/mpipeline.ml", line 17, characters 10-22
| Re-raised at Merlin_kernel__Mpipeline.timed_lazy in file "src/kernel/mpipeline.ml", line 19, characters 34-49
| Called from CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 31, characters 17-27
| Re-raised at CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 36, characters 4-11
| Called from Merlin_kernel__Mpipeline.input_source in file "src/kernel/mpipeline.ml", line 111, characters 25-46
| Called from Merlin_kernel__Mpipeline.with_pipeline.(fun) in file "src/kernel/mpipeline.ml", line 115, characters 39-55
| Called from Stdlib__Fun.protect in file "fun.ml", line 33, characters 8-15
| Re-raised at Stdlib__Fun.protect in file "fun.ml", line 38, characters 6-52
| Called from Merlin_kernel__Mocaml.with_state in file "src/kernel/mocaml.ml", line 18, characters 8-38
| Re-raised at Merlin_kernel__Mocaml.with_state in file "src/kernel/mocaml.ml", line 20, characters 42-53
| Called from Ocaml_lsp_server__Document.Single_pipeline.use.(fun) in file "ocaml-lsp-server/src/document.ml", line 147, characters 22-77
| Called from Stdune__Exn_with_backtrace.try_with in file "otherlibs/stdune/src/exn_with_backtrace.ml", line 9, characters 8-12
| Re-raised at Stdune__Exn.raise_with_backtrace in file "otherlibs/stdune/src/exn.ml" (inlined), line 38, characters 27-56
| Called from Stdune__Exn_with_backtrace.reraise in file "otherlibs/stdune/src/exn_with_backtrace.ml", line 20, characters 33-71
| Called from Fiber__Core.O.(>>|).(fun) in file "fiber/src/core.ml", line 250, characters 36-41
| Called from Fiber__Scheduler.exec in file "fiber/src/scheduler.ml", line 73, characters 8-11
\-----------------------------------------------------------------------

Pulling out my hair trying to find if this is a writing problem (ppxlib?) or a reading problem (merlin).

I fixed what I could in https://github.com/ocaml/merlin/pull/1723 but still there is an open problem with either ppxlib or merlin here.

[pitag-ha]

Thanks a lot, @jonahbeckford , both for the issue, and for already starting to fix it! The error you're getting now seems specific to the metapp PPX, doesn't it? Do you also get it when using other PPXs?

[jonahbeckford]

The error you're getting now seems specific to the metapp PPX, doesn't it? Do you also get it when using other PPXs?

Oops. ~I didn't update this ticket after I found the root cause.~ I did update this ticket, but I didn't make the update prominent. Fixing that now.

Root cause for metapp: it does not like local development of ppxlib since it parses the findlib reported ppxlib version and expects MAJOR.MINOR.PATH. With a local ppxlib findlib reports a git commit id based version number.

[jonahbeckford]

In the description I posted the next problem I encountered. It might belong elsewhere, but AFAIK there is a 50/50 chance it is a ppxlib issue (perhaps addressed by your PR comments). For visibility:

I fixed what I could in https://github.com/ocaml/merlin/pull/1723 but still there is an open problem with either ppxlib or merlin here.

It would make things much easier/quicker if someone who knew ppxlib could verify that the marshalled AST is being written correctly on Windows. Perhaps a test case with a known PPX that used -dump-ast or -as-pp? I realize that many people don't have access to a Windows machine, so this may not be possible.

[pitag-ha]

From the ppxlib side, this is fixed once https://github.com/ocaml-ppx/ppxlib/pull/465 is merged, right? The other two problems seem to be on metapp and merlin IIUC. Btw, have you opened an issue on metapp as well?

[jonahbeckford]

From the ppxlib side, this is fixed once #465 is merged, right? The other two problems seem to be on metapp and merlin IIUC. Btw, have you opened an issue on metapp as well?

I don't consider metapp to have a bug; it is just unusual. Yes, the two merlin issues have been opened and are closed/being-reviewed.

[NathanReb]

I guess we can close this now that #465 has been merged. Do do you confirm?

[jonahbeckford]

Closing. Thanks!