Support usage of `scala_artifact` addresses in `scalac_plugin` targets by alonsodomin · Pull Request #19205 · pantsbuild/pants

alonsodomin · 2023-05-31T15:55:25Z

This allows to reference a scala_artifact target in the artifact field of a scalac_plugin target and enables the usage of scalac plugins in workspaces that cross-compile with different versions of Scala.

For example like:

scala_artifact(
    name="acyclic_jar",
   group="com.lihaoyi",
   artifact="acyclic",
   version="0.2.1",
   resolve=parametrize("scala2.12", "scala2.13"),
)

scalac_plugin(
    name="acyclic",
    artifact=":acyclic_jar"
)

scala_sources(
    name="src",
    scalac_plugins=["acyclic"],
    resolve=parametrize("scala2.12", "scala2.13"),
)

Before this, artifact resolution will fail since scala_artifact is a target generator and it is parametrized, therefore Pants can't find the given artifact just by looking up the address to the generator.

alonsodomin · 2023-05-31T15:56:26Z

The solution feels a bit work-aroundish and could probably be implemented in the graph by adding a consumer_target: Target field to the WrappedTargetRequest but would like to hear opinions from others on other ways of implementing this before potentially impacting the graph in ways I can't predict.

stuhood

Thanks!

Non-blocking comment on the use of _TargetParametrizations.

stuhood · 2023-06-01T18:39:44Z

src/python/pants/backend/scala/compile/scalac_plugins.py

+@rule
+async def _resolve_scalac_plugin_artifact(


Since this code is invoked from a @rule which will already memoize "the plugins to use for a target", I think that it should probably be a rule helper (plain async def) instead of @rule. That would avoid defining the request type, and the return type wrapping.

src/python/pants/backend/scala/compile/scalac_plugins.py

stuhood · 2023-06-01T18:47:19Z

src/python/pants/backend/scala/compile/scalac_plugins.py

+    parametrizations = await Get(
+        _TargetParametrizations,
+        {
+            _TargetParametrizationsRequest(
+                address.maybe_convert_to_target_generator(),
+                description_of_origin=(
+                    f"the target generator {address.maybe_convert_to_target_generator()}"
+                ),
+            ): _TargetParametrizationsRequest,
+            environment_name: EnvironmentName,
+        },
+    )
+
+    target = parametrizations.get_subset(
+        address, request.consumer_target, field_defaults, target_types_to_generate_requests
+    )


Yea, as you've mentioned, _TargetParametrizations is a very awkward API, which is part of why it is still explicitly private. I'd like to find a cleaner way to expose the get_subset call... it may be that it is a good use-case for one of our first public rule helpers (rather than exposing the _TargetParametrizations type, which should maybe be an implementation detail).

get_subset is the fundamental operation, but it's possible that there is a better granularity to expose, such as one level up in resolve_dependencies: it's also possible that exposing resolve_dependencies as an API that can be used to expand any given list of dependencies would make sense?

In fact I did come up with this implementation by looking at the implementation of the resolve_dependencies rule in the graph. However getting the subset is only one step in the process, that results in getting the actual target generator, we still need the final generated target and it's why there is a call to parametrizations.generated_for right after.

Although exposing the subset will get a long way, because IIRC there is a dependency relationship between a target generator and its generated targets, so I guess I could resolve the dependencies of the generator and, in case I find none or more than one, then error there.

In a way, the field artifact in scalac_plugin is like a SingleDependencyField for which we expect to find only one target after expanding parameters and target generation. What makes it even more awkward is the fact that the consumer target, the one that carries the parameters over, is not the scalac_plugin itself, but the scala_sources target that lists the plugin names in the scalar_plugins field (a field that gives me SpecialCasedDependencies vibes, but that's a different topic).

So, in a way, everything here smells of this being a special case:

A target (scalac_plugin) that adds metadata to the build and it's meant to decorate another target (jvm_artifact/scala_artifact). And one that, in a way, feels like having a 1->1 dependency relationship with the decorated target.

A 1:1 target generator (scala_artifact->jvm_artifact)

Parametrized fields

A compile-only dependency between a scala_source(s) target and the jvm_artifact target generated, and the dependency happens via an intermediary target (the scalac_plugin)

Also, Scalac plugins can be globally defined in the scalac subsystem using a dict where the key is the resolve name.

In general, this is hard to generalise as only the Kotlin backend implements compiler plugins in a similar fashion and it's unlikely this scenario shows up in there too.

Sorry for the write up!

As a side note, there is another case that I'm aware of where having a field for a single dependency would be nice: The helm_deployment target. That target needs a reference to the Helm chart to deploy and there should be only one of those.

At first I wanted to model using a bespoke field but ended up asking users to add the address to the chart in the dependencies field as I wanted Pants to rebuild any code that may have changed. While this works well, it feels unpolished and the implementation is a bit awkward as it has to avoid a rule cycle so I always wanted to change that and in fact I may have a branch somewhere with some changes.

Co-authored-by: Stu Hood <stuhood@gmail.com>

…r target, not the consumer

This updates `changelog.py` to write the (non-internal) changes to the relevant release notes file directly, rather than requiring a human to copy-paste it in. For now, the file is just mutated, without committing. The internal changes are still printed for the human to deal with. For instance, `pants run src/python/pants_release/changelog.py -- --prior 2.18.0.dev1 --new 2.18.0.dev2` adds a new section to `src/python/pants/notes/2.18.x.md`: ```diff diff --git a/src/python/pants/notes/2.18.x.md b/src/python/pants/notes/2.18.x.md index e648a45..d6668a24b1 100644 --- a/src/python/pants/notes/2.18.x.md +++ b/src/python/pants/notes/2.18.x.md @@ -1,5 +1,35 @@ # 2.18.x Release Series +## 2.18.0.dev2 (Jun 14, 2023) + +### New Features + +* Include complete platforms for FaaS environments for more reliable building ([#19253](#19253)) + +* Add experimental support for Rustfmt ([#18842](#18842)) + +* Helm deployment chart field ([#19234](#19234)) + +### Plugin API Changes + +* Replace `include_special_cased_deps` flag with `should_traverse_deps_predicate` ([#19272](#19272)) + +### Bug Fixes + +* Raise an error if isort can't read a config file ([#19294](#19294)) + +* Improve handling of additional files in Helm unit tests ([#19263](#19263)) + +* Add taplo to the release ([#19258](#19258)) + +* Handle `from foo import *` wildcard imports in Rust dep inference parser ([#19249](#19249)) + +* Support usage of `scala_artifact` addresses in `scalac_plugin` targets ([#19205](#19205)) + +### Performance + +* `scandir` returns `Stat`s relative to its directory. ([#19246](#19246)) + ## 2.18.0.dev1 (Jun 02, 2023) ### New Features ``` This also moves it into the new `pants_release` root, adds some basic tests, and has it fetch the relevant branch directly, rather than prompting the user to do so. It also pulls out a `git.py` support module with helpers for exec-ing `git` as an external process. The commits are individually reviewable. This is a step towards automating more of the "start release" process, per #19279. After this core refactoring of the functionality, I think the next step is to combine it with the CONTRIBUTORS update and version bumping, and then after that string it all together on CI so that starting a release is fully automated.

Antonio Alonso Dominguez added 2 commits May 30, 2023 22:42

Added test case

e81986d

Resolve scalac plugin target via parametrizations

24df6f8

alonsodomin added backend: JVM JVM backend-related issues category:bugfix Bug fixes for released features labels May 31, 2023

alonsodomin requested review from Eric-Arellano, chrisjrn, stuhood and tdyas May 31, 2023 15:55

alonsodomin and others added 2 commits May 31, 2023 17:56

Merge branch 'main' into scalac_plugin_support_cross_compile

fdc0685

Print target address in error message

9a96f94

stuhood approved these changes Jun 1, 2023

View reviewed changes

alonsodomin and others added 4 commits June 2, 2023 10:07

Update src/python/pants/backend/scala/compile/scalac_plugins.py

cc19882

Co-authored-by: Stu Hood <stuhood@gmail.com>

Merge branch 'main' into scalac_plugin_support_cross_compile

c40ca05

Kotlinc plugins artifact addresses should be resolved locally to thei…

81de8f7

…r target, not the consumer

Switch to a rule helper

7e94592

alonsodomin merged commit 704f7ba into pantsbuild:main Jun 5, 2023

huonw mentioned this pull request Oct 26, 2023

scala: create scala_artifact target type #12969

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support usage of `scala_artifact` addresses in `scalac_plugin` targets#19205

Support usage of `scala_artifact` addresses in `scalac_plugin` targets#19205
alonsodomin merged 8 commits intopantsbuild:mainfrom
alonsodomin:scalac_plugin_support_cross_compile

alonsodomin commented May 31, 2023

Uh oh!

alonsodomin commented May 31, 2023

Uh oh!

stuhood left a comment

Uh oh!

stuhood Jun 1, 2023

Uh oh!

Uh oh!

stuhood Jun 1, 2023

Uh oh!

alonsodomin Jun 2, 2023 •

edited

Loading

Uh oh!

alonsodomin Jun 2, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@rule
		async def _resolve_scalac_plugin_artifact(

Uh oh!

Conversation

alonsodomin commented May 31, 2023

Uh oh!

alonsodomin commented May 31, 2023

Uh oh!

stuhood left a comment

Choose a reason for hiding this comment

Uh oh!

stuhood Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stuhood Jun 1, 2023

Choose a reason for hiding this comment

Uh oh!

alonsodomin Jun 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alonsodomin Jun 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alonsodomin Jun 2, 2023 •

edited

Loading

alonsodomin Jun 2, 2023 •

edited

Loading