graphql_ppx icon indicating copy to clipboard operation
graphql_ppx copied to clipboard

Ast mapper version is not compat with bs-platform v6

Open Coobaha opened this issue 6 years ago • 8 comments

Hi @mhallin,

I've tried to use new bs-platform and it's not compat with graphql_ppx

bsb output is

Failure("Ast_mapper: OCaml version mismatch or malformed input")
File "~/demo/src/Demo.re", line 1:
Error: Error while running external preprocessor
Command line: ~/demo/node_modules/graphql_ppx/ppx '/var/folders/7_/7cxq24hd50vdm4xvrwjl959wx8xlpw/T/camlppxc7a198' '/var/folders/7_/7cxq24hd50vdm4xvrwjl959wx8xlpw/T/camlppx72e796'

My package.json

  "devDependencies": {
    "bs-platform": "6.0.0"
  },
  "dependencies": {
    "graphql_ppx": "^0.2.8"
  }

Coobaha avatar Apr 13 '19 08:04 Coobaha

https://github.com/mhallin/graphql_ppx/blob/master/src/bucklescript/graphql_ppx.ml#L1-L2

We're doing this conversion to downgrade to 402. I'm wondering if it's a matter of dropping it or changing to OCaml_406

baransu avatar Apr 13 '19 14:04 baransu

Ok. It's not that simple 😄

baransu avatar Apr 13 '19 14:04 baransu

It looks to me as though no code changes are necessary. The executable probably just needs to be compiled against OCaml 4.06. It's using OCaml-migrate-parsetree so it should just work.

anmonteiro avatar Apr 18 '19 10:04 anmonteiro

I just verified my assumption that no code changes are necessary.

I built the PPX with OCaml 4.06 and copied the binary to my node_modules in a project that depends on BuckleScript 6.X.

graphql_ppx works as expected in such scenario.

anmonteiro avatar Apr 18 '19 11:04 anmonteiro

@anmonteiro DO you think there is a sane way to have both 5.X and 6.X support? From what I tested when compiling with OCaml 4.06 it fails with 5.X. I assume we have to switch in CI and build two versions of every binary for both 5.X and 6.X.

baransu avatar Apr 19 '19 21:04 baransu

@baransu yeah, we are always going to need 2 binaries. The best alternative I see is to do what I recently did for the ReasonReact repo (https://github.com/reasonml/reason-react/pull/392) and use 2 esy files, one for each version of the OCaml compiler

anmonteiro avatar Apr 19 '19 22:04 anmonteiro

@anmonteiro That's nice. Thanks!

baransu avatar Apr 19 '19 22:04 baransu

It's possible to have one binary. I am doing that in Bisect_ppx, using a patched ocaml-migrate-parsetree (which I should probably just release).

aantron avatar Jan 31 '20 14:01 aantron