Move default C flags to the :standard set

[voodoos]

This PR implements RFC #3718

ocamlc_cflags and ocamlc_cppflags from ocamlc_config are now part of the :standard set of flags (it was already the case for the first).
Moreover they are not systematically added any-more to the compiler command line, giving back entire control over c-flags to the users.

These changes may break projects which made the assumption that dune (or ocamlc) added these flags to the command line, which might be considered a bad practice. The change in this PR will activate only when the option new_foreign_flags_handling is activated in the dune-project dune lang >= 2.8 is used.

@jeremiedimino, @rgrinberg : an other compatibility path would be to add an option to the dune-project and only default this behaviour in dune 3. However I believe it might be a good thing to push users who forgot to keep the :standard flags to update their projects' configurations.

[rgrinberg]

an other compatibility path would be to add an option to the dune-project and only default this behaviour in dune 3. However I believe it might be a good thing to push users who forgot to keep the :standard flags to update their projects' configurations.

That might be our only option. Otherwise, users can't freely upgrade to (lang dune 2.8)

[ghost]

I agree with Rudi.

However I believe it might be a good thing to push users who forgot to keep the :standard flags to update their projects' configurations.

Agreed. Perhaps we can introduce a warning for this?

[emillon]

Thanks for this PR.

I think that #3718 makes a good job at describing where flags are sourced from. I think a text description of this would be a nice addition to the manual.

I also second @rgrinberg 's suggestion to have a way for configurator to use the same flags.

[rgrinberg]

If we make the change to configurator, how do we version it?

If we make the change to configurator, we need to be mindful of the behavior of:

• (always_add_cflags false), configurator >= 2.8
• (always_add_cflags false), configurator < 2.7

It seems like our hands are tied, since the behavior is always going to be dependent on the version of configurator being used.

The simplest way is to just version people's programs. For example, change Configurator.create to:

type cflags =
  | Always_add
  | Ask_dune
val create :
     ?dest_dir:string
  -> ?ocamlc:string
  -> ?log:(string -> unit)
  -> ?cflags:cflags
  -> string (** name, such as library name *)
  -> t

That's super safe, but will mean that upgrading is going to be a long process.

[ghost]

@rgrinberg we just had a chat about this with @voodoos. The change you propose to confiugurator makes sense to me, however they are not straightforward. IIUC, they require Dune communicating the default C flags as customised by the user in a given directory to configurator, and Dune tracking this dependency so that it re-runs configurator if the user changes the C flags. This is something we currently don't know how to do.

Moreover, were planning to look at the more general question of "dune configure" next.

So let's skip the configurator integration for now, but we'll keep all that in mind when working on the design of "dune configure".

[voodoos]

I added a warning that triggers if all the following condition are true:

• Dune lang is >= 2.8
• The new always_add_cflags is not defined
• The :standard set of flags was overriden

The goal is to raise awareness about Dune's forced c-flags and guide the users toward the new default which will be always_add_cflags false.

[voodoos]

I just rebased on master and renamed the option to match the choice done in the twin PR #3802 : future_c_and_cxx_flags_handling.

I feel like this PR is ready to be merged, do you agree @rgrinberg, @jeremiedimino ?
If we merge it we will need to merge #3802 before the next release.

[rgrinberg]

Looks good apart from some nits. Let's pick a better name as well.

[rgrinberg]

We handle warnings in quite an adhoc way unfortunately. It's something that I'd like to improve when I get the chance. Feel free to work on it if you're interested

[voodoos]

This PR is now ready however we are waiting for it's CXX sibling: #3802
I changed the target milestone to 2.9.

[voodoos]

@ejgallego same as #3802