mformat: try to detect meson.format in parent directories #14178
Conversation
mesonbuild/mformat.py
Outdated
| default_config_path = sources[0].parent / 'meson.format' | ||
| if default_config_path.exists(): | ||
| options.configuration = default_config_path | ||
| current_path = Path.cwd() |
There was a problem hiding this comment.
Searching from cwd is wrong, as you could invoke the command from an unrelated directory.
An option could be to search in parents of the given source file. My concern however is that you have no way to know when you reached the sources root.
There was a problem hiding this comment.
First, when searching upwards and encountering meson.format, the search should stop. If it is not found, I can think of two methods to determine the source root.
- Use some heuristic methods, such as stopping the search when encountering files like
.git. This method is the simplest and less accurate, but it is at least better than the current approach of only looking formeson.formatinsources[0].parent. - First, search for
meson.buildfiles in each parent directory. If found, parse the file to check if it is the top-levelmeson.build(determined by whether it starts withproject()). In my understanding, this method is accurate but less efficient.
There was a problem hiding this comment.
Iterating the parents of the current source is probably the way to go, even if we could potentially go outside the source root.
There was a problem hiding this comment.
This could lead to a problem: consider a repository A that does not contain a meson.format file. If it exists as a submodule within a repository B that does contain a meson.format file, running meson format on A would use the meson.format from repository B. This might not align with the user's intentions.
But at least it's better than only use meson.format in sources[0].parent. I'll update this PR.
There was a problem hiding this comment.
First, search for
meson.buildfiles in each parent directory. If found, parse the file to check if it is the top-levelmeson.build(determined by whether it starts withproject()). In my understanding, this method is accurate but less efficient.
That is what we do in mesonbuild/interpreterbase/interpreterbase.py, see sanity_check_ast. But it does require instantiating mparser.Parser() once for each file...
wrvsrx
force-pushed
the
fix-mformat
branch
2 times, most recently
from
6928bd1 to
7338dae
Compare
wrvsrx
force-pushed
the
fix-mformat
branch
3 times, most recently
from
f8b1557 to
45d54ca
Compare
There was a problem hiding this comment.
You should not rebuild a Formatter object for every source file. This will be slow, especially when running recursively on big projects.
Also, parsing a different config file for each source file is a breaking change. If this is really what we want, this should be documented.
If we really want to do this, we should implement something similar to what is done for .editorconfig. A caching mechanism would also be something to consider in that case.
|
Maybe I should only find the |
Yes, that's what I think. Or iterate on parents of all of the given sources until you find one, but outside of the loop that actually do the formatting, because in that loop, sources is a queue where all child items are added when doing the recursion. |
|
@bruchar1 When you think that this is ready ping me and I'll merge |
Fix #14167. This PR makes mformat search
meson.formatin parent directories.