[WIP] configurator: add write_lines function

[avsm]

The write_flags only works with (:include directives, and it
is also useful to be able to write a list of lines so that the
discovered information can be used in variable expansion actions.

For example, ocaml-yaml discovers CFLAGS and then directly has
(run ${CC} ${read-lines:cflags}) actions that use this new
write_lines function to list cflags instead of s-expressions.
They must be line-by-line or else variable expansion doesnt work
since CFLAGS contain spaces.

Signed-off-by: Anil Madhavapeddy anil@recoil.org

[Chris00]

I used something like that in one of my packages as well.

[rgrinberg]

Use Io.write_lines please.

[ghost]

BTW, it's not urgent but one day we should deprecate (:include ...) and add support for %{read-lines:<file>} in flags. (:include ...) was added long before ${read-lines:...} and I don't think it's useful to have both.

[Chris00]

we should deprecate (:include ...)

Agreed. We should deprecate write_flags at the same time.

[rgrinberg]

Won't include be still useful when we have more dynamic rules. E.g. we'd be able to include an entire preprocess specification dynamically. Or include a list of files to be installed with their installed names, which has an sexp based dsl syntax

[avsm]

Hm, I ran into a quoting issue when using this, so this might be worth revising a little.

A cflags file contains (e.g.):

-O2 -Wall -I/usr/local/include/mysql

This then needs to be passed as individual atoms to the build action, which only happens (I think) when we actually write it as follows:

-O2
-Wall
-I/usr/local/include/mysql

...and use the ${read_lines action. Using {$read results in the whole block being passed quoted to the build action and the build failing.

As a result, ocaml-yaml now uses this fragment to generate the file:

    let fout = open_out "cflags" in
    String.iter (fun c ->
      let c = if c = ' ' then '\n' else c in
      output_char fout c
    ) cflags;
    close_out fout

Should we adapt a similar function to split newlines into individual lines, or is there some other way to unquote strings with spaces?

[ghost]

We have the three following functions in Stdune.String:

val extract_words : t -> is_word_char:(char -> bool) -> t list
val extract_comma_space_separated_words : t -> t list
val extract_blank_separated_words : t -> t list

We could expose and document them in Configurator.

[avsm]

Does the interface I've pushed make sense? I moved the Flags handling into its own module and made the write_flags explicitly mention include (since the difference between include and rule actions is a bit confusing). If this works I'll rebase and update the docs.

[rgrinberg]

Looks good. Though if the plan is to get rid of (:include ..) we should just do it rather than document and have special support for it in configurator.

@diml are we convinced that :include has to go?

[ghost]

@diml are we convinced that :include has to go?

I'm convinced it has to go

[Chris00]

@diml are we convinced that :include has to go?

I think so too if that matters.

[rgrinberg]

The final verdict was that (:include ..) will stay in 1.0. We might as well support it properly in configurator.

@avsm would you mind rebasing this PR? It looks ready.

[ghost]

One last thing: can you replace atom_or_quoted_string by (fun x -> Quoted_string x)? This will ensure that the produced files are compatible with both the jbuild and dune lexical conventions.

[ghost]

I did this in #967

[rgrinberg]

Thanks @diml. I will resolve the conflicts and merge this PR manually