Looking for feedback: is it wise to try to require...
# general
f
Looking for feedback: is it wise to try to require that all files under a directory be added to some target? I know
./pants tailor
will take care of things for which the backends are installed, but I'm thinking of also requiring that even text files or json files or whatever be at least lumped into
files()
targets so that nothing is really unknown to Pants. Could this backfire?
b
Other than worse performance, and maybe some upgrade headache, I doubt it'll "backfire". Oh and pants only warns when globs stop matching so that's annoying (for now)
c
ooh, this would have saved me a bit of frustration. I think implementing this as a tailor goal in a plugin would be fairly easy (tailor gets a list of unowned files, could just emit files targets for each of them). Just have to be careful to put it last, I guess? I wonder if implementing this as a linter which warns on unowned (but not ignored) files would be possible. This would allow us to not generate the targets.
b
Oh interesting bit on tailor, it only ever adds target generators. So every time you add files, your BUILD will grow 🤔
📈 1
f
We don’t enforce having all files being owned by a target; I think it’s safe to assume there will be files that don’t make sense to be owned by Pants in any capacity and I don’t think it’s practical to have Pants own them without ever being used in any build. What we do instead is that we search for Python modules that are part of the source control, but are not used by Pants. This could be a test module that is not globbed (due to incorrect naming pattern) or a source module in a directory that user forgot to list under target’s sources.
on a related note, we also run: •
./pants --files-not-found-behavior=error filedeps ::
to find all unmatched globs from
sources
field •
./pants --owners-not-found-behavior=error list project/**/*.py
on some projects to identify source files that are not owned
f
I guess the question I was trying to ask is "is it wise to require that all files within given directories be either owned or _ignored_"? Seems to be that people seem to like these kinds of checks
b
I think that boils down to you and your org. I certainly like the idea. But I also think I wouldn't like the execution, and neither would my engs,
✔️ 2
f
I think the issue I want to deal with is some module needing a
config.json
to load but that file just being dropped in there and unused. Or said
config.json
is really critical for some test, but we forget to add it and then someone changes it and it doesn't run tests and breaks things. I don't know if forcing file targets here is a good idea either, it might lead to blanket ignores or just
files()
targets with nothing depending on them just as easily. It's why I was trolling for opinions
b
If
config.json
is critical to a test, but it's existence isn't tested... 🙂
f
we don't yet execute tests in hermetic directories
b
By default we load resources from disk using a helper, which among other things, asserts the file exists (which can be disabled)
f
We're not really fully using the python backend yet to test, since we use a giant source tree + rpms to load deps, so the usual pattern of "if you forget to declare your dep, your code won't even load the file under test" doesn't work for us (yet)
👍 1
🤔 1
c
I think this is similar to how the example-python repo has a task to test that there aren't any files that tailor could grab. It uses a GitHub action with the command
./pants tailor --check update-build-files --check
. I could see adding the commands above to that check, and maybe as a pre-commit hook.
f
the commands above don't really work as expected anymore, since
--owners-not-found-behavior
is deprecated in 2.13, and passing recursive globs seems to blow up (this could be a bug though)
c
I could also see macros being useful in some cases. For us, for example, we have a bunch of plugins each with a sample config.json . We use the sample config in tests. Providing a macro like
munger_plugin
which creates the
file
target could also be a solution.