ocaml / ocamlbuild

The legacy OCamlbuild build manager
Other
122 stars 81 forks source link

Propagate -keep-locs and -keep-docs in all compiler invocations #258

Closed gasche closed 7 years ago

gasche commented 7 years ago

Fixes #256

jacquev6 commented 7 years ago

Thanks! That was quick :)

jacquev6 commented 7 years ago

That was quick but this doesn't work :-/ Using the gist I linked in #256, after path-pinning the keep-locs-docs branch:

$ ./do.sh 
/home/vincent/.opam/4.05.0/bin/ocamldep.opt -keep-docs -pp -keep-docs -modules Content.mli > Content.mli.depends
+ /home/vincent/.opam/4.05.0/bin/ocamldep.opt -keep-docs -pp -keep-docs -modules Content.mli > Content.mli.depends
/home/vincent/.opam/4.05.0/bin/ocamldep.opt: unknown option '-keep-docs'.

Looks like adding flag ["ocaml"; "pack"; "keep_docs";] (A "-keep-docs"); and flag ["ocaml"; "pack"; "keep_locs";] (A "-keep-locs"); would work.

gasche commented 7 years ago

Indeed. I tried to go for a broader scope of flag application to avoid under-shooting, but I forgot about the ocamldep invocation. I need a new iteration and a testing strategy. Thanks!

jacquev6 commented 7 years ago

Thank you for your time!

gasche commented 7 years ago

I updated the patch.

I would like to write a testsuite but I don't know of any easy-to-implement way to automate the check that keep-loc is passed (within our current testsuite). The only thing I found was that if I compile a pack from several modules, then edit one of the .mli to add a new empty line at the start, then location changes and the hash of the result packed .cmi should change as a result. But our testsuite encourages us to perform a single build action and observe the result (or build error), so this is impractical. I think I will leave this without a unit test.

jacquev6 commented 7 years ago

Thanks! I'm using it with

opam pin add ocamlbuild https://github.com/gasche/ocamlbuild.git#keep-locs-docs

and everything looks fine.

gasche commented 7 years ago

Merged, thanks a lot for the report and feedback.