Does pants have any native support for handling (p...
# general
p
Does pants have any native support for handling (prob by ignoring) directories that are git submodules? Or perhaps there's a better approach to solving my problem: One of my test fixtures has to be a git submodule, as the code under test runs
git worktree
. For other directories with similar contents, I can just import something (typically a PATH var to that directory generated with an test infra function). But if I do that across the git submodule boundary, python will fail to import if the git submodule isn't checked out, and pants will complain about any dependencies on that empty directory. (more in the thread)
for the similar folders (that are not git submodules), I'm doing this: this fixture file (a BUILD file makes this depend on all the relevant fixture metadata): https://github.com/st2sandbox/st2/blob/pants/st2tests/st2tests/fixtures/packs/dummy_pack_9/fixture.py imported in the test file: https://github.com/st2sandbox/st2/blob/pants/contrib/runners/python_runner/tests/unit/test_pythonrunner.py#L45 And here's where that imported var is used: https://github.com/st2sandbox/st2/blob/pants/contrib/runners/python_runner/tests/unit/test_pythonrunner.py#L62-L63
And the comparable bit in the test file that uses
test_content_version
git submodule directory is here: https://github.com/st2sandbox/st2/blob/pants/contrib/runners/python_runner/tests/unit/test_pythonrunner.py#L62-L63 With this utility test function call here: https://github.com/st2sandbox/st2/blob/pants/contrib/runners/python_runner/tests/unit/test_pythonrunner.py#L108 the utility function here checks to make sure this submodule is checked out before running the relevant tests: https://github.com/st2sandbox/st2/blob/pants/st2tests/st2tests/fixturesloader.py#L451-L465
How can I tell pants about this dependency on a git submodule?
e
So you want to allow the submodule to be checked out or not - dealer's choice - and then have the tests run or not accordingly? You don;t want to iron fist and force folks to check out the submodule?
One of my test fixtures has to be a git submodule, as the code under test runs 
git worktree
 .
Just to chase down all avenues - "... as the " - the one doesn;t obviously follow from the other. A test can create its own ephemeral git repository with
git init
etc in a tmp dir to then exercise
git worktree
against.
If you must have a git submodule for this, the only quick thing I can think of is it be nested 1 level below a dir that contains a BUILD file that uses recursive globs and maybe includes a dummy file if needed to stop Pants complaining. When the git submodule subdir is hydrated (checked out), the globs capture those files. When its not, the globs capture nothing, except perhaps the dummy file if needed to appease Pants.
p
I'm trying to introduce pants without re-architecting all the tests -- introducing as few changes as possible. I explain why it is a git submodule not to say that's the only way but to say that's probably why whoever wrote the tests wrote them that way.
checked out or not - dealer's choice
I think the idea is that it's required - you must checkout the submodule. The utility function serves to raise a helpful error message before running the tests that need it if it's not there. https://github.com/st2sandbox/st2/blob/pants/st2tests/st2tests/fixturesloader.py#L151-L157 Next time I work on this (my eyes are drooping ... bed is calling 😛 ) I need to find a way to accomplish that "fail nicely" purpose within the context of pants.
I wonder if I can use a symlink to improve this somehow.
OK. I think I found part of a decent way to handle this. Add a sibling directory with the BUILD file that includes anything needed from the submodule (which won't have BUILD files in it)
Copy code
~/p/st2sandbox/st2.git/st2tests/st2tests/fixtures/packs $ cat test_content_version_fixture/BUILD
pack_metadata(
    name="metadata",
    sources=[
        "st2tests/st2tests/fixtures/packs/test_content_version/pack.yaml",
        "st2tests/st2tests/fixtures/packs/test_content_version/**/*.yaml",
        "st2tests/st2tests/fixtures/packs/test_content_version/icon.png",
    ],
)

python_library(
    name="test_content_version",
    dependencies=[":metadata"],
    sources=[
        "st2tests/st2tests/fixtures/packs/test_content_version/**/*.py",
    ],
)

python_library(
    dependencies=[":test_content_version"],
)
That of course leads to some warnings about unmatched globs:
Copy code
12:49:28.19 [WARN] Unmatched globs from st2tests/st2tests/fixtures/packs/test_content_version_fixture:metadata's `sources` field: ["st2tests/st2tests/fixtures/packs/test_content_version_fixture/st2tests/st2tests/fixtures/packs/test_content_version/**/*.yaml", "st2tests/st2tests/fixtures/packs/test_content_version_fixture/st2tests/st2tests/fixtures/packs/test_content_version/icon.png", "st2tests/st2tests/fixtures/packs/test_content_version_fixture/st2tests/st2tests/fixtures/packs/test_content_version/pack.yaml"]

Do the file(s) exist? If so, check if the file(s) are in your `.gitignore` or the global `pants_ignore` option, which may result in Pants not being able to see the file(s) even though they exist on disk. Refer to (<https://www.pantsbuild.org/v2.6/docs/troubleshooting#pants-cannot-find-a-file-in-your-project>).
12:49:28.20 [WARN] Unmatched glob from st2tests/st2tests/fixtures/packs/test_content_version_fixture:test_content_version's `sources` field: "st2tests/st2tests/fixtures/packs/test_content_version_fixture/st2tests/st2tests/fixtures/packs/test_content_version/**/*.py"

Do the file(s) exist? If so, check if the file(s) are in your `.gitignore` or the global `pants_ignore` option, which may result in Pants not being able to see the file(s) even though they exist on disk. Refer to (<https://www.pantsbuild.org/v2.6/docs/troubleshooting#pants-cannot-find-a-file-in-your-project>).
Is there any way to enhance that error message from within a plugin? I'd like to add a note, for that directory, that explains how to initialize the git submodule.
I see where the error is generated. Looks like it is in rust so there's probably no way to add more context to the warning: https://github.com/pantsbuild/pants/blob/main/src/rust/engine/fs/src/glob_matching.rs#L509-L553
Here's the additional context: https://github.com/pantsbuild/pants/blob/main/src/rust/engine/src/nodes.rs#L497-L504 I wonder if it would be possible to add a field to the target like
unmatched_globs_additional_context
so that the target can provide additional information if the glob doesn't match. Or maybe a function that wraps the sources field to inject that context:
sources=ctx(["my.glob.*"], on_unmatched_globs="You need to do git submodule ...")
@happy-kitchen-89482 wdyt?
h
There is a way of adding context to unmatched globs via `description_of_origin`: https://github.com/pantsbuild/pants/blob/64a69b8e1f596308895153222c2b5e1377ccd5d3/src/python/pants/engine/fs.py#L119 Right now this is set at the call site, I'm not sure about setting it from target information. But certainly something we could consider if we saw more general uses for it. @hundreds-father-404 WDYT?
Basically, maybe here we could check if the target itself provides any extra verbiage
p
ok. I wonder if there's a way to use that from a plugin now. Maybe a target that grabs the sources globs from all the targets in the same BUILD file, and then runs them through a customized
PathGlobs
with a custom
description_of_origin
. wdyt?
Creating the target is simple enough - but I'm not sure what rule I could register that would get triggered simply because a target exists in a BUILD file.
wow. I just looked at the dependency graph for the
dependencies
goal... wow
h
I'm not sure I understand what you're suggesting.
@hundreds-father-404 knows this part of the codebase far better than I do though
h
Yeah that would be tricky to allow plugins to change what the warning/error message is for unmatched source globs. It's possible, but would require lots of special-cased code in pants's core to expose a plugin hook and consume it My first thought was Danny's WIP PR to allow plugins to try and catch exceptions in rules, which would allow you to overwrite the message. But that won't work - the rule for hydrating sources is used in lots of places in Pants's core and you would need to add try catches to each call site Instead, the only way I see this happening is adding a plugin hook of some type
p
Here is how I attempted to inject that error message... but it doesn't work 😛 the rule never actually gets called. https://github.com/st2sandbox/st2/commit/4712c96fc3f6418a7a14b7bb06abff072b54e205
wdyt? Is there a way to make this hacky target work?
maybe inject dependencies isn't the best way to do this. Perhaps I can piggy back on something else that needs to happen before or with dependency resolution?
Maybe something that makes the rule run for some of the goals (dependencies, dependees, test)
h
Oh! You can subclass the Sources field and implement the class method validate_snapshot - error if things aren't what you expect. The warning will still trigger before unfortunately, but you can at least then error with good instructions
👍 1
p
I'm fine with the warning still showing up as long as I can print the instructions. So with that sources subclass, I won't have to add a rule at all, right? Just the target and fields.
h
Yep, exactly
p
Hmm. Using
Sources.validate_resolved_files()
doesn't work:
Copy code
original_tgt: WrappedTarget = await Get(
                                 ^
SyntaxError: 'await' outside async function
https://github.com/st2sandbox/st2/commit/c366c3b0fd5b173de6f5d0c606460c17d808f73c#diff-ccb0f023e13c60e488a16c3232[…]5a5bbd8c995c009b437e521df1729R35
How could I deal with that?
h
Ah yeah, it can't use the rules API. Note that the globs are already resolved, no need to call
PathGlobs
again. I was imagining that you'd check something like
if self.address.spec_path == "some_dir" and not files
. That is, if it's the problematic target(s), see if the sources are missing or not
p
So, I'll need to extend the targets I want to add the error message to, maybe with a mixin that adds the unmatched_globs behavior (not a separate unmatched_globs target that depends on the targets with the real sources lists).
OK. I'm working on this, and I think I almost have it. From within
Sources.validate_resolved_files()
can I access another field on the same target? The closest I can find is
self.address
, but I can't turn that into a
Target
without the rules api afaict.
oh. I see where fields get instantiated in
Target.__init__()
So, it looks like the Field only gets the value and address. There is no way to get the error message from a different field.
So, I would have to somehow pass the error message in through the field's value... which might look odd. Hmm.
h
Yeah unfortunately you cannot access another field without the rules API, at least currently But taking a step back to the top of the thread, what's the intended behavior you want? I'm wondering if John's solution works: https://pantsbuild.slack.com/archives/C046T6T9U/p1622614433212800?thread_ts=1622611477.211300&amp;channel=C046T6T9U&amp;message_ts=1622614433.212800
p
I didn't write those tests, and trying to rewrite them is a whole other can of worms that I'd rather avoid. The more I change the tests, the harder it will be to get buy-in to merge pants into the main StackStorm.
So, technically, changing the tests is preferrable, but politically, ... Not so much.
h
Oops, sorry, linked to wrong message! I get what you mean there Iiuc the above thread correctly, you don't want to require folks to init the git submodule, right? And the tests will already error if they try to run them? https://pantsbuild.slack.com/archives/C046T6T9U/p1622614612213000?thread_ts=1622611477.211300&amp;cid=C046T6T9U
p
I'm fine requiring them to init the submodule as long as I can print a good error message with instructions on what is missing.
Here's my next version: https://github.com/st2sandbox/st2/blob/pants/st2tests/st2tests/fixtures/packs/test_content_version_fixture/BUILD But the error thrown here isn't caught like I expected where I override
Target.__init__()
here: https://github.com/st2sandbox/st2/blob/pants/pants-plugins/unmatched_globs/target_types.py#L61-L69 I get this:
Copy code
Traceback (most recent call last):
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/bin/local_pants_runner.py", line 234, in _run_inner
    return self._perform_run(goals)
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/bin/local_pants_runner.py", line 173, in _perform_run
    return self._perform_run_body(goals, poll=False)
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/bin/local_pants_runner.py", line 195, in _perform_run_body
    poll_delay=(0.1 if poll else None),
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/init/engine_initializer.py", line 136, in run_goal_rules
    goal_product, params, poll=poll, poll_delay=poll_delay
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/engine/internals/scheduler.py", line 530, in run_goal_rule
    self._raise_on_error([t for _, t in throws])
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/engine/internals/scheduler.py", line 494, in _raise_on_error
    wrapped_exceptions=tuple(t.exc for t in throws),
pants.engine.internals.scheduler.ExecutionError: 1 Exception encountered:

Engine traceback:
  in select
  in pants.backend.project_info.dependencies.dependencies
  in pants.engine.internals.graph.resolve_targets (st2tests/st2tests/fixtures/packs/test_content_version_fixture)
  in pants.engine.internals.graph.resolve_unexpanded_targets (st2tests/st2tests/fixtures/packs/test_content_version_fixture)
  in pants.engine.internals.graph.resolve_dependencies (st2tests/st2tests/fixtures/packs/test_content_version_fixture)
  in pants.backend.python.dependency_inference.rules.infer_python_dependencies_via_imports (st2tests/st2tests/fixtures/packs/test_content_version_fixture)
  in pants.backend.python.dependency_inference.module_mapper.map_module_to_address
  in pants.backend.python.dependency_inference.module_mapper.merge_first_party_module_mappings
  in pants.backend.python.dependency_inference.module_mapper.map_first_party_python_targets_to_modules
  in pants.engine.internals.graph.resolve_targets
  in pants.engine.internals.graph.generate_subtargets (st2tests/st2tests/fixtures/packs/test_content_version_fixture:test_content_version)
  in pants.engine.internals.graph.resolve_source_paths (st2tests/st2tests/fixtures/packs/test_content_version_fixture:test_content_version)
Traceback (most recent call last):
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/engine/internals/selectors.py", line 654, in native_engine_generator_send
    res = func.send(arg)
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/engine/internals/graph.py", line 645, in resolve_source_paths
    sources_field.validate_resolved_files(paths.files)
  File "/home/cognifloyd/p/st2sandbox/st2.git/pants-plugins/unmatched_globs/target_types.py", line 37, in validate_resolved_files
    raise UnmatchedGlobsError
unmatched_globs.target_types.UnmatchedGlobsError
Is there a way to catch that error so I can define the error message in the build file (which makes the purpose of that funky BUILD file much clearer)?
h
Hm so that mixin won't do what you expect. "Hydrating" source files is lazy and happens through the Target Rules API. It does not happen in the
Target
constructor. That try catch won't ever trigger how you want There is currently no plugin hook to try catch the
hydrate_sources
rule from
pants/engine/internals/graph.py
. Instead, I was envisioning something like:
Copy code
# BUILD
custom_tgt(sources=["maybe_exist/**"])
Copy code
class CustomSources(Sources):
   def validate_resolved_files(self, files: Sequence[str]) -> None:
       if files:
           return
       raise AssertionError("Init git submodule!")
That is, error if the files were not resolved, meaning we can assume the git submodule is not set up. Does that make sense?
p
That is what I'm doing, but I was trying to pull the
"Init git submodule!"
error message from the BUILD file somehow.
I can put the messaging in the plugin if I can't find a way to do that, I just thought it would be easier for people to reason about that odd BUILD file is the message was in it instead of in the plugin.
h
Ah yeah, right now the plugin hook can't access other fields from the target, so you'd want the message hardcoded in your plugin
p
Oh. I get it. Duh. I was trying to catch around
Target.__init__ -> Field.__init__
. But I'm not throwing the error in
Field.__init__
. Bummer. It was worth a shot.
👍 1
I assume there's no way to silence the traceback yet, right?
Copy code
11:03:21.38 [ERROR] Exception caught: (pants.engine.internals.scheduler.ExecutionError)
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/bin/local_pants_runner.py", line 234, in _run_inner
    return self._perform_run(goals)
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/bin/local_pants_runner.py", line 173, in _perform_run
    return self._perform_run_body(goals, poll=False)
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/bin/local_pants_runner.py", line 195, in _perform_run_body
    poll_delay=(0.1 if poll else None),
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/init/engine_initializer.py", line 136, in run_goal_rules
    goal_product, params, poll=poll, poll_delay=poll_delay
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/engine/internals/scheduler.py", line 530, in run_goal_rule
    self._raise_on_error([t for _, t in throws])
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev0_py37/lib/python3.7/site-packages/pants/engine/internals/scheduler.py", line 500, in _raise_on_error
    wrapped_exceptions=tuple(t.exc for t in throws),

Exception message: 1 Exception encountered:

  UnmatchedGlobsError: Instructions go here
h
Don't use --print-stacktrace
p
I'm not
Copy code
$ ./pants dependencies st2tests/st2tests/fixtures/packs/test_content_version_fixture
11:42:14.13 [ERROR] Exception caught: (pants.engine.internals.scheduler.ExecutionError)
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev1_py37/lib/python3.7/site-packages/pants/bin/local_pants_runner.py", line 234, in _run_inner
    return self._perform_run(goals)
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev1_py37/lib/python3.7/site-packages/pants/bin/local_pants_runner.py", line 173, in _perform_run
    return self._perform_run_body(goals, poll=False)
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev1_py37/lib/python3.7/site-packages/pants/bin/local_pants_runner.py", line 195, in _perform_run_body
    poll_delay=(0.1 if poll else None),
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev1_py37/lib/python3.7/site-packages/pants/init/engine_initializer.py", line 136, in run_goal_rules
    goal_product, params, poll=poll, poll_delay=poll_delay
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev1_py37/lib/python3.7/site-packages/pants/engine/internals/scheduler.py", line 530, in run_goal_rule
    self._raise_on_error([t for _, t in throws])
  File "/home/cognifloyd/.cache/pants/setup/bootstrap-Linux-x86_64/2.6.0.dev1_py37/lib/python3.7/site-packages/pants/engine/internals/scheduler.py", line 500, in _raise_on_error
    wrapped_exceptions=tuple(t.exc for t in throws),

Exception message: 1 Exception encountered:

  UnmatchedGlobsError: Instructions go here


(Use --print-stacktrace to see more error details.)
👍 1