Closed andyli closed 6 years ago
Hello, wouldn't it be possible to patch the existing bootstrap file instead of creating a new one?
On Mon, 21 May 2018, 08:02 UnixJunkie, notifications@github.com wrote:
Hello, wouldn't it be possible to patch the existing bootstrap file instead of creating a new one?
Hi. Yes. But if your intention is to keep it simple, I would suggest replacing the bootstrap script with bootstrap-bytecode, because the bytecode bootstrap binary will also work in native platforms. I can add a condition to create the final binary as native if possible.
It sounds reasonable to me. If possible, the final exe should be compiled to native. I guess that if ocamlopt.opt (or, ocamlopt) is present in the path,that would be your condition.
It should be good to go now. I've tested installing it with opam 4.04.0+bytecode-only
and 4.06.0
.
if @jeromemaloberti integrates this and tags a new obuild version, then the opam CI tests will test this with many different versions of the compiler (though I'm not sure if any of them is a bytecode-only compiler).
BTW, before tagging a release, remember to update the version number in obuild.obuild
, which is still at 0.1.9 right now.
The script is fine, I didn't test it though, however the change in prog.ml means that obuild will not use ocamlopt when it is available. I would prefer that the configuration correctly uses the right compiler. I will try to work on it.
@andyli , actually, could you try to change getOcamlOpt () in prog.ml line 35 to add "ocamlc" in the list ? It should fall back to ocamlc if ocamlopt is not found. It is a hack though.
I don't think my prog.ml change will affect using ocamlopt or not for actually building. That line is to call -config
, which from my testing, all ocamlc, ocamlopt, ocamlc.opt, and ocamlopt.opt output the same result.
I do fix an unrelated minor issue in the additional commit.
Yes, you're right. Another potential problem that worry me, is the default targets in gconf.ml, target_options_defaults. So, on platform with ocamlc only, the users will have to set explicitly executable-bytecode to true, and executable-native to false. Ideally, the configure command in the bootstrap should be the same on all platforms. I will think about it.
Yeah, I also considered making the default smart, but then I figured it could be done later. So, I kept this PR minimal.
Is there any problem with this PR? It is incremental that shouldn't break anything but enabled building obuild in bytecode-only platforms.
Sorry, I was on holiday, and I missed the latest commit.
Hi Jerome, ping me if you create a new tag, I will update the opam package, as usual.
andyli reminded that we need a new tag to publish the fix
Based on the work in #173. Fixes #174.