https://pantsbuild.org/ logo
Powered by
w

witty-crayon-22786

11/15/2021, 8:30 PM
@fast-nail-55400, @ancient-vegetable-10556: i’ve bootstrapped a java/scala install in https://github.com/twitter/finagle/compare/develop...toolchainlabs:toolchainlabs/pants-v2-spike
i’m currently triaging errors there, although i expect the breakdown to change dramatically after https://github.com/pantsbuild/pants/pull/13628
alright, i spent some time with this, and made good progress… about 20% compiling. but there are a few blockers, and i think i’m going to switch to something smaller/simpler:
• there is one legit cycle between java and scala, and i don’t want to tackle 13520 right now • there is annoying double-declaration of symbols in two/four packages, which would force a bunch of BUILD file edits
looking for another target now.
@fast-nail-55400: so, i’ve been using these criteria to choose: are thirdparty dependencies clearly defined? 3rdparty to source file ratio? does it have scala-java cycles?
and this is what my config for akka-http** looks like: https://github.com/akka/akka-http/compare/main...stuhood:stuhood/pants-v2-spike
the script 
create-thirdparty.sh
creates 3rdparty BUILD files from a 
build.sbt
file … very hackily
@ancient-vegetable-10556: i think that we’ll have a better idea of next tasks after some triage… but in the meantime, reviewing https://github.com/pantsbuild/pants/issues/13621 , and grabbing https://github.com/pantsbuild/pants/issues/13596 would probably be helpful? as soon as this triage turns something else up, we’ll probably have you switch though.
a

ancient-vegetable-10556

11/16/2021, 8:10 PM
Sure
w

witty-crayon-22786

11/16/2021, 8:10 PM
@ancient-vegetable-10556: also, do you have any interest in writing the blog post? fine if not
i’ve been assuming i would, but
a

ancient-vegetable-10556

11/16/2021, 8:11 PM
Trying to resolve a test failure in the exports work right now, but yes, I can take on some of those tickets, and I could definitely start on the blog post
w

witty-crayon-22786

11/16/2021, 8:12 PM
ok, yea: that sounds great then. after landing exports, would prioritize the post over other everything else unless Tom and I find something egregious
a

ancient-vegetable-10556

11/16/2021, 8:12 PM
OK
w

witty-crayon-22786

11/16/2021, 8:12 PM
just expect me to have Opinions about the post! 😃
a

ancient-vegetable-10556

11/16/2021, 8:12 PM
of course!
Maybe you can help me with the test failure? I think it’s down to a misconfiguration of 
rule_runner
in 
javac_tests
, but there’s so many errors I can’t figure out which thing I’m missing
w

witty-crayon-22786

11/16/2021, 8:14 PM
i can in about 30 mins
a

ancient-vegetable-10556

11/16/2021, 8:14 PM
I’ll keep bashing
Yeah, I’ve got nothing
w

witty-crayon-22786

11/16/2021, 8:48 PM
k, looking
a

ancient-vegetable-10556

11/16/2021, 8:51 PM
and it’s 
java_parser_test
, not 
javac_test
, but still
w

witty-crayon-22786

11/16/2021, 8:51 PM
and junit, yea
a

ancient-vegetable-10556

11/16/2021, 8:51 PM
I don’t get how to debug something like this
w

witty-crayon-22786

11/16/2021, 8:52 PM
fwiw, what i’m going to do is look at what new types are depended on by existing rules, and then make sure that those existing rules are re-exporting the rules that provide those types
(the error messages are ~useless, and i’ve made progress on https://github.com/pantsbuild/pants/issues/11269 to fix them, but all that’s on hold until after the language push)
a

ancient-vegetable-10556

11/16/2021, 8:53 PM
w

witty-crayon-22786

11/16/2021, 8:54 PM
yea. that means that 
javac
now depends on the inference rules
a

ancient-vegetable-10556

11/16/2021, 8:54 PM
It does, yes. That was as far as I got, but it wasn’t clear to me how to tell 
rule_runner
about that dependency
w

witty-crayon-22786

11/16/2021, 8:55 PM
but the import parser tests don’t use javac, so that one isn’t likely to be the culprit
a

ancient-vegetable-10556

11/16/2021, 8:56 PM
Right, but aren’t we looking at `java_parser_test`’s configuration, or is that irrelevant here?
w

witty-crayon-22786

11/16/2021, 8:57 PM
we are
…so, 
ParsedJavaImports
is orphaned. looks like 
src/python/pants/backend/java/dependency_inference/import_parser.py
(and its test) can be deleted entirely
a

ancient-vegetable-10556

11/16/2021, 9:02 PM
OK, let me start there.
Makes no changes when running 
java_parser_test
w

witty-crayon-22786

11/16/2021, 9:03 PM
yea, but it “fixes” 
import_parser_test.py
which was also broken
Copy code
𐄂 src/python/pants/backend/java/dependency_inference/import_parser_test.py:tests failed.
𐄂 src/python/pants/backend/java/dependency_inference/java_parser_test.py:tests failed.
𐄂 src/python/pants/backend/java/test/junit_test.py:tests failed.
a

ancient-vegetable-10556

11/16/2021, 9:04 PM
Sure. It’ll be failing for the same reason as 
java_parser_test
is failing, which is something to do with the new types I added
w

witty-crayon-22786

11/16/2021, 9:10 PM
@ancient-vegetable-10556: 
junit_test.py
is probably because it doesn’t use the inference rules. now that 
javac
depends on them, it should probably export them
java_parser_test
is still a mystery though.
OH
a

ancient-vegetable-10556

11/16/2021, 9:10 PM
This is in 
rules
?
w

witty-crayon-22786

11/16/2021, 9:11 PM
@ancient-vegetable-10556: yes, javac’s rules
so: this was wrong: https://pantsbuild.slack.com/archives/C0D7TNJHL/p1637096103004600?thread_ts=1637008204.475100&cid=C0D7TNJHL … 
src/python/pants/backend/java/dependency_inference/java_parser_test.py
does depend on the javac rules for some reason. it probably doesn’t need to. but exporting the new dependency for 
javac
might fix it too
a

ancient-vegetable-10556

11/16/2021, 9:12 PM
OK
w

witty-crayon-22786

11/16/2021, 9:12 PM
…yea. just deleting the 
javac
dep for 
java_parser_test.py
works.
but to fix 
junit
, should export the inference rules from javac.
a

ancient-vegetable-10556

11/16/2021, 9:12 PM
trying that now
seems to fix junit (good)
w

witty-crayon-22786

11/16/2021, 9:13 PM
and then:
Copy code
diff --git a/src/python/pants/backend/java/dependency_inference/java_parser_test.py b/src/python/pants/backend/java/dependency_inference/java_parser_test.py
index caa2e9c049..0682121483 100644
--- a/src/python/pants/backend/java/dependency_inference/java_parser_test.py
+++ b/src/python/pants/backend/java/dependency_inference/java_parser_test.py
@@ -7,7 +7,6 @@ from textwrap import dedent
 
 import pytest
 
-from pants.backend.java.compile.javac import rules as javac_rules
 from pants.backend.java.dependency_inference.java_parser import (
     FallibleJavaSourceDependencyAnalysisResult,
 )
@@ -43,7 +42,6 @@ def rule_runner() -> RuleRunner:
             *external_tool_rules(),
             *java_parser_launcher_rules(),
             *java_parser_rules(),
-            *javac_rules(),
             *source_files.rules(),
             *util_rules(),
             *jdk_rules.rules(),
a

ancient-vegetable-10556

11/16/2021, 9:15 PM
Copy code
BLAH
@witty-crayon-22786 so it’s not clear what made that work, but thank you for unblocking
w

witty-crayon-22786

11/16/2021, 9:19 PM
javac
now depends on the inference rules
and all three of these tests depended on javac, but not the inference rules
a

ancient-vegetable-10556

11/16/2021, 9:20 PM
ok
w

witty-crayon-22786

11/16/2021, 9:21 PM
one was dead code (import parser), one actually needed javac (junit), and one didn’t even need javac (java parser)
Powered by