Closed lahosken closed 9 years ago
areitz
commented
9 years ago I actually think this is a more general problem. Let's say you have a java_thrift_library() target, and you try to publish it. Pants will happily run -- but it won't actually publish anything. However, if you create a dummy target(), which depends on your java_thrift_library() target, and publish that, then pants will actually publish your thrift.
jsirois
commented
9 years ago I don't believe Andy's example unless the dummy target is_jvm. This behavior has existed from day 1 of pants when pants kicked the ant backend - so this is ~2011 vintage behavior. We've discussed this at Twitter numerous times - where we is me and various tweeps on devprod and otherwise, as well as in several pants summits. The CodeGen baseclass that ProtobufGen and ApacheThriftGen inherit from determines languages to generate code from from inbound target edges in the active graph. In other words, if I compile a set of targets where one is python and it depends on the thrift, another is java and it depends on the same thrift, ApacheThriftGen will generate python and java code. If the active target graph has no-one depending on the thrift nothing is generated. Its this behavior you are witnessing. Today you must supply the langs to generate for on the CLI if you try to act on thrift targets with the apache compiler directly. Everyone agrees this behavior is confusing to folks and needs to go. No one so far has stepped up to fix ... over the span of 2+ years of complaints!
areitz
commented
9 years ago Hi @jsirois,
While I agree with what you're saying in principle, I didn't have to work too hard to repro the behavior that I'm seeing inside Twitter, strictly in OS pants. On SHA d21080bb4716e13383aed02b85893cc756b0c696, I applied this diff:
[prometheus pants (master)]$ git diff
diff --git a/examples/src/thrift/com/pants/examples/distance/BUILD b/examples/src/thrift/com/pants/examples/distance/BUILD
index 24d08b7..04696ca 100644
--- a/examples/src/thrift/com/pants/examples/distance/BUILD
+++ b/examples/src/thrift/com/pants/examples/distance/BUILD
@@ -4,8 +4,16 @@
# trivial example of "generally-useful" thrift to include in other thrift
# (to see how this is included, cd ../precipitation)
+target(name='no_cities_to_love', dependencies=[':distance-java'])
+
java_thrift_library(name='distance-java',
sources=['distance.thrift'],
+ provides=artifact(org='com.pants.examples',
+ name='distance-thrift-java',
+ repo=public),
+ compiler='scrooge',
+ language='java',
+ rpc_style='finagle',
)
python_thrift_library(name='distance-python',And when I run this command, pants succeeds without actually publishing any thrift:
[prometheus pants (master)]$ ./pants publish examples/src/thrift/com/pants/examples/distance:distance-java --publish-local=~/Desktop/pantsbuild --no-publish-dryrun
INFO] Detected git repository at /Users/areitz/workspace/pants on branch master
14:14:35 00:00 [main]
(To run a reporting server: ./pants server)
14:14:35 00:00 [bootstrap]
14:14:35 00:00 [setup]
14:14:35 00:00 [parse]
Executing tasks in goals: bootstrap -> imports -> unpack-jars -> deferred-sources -> gen -> resolve -> compile -> resources -> doc -> jar -> publish
14:14:35 00:00 [bootstrap]
14:14:35 00:00 [bootstrap-jvm-tools]
14:14:35 00:00 [imports]
14:14:35 00:00 [ivy-imports]
14:14:35 00:00 [unpack-jars]
14:14:35 00:00 [unpack-jars]
14:14:35 00:00 [deferred-sources]
14:14:35 00:00 [deferred-sources]
14:14:35 00:00 [gen]
14:14:35 00:00 [thrift]
14:14:35 00:00 [protoc]
14:14:35 00:00 [antlr]
14:14:35 00:00 [ragel]
14:14:35 00:00 [jaxb]
14:14:35 00:00 [wire]
14:14:35 00:00 [aapt]
14:14:35 00:00 [scrooge]
14:14:35 00:00 [resolve]
14:14:35 00:00 [ivy]
14:14:35 00:00 [compile]
14:14:35 00:00 [compile]
14:14:35 00:00 [jvm]
14:14:35 00:00 [jvm-compilers]
14:14:35 00:00 [python-eval]
14:14:35 00:00 [eval-targets]
14:14:35 00:00 [checkstyle]
14:14:35 00:00 [scalastyle]
14:14:35 00:00 [resources]
14:14:35 00:00 [prepare]
14:14:35 00:00 [doc]
14:14:35 00:00 [javadoc]
14:14:35 00:00 [scaladoc]
14:14:35 00:00 [jar]
14:14:35 00:00 [jar]
14:14:35 00:00 [jar-create]
14:14:35 00:00 [publish]
14:14:35 00:00 [publish]Skipping check for a clean master in test mode.
SUCCESSHowever, publishing the dummy target actually does stuff:
[prometheus pants (master)]$ ./pants publish examples/src/thrift/com/pants/examples/distance:no_cities_to_love --publish-local=~/Desktop/pantsbuild --no-publish-dryrun
INFO] Detected git repository at /Users/areitz/workspace/pants on branch master
14:15:32 00:00 [main]
(To run a reporting server: ./pants server)
14:15:32 00:00 [bootstrap]
14:15:32 00:00 [setup]
14:15:32 00:00 [parse]
Executing tasks in goals: bootstrap -> imports -> unpack-jars -> deferred-sources -> gen -> resolve -> compile -> resources -> jar -> doc -> publish
14:15:32 00:00 [bootstrap]
14:15:32 00:00 [bootstrap-jvm-tools]
14:15:32 00:00 [imports]
14:15:32 00:00 [ivy-imports]
14:15:32 00:00 [unpack-jars]
14:15:32 00:00 [unpack-jars]
14:15:32 00:00 [deferred-sources]
14:15:32 00:00 [deferred-sources]
14:15:32 00:00 [gen]
14:15:32 00:00 [thrift]
14:15:32 00:00 [protoc]
14:15:32 00:00 [antlr]
14:15:32 00:00 [ragel]
14:15:33 00:01 [jaxb]
14:15:33 00:01 [wire]
14:15:33 00:01 [aapt]
14:15:33 00:01 [scrooge]
14:15:33 00:01 [resolve]
14:15:33 00:01 [ivy]
14:15:33 00:01 [compile]
14:15:33 00:01 [compile]
14:15:33 00:01 [jvm]
14:15:33 00:01 [jvm-compilers]
14:15:33 00:01 [python-eval]
14:15:33 00:01 [eval-targets]
14:15:33 00:01 [checkstyle]
14:15:33 00:01 [scalastyle]
14:15:33 00:01 [resources]
14:15:33 00:01 [prepare]
14:15:33 00:01 [jar]
14:15:33 00:01 [jar]
14:15:33 00:01 [jar-create].
14:15:33 00:01 [doc]
14:15:33 00:01 [javadoc]
14:15:33 00:01 [scaladoc]
14:15:33 00:01 [publish]
14:15:33 00:01 [publish]Skipping check for a clean master in test mode.
Changes for com.pants.examples#distance-thrift-java since Semver(0.0.0) @ None:
Direct dependencies changed.
Publish com.pants.examples#distance-thrift-java with revision Semver(0.0.1-SNAPSHOT) ? [y|N] y
14:16:31 00:59 [jar-tool]
14:16:31 00:59 [jar-publish]WARNING] Scrubbing JAVA_TOOL_OPTIONS=-Dfile.encoding=utf8
SUCCESS
[prometheus pants (master)]$ ls -la ~/Desktop/pantsbuild/com/pants/examples/distance-thrift-java/0.0.1-SNAPSHOT/
total 104
drwxr-xr-x+ 14 areitz staff 476 Feb 24 14:01 .
drwxr-xr-x+ 3 areitz staff 102 Feb 24 14:01 ..
-rw-r--r--+ 1 areitz staff 4009 Feb 24 14:16 distance-thrift-java-0.0.1-SNAPSHOT-sources.jar
-rw-r--r--+ 1 areitz staff 32 Feb 24 14:16 distance-thrift-java-0.0.1-SNAPSHOT-sources.jar.md5
-rw-r--r--+ 1 areitz staff 40 Feb 24 14:16 distance-thrift-java-0.0.1-SNAPSHOT-sources.jar.sha1
-rw-r--r--+ 1 areitz staff 8088 Feb 24 14:16 distance-thrift-java-0.0.1-SNAPSHOT.jar
-rw-r--r--+ 1 areitz staff 32 Feb 24 14:16 distance-thrift-java-0.0.1-SNAPSHOT.jar.md5
-rw-r--r--+ 1 areitz staff 40 Feb 24 14:16 distance-thrift-java-0.0.1-SNAPSHOT.jar.sha1
-rw-r--r--+ 1 areitz staff 1412 Feb 24 14:16 distance-thrift-java-0.0.1-SNAPSHOT.pom
-rw-r--r--+ 1 areitz staff 32 Feb 24 14:16 distance-thrift-java-0.0.1-SNAPSHOT.pom.md5
-rw-r--r--+ 1 areitz staff 40 Feb 24 14:16 distance-thrift-java-0.0.1-SNAPSHOT.pom.sha1
-rw-r--r--+ 1 areitz staff 1418 Feb 24 14:16 ivy-0.0.1-SNAPSHOT.xml
-rw-r--r--+ 1 areitz staff 32 Feb 24 14:16 ivy-0.0.1-SNAPSHOT.xml.md5
-rw-r--r--+ 1 areitz staff 40 Feb 24 14:16 ivy-0.0.1-SNAPSHOT.xml.sha1I think that java_thrift_library() should be is_jvm, especially if the language is set to java. I didn't really dig any more into this -- it could be related to using the scrooge compiler.
jsirois
commented
9 years ago It has everything to do with the scrooge compiler. ScroogeGen does not inherit from CodeGen and so does not have the behavior I described (except it sort of does for unrelated reasons - see below :/). Larry filed this issue as a non-Tweep, so I assume he is talking about OSS behavior. In OSS pants, ApacheThriftGen is the default compiler for thfit with lang 'java'. Its configured to be the opposite inside Twitter walls (or at least it was in birdcage).
Now ScroogeGen does suffer a similar fate - but not intentionally like CodeGen descendants:
jsirois@gill ~/dev/3rdparty/pants (master *) $ git diff
diff --git a/contrib/scrooge/src/python/pants/contrib/scrooge/tasks/scrooge_gen.py b/contrib/scrooge/src/python/pants/contrib/scrooge/tasks/scrooge_gen.py
index 3973317..ff66dc7 100644
--- a/contrib/scrooge/src/python/pants/contrib/scrooge/tasks/scrooge_gen.py
+++ b/contrib/scrooge/src/python/pants/contrib/scrooge/tasks/scrooge_gen.py
@@ -130,7 +130,9 @@ class ScroogeGen(NailgunTask, JvmToolTaskMixin):
for vt in invalidation_check.invalid_vts:
invalid_targets.extend(vt.targets)
+ print('>>> invalid targets: {}'.format(invalid_targets))
import_paths, changed_srcs = calculate_compile_sources(invalid_targets, self.is_gentarget)
+ print('>>> import paths: {} changed_srcs: {}'.format(import_paths, changed_srcs))
outdir = self._outdir(partial_cmd)
if changed_srcs:
args = []
@@ -240,6 +242,10 @@ class ScroogeGen(NailgunTask, JvmToolTaskMixin):
tgt.add_labels('codegen')
for dependee in dependees:
dependee.inject_dependency(tgt.address)
+ print('\n>>> injected target {}'.format(tgt.address.spec))
+ print(' with br sources {}'.format(tgt.sources_relative_to_buildroot()))
+ print(' with sr sources {}'.format(list(tgt.sources_relative_to_source_root())))
+ print(' as a dependency of {}'.format(dependees))
return tgt
def parse_gen_file_map(self, gen_file_map_path, outdir):
diff --git a/examples/src/thrift/com/pants/examples/distance/BUILD b/examples/src/thrift/com/pants/examples/distance/BUILD
index 24d08b7..04696ca 100644
--- a/examples/src/thrift/com/pants/examples/distance/BUILD
+++ b/examples/src/thrift/com/pants/examples/distance/BUILD
@@ -4,8 +4,16 @@
# trivial example of "generally-useful" thrift to include in other thrift
# (to see how this is included, cd ../precipitation)
+target(name='no_cities_to_love', dependencies=[':distance-java'])
+
java_thrift_library(name='distance-java',
sources=['distance.thrift'],
+ provides=artifact(org='com.pants.examples',
+ name='distance-thrift-java',
+ repo=public),
+ compiler='scrooge',
+ language='java',
+ rpc_style='finagle',
)
python_thrift_library(name='distance-python',
diff --git a/src/python/pants/backend/core/tasks/group_task.py b/src/python/pants/backend/core/tasks/group_task.py
index af3f7d7..dfeed52 100644
--- a/src/python/pants/backend/core/tasks/group_task.py
+++ b/src/python/pants/backend/core/tasks/group_task.py
@@ -300,6 +300,7 @@ class GroupTask(Task):
# built.
ordered_chunks = []
chunks_by_member = defaultdict(list)
+ print('>>> universe of targets:\n\t{}'.format('\n\t'.join(t.address.spec for t in self.context.targets())))
for group_member, chunk in GroupIterator(self.context.targets(), self._group_members):
ordered_chunks.append((group_member, chunk))
chunks_by_member[group_member].append(chunk)
jsirois@gill ~/dev/3rdparty/pants (master *) $ pants.dev clean-all compile examples/src/thrift/com/pants/examples/distance:distance-java -ldebug
...
13:16:14 00:00 [gen]
13:16:14 00:00 [thrift]
13:16:14 00:00 [protoc]DEBUG] Selected protoc binary bootstrapped to: /home/jsirois/.cache/pants/bin/protobuf/linux/x86_64/2.4.1/protoc
13:16:14 00:00 [antlr]
13:16:14 00:00 [ragel]
13:16:14 00:00 [jaxb]
13:16:14 00:00 [wire]
13:16:14 00:00 [aapt]
13:16:14 00:00 [scrooge]
Invalidated 1 target.>>> invalid targets: [JavaThriftLibrary(BuildFileAddress(/home/jsirois/dev/3rdparty/pants/examples/src/thrift/com/pants/examples/distance/BUILD, distance-java))]
>>> import paths: set([u'examples/src/thrift']) changed_srcs: set([u'examples/src/thrift/com/pants/examples/distance/distance.thrift'])
...
>>> injected target .pants.d/gen/scrooge/java-finagle:examples.src.thrift.com.pants.examples.distance.distance-java
with br sources [u'.pants.d/gen/scrooge/java-finagle/com/pants/examples/distance/thriftjava/Distance.java']
with sr sources [u'com/pants/examples/distance/thriftjava/Distance.java']
as a dependency of []
13:16:16 00:02 [resolve]
13:16:16 00:02 [ivy]
13:16:16 00:02 [compile]
13:16:16 00:02 [compile]
13:16:16 00:02 [jvm]
13:16:16 00:02 [jvm-compilers]>>> universe of targets:
examples/src/thrift/com/pants/examples/distance:distance-java
::: created chunks(0)
13:16:16 00:02 [python-eval]
13:16:16 00:02 [eval-targets]
13:16:16 00:02 [checkstyle]
13:16:16 00:02 [scalastyle]
Non synthetic scala targets to be checked:
Non excluded scala sources to be checked:
SUCCESSSo scrooge gen fails to add the synthetic java library to any dependees because there are none (your dummy target adds one and that's why this works). More importantly, the synthetic target it adds to the context does not show up in self.context.targets(). That is a behavior change that came from the big target refactor in May 2014 and thus broke ScroogeGen at that time. If you read the code for add_new_target and targets in Context pre-refactor [1], you'll see the synthetic target would have shown up in self.context.targets() at that point in time.
I encourage less thought and more code reading here. Thinking is vastly over-rated.
dturner-tw
commented
9 years ago I believe https://rbcommons.com/s/twitter/r/1840/ fixes this.
to see...
edit src/thrift/com/pants/examples/distance/distance.thrift:
$ ./pants goal compile src/thrift/com/pants/examples/distance:distance-java
Happily says
$ ./pants goal test src/thrift/com/pants/examples/distance:distance-java gets SUCCESS $ ./pants goal compile tests/java/com/pants/examples/usethrift # <- test which deps on distance:java gets an ERROR (whew!)
Mmmaybe this is a problem w/Apache-thrift integration. I say this because putting a compiler='scrooge', into the BUILD target makes a compile happen. (if your thrift is bad, you get an error. if your thrift is good, you get class files in .pants.d.)