Octachron / codept

Contextual Ocaml DEPendencies Tool: alternative ocaml dependency analyzer
Other
59 stars 10 forks source link

dune runtest failure on Windows: backslash/forward slashes #42

Closed jonahbeckford closed 1 month ago

jonahbeckford commented 1 month ago

(I don't know how to solve this completely, so won't be submitting a PR at this time.)

The reference files have Unix paths like https://github.com/Octachron/codept/blob/8c37f498f8c88d447b48fb8d312acb5e8a1b013e/tests/complex/split_unit/reference:

{
"version" : [0, 11, 0],
"dependencies" :
  [{ "file" : "split_unit/intf/a.mli" }, { "file" : "split_unit/impl/a.ml" }],
"local" :
  [{
   "module" : ["A"],
   "ml" : "split_unit/impl/a.ml",
   "mli" : "split_unit/intf/a.mli"
   }]
}

On Windows that will be generated as:

{
"version" : [0, 11, 0],
"dependencies" :
  [{ "file" : "split_unit\\intf\a.mli" },
  { "file" : "split_unit\\impl\a.ml" }],
"local" :
  [{
   "module" : ["A"],
   "ml" : "split_unit\\impl\a.ml",
   "mli" : "split_unit\\intf\a.mli"
   }]
}

There are three problems:

  1. The \ versus /. That can be fixed by replacing these slashes before doing comparisons in tests. Easy
  2. The JSON encoding is not valid. It should be "split_unit\\impl\\a.ml" not "split_unit\\impl\a.ml" (notice the last backslash is not escaped). Not sure where this is happening, but doesn't sound hard to fix
  3. Since the JSON seems to use Format.pp*, it will have different line breaks based on the length of the escaped \\ backslash versus /. Notice that the dependencies is on two lines in Windows, but only one on Unix. Hard.
Octachron commented 1 month ago

For the third point, I am thinking of adding a -pretty-printing-mode option to switch to a more regular printing mode for tests.

Octachron commented 1 month ago

Implemented as a simple-json option to the -format flag

Octachron commented 1 month ago

The Windows CI passes now, but due to a hole of the json implementation that doesn't escape characters at all. I will fix that in time, but I am not sure in which context "half-escaped" strings appears.

jonahbeckford commented 1 month ago

Thanks!! I will check it out tomorrow and will close this issue if it is not closed by then.