https://pantsbuild.org/ logo
Join Slack
Powered by
There might be docs missing in <https://www.pantsb...
# development
p
There might be docs missing in https://www.pantsbuild.org/2.22/docs/writing-plugins/common-plugin-tasks/plugin-upgrade-guide My in-repo plugin tests are failing in the rule_runner -> native options parser with:
Copy code
E       ValueError: No build root detected for the current directory of /tmp/pants-sandbox-LsFSaZ. Pants detects the build root by looking for at least one file from pants.toml, BUILDROOT, BUILD_ROOT in the cwd and its ancestors. If you have none of these files, you can create an empty file in your build root.
With pants 2.22 do I need to change how I use the 
RuleRunner
?
āœ… 1
w
I dont think the upgrade guide has been updated since 2.17
p
I assume that means no significant plugin API changes have been made since 2.17. At least for st2's in-repo plugins, that is true for 2.18-2.21. It's just 2.22 that I'm seeing an issue updating.
w
Didn't we remove @rule_helper in 2.18? I recall running into an undocumented breaking change at some point
p
Hmm. I don't know, as I don't think I used 
@rule_helper
šŸ˜›
w
I think that's why I wrote this in the first place šŸ˜† I haven't had any issues with rule runner, though, so can't help there
p
w
oof
p
Copy code
/home/runner/.cache/pants/named_caches/pex_root/venvs/s/3e4d8434/venv/lib/python3.9/site-packages/pants/testutil/rule_runner.py:313: in __init__
    self.options_bootstrapper = self.create_options_bootstrapper(args=bootstrap_args, env=None)
/home/runner/.cache/pants/named_caches/pex_root/venvs/s/3e4d8434/venv/lib/python3.9/site-packages/pants/testutil/rule_runner.py:443: in create_options_bootstrapper
    return create_options_bootstrapper(args=args, env=env)
/home/runner/.cache/pants/named_caches/pex_root/venvs/s/3e4d8434/venv/lib/python3.9/site-packages/pants/testutil/option_util.py:18: in create_options_bootstrapper
    return OptionsBootstrapper.create(
/home/runner/.cache/pants/named_caches/pex_root/venvs/s/3e4d8434/venv/lib/python3.9/site-packages/pants/option/options_bootstrapper.py:145: in create
    initial_bootstrap_options = cls.parse_bootstrap_options(
/home/runner/.cache/pants/named_caches/pex_root/venvs/s/3e4d8434/venv/lib/python3.9/site-packages/pants/option/options_bootstrapper.py:95: in parse_bootstrap_options
    bootstrap_options = Options.create(
/home/runner/.cache/pants/named_caches/pex_root/venvs/s/3e4d8434/venv/lib/python3.9/site-packages/pants/option/options.py:166: in create
    native_parser = NativeOptionParser(args, env, config.sources(), allow_pantsrc=True)
/home/runner/.cache/pants/named_caches/pex_root/venvs/s/3e4d8434/venv/lib/python3.9/site-packages/pants/option/native_options.py:47: ValueError
       self._native_parser = native_engine.PyOptionParser(
I just looked at adding a 
BUILDROOT
file to some of my tests, but it dies will initing 
RuleRunner
before I've had a chance to write any files šŸ˜•
@happy-kitchen-89482 Have you seen any option parser problems w/ 2.22 like this?
afaict, RuleRunner can't work with the native options parser... why are any of the tests passing? šŸ˜– RulerRunner inits the build root as an empty directory (no sentinel files): https://github.com/pantsbuild/pants/blob/ecf0a7f30800db1bcaf17079f3595a2ae8587964/src/python/pants/testutil/rule_runner.py#L266-L277 Then it bypasses the build root look up logic (in python), directly initializing the singleton 
BuildRoot
here: https://github.com/pantsbuild/pants/blob/ecf0a7f30800db1bcaf17079f3595a2ae8587964/src/python/pants/testutil/rule_runner.py#L280 But then, the failure happens when initializing the options bootstrapper, because the rust layer does not respect that singleton 
BuildRoot
python object. That initialization starts here AFTER the 
BuildRoot
has already been configured: https://github.com/pantsbuild/pants/blob/ecf0a7f30800db1bcaf17079f3595a2ae8587964/src/python/pants/testutil/rule_runner.py#L313 The options bootstrapper finishes setting up the python options parser, and then just (randomly?) creates a 
NativeOptionParser
simply to ensure it works with the same args. https://github.com/pantsbuild/pants/blob/ecf0a7f30800db1bcaf17079f3595a2ae8587964/src/python/pants/option/options.py#L166 This is where the rust code throws 
ValueError
if OptionParser init fails (note that the 
buildroot
kwarg is explicitly set to 
None
): https://github.com/pantsbuild/pants/blob/ecf0a7f30800db1bcaf17079f3595a2ae8587964/src/rust/engine/src/externs/options.rs#L184-L193 The first thing the option parser does is search for the build root, which fails (since the RuleRunner has only created an empty directory): https://github.com/pantsbuild/pants/blob/ecf0a7f30800db1bcaf17079f3595a2ae8587964/src/rust/engine/options/src/lib.rs#L284 Maybe the 
RuleRunner
should add the 
PANTS_BUILDROOT_OVERRIDE
env var to bypass the search logic? Or is there some way for the rust layer to find the 
BuildRoot
python singleton object? https://github.com/pantsbuild/pants/blob/ecf0a7f30800db1bcaf17079f3595a2ae8587964/src/rust/engine/options/src/build_root.rs#L24
h
Hmm, I haven't seen this before, and as you say, the tests all pass.
I'll take a moment to internalize your discoveries above
p
@happy-kitchen-89482 is it better to pass buildroot into 
NativeOptionParser
(essentially bypassing the 
BuildRoot::find
logic in rust), or set 
PANTS_BUILDROOT_OVERRIDE
in the 
RuleRunner
, or have 
RuleRunner
touch a 
BUILD_ROOT
file in the sandbox? Thanks for looking... Take your time. I started drafting this message before you responded šŸ˜›
h
Hmmm... probably best to have RuleRunner touch a BUILD_ROOT file I think
p
Will that mess up any tests that need to control the existence of that file?
h
I do not know, but I don't think there would be many of those
p
looks like the build root tests do not use RuleRunner... The only test that uses the RuleRunner and touches a BuildRoot file is: https://github.com/pantsbuild/pants/blob/main/src/python/pants/vcs/git_test.py#L454
Trying to understand that test leads to another interesting bit. The 
RuleRunner
does not change the cwd to the temp directory it created as a build_root, which is why that test can generate its own build root and then create RuleRunner. Once the test calls set_options, the build_root gets re-calculated based on cwd. So, I think the cwd is the pytest sandbox instead of RuleRunner's build_root tmp dir. Maybe the 
pants.toml
or 
BUILD_ROOT
files are ending up in the pytest sandbox when running in the pants repo?
Yes! The tests in the 
pantsbuild/pants
repo are passing because the 
BUILD_ROOT
file is a dependency of 
src/python/pants/testutil/rule_runner.py
, meaning every test that uses 
RuleRunner
has the 
BUILD_ROOT
file in the pytest sandbox!
Does anyone have a project with in-repo pants plugins that have tests that use 
RuleRunner
? If so, please try pants 2.22.0.rc0. Does RuleRunner work for you? If the tests work, do you have a 
BUILD_ROOT
or 
BUILDROOT
file in the root of your repo?
h
LOL, so basically the actual build root is the sandbox and not the (apparently superfluous) tmpdir?
p
No. The build root is used for the actual test. Because RuleRunner overrides the path in the global singleton 
BuildRoot
, everything in the python layer will see the tmp directory as the build root, even though cwd is the pytest sandbox.
I just tested my hypothesis in the st2 repo by changing my 
rule_runner
pytest fixture to make it add a 
BUILDROOT
file to the cwd before initializing the `RuleRunner`:
Copy code
@pytest.fixture
def rule_runner() -> RuleRunner:
    # add BUILDROOT file in the pytest sandbox to deal with the rust options layer
    touch("BUILDROOT")
    return RuleRunner(
And then I ran the tests (successfully!) with 
--keep-sandboxes=always
. Yes. The 
BUILDROOT
file (which does not exist in the st2 repo) gets added to the root of the pytest sandbox:
Copy code
$ pants --keep-sandboxes=always test pants-plugins/api_spec/rules_test.py 
14:30:59.91 [INFO] Preserving local process execution dir /tmp/pants-sandbox-AFG2bb for Run Pytest for pants-plugins/api_spec/rules_test.py:tests
14:31:10.27 [INFO] Completed: Run Pytest - pants-plugins/api_spec/rules_test.py:tests - succeeded.

āœ“ pants-plugins/api_spec/rules_test.py:tests succeeded in 10.36s.
$ ls /tmp/pants-sandbox-AFG2bb
BUILDROOT  extra-output  local_dists.pex  pants-plugins  pants-plugins.api_spec.rules_test.py.tests.xml  pyproject.toml  pytest.pex  pytest_runner.pex  pytest_runner.pex_bin_python_shim.sh  pytest_runner.pex_pex_shim.sh  requirements.pex  __run.sh
Is there any reason for 
RuleRunner
to depend on 
//BUILD_ROOT:files
? I've traced the git history back to https://github.com/pantsbuild/pants/commit/a0bcc2314c486fcaa34de9401c29890799ead1ba#diff-d031e5fbb87740ea8f711edcd[ā€¦]8bc5e983001013f3bccf590da487R35 And I'm not sure this has much to do with the current RuleRunner. I think we should remove that dependency. Doing that shows the same failure that I'm experiencing in the st2 repo on the 2.22.x branch, BUT the tests pass on the 2.21.x branch. (or at least, running the tests locally for 
src/python/pants/backend/pants/::
fails on 2.22 and passes on 2.21).
n
Just ran into this exact 
RuleRunner
issue as well when running the integration tests for our internal plugins on Pants 2.23.0.dev2 instead of 2.21.0dev1.
p
Thank you for confirming that it's not just me. I'm working on a fix...
a
Bumping this because it's approved and blocking my dbt backend testing (I have a dependency on pants >=2.23 due to this bug fix).
p
@happy-kitchen-89482 if you have a chance could you review the PR? I want your input before I merge.
šŸ‘€ 1
https://github.com/pantsbuild/pants/pull/21112 has been merged to main and will be included in the next 2.23 dev release. https://github.com/pantsbuild/pants/pull/21121 has been merged to 2.22.x and will be included in the next 2.22 release candidate.
šŸ™ 1
4 Views
Open in Slack
PreviousNext
Powered by