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

Compilation issue with yojson 2.x.x #159

Open
SnarkBoojum opened this issue Aug 15, 2022 · 10 comments
Open

Compilation issue with yojson 2.x.x #159

SnarkBoojum opened this issue Aug 15, 2022 · 10 comments

Comments

@SnarkBoojum
Copy link

If one compiles a more recent yojson (2.0.2 here), then use it against ocaml-atd 2.9.1, then the resulting lib breaks elpi:

File "src/elpi_trace_elaborator.ml", line 668, characters 14-16:
668 |   write_trace ob cards;
                    ^^
Error: This expression has type Bi_outbuf.t
       but an expression was expected of type Buffer.t

because indeed in the 2.x.x series, yojson doesn't use biniou anymore, so Bi_outbuf isn't there anymore.

I noticed it because I'm looking into updating yojson in Debian, so I could find everything that breaks and report to the various upstreams.

I'll try to provide a patch.

@xgqt
Copy link

xgqt commented Oct 23, 2022

I just ran into this issue on Gentoo.

@SnarkBoojum have you had time to see if this can be patched?

@SnarkBoojum
Copy link
Author

I have no clue where write_trace comes from... the three lines can probably replaced with something like List.map (fun card -> Yojson.Safe.to_channel stdout card) cards ; but first cards needs to be turned into Yojson.Safe.t and I don't know how.

@SnarkBoojum
Copy link
Author

(now that I think of it, write_trace is probably from trace.atd)

@SnarkBoojum
Copy link
Author

Here is a first draft ; some output is different so it breaks some tests, but it doesn't look insane:

--- elpi.orig/src/elpi_trace_elaborator.ml
+++ elpi/src/elpi_trace_elaborator.ml
@@ -664,6 +664,6 @@
 
   let cards = Trace.cards steps ~stack_frames ~aggregated_goal_success ~goal_text ~goal_attempts in
 
-  let ob = Bi_outbuf.create_channel_writer stdout in
-  write_trace ob cards;
-  Bi_outbuf.flush_channel_writer ob
+  let buf = Buffer.create 1000 in
+  write_trace buf cards;
+  Buffer.output_buffer stdout buf

@gares
Copy link
Contributor

gares commented Oct 24, 2022

some output is different so it breaks some tests

how? (not that I did a release on Friday since the test suite was broken)

@SnarkBoojum
Copy link
Author

Well, using a different yojson means pretty printing has some slight differences ; for example:

--- elpi.orig/tests/sources/trace.elab.json
+++ elpi/tests/sources/trace.elab.json
@@ -271,8 +271,7 @@
                 }
               ],
               "events": [
-                [ "Assign", "A0 := 1" ],
-                [ "Assign", "A1 := 2 + 3" ]
+                [ "Assign", "A0 := 1" ], [ "Assign", "A1 := 2 + 3" ]
               ]
             },
             "siblings": [ { "goal_text": "calc (2 + 3) 1", "goal_id": 8 } ],

@gares
Copy link
Contributor

gares commented Oct 24, 2022

Oh, but this change is more about ydump than your patch. Maybe it has a flag to stick to the old format?

@gares
Copy link
Contributor

gares commented Oct 24, 2022

(yojson prints a single line, I then run ydump on it to make the test reference readable)

@SnarkBoojum
Copy link
Author

It's not backward-compatible ; in fact I'm using yojson 2.0.2 and atd 2.10.0.

@SnarkBoojum
Copy link
Author

I added a commit to tighten the dependency on atd.

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

No branches or pull requests

3 participants