Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

RFC: Introduce build rules for building GWT applications. #109

Closed
bolinfest opened this issue Apr 29, 2014 · 58 comments
Closed

RFC: Introduce build rules for building GWT applications. #109

bolinfest opened this issue Apr 29, 2014 · 58 comments

Comments

@bolinfest
Copy link
Contributor

I'm looking into adding build rules for building GWT applications. I'm curious what folks think the arguments to the rules should look like.

  • Do we need a special gwt_library() rule, or can we just use java_library()? My intuition is that if you use JSNI in your Java code, then you need to classify that as a gwt_library().
  • How do you deal with dependencies that have dual implementations: one for the JRE and one for JavaScript? For example, you may have a subgraph of dependencies that transitively depend on Guava. Then at the top, you have a gwt_binary() target and a java_binary() target? How do you ensure that the gwt_binary() pulls in guava-gwt-17.0.jar while the java_binary() pulls in guava-17.0.jar? One option is to have parallel versions of each subgraph, but that seems messy.
  • How can we design gwt_binary() (or this whole system of rules) so that it is easy to run the GWT app in developer mode? Ideally, a developer should not have to run buck build after every code modification in order to reload the GWT app. The existing edit-refresh-test cycle needs to be preserved.
@bolinfest
Copy link
Contributor Author

One argument that gwt_library() would have beyond an ordinary java_library() rule is that of a module_file argument to specify a .gwt.xml file associated with the rule.

@bolinfest
Copy link
Contributor Author

The Gerrit project (which uses Buck!) has defined gwt_module() and gwt_application() macros in https://gerrit.googlesource.com/gerrit/+/master/tools/default.defs.

@skybrian
Copy link

I don't think JSNI is too much of an issue since it's designed so you can compile it as Java. It will be a native method with no implementation.

In the build tool's implementation, you probably want to have separate, mostly-parallel DAG's of build actions to perform for Java and GWT, so that the GWT output doesn't have to be rebuilt when you only want Java. However, it's annoying to have to maintain separate rules for GWT and non-GWT library variants in build files. So I think java_library should handle both, but you will need special attributes for GWT. For example, there may be gwt-only dependencies, Java-only dependencies, and shared dependencies. It might also be good to distinguish GWT generator source code (typically in a "rebind" directory) and dependencies needed only by the generator.

Dev Mode is slowly going away so I wouldn't worry about that too much. For the GWT compiler and Super Dev Mode, you need to construct a Java classpath containing all the dependencies and use that to start the GWT tool. (We are also working on incremental compilation for GWT, but that's not far enough along to worry about quite yet.)

Another issue for (Super)DevMode is that you want to put the original source files into the classpath, so that when the user edits a file, the GWT compiler sees the changes without having to run buck. However, the GWT compiler doesn't recompile code used by generators, so the bytecode for generators has to be in the classpath. (In addition, some GWT generators assume Java bytecode exists so they can use Class.forName() and reflection.)

@tbroyer
Copy link

tbroyer commented Apr 29, 2014

There are three kind of GWT artifacts: shared libraries (shared between JVM -e.g. server, or Android client- that is), client-only libraries, and applications.

The difference between client and shared libs is about where the classes go. Shared libs have most classes in a non-GWT jar, and another GWT-specific jar with GWT-specific files and classes and the gwt.xml. Client-only libs on the other hand do not need that separation.
Applications are the result of compiling a GWT module to JS.

Shared libraries are the toughest ones to deal with, for the reasons you gave. My mind has been corrupted by Maven and the like, but I think a parallel tree is what works best: shared1-gwt depends on shared1, shared2 depends on shared1, shared2-gwt depends on shared2 and shared1-gwt.
Ideally, a build rule would have 2 outputs, and the gwt one would depend on the gwt variants of the rule dependencies.

Also, if a GWT library (client or shared) has a single gwt.xml, it's possible to generate/merge from the rule dependencies.

I started doing something along these lines for Maven (where separation is strict), an was thinking of doing the same for Gradle (where you could generate both jars if a shared lib from the same project).

@tbroyer
Copy link

tbroyer commented Apr 29, 2014

As for dev mode, first you'll want to use superdevmode nowadays.

Size of the classpath can have a big impact on performance, so you don't want to put your source tree there. Following Buck's strategy for other things, you'll rather symlink files. For a better edit-refresh-cycle (including when adding files), maybe you'd rather symlink directories instead.

And you'll of course need to use the gwt.xml too.

@spearce
Copy link
Contributor

spearce commented Apr 29, 2014

On Tue, Apr 29, 2014 at 12:06 PM, bolinfest notifications@github.comwrote:

The Gerrit project (which uses Buck!) has defined gwt_module() and
gwt_application() macros in
https://gerrit.googlesource.com/gerrit/+/master/tools/default.defs.

The comments from Brian and Thomas about needing a dual graph are correct.
Gerrit has to define a dual graph for some libraries, for example:

https://gerrit.googlesource.com/gerrit/+/HEAD/gerrit-gwtexpui/BUCK

We define a java_library(name='server') that has some overlap in srcs with
gwt_module() in the same BUCK file. This is awkward to use but mostly
sensical. From a gwt_module() or gwt_application() we link to a
gwt_module(), from a java_library() or java_application() we link to the
java_library().

For us gwt_module() is defined as a simple rule to collect the sources into
a JAR for inclusion in a CLASSPATH later when the GWT compiler is invoked.
For performance reasons we do not invoke javac here and instead defer Java
language verification to the GWT compiler. The dual graph means the same
source files may be built in parallel by a java_library() that does invoke
javac. This is one reason we don't run javac from a gwt_module() rule.

For runtime "edit-reload-test" cycle our application server knows how to
reinvoke buck to regenerate the JavaScript code for the page:

https://gerrit.googlesource.com/gerrit/+/HEAD/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
see ~line 568 method useDeveloperBuild()

We have a genrule() in buck output a properties file that the server can
read passing it the environment necessary to relaunch buck. We run Buck in
a filter on the URL that serves the JS, allowing Buck to verify the JS is
up-to-date after an edit before serving. In a draft mode compile this is
O(22 seconds) after edit to serve the JS. Sluggish but preserves the
edit-refresh cycle developers are used to.

@bolinfest
Copy link
Contributor Author

Thanks everyone: this is all incredibly helpful!

I haven't started playing around with SuperDevMode, but I certainly want to
support it. How hard would it be to modify it so that, as an option,
instead of reading files from a classpath, it can take in a manifest file
that lists all of the files that it would normally look up as classpath
entries?

The reason I suggest this is that it would be a lot easier to do things "in
place" without compromising your directory structure. For example, suppose
I had:

java/com/example/stuff/Shared.java
java/com/example/stuff/client/Client.java
java/com/example/stuff/server/Server.java

If I included java/com/example/stuff/ as a classpath entry, then that would
include java/com/example/stuff/server/Server.java, which I do not want.
Alternatively, I could create a symlink farm or copy files into a temp
directory, but that's a bunch of I/O that I could avoid if I could just
generate a file with the contents:

java/com/example/stuff/Shared.java
java/com/example/stuff/client/Client.java

and then pass the path of that manifest file as an input to SuperDevMode.
It would be up to me to regenerate that manifest when files are added or
removed.

Thoughts?
Michael

On Tue, Apr 29, 2014 at 3:02 PM, Shawn O. Pearce
notifications@github.comwrote:

On Tue, Apr 29, 2014 at 12:06 PM, bolinfest <notifications@github.com

wrote:

The Gerrit project (which uses Buck!) has defined gwt_module() and
gwt_application() macros in
https://gerrit.googlesource.com/gerrit/+/master/tools/default.defs.

The comments from Brian and Thomas about needing a dual graph are correct.
Gerrit has to define a dual graph for some libraries, for example:

https://gerrit.googlesource.com/gerrit/+/HEAD/gerrit-gwtexpui/BUCK

We define a java_library(name='server') that has some overlap in srcs with
gwt_module() in the same BUCK file. This is awkward to use but mostly
sensical. From a gwt_module() or gwt_application() we link to a
gwt_module(), from a java_library() or java_application() we link to the
java_library().

For us gwt_module() is defined as a simple rule to collect the sources into
a JAR for inclusion in a CLASSPATH later when the GWT compiler is invoked.
For performance reasons we do not invoke javac here and instead defer Java
language verification to the GWT compiler. The dual graph means the same
source files may be built in parallel by a java_library() that does invoke
javac. This is one reason we don't run javac from a gwt_module() rule.

For runtime "edit-reload-test" cycle our application server knows how to
reinvoke buck to regenerate the JavaScript code for the page:

https://gerrit.googlesource.com/gerrit/+/HEAD/gerrit-pgm/src/main/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
see ~line 568 method useDeveloperBuild()

We have a genrule() in buck output a properties file that the server can
read passing it the environment necessary to relaunch buck. We run Buck in
a filter on the URL that serves the JS, allowing Buck to verify the JS is
up-to-date after an edit before serving. In a draft mode compile this is
O(22 seconds) after edit to serve the JS. Sluggish but preserves the
edit-refresh cycle developers are used to.


Reply to this email directly or view it on GitHubhttps://github.com//issues/109#issuecomment-41737961
.

@skybrian
Copy link

Sure, adding support for a sourcefile manifest wouldn't be too hard. However, in a way, the gwt.xml files are already a manifest if you don't use globs. Would Buck be generating them?

Also, you probably want to build using the GWT compiler's -strict flag (also known as -failOnErrors) by default; this will help ensure you get compile errors if your gwt.xml files include too much.

@tbroyer
Copy link

tbroyer commented Apr 30, 2014

I think we should change GWT to load from a dedicated class loader ultimately.

It could be set to the system class loader by default for backwards compatibility, but if it's swappable then it becomes possible to pass a classpath as argument to the GWT compiler/superdevmode/etc. and have it construct a URLClassLoader; and you could then also swap in a specialized class loader that would respect Buck's srcs and resources (and in Maven and Gradle plugins, we could then do something similar too, without the need to fork a new JVM to trim the classpath).

That said, for that to happen, we'd probably first have to remove classpath scanning from GWT (the custom class loader for Buck would then be about ensuring some file that isn't in the srcs or resources won't ever be loaded by GWT; rather than limiting the size of the classpath to be scanned)

@bolinfest
Copy link
Contributor Author

I'm learning as I go with all things GWT right now, so please bear with me.

@tbroyer "Also, if a GWT library (client or shared) has a single gwt.xml, it's possible to generate/merge from the rule dependencies."

Is it possible for a GWT library (or "module" in your parlance?) to have more than one gwt.xml file? From a build perspective, it appears that they are just resources in a JAR file, so I guess there is no reason why there cannot be more than one. Is there ever a case where additional metadata about the gwt.xml files needs to be written to the JAR's manifest or anywhere else? The answer to this question determines whether a java_library() should (1) take a single gwt_xml argument as a string, (2) take a single gwt_xml argument as a list of strings, or (3) not special-case gwt_xml as its own argument and require developers to use the existing resources argument.

@skybrian "However, the GWT compiler doesn't recompile code used by generators, so the bytecode for generators has to be in the classpath. (In addition, some GWT generators assume Java bytecode exists so they can use Class.forName() and reflection.)"

Does the GWT compiler normally need the .class files for the .java files for which it is generating JavaScript? Or is that only for the special generator case? When the .class files are not needed, it would be nice if Buck could avoid running javac (assuming it doesn't add any value). (@spearce I believe this is what you are doing in https://gerrit.googlesource.com/gerrit/+/master/tools/default.defs by having gwt_module() generate a java_library() that treats all of its srcs as resources.)

@skybrian After playing around with things myself, I agree that having separate java_library() and gwt_library() rules are absolutely a pain to maintain, and that it makes more sense to offer additional, optional arguments on a java_library() to support GWT. Here's my primary hangup (consider the following scenario):

java_library(
  name = 'lib1',
  srcs = glob(['lib1/**/*.java']),
)

java_library(
  name = 'lib2',
  srcs = glob(['lib2/**/*.java']),
  deps = [ ':lib1', ],
)

gwt_binary(
  name = 'app',
  module = 'com.example.client.TheClient',
  deps = [ ':lib2', ],
)

And you run the following command:

buck build //:app

You might expect that this would not run javac at all: it should just run com.google.gwt.dev.Compiler to generate the JavaScript from the Java code. The problem is that Buck has a strong invariant where every rule listed in a rule's deps is built before the rule itself. This means that it will build :lib2 and therefore :lib1, which means running javac twice.

We have worked hard to avoid a situation where a rule is allowed to "look upwards" to decide how to build its deps based on what the ultimate thing it is trying to build (e.g., java_binary() vs. gwt_binary()). Therefore, I would not want to add semantics to Buck where a java_library() decides not to javac if it is in the transitive deps of a gwt_binary() but not a java_binary().

We have been able to avoid the "look upwards" thing using a technique we call graph_enhancement, which I won't get into here (it probably deserves its own write-up on http://facebook.github.io/buck/). Under the hood, for a java_library named //foo:bar, we could generate a rule whose name was //foo:bar#srcs_plus_resources that would be a node in the build graph with no edges. Its output would be a JAR containing the srcs and resources of the java_library() rule.

The gwt_binary() would have a separate argument named src_deps (or something like that: I'm open to suggestions) that you would use instead of deps (so that deps maintains its original semantics). When building a gwt_binary() rule, it would traverse its src_deps, looking for the set of reachable java_library()-ish rules (which would include things like prebuilt_jar). Upon finding those, it would take the build target of each rule and tack on a #srcs_plus_resources to it, and add all such targets to its deps. This would connect those #srcs_plus_resources nodes in the build graph that previously had no edges to or from them.

Though this would allow us to reuse java_library(), the one benefit I could see to having a separate gwt_xml argument for java_library() (instead of requiring developers to overload the existing resources argument) would be that we would not add the .gwt.xml files to the JAR generate by java_library() with all of the .class files.

What do folks think of this approach? Currently I'm skirting some issues like guava-17.0.jar vs. guava-gwt-17.0.jar, but I'll focus on that after.

@skybrian
Copy link

skybrian commented May 1, 2014

On Wed, Apr 30, 2014 at 4:50 PM, bolinfest notifications@github.com wrote:

I'm learning as I go with all things GWT right now, so please bear with me.

@tbroyer https://github.com/tbroyer "Also, if a GWT library (client or
shared) has a single gwt.xml, it's possible to generate/merge from the rule
dependencies."

Is it possible for a GWT library (or "module" in your parlance?) to have
more than one gwt.xml file?

What we call a "GWT module" is by definition one gwt.xml file and whatever
source code it includes. However, any given jar file can have multiple GWT
modules within it. For example, gwt-user.jar in the SDK has many GWT
modules.

For prebuilt jar files, you might want to have a gwt_import rule that takes
a jar file and the name of the module to use.

From a build perspective, it appears that they are just resources in a
JAR file, so I guess there is no reason why there cannot be more than one.
Is there ever a case where additional metadata about the gwt.xml files
needs to be written to the JAR's manifest or anywhere else?

Normally nothing special is needed in the jar's manifest. When you start up
the GWT compiler, you give it the name of the top-level module you want and
it looks for that gwt.xml file in the classpath (looks it up as a
resource), and finds the transitive dependencies from there. Once it finds
all the modules, it compiles all their source code together (ignoring
module boundaries).

As a result, if you just compile using the GWT compiler, you won't catch
errors where one library depends on another one but doesn't declare the
dependency. Also, GWT modules can have circular dependencies, which is
probably a bad idea.

For incremental compilation, we will need to clean this up. In Buck, it's
probably a good idea to be stricter about dependencies from the beginning.

The answer to this question determines whether a java_library() should
(1) take a single gwt_xml argument as a string, (2) take a single gwt_xmlargument as a list of strings, or (3) not special-case
gwt_xml as its own argument and require developers to use the existing
resources argument.

If you use gwt.xml files as-is, there's nothing to keep their dependencies
in sync with the declared dependencies in Buck. So our usual approach isn't
to check in the entire gwt.xml file. Instead we check in a fragment
(everything but the dependencies) , and generate the part of the gwt.xml
file containing the deps. But you could also write automatic checks to
verify they're in sync, or generate the entire gwt.xml file from
GWT-specific attributes.

@skybrian https://github.com/skybrian "However, the GWT compiler
doesn't recompile code used by generators, so the bytecode for generators
has to be in the classpath. (In addition, some GWT generators assume Java
bytecode exists so they can use Class.forName() and reflection.)"

Does the GWT compiler normally need the .class files for the .java files
for which it is generating JavaScript? Or is that only for the special
generator case?

The two cases I know of are generators using reflection and the GWT
compiler itself needs to load annotation classes. It's rare enough that in
Super Dev Mode, we can get away with adding source code without adding it
to the classpath (using the -src flag), but unfortunately doesn't work in
all cases.

When the .class files are not needed, it would be nice if Buck could
avoid running javac (assuming it doesn't add any value). (@spearcehttps://github.com/spearceI believe this is what you are doing in
https://gerrit.googlesource.com/gerrit/+/master/tools/default.defs by
having gwt_module() generate a java_library() that treats all of its srcsas
resources.)

It does add a fair bit of value since javac will report better compiler
errors than the GWT compiler.

We have been able to avoid the "look upwards" thing using a technique we
call graph_enhancement, which I won't get into here (it probably
deserves its own write-up on http://facebook.github.io/buck/). Under the
hood, for a java_library named //foo:bar, we could generate a rule whose
name was //foo:bar#srcs_plus_resources that would be a node in the build
graph with no edges. Its output would be a JAR containing the srcs and
resources of the java_library() rule.

Yes, something like graph enhancement is similar to what I was thinking
about. You need parallel build graphs in the implementation but the user
shouldn't have to have near-duplicate rules in the build file.

However, incremental compiles will be a problem; there's currently no way
to do it without a "look upwards". This is because any GWT module can
contribute configuration properties (last one wins in a post-order
traversal) and these properties are global. You need a separate pass over
the dependency tree to collect the properties and then to compile each
library. We're still figuring that out ourselves, so perhaps punt on that
for now.

  • Brian

@tbroyer
Copy link

tbroyer commented May 1, 2014

On Thu, May 1, 2014 at 6:22 PM, Brian Slesinsky notifications@github.comwrote:

On Wed, Apr 30, 2014 at 4:50 PM, bolinfest notifications@github.com
wrote:

I'm learning as I go with all things GWT right now, so please bear with
me.

@tbroyer https://github.com/tbroyer "Also, if a GWT library (client or

shared) has a single gwt.xml, it's possible to generate/merge from the
rule
dependencies."

Is it possible for a GWT library (or "module" in your parlance?) to have
more than one gwt.xml file?

What we call a "GWT module" is by definition one gwt.xml file and whatever
source code it includes. However, any given jar file can have multiple GWT
modules within it. For example, gwt-user.jar in the SDK has many GWT
modules.

For prebuilt jar files, you might want to have a gwt_import rule that takes
a jar file and the name of the module to use.

From a build perspective, it appears that they are just resources in a
JAR file, so I guess there is no reason why there cannot be more than
one.
Is there ever a case where additional metadata about the gwt.xml files
needs to be written to the JAR's manifest or anywhere else?

Normally nothing special is needed in the jar's manifest. When you start up
the GWT compiler, you give it the name of the top-level module you want and
it looks for that gwt.xml file in the classpath (looks it up as a
resource), and finds the transitive dependencies from there. Once it finds
all the modules, it compiles all their source code together (ignoring
module boundaries).

As a result, if you just compile using the GWT compiler, you won't catch
errors where one library depends on another one but doesn't declare the
dependency. Also, GWT modules can have circular dependencies, which is
probably a bad idea.

For incremental compilation, we will need to clean this up. In Buck, it's
probably a good idea to be stricter about dependencies from the beginning.

This is why I said earlier that you'd want to symlink files like you do for
javac, to make sure you don't cross boundaries (launching GWT –compiler or
(super)devmode– using the sources would be different from launching it
using the packaged files).
For example, you could have:

  • src/lib1/BUCK defines a lib1 library with Lib1.gwt.xml and client/**
  • src/lib1/Lib1.gwt.xml
  • src/lib1/client/…
  • src/lib2/BUCK defines lib2 library with Lib2.gwt.xml and client/**, but
    missing the //lib1:lib1 dependency
  • src/lib2/Lib2.gwt.xml contains (i.e; depends
    on lib1/Lib1.gwt.xml)
  • src/lib2/client/…

If you put src/ on the classpath, then GWT will find lib1/* despite the
missing dependency.
If you package things in JARs (or symlink files into buck-out/) then
lib1/
* won't be in the classpath and you'll have an error. Add the missing
dependency from //lib2:lib2 to //lib1:lib1 to fix it.

Computing the GWT dependencies from Buck dependencies (see below) frees you
from having to keep GWT and Buck deps synchronized.

The answer to this question determines whether a java_library() should

(1) take a single gwt_xml argument as a string, (2) take a single
gwt_xmlargument as a list of strings, or (3) not special-case

gwt_xml as its own argument and require developers to use the existing
resources argument.

If you use gwt.xml files as-is, there's nothing to keep their dependencies
in sync with the declared dependencies in Buck. So our usual approach isn't
to check in the entire gwt.xml file. Instead we check in a fragment
(everything but the dependencies) , and generate the part of the gwt.xml
file containing the deps. But you could also write automatic checks to
verify they're in sync, or generate the entire gwt.xml file from
GWT-specific attributes.

I tried to do something similar for Maven, but it has to look at a file
within the JAR to determine the module contained within (assuming there's
only one; otherwise the dependency cannot be determined).
The file is META-INF/gwt/mainModule and contains the name of the "main
module" of the dependency. Adding the dependency to your pom.xml is enough
for the GWT maven plugin (net.ltgt.gwt.maven:gwt-maven-plugin, not
org.codehaus.mojo:gwt-maven-plugin) to generate the .
Contrary to Google, the plugin uses a complete XML document as base
(defaults to src/main/module.gwt.xml) rather than a fragment, and merges in
rules.

@skybrian https://github.com/skybrian "However, the GWT compiler

doesn't recompile code used by generators, so the bytecode for generators
has to be in the classpath. (In addition, some GWT generators assume Java
bytecode exists so they can use Class.forName() and reflection.)"

Does the GWT compiler normally need the .class files for the .java files
for which it is generating JavaScript? Or is that only for the special
generator case?

The two cases I know of are generators using reflection and the GWT
compiler itself needs to load annotation classes. It's rare enough that in
Super Dev Mode, we can get away with adding source code without adding it
to the classpath (using the -src flag), but unfortunately doesn't work in
all cases.

There's also the case of classes referenced by annotations (e.g.
@MyAnnotation(someProperty=SomeClass.class) )

When the .class files are not needed, it would be nice if Buck could
avoid running javac (assuming it doesn't add any value). (@spearce<
https://github.com/spearce>I believe this is what you are doing in

https://gerrit.googlesource.com/gerrit/+/master/tools/default.defs by
having gwt_module() generate a java_library() that treats all of its
srcsas
resources.)

It does add a fair bit of value since javac will report better compiler
errors than the GWT compiler.

You'll also need to javac the classes if you test them.

Thomas Broyer
/tɔ.ma.bʁwa.je/ http://xn--nna.ma.xn--bwa-xxb.je/

@bolinfest
Copy link
Contributor Author

Thanks for all of the info! I am actively working on a naive implementation of gwt_binary() that can graph-enhance a graph of java_library() rules so it can build a GWT application using the GWT compiler without running all of the intermediate javac commands (modulo the annotation issues that you raised).

I think that it would make sense to have an argument to gwt_binary() that declares whether you want to javac your deps or not. That way, you can have a "fast" mode that calls the GWT compiler directly, or a "slow" mode that runs javac and gives you better errors.

I think it's important for me to land the initial version and then iterate on it from there. It hear what you're saying and it definitely makes sense for Buck to generate the appropriate bits of the .gwt.xml files from its dependency graph, though the initial version that I land won't do that. Once I lay the groundwork, you could also feel free to start sending pull requests my way!

@bolinfest
Copy link
Contributor Author

I am still working on getting my diffs on gwt_binary() through review. Once those are out, hopefully we can have a more concrete discussion.

However, while we're waiting for those, it seems like it currently takes ~20s for the GWT compiler to run on a modest size app (yes, that is with using -optimize 0). I want faster builds and I've read up about Super Dev Mode (http://www.gwtproject.org/articles/superdevmode.html), but I'm reluctant to invest in the time to set it up because that doc is from 2012 and I haven't seen anything that says that it's good to go yet, so it doesn't sound like it's ready.

For those of you who are using it, does it get edit-refresh-test cycles down from ~20s to 1s? Historically, I am a JavaScript developer, so although I love IDEs and static typing, I don't know if I would opt for that in favor of faster dev cycles. For the JS/Closure build tool that I wrote (http://plovr.com/), you can control the level of compilation that you want via query parameters. Most of the time, you want "raw" mode where the Closure Compiler does absolutely nothing and you have one <script> tag per input file. Historically, this could have been prohibitively slow, but modern browsers like Chrome can load hundreds of <script> tags with ease. Using SHA-1s of the file contents as etags can make things even faster.

I don't know how GWT works, but can it be designed to work this way? That is, can each input .java file be translated to a JavaScript file independently? Then CodeServer can cache these transformations and serve each as its own <script> tag. I would expect that this makes iterative development extremely fast, so I hope that this is how Super Dev Mode works.

Admittedly, this might forgo a lot of error-checking, but I am relying on the IDE to do most of my static checking, as is. I could always run a full [slow] build if I am seeing things that I do not expect in the browser because I am playing fast and loose with the "raw" mode that I described.

@skybrian
Copy link

skybrian commented May 6, 2014

Super Dev Mode as it exists today is basically a wrapper around draft mode. (Note that -draft is not quite the same as -optimize 0; it sets some additional flags). It gets a bit of speed on subsequent compiles because it's running the compiler in the same JVM, so some things get cached. But it's fundamentally not that different from -draft.

The compiler improvements you describe are basically what we call "incremental compilation". John Stalcup has been working on this for many months and is now adding it to Super Dev Mode; it's available on trunk behind the "-incremental" flag. However, many modules (including within the GWT SDK itself) don't work with it yet. Also, it's not always faster yet; we're currently focused on making the migration smoother.

We did Super Dev Mode first because we control the entire build process that way, but it's definitely a goal to make incremental compilation work with other build systems. It would be great if we could support it in Buck. But it's a little early to get started, unless you want to jump into working on GWT's internals. (It might be worth looking into it and giving us feedback on the design.)

There will be some significant improvements to Super Dev Mode in GWT 2.7 which we're planning on releasing around June. Also, we're currently releasing GWT 2.6.1 but that's just a bugfix release. (The main change for Super Dev Mode in 2.6.1 is getting it to work on Windows.)

@skybrian
Copy link

skybrian commented May 6, 2014

Also, this isn't going to help get you from 20s to 1s, but it's an important speedup for larger GWT projects: it's possible to optimize each permutation in a separate JVM, in parallel.

The entry points for that are com.google.gwt.dev.Precompile for the first phase, com.google.gwt.dev.CompileOnePerm to compile a permutation (this can be run in parallel), and com.google.gwt.dev.Link to assemble the permutations into a single output directory.

In Google's build system, we use this to compile each permutation on a different machine.

bolinfest added a commit that referenced this issue May 9, 2014
Summary:
Implement `gwt_binary()` as a result of the discussion at:
#109

Using `gwt_binary()` looks like the following:

```
gwt_binary(
  name = 'my_app',
  modules = [
    'com.example.client.TheClient', # GWT module to build.
  ],
  style = 'PRETTY',
  draft_compile = True,
  optimize = 0,
  local_workers = 4,
  module_deps = [
    # List of java_library() build targets that function as GWT modules.
  ],
  deps = [
    # Traditional deps.
    # One of these should be a java_library() (or prebuilt_jar()) rule that
    # has the definition of the com.google.gwt.dev.Compiler class.
  ],
)
```

For each Java library (i.e., `java_library()` or `prebuilt_jar()`) in `module_deps`,
a source JAR is generated and is included on the classpath when running
the GWT compiler. Therefore, building a `gwt_binary()` does not entail
running `javac` to build each of the rules listed in `module_deps`. If you want
to make sure that `javac` is run for your library, you can always list your Java library in `deps`.
(In the future, I will probably add an argument to `gwt_binary()` to specify
whether `module_deps` get built like regular deps.)

Note that the rules in the `module_deps` list will likely want to include their
`.gwt.xml` files as Java resources. As such, you might want to define the following
macro in your build:

```
import copy

def gwt_module(
    gwt_xml=None,
    **kwargs
    ):
  """A GWT library is defined by its .java source code, a .gwt.xml module file, and resources."""
  java_kwargs = copy.copy(kwargs)
  if not 'resources' in java_kwargs:
    java_kwargs['resources'] = []
  if gwt_xml:
    java_kwargs['resources'] += [gwt_xml]
  java_library(**java_kwargs)
```

Going forward, whether we support a `gwt_module()` rule or just add a `gwt_xml` argument to `java_library()`
is open for debate. For now, we would like to add support for GWT while introducing as few
new concepts as possible to Buck.

This diff also introduces a `gwt_jar` argument to `prebuilt_jar`. This is useful for
defining Guava:

```
prebuilt_jar(
  name = 'guava',
  binary_jar = 'guava-17.0.jar',
  source_jar = 'guava-17.0-sources.jar',
  gwt_jar = 'guava-gwt-17.0.jar',
  visibility = [
    'PUBLIC',
  ],
)
```

This is imperative when you have an alternative GWT implementation of a Java library.
Specifically, when you have one implementation that contains JSNI, and one implementation that
contains an implementation that cannot be translated to JS by the GWT compiler.

Finally, here is an example of using macros to create "debug" and "release" targets for the same GWT application:

```
java_library(
  name = 'debug_module',
  resources = [ 'Example_debug.gwt.xml', ],
)

java_library(
  name = 'release_module',
  resources = [ 'Example_release.gwt.xml', ]
)

def create_gwt_binary_rules(name):
  for is_debug in [True, False]:
    gwt_binary(
      name = name + '_' + ('debug' if is_debug else 'release'),
      style = 'PRETTY' if is_debug else 'OBF',
      draft_compile = is_debug,
      optimize = 0 if is_debug else 9,
      local_workers = 3 if is_debug else 8,
      modules = [
        'com.example.Example_' + ('debug' if is_debug else 'release'),
      ],
      module_deps = [
        # List the common module_deps here.
      ] + [':debug_module' if is_debug else ':release_module'],
      deps = [
        # List the common deps here.
      ],
    )

# Creates :example_debug and :example_release.
create_gwt_binary_rules('example')
```

Test Plan: Retrofit an existing GWT application with `BUCK` build files and build it using Buck.
@bolinfest
Copy link
Contributor Author

I apologize that this took so long, but I just landed an implementation of gwt_binary() to make this discussion more concrete: 2af62f4. The commit message contains details about how to use it. I don't want to document it on http://facebook.github.io/buck/ until I get some external confirmation that this is the right way to go.

@bolinfest
Copy link
Contributor Author

Are relative paths in @Source supposed to work? Someone on my team recently introduced a @Source that includes ../../packagename/File.css and I'm seeing errors from the GWT compiler along the lines of:

[ERROR] Resource ../../packagename/File.css not found. Is the name specified as Class.getResource() would expect?

Is this because I am passing source jars to com.google.gwt.dev.Compiler that do not contain any .class files? If so, this seems like a bug.

I'm hoping to make Buck build GWT apps quickly by avoiding javac altogether, which means not generating .class files.

@bolinfest
Copy link
Contributor Author

It seems that if I use an "absolute path" with no leading slash, then things work.

@spearce
Copy link
Contributor

spearce commented May 13, 2014

I just tried using 2af62f4 to build Gerrit Code Review. It appears to work, but we lost some speed.

Gerrit passes -Dgwt.normalizeTimestamps=true -war $OUT to the compiler to have it do the ZIP creation, rather than writing temporary files that are zipped in the caller. This saved us a few seconds on build time.

We used to pass -XdisableClassMetadata -XdisableCastChecking, but there is no way to do this from gwt_binary().

I am confused about why the modules argument to gwt_binary() is a list. IIRC the compiler only accepts one module at a time for compilation, the top level that defines the "main" of the JavaScript application.

The resulting .war is 2M larger for us. It looks like the WEB-INF/deploy/gerrit_ui/symbolMaps directory is now being packaged, while before we put them into a throw-away location using -deploy flag to the GWT compiler. We may be unique by not using this data.

There is no way to set the heap memory of the JVM running the GWT compiler. Gerrit had to pass -Xmx512m, as some platforms our developers use were getting too small of a heap by default.

@spearce
Copy link
Contributor

spearce commented May 13, 2014

Gerrit also used to pass -strict to the GWT compiler, but it looks like this is not an option recognized by gwt_binary() in 2af62f4.

@shs96c
Copy link
Contributor

shs96c commented May 13, 2014

All helpful feedback. Thank you. I think some of the points you raise are
just that we're relative newbies at compiling GWT: it'd be pretty easy to
make the module a single entry rather than a set. It sounds like some kind
of "compiler_flags" parameter would be useful for you (I'm guessing the -X*
options are ones that aren't commonly used?) as well as a "jvm_flags" too.
Would that be the appropriate thing to do?

The alternative to the "jvm_flags" would be to have a parameter in your
.buckconfig. That feels a little cleaner to me, since it removes the need
for every gwt target to consider whether or not it's going to get big.

Regards,

Simon

On Wed, May 14, 2014 at 12:09 AM, Shawn O. Pearce
notifications@github.comwrote:

Gerrit also used to pass -strict to the GWT compiler, but it looks like
this is not an option recognized by gwt_binary() in 2af62f42af62f4
.


Reply to this email directly or view it on GitHubhttps://github.com//issues/109#issuecomment-43024641
.

@spearce
Copy link
Contributor

spearce commented May 13, 2014

On Tue, May 13, 2014 at 4:37 PM, Simon Stewart notifications@github.com wrote:

All helpful feedback. Thank you. I think some of the points you raise are
just that we're relative newbies at compiling GWT: it'd be pretty easy to
make the module a single entry rather than a set.

I think a single entry makes sense, but maybe some of the other GWT
folks on this issue can verify.

It sounds like some kind
of "compiler_flags" parameter would be useful for you (I'm guessing the -X*
options are ones that aren't commonly used?)

Right these are not common so a generic flag parameter for the
uncommon ones would be great.

as well as a "jvm_flags" too.
Would that be the appropriate thing to do?

Yes, but the global setting may be better:

The alternative to the "jvm_flags" would be to have a parameter in your
.buckconfig. That feels a little cleaner to me, since it removes the need
for every gwt target to consider whether or not it's going to get big.

+1, this may be cleaner.

@tbroyer
Copy link

tbroyer commented May 14, 2014

I am confused about why the modules argument to gwt_binary() is a list. IIRC the compiler only accepts one module at a time for compilation, the top level that defines the "main" of the JavaScript application.

You can pass several modules to the GWT compiler. It outputs each one's output to a different directory within the -war. I've never needed it, but I suppose it could speed things up compared to separate compilations by sharing internal state between modules. It might become moot with incremental compilation being added to GWT though. @skybrian can probably tell you more about it.

The resulting .war is 2M larger for us. It looks like the WEB-INF/deploy/gerrit_ui/symbolMaps directory is now being packaged, while before we put them into a throw-away location using -deploy flag to the GWT compiler. We may be unique by not using this data.

You're far from unique.

symbolMaps are useful for deobfuscating stacktraces logged by the client, but a) I suspect not many people actually send logs to the server and b) it doesn't work out of the box as it needs some configuration: both GWT-RPC-based RemoteLoggingService and RequestFactory-based Logging only read symbol maps from the file system whereas WEB-INF/deploy is generally not extracted, or into temporary directories whose name is hard to guess; the alternative is to use JsonLogRecord{Client,Server}Util from your code.

Gerrit also used to pass -strict to the GWT compiler

Brian proposed this should be the default.

@bolinfest
Copy link
Contributor Author

@spearce Awesome: thanks for the feedback! I will try to add options for these things and land this today.

I have been doing specific arguments instead of a general "compiler args" list because:

(1) When I finally write up documentation, it is easier for users to see all of the available options.
(2) If we want Buck to change its behavior based on any of these options, I think it would be easier to check the option explicitly rather than try to parse a list of complier args and infer the right thing.

What do you think?

bolinfest added a commit that referenced this issue May 14, 2014
Summary:
This is in response to Shawn Pearce's feedback on
#109.
Hopefully this addresses all of the issues.

Test Plan: Retrofit an existing GWT application with `BUCK` build files and build it using Buck.
@bolinfest
Copy link
Contributor Author

@spearce OK, I think that I addressed everything in your feedback in 5a9d2a4. Could you try to patch it in and see if you can use gwt_binary() without sacrificing performance?

@skybrian
Copy link

I think we do need a catch-all for various GWT compiler arguments, which should be appended at the end. (In the GWT compiler, flags that come later in the list override flags that come earlier.) There are lots of flags, some are experimental, and we do change them in new GWT releases (while trying to remain backward compatible). You shouldn't have to update Buck (and its documentation) every time we do a GWT release; instead link to the GWT compiler's docs. [1] Also, Buck users should be able to use nightly builds of GWT if they like and try out experimental compiler flags without having to change Buck, so they can give the GWT team feedback on release candidates.

Of course there are some compiler options that Buck needs to know about, so I suggest just adding attributes for those. (In particular, controlling the compiler's inputs and outputs.)

Regarding JVM flags: we do have a compiler_jvm_flags attribute in blaze. It seems to be used solely for tinkering with JVM performance to make GWT compiles go faster, so I don't have a strong opinion; it could start out as a global flag for now.

[1] http://www.gwtproject.org/doc/latest/DevGuideCompilingAndDebugging.html at the bottom.
(This page is out of date and we should probably have a separate "man page" for the GWT compiler that everyone can link to.)

@davido
Copy link
Contributor

davido commented May 14, 2014

@bolinfest Thanks. Looks good. Performance is OK now. One question, If i'm reading the code correctly the default for local_workers is 2? Why not:

factor * Runtime.getRuntime().availableProcessors();

Before switch to gwt_binary() we called GWT compiler from python code with:

str(cpu_count())

but now i think it's more cleaner to do it from gwt_binary() and not to from the client code.

@davido
Copy link
Contributor

davido commented May 16, 2014

@bolinfest Thanks for adding -Dgwt.normalizeTimestamps=true and changing the output file to .zip.

Are there any other regressions in moving to gwt_binary()?

I have an issue with recovering from compile errors in GWT code:

  • conduct an error in GWT code
  • compile with buck build //gerrit-gwtui:ui_safari, note that output file is not created.
  • revert the error
  • call the target again:
$ buck build //gerrit-gwtui:ui_safari && file `buck targets --show_output //gerrit-gwtui:ui_safari | awk '{print $2}'`
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
[-] PARSING BUILD FILES...FINISHED 0,6s
[-] BUILDING...FINISHED 0,3s
Log:
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
[-] PARSING BUILD FILES...FINISHED 0,6s
buck-out/gen/gerrit-gwtui/__gwt_binary_ui_safari__/ui_safari.zip: ERROR: cannot open `buck-out/gen/gerrit-gwtui/__gwt_binary_ui_safari__/ui_safari.zip' (No such file or directory)

To recover from this situation, i've found that only buck clean helps so far:

$ buck clean 
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
davido@linux-ucwl:~/projects/gerrit (upgrade-buck %)$ buck build //gerrit-gwtui:ui_safari && file `buck targets --show_output //gerrit-gwtui:ui_safari | awk '{print $2}'`
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
[-] PARSING BUILD FILES...FINISHED 0,6s
[-] BUILDING...FINISHED 1,2s
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
[-] PARSING BUILD FILES...FINISHED 0,6s
buck-out/gen/gerrit-gwtui/__gwt_binary_ui_safari__/ui_safari.zip: Zip archive data, at least v1.0 to extract Java Jar file data (zip)

The output file seems not to be created from the cache after GWT error. We are using the following caching strategy:

[cache]
  mode = dir
  dir = buck-out/cache

@bolinfest
Copy link
Contributor Author

I don't follow: in your first example, I only see one command run.
On May 16, 2014 12:17 AM, "David Ostrovsky" notifications@github.com
wrote:

@bolinfest https://github.com/bolinfest Thanks for adding
-Dgwt.normalizeTimestamps=true and changing the output file to .zip. I
have an issue with recovering from compile errors in GWT code:

  • conduct an error in GWT code
  • compile with buck build //gerrit-gwtui:ui_safari, note that output
    file is not created.
  • revert the error
  • call the target again:

$ buck build //gerrit-gwtui:ui_safari && file buck targets --show_output //gerrit-gwtui:ui_safari | awk '{print $2}'
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
[-] PARSING BUILD FILES...FINISHED 0,6s
[-] BUILDING...FINISHED 0,3s
Log:
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
[-] PARSING BUILD FILES...FINISHED 0,6s
buck-out/gen/gerrit-gwtui/gwt_binary_ui_safari/ui_safari.zip: ERROR: cannot open `buck-out/gen/gerrit-gwtui/gwt_binary_ui_safari/ui_safari.zip' (No such file or directory)

To recover from this situation, i've found that only buck clean helps so
far:

$ buck clean
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
davido@linux-ucwl:~/projects/gerrit (upgrade-buck %)$ buck build //gerrit-gwtui:ui_safari && file buck targets --show_output //gerrit-gwtui:ui_safari | awk '{print $2}'
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
[-] PARSING BUILD FILES...FINISHED 0,6s
[-] BUILDING...FINISHED 1,2s
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
[-] PARSING BUILD FILES...FINISHED 0,6s
buck-out/gen/gerrit-gwtui/gwt_binary_ui_safari/ui_safari.zip: Zip archive data, at least v1.0 to extract Java Jar file data (zip)

The output file seems not to be created from the cache after GWT error. We
are using the following caching strategy:

[cache]
mode = dir
dir = buck-out/cache


Reply to this email directly or view it on GitHubhttps://github.com//issues/109#issuecomment-43303784
.

@davido
Copy link
Contributor

davido commented May 16, 2014

I don't follow: in your first example, I only see one command run.

OK, first successful buck run was missing:

  • buck build //gerrit-gwtui:ui_safari => successful, output file was created
  • create an error in GWT class
  • compile with buck build //gerrit-gwtui:ui_safari => the error is reported and the output file is removed
  • revert the error
  • repeat buck build //gerrit-gwtui:ui_safari => successful, still output file is missing

@bolinfest
Copy link
Contributor Author

Why shouldn't it output a file if you removed the error?
On May 16, 2014 8:58 AM, "David Ostrovsky" notifications@github.com wrote:

I don't follow: in your first example, I only see one command run.

OK, first successful buck run was missing:

  • buck build //gerrit-gwtui:ui_safari => successful, output file was
    created
  • create an error in GWT class
  • compile with buck build //gerrit-gwtui:ui_safari, note that output
    file is removed
  • revert the error
  • repeat buck build //gerrit-gwtui:ui_safari => successful, still
    output file


Reply to this email directly or view it on GitHubhttps://github.com//issues/109#issuecomment-43348258
.

@bolinfest
Copy link
Contributor Author

@davido I think I misread. So you're saying that after the third call to buck build, the build succeeded, but there was no .war file?

@bolinfest
Copy link
Contributor Author

OK, I can repro this. Investigating..

@davido
Copy link
Contributor

davido commented May 16, 2014

@bolinfest Exactly. That's what i am saying. It looks like it isn't get compiled new, normal compile time would be around 30 sec. but the third call after revert of my error change is ready after 1 sec. So i think that something is going wrong with cache handling. If you prefer i could create a reproducer.

@bolinfest
Copy link
Contributor Author

What is really weird is that I looked at the cache key from the build trace (d5be56604a588215350cb925011182dae9ecc77a) and did unzip -l buck-cache/d5be56604a588215350cb925011182dae9ecc77a and I see that debug.zip did get archived. So if we get a rule cache hit, then everything in that zip should be getting written to the filesystem.

@davido
Copy link
Contributor

davido commented May 16, 2014

Unfortunately it doesn't work (yet).

@bolinfest
Copy link
Contributor Author

@davido I believe this is not specific to Buck. I suspect these lines in https://github.com/facebook/buck/blob/master/src/com/facebook/buck/rules/CachingBuildEngine.java are to blame:

    // If the RuleKeys match, then there is nothing to build.
    if (ruleKey.equals(cachedRuleKey.orNull())) {
      context.logBuildInfo("[UNCHANGED %s]", rule.getFullyQualifiedName());
      return new BuildResult(BuildRuleSuccess.Type.MATCHING_RULE_KEY,
          CacheResult.LOCAL_KEY_UNCHANGED_HIT);
    }

Or rather, this doing the right thing, but if the build rule fails to build, then we need to make sure that we delete the existing RULE_KEY and RULE_KEY_NO_DEPS files. It looks like those are still lying around after I have a failed build.

@bolinfest
Copy link
Contributor Author

Ha: look at the TODO I wrote for myself on line 224.

@davido
Copy link
Contributor

davido commented May 16, 2014

@bolinfest

I believe this is not specific to Buck.

You mean it's not specific to the gwt_binary()?

Ha: look at the TODO I wrote for myself on line 224.

// TODO(mbolin): Delete all genfiles and metadata, as they are not guaranteed to be
// valid at this point?

;-)

bolinfest added a commit that referenced this issue May 16, 2014
Summary:
A concrete failure case was reported in the wild:
#109.

Test Plan:
Very straightforward repro base:
* Build a GWT app, verify the .zip file is generated.
* Introduce a breaking change and try to rebuild the GWT app.
* Verify that the .zip file, `metadata/RULE_KEY` and `metadata/RULE_KEY_NO_DEPS` files are deleted.
* Revert the breaking change so it matches the original rule key.
* Build the GWT app again.
* Verify that the .zip file, `metadata/RULE_KEY` and `metadata/RULE_KEY_NO_DEPS` files are restored.
Also ran `buck test --all` locally and verified that all tests passed.
@bolinfest
Copy link
Contributor Author

@davido Fixed in 75000f4

@davido
Copy link
Contributor

davido commented May 16, 2014

@bolinfest Thanks, works like a charm:

$ git checkout gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.ui.xml
$ buck build safari && file `buck targets --show_output //gerrit-gwtui:ui_safari | awk '{print $2}'`
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
[-] PARSING BUILD FILES...FINISHED 1,1s
[-] BUILDING...FINISHED 0,8s
Log:
Watchman not found, please install when using buckd.
See https://github.com/facebook/watchman for details.
[-] PARSING BUILD FILES...FINISHED 0,6s
buck-out/gen/gerrit-gwtui/__gwt_binary_ui_safari__/ui_safari.zip: Zip archive data, at least v1.0 to extract Java Jar file data (zip)

@davido
Copy link
Contributor

davido commented May 16, 2014

Are you able to run GWT tests with the ordinary java_test() rule?

We have only very few GWT specific tests and we are using GWT test utils library [1] :

java_test(
  name = 'ui_tests',
  srcs = glob(['src/test/java/**/*.java']),
  resources = glob(['src/test/resources/**/*']) + [
    'src/main/java/com/google/gerrit/GerritGwtUI.gwt.xml',
  ],
  deps = [
    ':ui_module',
    '//gerrit-common:client',
    '//gerrit-extension-api:client',
    '//lib:junit',
    '//lib/gwt:dev',
    '//lib/gwt:user',
    '//lib/gwt:gwt-test-utils',
    '//lib/jgit:jgit',
  ],
  source_under_test = [':ui_module'],
  vm_args = ['-Xmx512m'],
  visibility = ['//tools/eclipse:classpath'],
)

[1] https://github.com/gwt-test-utils/gwt-test-utils

@davido
Copy link
Contributor

davido commented May 20, 2014

@bolinfest Just to let you know: gwt_binary() migration series was merged tonight. Thanks again!

The only three custom Buck hacks in Gerrit itself and its eco systems are:

openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue May 20, 2014
Migrate gwt build tool chain to built in gwt_binary() rule [1].

[1] facebook/buck#109

Change-Id: I1f13f4d29864bc7e7278f8a2b2e39c882acf44aa
@davido
Copy link
Contributor

davido commented Jan 23, 2015

I've spent some time to understand why small extension to Gerrit GWT Plugin API worked as expected in standalone build mode, but failed to compile in Gerrit tree mode: [1]. Classpath of GWT compiler invocation was polluted with dozens of pseudo GWT modules, that interfered with this new extension.

What happened? The easiest way to see the problem is to check the source code: [2].

It turns out (it wasn't really clear to me) that the implementation reconstructs GWT modules from the dependency graph. In Gerrit tree build //gerrit-plugin-api:lib that was passed here, resolved to dozens of rules and thus the whole Gerrit server stuff was included as GWT modules and passed to the GWT compiler. Even though we used provided deps.

In standalone mode, that makes use of bucklets, the implementation is different: plugin-api.jar is fetched from Central and is used as module deps. Dependency graph here is empty, so nothing is passed in addition to GWT compiler and the classpath wasn't polluted.

I think that the reachability resolution algorithm should differentiate between provided deps and normal deps. Another option to consider is to make dependency module resolution configurable.

[1] https://gerrit-review.googlesource.com/#/c/63489
[2] https://github.com/facebook/buck/blob/master/src/com/facebook/buck/gwt/GwtBinaryDescription.java#L115,L138

openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue Jan 27, 2015
As explained in [1], [2] in tree build mode is currently broken for
plugins that expose GWT modules.

To rectify, isolate the libraries that used for building regular and
GWT modules. While regular modules need the whole Plugin API as their
dependency, GWT modules should only consume pre-defined set of GWT
API modules.

[1] facebook/buck#109
[2] https://gerrit-review.googlesource.com/63489

Change-Id: I4694745254ec0f2d9578d79b62110241d4b8b429
@Coneko
Copy link

Coneko commented Feb 2, 2015

@davido: I've opened a new issue for that: #239

I think this one can be closed now that GWT rules are in.

@BruceZu
Copy link

BruceZu commented Feb 5, 2015

“ I don't want to document it on http://facebook.github.io/buck/ until I get some external confirmation that this is the right way to go.”

@bolinfest:Is it the time to document it?

@tbroyer
Copy link

tbroyer commented Feb 5, 2015

FYI, I copied from Buck how you configure GWT for my gwt-maven-plugin (very few properties exposed, and compilerArgs, jvmArgs and systemProperties for further customization needs).

Please note however that strict has been renamed to failOnError in GWT, with strict still being recognized as an alias. Maybe you'd want to use the new naming before documenting it.
Also, GWT 2.8 will add --setProperty (gwtproject/gwt@d4f4f42) that you might want to eventually expose on the build rule.

davido added a commit to davido/bucklets that referenced this issue Mar 21, 2015
As explained in [1], [2] in tree build mode is currently broken for
plugins that expose GWT modules.

To rectify, isolate the libraries that used for building regular and
GWT modules. While regular modules need the whole Plugin API as their
dependency, GWT modules should only consume pre-defined set of GWT
API modules.

[1] facebook/buck#109
[2] https://gerrit-review.googlesource.com/63489

Change-Id: I5943bec40e659421b746de92397468b8ad7420ab
@sdwilsh
Copy link
Contributor

sdwilsh commented May 18, 2015

We now have integration tests for this provided by @shs96c as well. If someone wanted to submit a pull requests for documentation for these rules, we'd happily take it.

@sdwilsh sdwilsh closed this as completed May 18, 2015
lucamilanesio pushed a commit to GerritCodeReview/gerrit that referenced this issue Sep 21, 2016
Port Buck gwt_binary() native rule: [1] to Skylark.

TEST PLAN:

  bazel build gerrit-gwtui:ui_optdbg

* [1] facebook/buck#109

Change-Id: I24d11f10928a8000304bc4233156ddc50fad8931
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants