Allow obuild to be safe-string compatible

[kit-ty-kate]

Currently obuild doesn't compile with OCaml 4.06 or any OCaml releases with safe-string enabled. This PR fixes this.

Unfortunately, I didn't managed to be compatible with OCaml < 4.02. If it's not a problem it can be merged, otherwise it requires some more work (if anybody want to give it a try).

[UnixJunkie]

I will try it.

[jeromemaloberti]

I will try it too, but it looks good. My only concern is about the compatibility with ocaml < 4.02. It should be possible to add a module Bytes that conditionaly compile and link.

[kit-ty-kate]

Such a module can be taken from the bytes compatibility package here: https://gitlab.camlcity.org/gerd/lib-findlib/blob/master/src/bytes/bytes.ml You can either choose to take it as is (the license is permissive) or to rely on ocamlfind

[kit-ty-kate]

I tried to import the Bytes module but I couldn't get over the bootstrap phase (modifying the bootstrap file was fairly easy but obuild AFAIK doesn't support conditional compilation) So here is a commit that allows to handle older OCaml versions but relies on ocamlfind (base-bytes relies itself on ocamlfind)

[andyli]

Is this a blocking issue for OCaml 4.06 support?

[jeromemaloberti]

I pushed a changed inspired by this pull request, but using a compat module. Bootstrap detect the version of ocaml and copy the right file. Could you try it ? I only tested on linux so far, and my bash skills are quite limited.

[andyli]

Just tested on my Mac. The current master branch (668a70a5b1f14fb5a176cd3b81477f0bd3d5bd8a) was built successfully with OCaml 4.02.3/4.06.0.

[kit-ty-kate]

I tried on 3.12.1, 4.01.0, 4.02.3 and 4.06.0. Everything seems to work fine (on Linux at least). However, I believe testing for -safe-string would be a bit more resistant than <. That would give something like this:

if $OCAMLOPT -safe-string ; then
    echo "Using compat403.ml"
    cp -f compat403.ml ext/compat.ml
else
    echo "Using compat402.ml"
    cp compat402.ml ext/compat.ml
fi

I would personally also rename compat403 to compat402 and compat402 to compat401 since safe-string has been added in 4.02

[andyli]

For the record, testing for ocaml-version is the current practice of opam: https://github.com/ocaml/opam-repository/search?utf8=%E2%9C%93&q=safe-string&type=Commits

Either fix is fine with me, but I would like to see a 4.06 compatible release soon.

[kit-ty-kate]

@andyli I'm not sure to understand what you mean. The problem of using ocaml-version is that it only works if you use opam. Compilation from source is not handled. But yes, next release should probably restrain older versions of obuild anyway.

[UnixJunkie]

The master branch compiles fine on 4.06.0 (I'm on Ubuntu LTS). It would be nice to have a new obuild release since several packages will not be installable in opam if obuild doesn't support 4.06.0. You can ping me if you create a new tag and want me to update the opam package.

[andyli]

@jpdeplaix I didn't mean using an ocaml-version constraint in the opam file. I meant testing for ocaml version (in the build process, e.g. $OCAMLVER < "4.02.0" in obuild's bootstrap) is the current practice.

[jeromemaloberti]

@andyli , I Finally understood what you meant. I also prefer using ocaml version, but I will fix the file names, and replace cp with ln. I will also add a configure script to obuild to be able to work on obuild without using bootstrap. I hope I can make a new release by tonight.

[jeromemaloberti]

@UnixJunkie , I pushed a new tag, do you think you can update the opam package ? If you cannot, can you tell me how to do it ? Thank you.

[kit-ty-kate]

@jeromemaloberti Here it is: https://github.com/ocaml/opam-repository/pull/10800

[UnixJunkie]

@jeromemaloberti to update the opam package, you need to make a pull request in there: https://github.com/ocaml/opam-repository/tree/master/packages/obuild copy the latest obuild version directory, create a new one then just update the url file (latest tarball URL + its md5sum).

[UnixJunkie]

I'm closing this issue since it was resolved. Thanks to all the helping hands.