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

Windows: Fix 'output_value: not a binary channel' when writing the AST to stdout #57

Merged

Conversation

bryphe
Copy link
Contributor

@bryphe bryphe commented Dec 18, 2018

Issue: I was building the latest ppx_deriving_yojson on Windows, and hit this error:

  # esy-build-package: building: @opam/ppx_deriving_yojson@opam:3.3
    # esy-build-package: pwd: C:\Users\bryph\.esy\3_\b\opam__s__ppx__deriving__yojson-opam__c__3.3-f1cb9869
    # esy-build-package: running: "dune" "build" "-p" "ppx_deriving_yojson" "-j" "4"
         ppxfind src/ppx_deriving_yojson.pp.ml (exit 2)
    (cd _build/default && C:\Users\bryph\.esy\3_\i\opam__s__ppxfind-opam__c__1.2-3fa788d0\bin\ppxfind.exe -legacy ppx_tools.metaquot --as-pp src/ppx_deriving_yojson.ml) > _build/default/src/ppx_deriving_yojson.pp.ml
    Fatal error: exception Failure("output_value: not a binary channel")
    error: command failed: "dune" "build" "-p" "ppx_deriving_yojson" "-j" "4" (exited with 1)
    esy-build-package: exiting with errors above...

  building @opam/ppx_deriving_yojson@opam:3.3@d196be77

Defect: ocaml-migrate-parsetree uses output_value without ensuring the channel is on binary m ode. On OSX/Linux, this is fine, because binary mode is the default. However, on Windows, we need to ensure the channel is in binary mode for either the stdout or file case.

The reason I hit this now is because ppx_deriving_yojson depends on ppxfind in its latest release, which depends on ocaml-migrate-parsetree.

Fix: Call set_binary_mode_out oc true prior to using the marshalling calls. This is a no-op on OSX/Linux, but will ensure the channel is in the correct mode on Windows.

cc @dra27

@bryphe bryphe changed the title Windows: Fix failure on Windows when writing AST to stdout Windows: Fix 'output_value: not a binary channel' when writing the AST to stdout Dec 18, 2018
@ghost
Copy link

ghost commented Dec 18, 2018

Looks good, thanks!

@ghost ghost merged commit 09b37d1 into ocaml-ppx:master Dec 18, 2018
@ghost
Copy link

ghost commented Dec 18, 2018

@bryphe, if you can confirm that this is enough to fix Windows compatibility, I'll prepare a new release.

This pull request was closed.
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.

1 participant