[PATCH] D37838: [lit] Allow lit config files to have a .py extension

[David L. Jones via Phabricator via llvm-commits]

dlj accepted this revision.
dlj added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D37838#870412, @rnk wrote:

> In https://reviews.llvm.org/D37838#870372, @zturner wrote:
>
> > In https://reviews.llvm.org/D37838#870368, @rnk wrote:
> >
> > > `lit.cfg.py` still isn't a valid name for a python module. I had this idea that in the future we'd import the config module directly to simplify custom test formats defined in lit configuration files, which interact badly with multiprocessing.Pool. I guess that won't work well anyway since all the config files will have the same name, so we still have to move custom test formats out into well-named python modules.
> >
> >
> > Could we work around this by using `__import__` or something or searching for a module loader with a given path, and just assigning a dynamically generated name to the module?  Not trying to do anything like that with this patch by the way.
>
>
> I think we could do something crazy like that.
>
> > If it makes things simpler, I could call it `lit_cfg.py`?
>
> It probably won't make the __import__ stuff simpler, so leaving it as lit.cfg.py seems more consistent with our current lit.site.cfg naming.


Just to throw my hat into the ring here...

I don't think the structure of the lit.cfg, et. al. files are terribly amenable to being imported in Python. The lit configs are written as fallthrough scripts, and probably have too many assumptions about global state and global variables to make sense to import them using Python's import mechanics. They need to work more like a pasted textual inclusion, so they can run after the basic environment is set up, but within the pre-populated namespace. They don't really work as a standalone namespaced module, and the assumed presence of globals are (I think) the main showstopper.

>From that perspective, the illegal naming is almost more of a feature than a drawback. (I do think, however, that adding the extension for purposes of filename associations and editor support is reasonable.)


https://reviews.llvm.org/D37838