Irmin.Type.decode_json returns error on json-encoded variant types

[gowthamk]

Consider the following example:

open Lwt.Infix                                                                                           
open Irmin_unix
open Printf                                                                                              

type t = Empty | Node of int32 

let t =  
  let open Irmin.Type in                                                                                 
  variant "t" (fun b e -> function
      | Node a  -> b a                                                                                   
      | Empty -> e)                                                                                      
  |~ case1 "Node" int32 (fun x -> Node x)                                                                
  |~ case0 "Empty" Empty 
  |> sealv

let pp_json = Irmin.Type.pp_json ~minify:false t                                                         

let () = 
  let p = Empty in
  let s = Fmt.strf "%a\n" pp_json p in                                                                   
  let _ = printf "%s\n" s in                                                                             
  let decoder = Jsonm.decoder (`String s) in
  let res = Irmin.Type.decode_json t decoder in                                                          
  match res with
    | Ok _ -> printf "Result Ok\n"                                                                       
    | Error (`Msg s) -> printf "Result Error: %s\n" s 

I expect it to print Result OK because s is a json string. But, here's what I get:

$ocamlfind ocamlopt test2.ml -o test2 -package ezjsonm,irmin-unix,lwt.unix -linkpkg -thread
$./test2 
[                                                   
  "Empty"                                           
]                                                   

Result Error: line 1, character 1:                  
Found lexeme `As, but lexeme (`String | `Os) was expected 

Something is broken, or maybe I'm missing something? FYI, my Irmin packages are pinned to the 1.4 branch.

[emillon]

Hi,

It seems that you're parsing the wrong string: s is the output of the json pretty printing operation, so it is "[\n\"Empty\"]\n" (or something similar). 'String s denotes this string, which would explain that it cannot parse it.

I think that you need something like:

`A [`String "Empty"]

(an array with a single element which is the string "Empty"). You can of course call the JSON parser on s rather than writing this AST by hand.

Does that work?

[dinosaure]

Your code works with the current version of irmin.

EDIT: and it's work with 1.4 ... I can not reproduce the bug on my side.

[gowthamk]

Thanks for the reply. I see your point, but I am not sure that would solve the problem in the context it occurs. I use Irmin_unix.Git.FS.KV, which expects a Irmin.Contents.S. Looks like KV indeed uses Irmin.Contents.S.pp to serialize and write values to the disk, because when I give:

let pp = Irmin.Type.pp_json ~minify:false t

I see the following in the corresponding git repo:

$cd /tmp/repos/rbset.git/
$ls -lrt
total 8
-rw-------  1 gkaki  wheel  13 Feb 28 11:39 state
$cat state 
[
  "Empty"
]

Consequently, when I use Irmin.Type.decode_json in Irmin.Contents.S.of_string, I get the same error as above.

If pretty printing does indeed effect json_decode then maybe Irmin_unix.Git.FS.KV should not use Irmin.Contents.S.pp function to serialize and write values to the disk. Perhpas Irmin.Type should could a to_json function that I can pass to KV.

[gowthamk]

Hi @dinosaure,

I (manually) applied the commit 57c74c to the 1.4 branch, so may that's causing the problem? Except that one-line commit, everything comes from the 1.4 branch:

$opam pin list
irmin.1.4.0                       path  /Users/gkaki/git/pavani/Networks/project/ocaml-irmin/irmin
irmin-fs.1.3.0                    path  /Users/gkaki/git/pavani/Networks/project/ocaml-irmin/irmin
irmin-git.1.3.0                   path  /Users/gkaki/git/pavani/Networks/project/ocaml-irmin/irmin
irmin-http.1.3.3                  path  /Users/gkaki/git/pavani/Networks/project/ocaml-irmin/irmin
irmin-mem.1.3.0                   path  /Users/gkaki/git/pavani/Networks/project/ocaml-irmin/irmin
irmin-unix.1.3.3                  path  /Users/gkaki/git/pavani/Networks/project/ocaml-irmin/irmin

FYI, I made a symlink:

$ls -lrt ~/git/ocaml-irmin
lrwxr-xr-x  1 gkaki  staff  52 Dec 18 11:40 /Users/gkaki/git/ocaml-irmin -> /Users/gkaki/git/pavani/Networks/project/ocaml-irmin

And here's my 1.4 branch:

$cd ~/git/ocaml-irmin/irmin/
$git branch
* 1.4
$git log --name-status HEAD^..HEAD
commit 615364620f4233cb82a96144824eb6ad5d1104f0 (HEAD -> 1.4, tag: 1.4.0, origin/1.4)
Author: Thomas Gazagnaire <thomas@gazagnaire.org>
Date:   Wed Jun 6 15:33:23 2018 +0200

    Update CHANGES

M       CHANGES.md
$git status
On branch 1.4
Your branch is up to date with 'origin/1.4'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   src/irmin/type.ml

no changes added to commit (use "git add" and/or "git commit -a")
$git diff src/irmin/type.ml
diff --git a/src/irmin/type.ml b/src/irmin/type.ml
index 60751676..cc18b4ad 100644
--- a/src/irmin/type.ml
+++ b/src/irmin/type.ml
@@ -1076,7 +1076,7 @@ module Decode_json = struct

   let char e =
     lexeme e >>= function
-    | `String s when String.length s = 1 -> Ok (String.get s 1)
+    | `String s when String.length s = 1 -> Ok (String.get s 0)
     | l -> error e l "`String[1]"

   let int32 e = float e >|= Int32.of_float

Anything amiss in my setup? Thanks!

[dinosaure]

Yes, it's reproducible with 1.4.0 (and not 1.4). It seems that the bug is inside the encoder which want to encode As and Ae where the decoder does not want this kind of lexeme. I will try to find the diff :+1: .

[dinosaure]

Ok the bug is here: https://github.com/mirage/irmin/blob/1.4/src/irmin/type.ml#L675

We should not wrap the JSON value inside a list (which put As and Ae lexeme). To fix it, you just need to apply this patch. I don't know about the process to cherry-pick it at the version 1.4.

The good news is, in the current state of irmin, this bug does not appear.

[gowthamk]

Thanks @dinosaure! I applied the patch, and it did solve the problem. Closing this issue.

[dinosaure]

I re-open the issue (but thanks for your report) mostly because this patch like a patch-monkey and we should investigate more deeper about a proper solution than what I did - but it's the week end now :-P .

[samoht]

Closing as this is fixed in irmin 2.0.