WIP internals: allow Pants sources to depend only on certain 3rd-party requirements #19512

AlexTereshenkov · 2023-07-23T12:25:41Z

Work towards #19458

AlexTereshenkov · 2023-07-23T12:29:57Z

With this rule set, there are some warnings e.g. src/python/pants/explorer/server/graphql/context.py's dependency on 3rdparty/python#strawberry-graphql

as

pants/3rdparty/python/BUILD

Lines 36 to 49 in eb0e836

    
           __dependents_rules__( 
        
               (  # Only the explorer server may depend on these libraries 
        
                   ( 
        
                       "[fastapi]", 
        
                       "[starlette]", 
        
                       "[strawberry-graphql]", 
        
                       "[uvicorn]", 
        
                   ), 
        
                   "src/python/pants/explorer/server/**", 
        
                   "!*", 
        
               ), 
        
               # Free for all on the rest 
        
               ("*", "*"), 
        
           )

seems to be overridden by the new rule set I added. Using the extend=True won't help as they do no share a directory hierarchy.

I wonder if we shall exclude the explorer/ from the dependencies rule, but it feels wrong since we have already declared that this package may import those 3rd-party requirements. @kaos

AlexTereshenkov · 2023-07-23T12:31:36Z

src/python/pants/BUILD

+        # can depend on any 3rd party requirement as they aren't included in the final artifact
+        (
+            {"type": python_sources},
+            PANTS_ARTIFACT_DEPENDENCIES,


@kaos this is where having #19507 implemented would come in handy, I think. A tuple is not terrible, but it can't be accessed outside of this BUILD file, right?

@kaos this is where having #19507 implemented would come in handy, I think. A tuple is not terrible, but it can't be accessed outside of this BUILD file, right?

Will reply to this first, before having looked at the rest.

The tuple could be put in a macro, as such it would be globally available.

I would prefer to have the "rule" (deciding fact) of what we may depend on (or not) closer to where it is declared.

Imagine I were to add a new 3rd party dependency, I would look at the others already there and do the same, most definitely overlooking to modify this allow list here.

I think an alternative could be to instead use tags to mark which 3rd party deps are allowed to be used where/how. That way the rules may be enforced here and else where as applicable, but the properties for each dep would be declared along with the BUILD file for the 3rd parties where they belong.

…quirements

AlexTereshenkov · 2023-08-31T08:53:12Z

With the current ruleset, I get some violations, unexpectedly:

09:51:40.62 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/query/rules.py's dependency on 3rdparty/python#strawberry-graphql
09:51:40.67 [WARN] Dependency rule violation for src/python/pants/testutil/__init__.py's dependency on 3rdparty/python#pytest
09:51:40.91 [WARN] Dependency rule violation for src/python/pants/backend/terraform/testutil.py's dependency on 3rdparty/python#pytest
09:51:41.22 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/query/root.py's dependency on 3rdparty/python#strawberry-graphql
09:51:41.47 [WARN] Dependency rule violation for src/python/pants/jvm/testutil.py's dependency on 3rdparty/python#pytest
09:51:41.75 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/context.py's dependency on 3rdparty/python#strawberry-graphql
09:51:41.77 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/setup.py's dependency on 3rdparty/python#starlette
09:51:41.77 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/setup.py's dependency on 3rdparty/python#strawberry-graphql
09:51:42.04 [WARN] Dependency rule violation for src/python/pants/testutil/skip_utils.py's dependency on 3rdparty/python#pytest
09:51:42.21 [WARN] Dependency rule violation for src/python/pants/testutil/python_interpreter_selection.py's dependency on 3rdparty/python#pytest
09:51:42.33 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/field_types.py's dependency on 3rdparty/python#strawberry-graphql
09:51:42.55 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/query/targets.py's dependency on 3rdparty/python#strawberry-graphql
09:51:42.67 [WARN] Dependency rule violation for src/python/pants/explorer/server/uvicorn.py's dependency on 3rdparty/python#fastapi
09:51:42.67 [WARN] Dependency rule violation for src/python/pants/explorer/server/uvicorn.py's dependency on 3rdparty/python#uvicorn
09:51:44.65 [WARN] Dependency rule violation for src/python/pants/testutil/pants_integration_test.py's dependency on 3rdparty/python#pytest

kaos · 2023-08-31T14:48:30Z

src/python/pants/BUILD

+        # published Pants artifact will require; test modules do not have any restrictions and
+        # can depend on any 3rd party requirement as they aren't included in the final artifact
+        (
+            {"type": python_sources},


to follow the style of the previous rule set.

Suggested change

{"type": python_sources},

(python_sources, python_source),

kaos · 2023-08-31T14:53:26Z

With this rule set, there are some warnings e.g. src/python/pants/explorer/server/graphql/context.py's dependency on 3rdparty/python#strawberry-graphql

as

pants/3rdparty/python/BUILD

Lines 36 to 49 in eb0e836

__dependents_rules__(

( # Only the explorer server may depend on these libraries

(

"[fastapi]",

"[starlette]",

"[strawberry-graphql]",

"[uvicorn]",

),

"src/python/pants/explorer/server/**",

"!*",

),

# Free for all on the rest

("*", "*"),

)

seems to be overridden by the new rule set I added. Using the extend=True won't help as they do no share a directory hierarchy.

Not overridden, as it is dependencies rules vs dependents rules so there's no overlap. And extend does not apply for the same reason, regardless of directory structure in play.

I wonder if we shall exclude the explorer/ from the dependencies rule, but it feels wrong since we have already declared that this package may import those 3rd-party requirements. @kaos

Perhaps place the exception for the dependencies rules in src/python/pants/explorer/BUILD, leaving the top default rules clean.

kaos · 2023-08-31T15:10:33Z

With the current ruleset, I get some violations, unexpectedly:

09:51:40.62 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/query/rules.py's dependency on 3rdparty/python#strawberry-graphql

a) This (and all other ../explorer/ warnings) was expected according to the above, right.. ?

09:51:40.67 [WARN] Dependency rule violation for src/python/pants/testutil/init.py's dependency on 3rdparty/python#pytest

b) I think this is a regular python_source so it falls into the source rule set rather than the test modules rule set.

09:51:40.91 [WARN] Dependency rule violation for src/python/pants/backend/terraform/testutil.py's dependency on 3rdparty/python#pytest

Same as (b).

09:51:41.22 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/query/root.py's dependency on 3rdparty/python#strawberry-graphql

Same as (a).

09:51:41.47 [WARN] Dependency rule violation for src/python/pants/jvm/testutil.py's dependency on 3rdparty/python#pytest

Same as (b).

09:51:41.75 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/context.py's dependency on 3rdparty/python#strawberry-graphql

Same as (a).

09:51:41.77 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/setup.py's dependency on 3rdparty/python#starlette

Same as (a).

09:51:41.77 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/setup.py's dependency on 3rdparty/python#strawberry-graphql

Same as (a).

09:51:42.04 [WARN] Dependency rule violation for src/python/pants/testutil/skip_utils.py's dependency on 3rdparty/python#pytest

Same as (b).

09:51:42.21 [WARN] Dependency rule violation for src/python/pants/testutil/python_interpreter_selection.py's dependency on 3rdparty/python#pytest

Same as (b).

09:51:42.33 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/field_types.py's dependency on 3rdparty/python#strawberry-graphql

Same as (a).

09:51:42.55 [WARN] Dependency rule violation for src/python/pants/explorer/server/graphql/query/targets.py's dependency on 3rdparty/python#strawberry-graphql

Same as (a).

09:51:42.67 [WARN] Dependency rule violation for src/python/pants/explorer/server/uvicorn.py's dependency on 3rdparty/python#fastapi

Same as (a).

09:51:42.67 [WARN] Dependency rule violation for src/python/pants/explorer/server/uvicorn.py's dependency on 3rdparty/python#uvicorn

Same as (a).

09:51:44.65 [WARN] Dependency rule violation for src/python/pants/testutil/pants_integration_test.py's dependency on 3rdparty/python#pytest

Same as (b).

So there are two root issues. The missing exception from the default rules for the explorer (issue a), and not treating test util code as test code (issue b). The latter is trickier as it is in fact python_source targets. What I've done is to use tags for these and use that tag in the rules. That way it's easy to just apply a test-util tag to all sources in the testutil module for instance.

AlexTereshenkov added the category:internal CI, fixes for not-yet-released features, etc. label Jul 23, 2023

AlexTereshenkov changed the title ~~internals: allow Pants sources to depend only on certain 3rd-party requirements~~ WIP internals: allow Pants sources to depend only on certain 3rd-party requirements Jul 23, 2023

AlexTereshenkov commented Jul 23, 2023

View reviewed changes

AlexTereshenkov requested a review from kaos July 23, 2023 12:31

internals: allow Pants sources to depend only on certain 3rd-party re…

b1f2895

…quirements

AlexTereshenkov force-pushed the internals/add-visibility-rules-pants-3rdparty branch from e77e47b to b1f2895 Compare August 31, 2023 08:54

kaos reviewed Aug 31, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP internals: allow Pants sources to depend only on certain 3rd-party requirements #19512

WIP internals: allow Pants sources to depend only on certain 3rd-party requirements #19512

AlexTereshenkov commented Jul 23, 2023 •

edited

Loading

AlexTereshenkov commented Jul 23, 2023 •

edited

Loading

AlexTereshenkov Jul 23, 2023 •

edited

Loading

kaos Aug 31, 2023

kaos Aug 31, 2023 •

edited

Loading

AlexTereshenkov commented Aug 31, 2023

kaos Aug 31, 2023

kaos commented Aug 31, 2023

kaos commented Aug 31, 2023 •

edited

Loading

WIP internals: allow Pants sources to depend only on certain 3rd-party requirements #19512

Are you sure you want to change the base?

WIP internals: allow Pants sources to depend only on certain 3rd-party requirements #19512

Conversation

AlexTereshenkov commented Jul 23, 2023 • edited Loading

AlexTereshenkov commented Jul 23, 2023 • edited Loading

AlexTereshenkov Jul 23, 2023 • edited Loading

Choose a reason for hiding this comment

kaos Aug 31, 2023

Choose a reason for hiding this comment

kaos Aug 31, 2023 • edited Loading

Choose a reason for hiding this comment

AlexTereshenkov commented Aug 31, 2023

kaos Aug 31, 2023

Choose a reason for hiding this comment

kaos commented Aug 31, 2023

kaos commented Aug 31, 2023 • edited Loading

AlexTereshenkov commented Jul 23, 2023 •

edited

Loading

AlexTereshenkov commented Jul 23, 2023 •

edited

Loading

AlexTereshenkov Jul 23, 2023 •

edited

Loading

kaos Aug 31, 2023 •

edited

Loading

kaos commented Aug 31, 2023 •

edited

Loading