https://pantsbuild.org/ logo
#general
Title
# general
n

narrow-vegetable-37489

02/02/2022, 11:51 AM
Hmm, running into some issues with Pants 2.10.0.dev2. See thread...
As instructed by the deprecation warning, I run the following command:
Copy code
./pants update-build-files --fix-python-macros
This results in the following diff:
Copy code
diff --git a/vendor/BUILD b/vendor/BUILD
index 92a8134..c577e67 100644
--- a/vendor/BUILD
+++ b/vendor/BUILD
@@ -9,8 +9,8 @@ poetry_requirements(
         "setuptools": ["easy_install", "pkg_resources", "setuptools"],
     },
     overrides={
-        "openapi-core": {"dependencies": [":setuptools"]},
-        "protobuf": {"dependencies": [":types-protobuf"]},
-        "starlette": {"dependencies": [":requests"]},
+        "openapi-core": {"dependencies": ["#setuptools"]},
+        "protobuf": {"dependencies": ["#types-protobuf"]},
+        "starlette": {"dependencies": ["#requests"]},
     },
 )
Adding
use_deprecated_python_macros = false
to
pants.toml
and then running
./pants test ::
gives the following error:
Copy code
12:44:06.49 [ERROR] 1 Exception encountered:

  ResolveError: 'protobuf' was not found in namespace 'vendor'. Did you mean one of:
  :vendor
Not sure where to start debugging this.
f

fast-nail-55400

02/02/2022, 12:07 PM
so this is a bug. the modification switched the dependencies from
:foo
(with a colon) to
#foo
(with a # which is the syntax for the name of a generator), although not sure that is valid reference
could you open an issue?
in the meantime, you could work around this by trying an address like
:#requests
or
:vendor#requests
for the dependencies. those might work.
although I may be wrong. the error message is
ResolveError: 'protobuf' was not found in namespace 'vendor'. Did you mean one of:
which suggests that the keys in
overrides
no longer reference targets since the targets are now “generated targets” under vendor, as in
vendor#protobuf
instead of being macro-generated targets.
which would be a
protobuf
target as a sibling to the
vendor:vendor
target
either way, it’s a bug
n

narrow-vegetable-37489

02/02/2022, 1:52 PM
Alright, thanks! I'll open an issue.
h

hundreds-father-404

02/02/2022, 4:42 PM
I'm not sure the
overrides
part is the issue. @narrow-vegetable-37489 it sounds like you're using Protobuf -- have you set
[python-protobuf].runtime_dependencies
?
I'm not sure the overrides part is the issue.
Relative addresses work with generated targets, it's correct last I checked to convert here
:foo
to
#foo
n

narrow-vegetable-37489

02/02/2022, 5:28 PM
@hundreds-father-404 Ah, yes. The following diff seems to fix the issue:
Copy code
diff --git a/pants.toml b/pants.toml
index f86a503..5a8bfa5 100644
--- a/pants.toml
+++ b/pants.toml
@@ -112,8 +112,8 @@ requirement_constraints = "vendor/lock/constraints.txt"
 [python-protobuf]
 mypy_plugin = true
 runtime_dependencies = [
-    "vendor:grpcio",
-    "vendor:protobuf",
+    "vendor#grpcio",
+    "vendor#protobuf",
 ]

 [source]
h

hundreds-father-404

02/02/2022, 5:32 PM
Excellent! Thanks so much for reporting this. It would take a bit of engineering work to have Pants automatically update
pants.toml
because
update-build-files
does not yet have the mechanism to overwrite the file, and it would also be a misnomer to do that What do you think about instead us logging at ERROR level that you need to make these N edits to your pants.toml? (The much better fix is for the error message you encountered to say the source of the bad target reference. That is suprisingly complicated to do because every call site needs to be updated to pass in that information. If you're familiar with React, it's the same problem of when you want to pass info down to a deeply nested component via props; what Context solves)
n

narrow-vegetable-37489

02/02/2022, 7:14 PM
I think a simple hint after the migration that you have to manually update any addresses targeting the
poetry_requirements
packages to use
#
instead of
:
would've been enough for me (because I assume this issue isn't exclusive to
runtime_dependencies
but rather any address that targets
(python|poetry)_requirements
packages?). I did a quick search for
protobuf
in all BUILD files when I encountered the error assuming I had to do some manual tinkering somewhere, but there were no addresses targeting it. Sadly I didn't think of checking pants.toml as well :)
h

hundreds-father-404

02/02/2022, 7:43 PM
It shouldn't be necessary to manually update anything in BUILD files - the script should handle them all. And it would be really difficult/tedius if we made you audit all your BUILD files. I think pants.toml is the only oversight we had
K I made progress on this, but realized naively scanning
pants.toml
is more complex than expected because we need to iterate over every distinct string, like handle the nuances of:
Copy code
[my-scope]
str_opt = "//:addr"
list_opt = ["not-addr", "//:addr"]
We can't simply run our detection heuristic over
toml_contents
as a single string -- So instead I'm thinking we have a hardcoded list of options where we believe you're likely to use a Python address. We'll check the fully normalized option values A benefit of this approach is we can catch when the option is set via env var and CLI args, not jsut pants.toml A downside is our error log will not tell you the origin of that particular option setting. It will tell you that
[python-protobuf].runtime_dependencies
needs updating, but you need to trace down if it's from env var vs CLI vs config file Does that sound reasonable @narrow-vegetable-37489? I really want to mitigate this migration being confusing, I know it's disruptive
Cool. Looks like the only dangerous option is
[python-protobuf].runtime_dependencies
. Maybe
[mypy].source_plugins
, although we instruct you to not point that at
python_requirement