Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[internal] Unify JVM thirdparty resolves #13771

Merged
merged 4 commits into from
Dec 1, 2021
Merged

[internal] Unify JVM thirdparty resolves #13771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7860840
Set a default resolve, and move lockfiles to thirdparty.
stuhood Dec 1, 2021
66d6e3c
Switch to inferred dependencies for `java_parser`.
stuhood Dec 1, 2021
a530817
Define a target for `scala_parser`.
stuhood Dec 1, 2021
6056483
Review feedback.
stuhood Dec 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions 3rdparty/jvm/com/fasterxml/jackson/core/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

jvm_artifact(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would probably be a better example to have all of the com.fasterxml.jackson declarations in the same file -- from a maintenance point of view, the versions of these packages are usually kept in lockstep, and having to change a version in multiple files seems obtuse.

Copy link
Member Author

@stuhood stuhood Dec 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, there's an interesting tradeoff there right now which I think that we can maybe remove via #13767: it's not possible to share a variable between different BUILD files without a macro. Allowing macros to be declared hierarchically would help to keep these from going out of sync.

But from experience with a thirdparty layout that had exceptions to the org/group-as-hierarchy (Twitter had ~1k direct thirdparty deps): folks ended up confused about where to put things, and so you ended up with multiple copies of some libraries in parent/child directories. If anything, I'd love to be able to enforce it, although that probably goes too far for casual usage.

I'll ... add a comment for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unfortunate that Jackson has such a fine-grained group structure, which still doesn't make a whole lot of sense to me, tbqh. It really doesn't lend itself well to this multiple files situation.

name="jackson-databind",
group="com.fasterxml.jackson.core",
artifact="jackson-databind",
# NB: Keep in sync with all other `3rdparty/jvm/com/fasterxml/jackson` libraries.
# See https://github.com/pantsbuild/pants/issues/13767 for how this might be deduped.
version="2.12.4",
)
11 changes: 11 additions & 0 deletions 3rdparty/jvm/com/fasterxml/jackson/datatype/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

jvm_artifact(
name="jackson-datatype-jdk8",
group="com.fasterxml.jackson.datatype",
artifact="jackson-datatype-jdk8",
# NB: Keep in sync with all other `3rdparty/jvm/com/fasterxml/jackson` libraries.
# See https://github.com/pantsbuild/pants/issues/13767 for how this might be deduped.
version="2.12.4",
)
9 changes: 9 additions & 0 deletions 3rdparty/jvm/com/github/javaparser/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

jvm_artifact(
name="javaparser-symbol-solver-core",
group="com.github.javaparser",
artifact="javaparser-symbol-solver-core",
version="3.23.0",
)
9 changes: 9 additions & 0 deletions 3rdparty/jvm/io/circe/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

jvm_artifact(
name="circe-generic",
group="io.circe",
artifact="circe-generic_2.13",
version="0.14.1",
)
10 changes: 10 additions & 0 deletions 3rdparty/jvm/org/scalameta/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

jvm_artifact(
name="scalameta",
group="org.scalameta",
artifact="scalameta_2.13",
version="4.4.30",
packages=["scala.meta.**"],
)
0 ...ild/example/app/coursier_resolve.lockfile → 3rdparty/jvm/testprojects.lockfile
View file
Open in desktop
File renamed without changes.
14 changes: 11 additions & 3 deletions pants.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,18 @@ config = "@build-support/regexes/config.yaml"
[generate-lockfiles]
custom_command = "build-support/bin/generate_all_lockfiles.sh"

[jvm]
default_resolve = "jvm_testprojects"

[jvm.resolves]
# TODO: These should move into 3rdparty, as should jvm_artifact definitions.
exampleapp = "testprojects/src/jvm/org/pantsbuild/example/app/coursier_resolve.lockfile"
pants_java_parser = "src/python/pants/backend/java/dependency_inference/coursier_resolve.lockfile"
# A shared resolve for all testproject/example code. Because this is not shipped with Pants
# binaries, it requires no isolation.
jvm_testprojects = "3rdparty/jvm/testprojects.lockfile"
# A resolve for the java_parser, which is shipped with Pants, and invoked with its own isolated
# classpath. Consequently, we isolate it to its own lockfile.
java_parser = "src/python/pants/backend/java/dependency_inference/java_parser.lockfile"
# Has the same isolation requirements as `java_parser`.
scala_parser = "src/python/pants/backend/scala/dependency_inference/scala_parser.lockfile"

[toolchain-setup]
repo = "pants"
Expand Down
43 changes: 4 additions & 39 deletions src/python/pants/backend/java/dependency_inference/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,47 +1,12 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_sources(dependencies=[":java_src"])
python_sources(dependencies=[":java_resources"])
resources(name="java_resources", sources=["*.java", "java_parser.lockfile"])
python_tests(name="tests", timeout=240)

resources(name="java_src", sources=["*.java"])

#
# Targets for developing on the Java parser outside of engine rules.
#

java_sources(
name="javaparser",
compatible_resolves=[
"pants_java_parser",
],
dependencies=[
# Update in concert with `java_parser_artifact_requirements`.
# TODO: Move these definitions to thirdparty, and add to the thirdparty mapping to
# enable inference.
":com.github.javaparser_javaparser-symbol-solver-core",
":com.fasterxml.jackson.core_jackson-databind",
":com.fasterxml.jackson.datatype_jackson-datatype-jdk8",
],
)

jvm_artifact(
name="com.github.javaparser_javaparser-symbol-solver-core",
group="com.github.javaparser",
artifact="javaparser-symbol-solver-core",
version="3.23.0",
)

jvm_artifact(
name="com.fasterxml.jackson.core_jackson-databind",
group="com.fasterxml.jackson.core",
artifact="jackson-databind",
version="2.12.4",
)

jvm_artifact(
name="com.fasterxml.jackson.datatype_jackson-datatype-jdk8",
group="com.fasterxml.jackson.datatype",
artifact="jackson-datatype-jdk8",
version="2.12.4",
name="java_parser",
compatible_resolves=["java_parser"],
)
7 changes: 6 additions & 1 deletion src/python/pants/backend/java/dependency_inference/PantsJavaParserLauncher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,12 @@ class CompilationUnitAnalysis {
public final ArrayList<String> exportTypes;
}


/**
* TODO: The dependencies of this class are defined in two places:
* 1. `3rdparty/jvm` via import inference.
* 2. `java_parser_artifact_requirements`.
* See https://github.com/pantsbuild/pants/issues/13754.
*/
public class PantsJavaParserLauncher {
// Unwrap a `Type` and return the identifiers representing the "consumed" types.
private static List<String> unwrapIdentifiersForType(Type type) {
Expand Down
28 changes: 14 additions & 14 deletions ...dency_inference/coursier_resolve.lockfile → ...dependency_inference/java_parser.lockfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
},
"directDependencies": [],
"dependencies": [],
"file_name": "jackson-annotations-2.12.4.jar",
"file_name": "com.fasterxml.jackson.core_jackson-annotations_2.12.4.jar",
"file_digest": {
"fingerprint": "f6aa3706a875689b66cdac3334f65dfdb795ccfad4117bf072893b196ed1ec8e",
"serialized_bytes_length": 75705
Expand All @@ -23,7 +23,7 @@
},
"directDependencies": [],
"dependencies": [],
"file_name": "jackson-core-2.12.4.jar",
"file_name": "com.fasterxml.jackson.core_jackson-core_2.12.4.jar",
"file_digest": {
"fingerprint": "3506ce47ec2604ae2d80d79505f7cb374f718060639415c07d144adadd2d68a3",
"serialized_bytes_length": 365223
Expand Down Expand Up @@ -64,7 +64,7 @@
"packaging": "jar"
}
],
"file_name": "jackson-databind-2.12.4.jar",
"file_name": "com.fasterxml.jackson.core_jackson-databind_2.12.4.jar",
"file_digest": {
"fingerprint": "e99a7b4b89074bc689aabcd9eb1f2c1318b68cc5c34979daf3e34edc558c7a01",
"serialized_bytes_length": 1516044
Expand Down Expand Up @@ -111,7 +111,7 @@
"packaging": "jar"
}
],
"file_name": "jackson-datatype-jdk8-2.12.4.jar",
"file_name": "com.fasterxml.jackson.datatype_jackson-datatype-jdk8_2.12.4.jar",
"file_digest": {
"fingerprint": "00852350fc1503344723b590f1afe9593ab732fb5b035659b503b49bbea5c9b2",
"serialized_bytes_length": 34438
Expand All @@ -126,7 +126,7 @@
},
"directDependencies": [],
"dependencies": [],
"file_name": "javaparser-core-3.23.0.jar",
"file_name": "com.github.javaparser_javaparser-core_3.23.0.jar",
"file_digest": {
"fingerprint": "1d732f1b057b9e2ea69935180b5a6abf2c776f5e311a96651c3c03b0616032cc",
"serialized_bytes_length": 1308975
Expand Down Expand Up @@ -215,7 +215,7 @@
"packaging": "jar"
}
],
"file_name": "javaparser-symbol-solver-core-3.23.0.jar",
"file_name": "com.github.javaparser_javaparser-symbol-solver-core_3.23.0.jar",
"file_digest": {
"fingerprint": "a27067d886e72b35b4af31c8534f13ac54e924995f8f05ab2c3c4ab412ed3fb4",
"serialized_bytes_length": 480392
Expand All @@ -230,7 +230,7 @@
},
"directDependencies": [],
"dependencies": [],
"file_name": "jsr305-3.0.2.jar",
"file_name": "com.google.code.findbugs_jsr305_3.0.2.jar",
"file_digest": {
"fingerprint": "766ad2a0783f2687962c8ad74ceecc38a28b9f72a2d085ee438b7813e928d0c7",
"serialized_bytes_length": 19936
Expand All @@ -245,7 +245,7 @@
},
"directDependencies": [],
"dependencies": [],
"file_name": "error_prone_annotations-2.5.1.jar",
"file_name": "com.google.errorprone_error_prone_annotations_2.5.1.jar",
"file_digest": {
"fingerprint": "ff80626baaf12a09342befd4e84cba9d50662f5fcd7f7a9b3490a6b7cf87e66c",
"serialized_bytes_length": 13854
Expand All @@ -260,7 +260,7 @@
},
"directDependencies": [],
"dependencies": [],
"file_name": "failureaccess-1.0.1.jar",
"file_name": "com.google.guava_failureaccess_1.0.1.jar",
"file_digest": {
"fingerprint": "a171ee4c734dd2da837e4b16be9df4661afab72a41adaf31eb84dfdaf936ca26",
"serialized_bytes_length": 4617
Expand Down Expand Up @@ -349,7 +349,7 @@
"packaging": "jar"
}
],
"file_name": "guava-30.1.1-jre.jar",
"file_name": "com.google.guava_guava_30.1.1-jre.jar",
"file_digest": {
"fingerprint": "44ce229ce26d880bf3afc362bbfcec34d7e6903d195bbb1db9f3b6e0d9834f06",
"serialized_bytes_length": 2874025
Expand All @@ -364,7 +364,7 @@
},
"directDependencies": [],
"dependencies": [],
"file_name": "listenablefuture-9999.0-empty-to-avoid-conflict-with-guava.jar",
"file_name": "com.google.guava_listenablefuture_9999.0-empty-to-avoid-conflict-with-guava.jar",
"file_digest": {
"fingerprint": "b372a037d4230aa57fbeffdef30fd6123f9c0c2db85d0aced00c91b974f33f99",
"serialized_bytes_length": 2199
Expand All @@ -379,7 +379,7 @@
},
"directDependencies": [],
"dependencies": [],
"file_name": "j2objc-annotations-1.3.jar",
"file_name": "com.google.j2objc_j2objc-annotations_1.3.jar",
"file_digest": {
"fingerprint": "21af30c92267bd6122c0e0b4d20cccb6641a37eaf956c6540ec471d584e64a7b",
"serialized_bytes_length": 8781
Expand All @@ -394,7 +394,7 @@
},
"directDependencies": [],
"dependencies": [],
"file_name": "checker-qual-3.8.0.jar",
"file_name": "org.checkerframework_checker-qual_3.8.0.jar",
"file_digest": {
"fingerprint": "c88c2e6a5fdaeb9f26fcf879264042de8a9ee9d376e2477838feaabcfa44dda6",
"serialized_bytes_length": 230905
Expand All @@ -409,7 +409,7 @@
},
"directDependencies": [],
"dependencies": [],
"file_name": "javassist-3.28.0-GA.jar",
"file_name": "org.javassist_javassist_3.28.0-GA.jar",
"file_digest": {
"fingerprint": "57d0a9e9286f82f4eaa851125186997f811befce0e2060ff0a15a77f5a9dd9a7",
"serialized_bytes_length": 851531
Expand Down
10 changes: 8 additions & 2 deletions src/python/pants/backend/scala/dependency_inference/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_sources(dependencies=[":scala_sources"])
python_sources(dependencies=[":scala_resources"])
resources(name="scala_resources", sources=["*.scala", "scala_parser.lockfile"])

python_tests(name="tests", timeout=240)

resources(name="scala_sources", sources=["*.scala"])
# Targets for developing on the Scala parser outside of engine rules.
scala_sources(
name="scala_parser",
compatible_resolves=["scala_parser"],
)
6 changes: 6 additions & 0 deletions src/python/pants/backend/scala/dependency_inference/ScalaParser.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
/**
* TODO: The dependencies of this class are defined in two places:
* 1. `3rdparty/jvm` via import inference.
* 2. `SCALA_PARSER_ARTIFACT_REQUIREMENTS`.
* See https://github.com/pantsbuild/pants/issues/13754.
*/
package org.pantsbuild.backend.scala.dependency_inference

import io.circe._
Expand Down
Loading