Config module is weak

[ndmitchell]

The Config module doesn't support generated the config at runtime, and the Ninja lexical syntax means that all include statements are relative to the current directory, not the file doing the including, so you can't relocate directories.

I think if usingConfig took a Action (Map String String) as the first argument then everything would generalise nicely. You could then have better readConfig functions, but they would be entirely optional.

[ndmitchell]

Should usingConfigFile add a need so the config file can be generated? If it already exists, there is no harm. If it doesn't, generating now works.

[ndmitchell]

Note technically (because of the alwaysRerun and equality on the oracle) the usingConfigFile could be an orderOnly dependency - not sure if that makes a difference in practice.

[ndmitchell]

Since I reuse the Ninja config format, I should contact them about relative path specifications.

[ndmitchell]

I've contacted the Ninja people: https://groups.google.com/d/msg/ninja-build/0zizMcGhWwQ/pNKw0-F93ywJ

[kaoskorobase]

Hi Neal,

thanks for looking into this, I for one would love a more flexible Config module! Apart from the include issue you mention above I'd also like to note the following things I came across in the past:

• AFAIR usingConfigFile only allows to specify one config file per shake rules action; in this utility module I've tried to overcome that limitation and also added a way for passing a default environment of variable bindings. Please ignore the deps argument of withConfig; I always thought it would be much nicer if the config file parser could determine and return the config files it needs automatically.
• In our use of config files we practically only ever append a given configuration variable in order to keep configuration files composable. Currently the file format doesn't have a special syntax for appending and it can be very tedious and error prone (variable = ${variable} ... instead of variable += ...). But maybe such a change is out of scope for the Ninja file format. Btw, is there a reason Shake uses that specific config file format?

[ndmitchell]

So the reason for going with the Ninja syntax is that I wanted to avoid defining my own language (that's always some level of effort, and often results in unexpected design warts) and Shake already had a high-performance well-tested Ninja parser inside it. As we figure out what a better config language should look like, I'm OK with extending the Ninja syntax or giving up on it altogether.

The Config module is my attempt at guessing what a Config language should look like, based on my experience writing a single large config mechanism. In that case, I used CPP to preprocess everything, which gave me include files and also conditionals. The syntax for content was LHS = RHS. Any word on the RHS which didn't start with a + or - (which I made special syntax for include a library and include a flag) was treated as relative to the current directory. That works quite well, but is a bit hard-coded to a particular setup to be generally useful. I also automatically unioned all identical LHS values, which gives you append automatically.

For your example, once you have usingConfig take an Action, and a separate function to read a config file, writing things like yours is only a few lines of code. Maybe it should be a builtin though.

My strawman proposal is:

• Define a new config language which has LHS = RHS variable binding and some kind of include statement, but nothing else - no variables or substitution.
• Any word on the RHS which doesn't contain an = sign is treated as relative to the config file.

Example:

include MoreFiles.cfg
MyExecutable.exe = File1.c File2.c INCLUDEDIR=foo "FLAGS=-O2 -Wall"

I have a few worries though. There is no variable substitution (sometimes useful), no conditionality, its defining a new file format, and while being relative to the current file is sometimes useful, it's not always useful and the = rule is pretty arbitrary. Also you can't make the LHS relative to the current directory. Thoughts welcome.

[kaoskorobase]

Any word on the RHS which doesn't contain an = sign is treated as relative to the config file.

Do you mean that any word not including a = would be automatically appended to the current directory? That sounds quite inflexible to me. I also wouldn't want to lose variable interpolation.

Here's my proposal, although not very thought-through I must say:

• Variable interpolation with ${var}
• "Standard" escaping of spaces with \ or by enclosing in double quotes
• Escaping of newlines with \
• Overwriting assignment made explicit (e.g. :=)
• Concatenating assignment (e.g. +=)
• Directory of "current" config file available in special variable (e.g. ${CWD})
• Optional selective variable exports and imports

( x ) # Optional list of exported bindings
include ${CWD}/foo.cfg (x) # Optional selective import, special variable CWD
y := 12 # Assignment, y is local to the file
x += top\ file.h "${pwd}/local file.h" # Concatenation, x is exported

Anyway, maybe this is going too far for this particular issue.

[ndmitchell]

I think coming up with a good Config story is important - as builds get bigger, they need to move metadata out into a separate file, so this might end up being the "public face" of Shake to the non-Haskell users.

I'm not a fan of explicitly listing exporting bindings - that's going too close to being a real language. The Ninja approach of having include (like the text was pasted) and subninja (scoped) seems a possible approach, and much simpler. I wouldn't be adverse to some convention that a leading @ makes it a variable that is treated specially by the config language though, or even that only leading @ variables are returned by readConfigFile.

The presence of both concatenating and non-concatenating variables is a bit of a shame, but I guess might be required.

The inclusion of ${CWD} on an include file is no big deal. But if you are defining:

myexe = ${CWD}/foo.c ${CWD}/bar.c ${CWD}/baz.c

Then it seems the ${CWD} bits are dominating the real content. Maybe that's not fatal though.

[kaoskorobase]

I'm not a fan of explicitly listing exporting bindings - that's going too close to being a real language. The Ninja approach of having include (like the text was pasted) and subninja (scoped) seems a possible approach, and much simpler.

Oh yes, I forgot about subninja; does Shake currently support that keyword? The difference is that AFAIU subninja is all or nothing; bindings are always shadowing the parent's ones and you cannot modify the latter. It's probably not a serious limitation in practice.

The presence of both concatenating and non-concatenating variables is a bit of a shame, but I guess might be required.

We mostly use concatenation and assignment only for user-supplied variables where multiple assignment would indicate an error actually.

Then it seems the ${CWD} bits are dominating the real content.

I agree but I don't understand how you could have a variable containing something different than paths in your scheme. How could you avoid that in x = ThisValue AnotherValue the words get expanded to relative paths?

[ndmitchell]

See #279 which does a bit of work towards this.