Add -t output text option

[jonahbeckford]

Problem

Because cppo writes with open_out, CRLF are added to output files on Windows even when the input files are LF.

Context

• .cmi CRC checksums are computed on the marshalled signatures of .mli files. These CRC checksums are the source of the The files xxx.cmi and yyy.cmi makes inconsistent assumptions on interface Zzz ocamlc/ocamlopt errors.
• The marshalled signatures include the byte locations inside the .mli files (because the marshalled Types.signature indirectly references Lexer.position). Easy to see in the utop-full toplevel on any .cmi file:

  utop> require "compiler-libs" ;;
  utop> Cmi_format.read_cmi filename ;;
   (Types.Sig_modtype (<abstr>,
     {Types.mtd_type =
       Some
         (Types.Mty_signature
           [Types.Sig_value (<abstr>,
             {Types.val_type = <abstr>; val_kind = Types.Val_reg;
             val_loc =
               {Location.loc_start = {Lexing.pos_fname = "endianString.cppo.mli"; pos_lnum = 21; pos_bol = 1326; pos_cnum = 1328}; 
               loc_end = {Lexing.pos_fname = "endianString.cppo.mli"; pos_lnum = 21; pos_bol = 1326; pos_cnum = 1364}; 
               loc_ghost = false};
             val_attributes = []; val_uid = Types.Uid.Item {Types.Uid.comp_unit = "EndianString"; id = 0}},
             Types.Exported);
  

• CRLF inserted in .mli will therefore have different CRC checksums than original LF-ending .mli files

The net effect is that I can't share .cmi/.cma between Unix and Windows machines if CPPO is used as a preprocessor. The files xxx.cmi and yyy.cmi makes inconsistent assumptions on interface Zzz is the result.

Solution

Use open_out_bin. This PR hides it behind a command line option, but I actually think binary mode should always have been used.

[liyishuai]

Please add an example in the test directory.

[jonahbeckford]

Sorry, but I have no clue what would be added there!

Also ...

but I actually think binary mode should always have been used.

It seems like the code was incorrect from the beginning, and this PR adding a CLI option is overkill. Can we start with that first ... why would cppo ever need to add carriage returns if the source has no carriage returns?

[liyishuai]

It seems like the code was incorrect from the beginning, and this PR adding a CLI option is overkill. Can we start with that first ... why would cppo ever need to add carriage returns if the source has no carriage returns?

Yes this design makes more sense to me. We can change the default behavior to use open_out_bin, and provide some -text flag for backward compatibility.

[liyishuai]

Sorry, but I have no clue what would be added there!

1. Input files *.cppo.bin in LF and in CRLF;
2. Expected output files *.ref.bin when processed in binary mode and in text mode, on Windows and on Linux;
3. .gitattribute configuration that tells Git to treat *.bin files as-is.

[jonahbeckford]

Okay the changes are done.

One note ... two tests were already broken on Windows due to CRLF. More accurately, they were broken depending on the setting of core.autocrlf git global configuration on Windows. This PR does not fix those problems.

PS Y:\source\cppo> git reset --hard master
PS Y:\source\cppo> dune build -w --auto-promote
PS Y:\source\cppo> git diff
diff --git a/test/ext.ref b/test/ext.ref
index 4626b21..3459392 100644
--- a/test/ext.ref
+++ b/test/ext.ref
@@ -1,28 +1,28 @@
-# 1 "ext.cppo"
-hello
-nop
-#raqrkg
-qrs
-# 7 "ext.cppo"
-goodbye
-
-# 9
-(*
-hello
-#ext rot13
-abc
-\#endext
-def
-#endext
-goodbye
-
-#ext source
-#endext
-*)
-(*
-   Environment variables:
-     CPPO_FILE=ext.cppo
-     CPPO_FIRST_LINE=9
-     CPPO_LAST_LINE=11
-*)
-# 11
+# 1 "ext.cppo"^M
+hello^M
+nop^M
+#raqrkg^M
+qrs^M
+# 7 "ext.cppo"^M
+goodbye^M
+^M
+# 9^M
+(*^M
+hello^M
+#ext rot13^M
+abc^M
+\#endext^M
+def^M
+#endext^M
+goodbye^M
+^M
+#ext source^M
+#endext^M
+*)^M
+(*^M
+   Environment variables:^M
+     CPPO_FILE=ext.cppo^M
+     CPPO_FIRST_LINE=9^M
+     CPPO_LAST_LINE=11^M
+*)^M
+# 11^M
diff --git a/test/int_expansion_error.ref b/test/int_expansion_error.ref
index 7bd1d63..c1b8584 100644
--- a/test/int_expansion_error.ref
+++ b/test/int_expansion_error.ref
@@ -1,4 +1,4 @@
-Error: File "int_expansion_error.cppo", line 2, characters 4-7
-Error: Variable FOO found in cppo boolean expression must expand
-into an int literal, into a tuple of int literals,
-or into a variable with the same properties.
+Error: File "int_expansion_error.cppo", line 2, characters 4-7^M
+Error: Variable FOO found in cppo boolean expression must expand^M
+into an int literal, into a tuple of int literals,^M
+or into a variable with the same properties.^M

[liyishuai]

Tests failed on Linux and macOS:

• https://github.com/ocaml-community/cppo/actions/runs/12174752473/job/33958290268
• https://github.com/ocaml-community/cppo/actions/runs/12174752473/job/33958283238

[jonahbeckford]

It has become clear that the project was already broken on Windows, and that I shouldn't be doing this PR as I'm neither an expert nor a user of cppo. I'll keep the 4-character patch (appending _bin to open_out) in a pin or in my own repository as it unblocks me, but I won't be making any more changes to this PR. Feel free to close this PR or let another user try to make progress. Thanks.

[pmetzger]

So what do we do to either advance this or close it?

[liyishuai]

If someone could help fix the build and tests, then I'd approve for merging.