Pass ocamlc explicitly to configurator using DUNE_CONFIGURATOR

[rgrinberg]

The simplest way of obsoleting the -ocamlc flag. Some notes:

• We are likely to still want to keep -ocamlc for debugging, but ideally, warn the user that this isn't necessary. Should we use the INSIDE_DUNE variable in configurator to issue a warning when this flag is passed?

• Ideally, DUNE_CONFIGURATOR would be an s-expression containing all the config info dune wants to pass to configurator. But there's a couple of issues with this. The first one is that it's tough to write an sexp parser in configurator since the Sexp module is in dune itself. And the second issue is that we'd prefer to share the configuration record, but that would require a dependency between dune and configurator. To solve this, I guess we could make Sexp part of usexp (or a separate, internal lib) and then have dune depend on configurator to get the record definition. Not sure if this is right however.

Anyway, the contents of DUNE_CONFIGURATOR is an implementation detail and is easy to safely change. So we can always punt these decisions until later.

[ghost]

Sys.getenv_opt is not available on older versions of OCaml.

[rgrinberg]

Sys.getenv_opt is not available on older versions of OCaml.

Ah yes. We should probably add our own Sys module to stdune to workaround this stuff.

[ghost]

Should we use the INSIDE_DUNE variable in configurator to issue a warning when this flag is passed?

That seems fine.

Regarding passing a S-expression, I guess we would indeed to move some more code to usexp.

There are a lot of modules in src/ that are specialized, and when extracting them to sub-libraries, I think we should clean them up, document and generalize them a bit. That makes them easier to understand as standalone components. For the S-expression combinators, one thing I don't really like at the moment is that we end up with a discrepancy in the naming, with some functions that are named t_of_sexp/sexp_of_t and some that are named just t. I wanted to sort that out before moving them to usexp.

[ghost]

Ah yes. We should probably add our own Sys module to stdune to workaround this stuff.

Indeed

[rgrinberg]

Actually, we can just issue the warning when DUNE_CONFIGURATOR is set. This has the same effect.

[rgrinberg]

OK, I implemented the check for DUNE_CONFIGURATOR when parsing -ocamlc. However, I'm kind of tempted to just make -ocamlc a no-op and just emit a warning. $ jbuilder exec ./discover.exe -- -ocamlc foo would still emit a warning when manually testing. Sure, we could check that -ocamlc is actually different from DUNE_CONFIGURATOR, but this is complexity that doesn't seem to justify itself.

[ghost]

Do we even need to keep the argument at all? Users will need to edit their jbuild file to switch from configurator to jbuilder.configurator, so we might as well remove the argument at the same time.

[rgrinberg]

Yup let's just get rid of it.

…

On Thu, Apr 12, 2018, 4:48 PM Jérémie Dimino ***@***.***> wrote: Do we even need to keep the argument at all? Users will need to edit their jbuild file to switch from configurator to jbuilder.configurator, so we might as well remove the argument at the same time. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#695 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIe-wOdYyKzAbEfAvK6Slt2Csk8Cm24ks5tnyLhgaJpZM4TQJdD> .

[rgrinberg]

@diml ok it's gone.

[ghost]

Looks good