Closed as we decided to load build configs in the future from exactly one file named 'build_config.py'

see issue1016

@Patrick: Ah, I see (about the complexity). But I guess we need to handle
build_configs.py specially anyway (even if we stay with the current glob
pattern) to make sure it is processed first.

OK, so let's change the title of the issue to reflect the idea of going from a
glob to a single customization file. Florian, if you want to know more about the
discussion that we had on this, let's catch up on it in person. We discussed
various reasons why the glob is a bit problematic, also including the fact that
some customizations override build_configs.py and other don't, depending on the
filename, and that is not very obvious. Repeating the whole discussion in
email/writing would probably take an hour longer than in person. :-)

Regarding lowering implementation complexity, my preferred solution would be to
not merge the two files from within build.py, but rather to execute/include the
custom file from within build_configs.py.

This would simplify things from the perspective of build.py (though of course at
a complexity cost in build_config.py), and I think it would more nicely keep the
build configuration aspects in one place. Users like me are more likely to see
the code in build_configs.py rather than in build.py if they want to figure out
things related to the build configs, so for people like me that would make the
customization feature more easily discoverable. (I actually didn't know or
forgot that the customization feature existed previously, and I often check
build_configs.py.) Essentially, whenever you're tempted to misuse
build_configs.py for personal customization, you see right in front of you that
there is actually a special customization hook.


For example, getting the build configs in build.py could simply be:

import build_configs

and then using build_configs instead of CONFIGS later on. (This has an object
interface rather than a dict interface, so requires further changes, but they
tend to be equally easy/difficult in most cases. getattr(build_configs, x) is a
reasonable way to access something whose name is given in variable x.)

And the customization within build_configs could be implemented as something like:

try:
    from .custom_build_configs import *
except ImportError:
    pass

at the end of build_configs.py.

(All untested, of course.)

I'm not opposed to the idea in general but I think we should _only_ do it if it
reduces complexity. The code that I looked at added a lot of lines, so I was
opposed to that specific patch.

My intention not to lobby for this is actually the main reason for opening this
issue. My issue761 was about solving the inconsitency with the default builds
used. During the discussion the change of the build_configs came up and I wanted
to get it out of my issue :)

The complexity comes from the points that now we have a for loop over all files
matching the pattern. If no file matches it, then the loop simple does not loop. 
Restricting us to two files does not remove code, but adds X lines for x file
paths to define and 1 line in the loop to check if the current file exists.

Thanks, Jendrik! That matches my recollection.

@Patrick: I agree we discussed the point of moving away from Python
configuration to something that doesn't include code execution, but I think that
after all aspects were considered, the majority of us thought that overall the
tradeoff of implementation cost, gained security and lost flexibility wasn't
worth it. Hence, I think we should not move in this direction. Of course, if
there is more widespread support for this suggestion, we can revisit it -- feel
free to lobby for it and recruit some supporters if you think we should go in
this direction. :-)

@Florian: Regarding the suggested change of replacing the included glob pattern
with a single file, Patrick writes that you strongly dislike the change because
it adds complexity and removes a feature. Can you elaborate? I agree that it
removes a feature, but how does it increase complexity? I would say that it
reduces complexity.

As I recall, in the first discussion we also came to the conclusion that using a 
different format for defining build configs is not needed at the moment.

We discussed this in the last two Fast Downward meetings. Malte, Patrick and I 
participated in both meetings, Florian only in the second meeting. In the first 
meeting I believe we decided to remove the wildcard in favor of importing a 
single module.

In my opinion this change is a good idea because I don't think anybody needs 
multiple files for defining custom build configs and it makes things more 
secure.

The following two points are supported by msg6814
I remember that we spoke about disliking to execute arbitrary python code and
that describing the configurations in another format was discussed. In my
memory, we did not made a decision how this should look like.
The idea with one script file for the user was in an message of Malte. Not in
the discussion. In the diff Florian strongly disliked it, because it added
complexity and removed a feature.

This is not quite what I remember from the meeting. (Jendrik, did you keep
notes? Or did someone else? This may have been the meeting that Jendrik missed.)

According to my somewhat hazy memory, what I thought we decided was to limit the
importing of user-specified Python code to one module (I think
"custom_build_configs.py" or something like that, but I don't remember the
details), and to document that this import happens.

But I don't think we decided we want to get rid of user-customization-in-Python
altogether. After all, all build systems use similar features. We just don't
want to use very general wildcards for this, and we don't want this
customization of the build system to bleed into the fast-downward.py script itself.

Do I misremember? In that case I'd suggest bringing it back onto the agenda for
one of the next meetings.

As discussed at the pre previous FastDownward Meeting, executing arbitrary
python code to load the build configs is a security hazard and should be changed.