Fix Eclipse plugin build #465

tobous · 2020-04-29T00:56:14Z

Updates the Eclipse plugin build logic. The logic is now completely separate from the main google java format build. Instead of relying on the build artifacts of the core build, the needed google-java-format and guava build artifacts used for the build are now pulled from maven.

Updates google-java-format version used for the build to '1.9', the build JDK to 11, and removes old dependencies which are no longer used.

Updates tycho version to 1.7.0 in order to build with JDK > 8.

Updates the build guide in the README.

googlebot · 2020-04-29T00:56:20Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

tobous · 2020-04-29T00:57:58Z

This PR was initially created before the move to building with Java 11. After rebasing, the build fails due to the maven javadoc build with the following error:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.1.1:jar (attach-javadocs) on project google-java-format-eclipse-plugin: MavenReportException: Error while generating Javadoc: 
[ERROR] Exit code: 1 - javadoc: error - The code being documented uses packages in the unnamed module, but the packages defined in https://github.com/google/google-java-format/google-java-format/apidocs/ are in named modules.
[ERROR] 
[ERROR] Command line was: /lib/jvm/java-11-openjdk/bin/javadoc @options @packages
[ERROR] 
[ERROR] Refer to the generated Javadoc files in '/home/tobous/git/google-java-format/eclipse_plugin/target/apidocs' dir.

Even with the failure, the plugin jar is still build and seems to be completely functional. (And, if I remove the doc build, everything builds fine.)

I had a look at recent changes and found the adjustment to the core pom.xml made in 3c191c1#diff-357e4854869b2e21c38b1b437f11095a. I tried adding the same configuration to the pom.xml for the Eclipse plugin build without any success.

Any idea how to fix the javadoc build?

sormuras · 2020-04-29T01:50:43Z

Move to JDK 15 - it contains a fix for https://bugs.openjdk.java.net/browse/JDK-8240169

tobous · 2020-04-29T08:48:05Z

@googlebot I fixed it.

googlebot · 2020-04-29T08:48:10Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

m273d15 · 2020-04-29T08:52:49Z

@googlebot I consent.

googlebot · 2020-04-29T08:52:54Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

tobous · 2020-04-29T09:09:27Z

Move to JDK 15 - it contains a fix for https://bugs.openjdk.java.net/browse/JDK-8240169

@sormuras I am not really sure what you mean. As far as I can tell, the google java format core is also build using Java 11 and the javadoc build seems to work just fine. Only the javadoc build for the Eclipse plugin fails (i.e. if I disable the Eclipse build, the build passes).

Furthermore, wouldn't this set the minimal required Java version to 15 (which hasn't even been released yet)?

sormuras · 2020-04-29T09:17:33Z

The underlying problem for why "javadoc build for the Eclipse plugin fails" is due to the issue linked above. Upgrading to 15-ea could solve the problem, introduce other ones ... and is not an option today.

So, I guess, you have figure out the differences in how the core project and the Eclipse plugin configure their javadoc run.

tobous · 2020-04-29T09:54:11Z

As far as I can tell, the javadoc run is configured in the pom.xml files. As mentioned, I tried copying the configuration section of the core pom to the pom of the Eclipse build but it did not fix the build.
(Updating to the latest maven-javadoc-plugin release 3.2.0 did not help either.)

Would simply skipping the maven javadoc build for the eclipse plugin be an acceptable solution?

This can be easily done by adding <maven.javadoc.skip>true</maven.javadoc.skip> to the properties section of the Eclipse build pom.

tbroyer

This PR addresses at least 4 separate things at once:

documenting the JDK 11 requirement in the README → should address ErrorProne as a whole, not just the Eclipse plugin
change the way version is handled in the build (through ${revision}) → breaks deployment
building the Eclipse plugin when building GJF itself →if the Eclipse plugin uses the "public API" of GJF, it should IMO be considered a separate project and build against the latest released version of GJF (it should be possible to easily build a version of the plugin that uses a snapshot of GJF, but that shouldn't be the default behavior). The plugin might also need intermediate releases independent of GJF releases (due to bugs, or changes in Eclipse), so it shouldn't share the same version as the reactor (see also point above) and should build against a stable GJF version by default.
fixing the Eclipse plugin build (update Tycho version and configure m-compiler-p to target JDK 11)

At least some of those should be split into separate PRs IMO (or abandoned entirely, if you ask me)

README.md

pom.xml

tbroyer · 2020-04-29T11:01:36Z

eclipse_plugin/pom.xml

+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <configuration>
+          <source>11</source>


<release>11</release> would be better, particularly when building with more recent JDK versions as that guarantees that you won't also use JDK12+ APIs.

I copied this section from the current core pom.xml.

tobous

The thought behind always enabling the build for the eclipse plugin was to make breakages more apparent (i.e. it ensures that breakages along the way are not just discovered when trying to build the next release version). But, again, if you don't like it, it can be dropped form the PR.

tobous · 2020-04-29T11:22:01Z

eclipse_plugin/pom.xml

+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <configuration>
+          <source>11</source>


I copied this section from the current core pom.xml.

README.md

tobous · 2020-04-29T12:10:47Z

Added logic to skip maven javadoc build for google java formatter Eclipse plugin.

m273d15 · 2020-04-29T12:24:28Z

This PR addresses at least 4 separate things at once

True, I would:

remove the whole "revision" handling (I just used it, because in my test setup I wanted to avoid inconsistent version numbers)
reduce the PR to: "fixing the Eclipse plugin build (update Tycho version and configure m-compiler-p to target JDK 11)"
adjust the maven set-up to build with the current 1.8 version (from maven central not the local build)
remove the reference from the parent pom to eclipse (this would decouple the builds and you could move the eclipse build into a separate repo).

PhilippWendler · 2020-05-04T07:47:50Z

Given that there recently was another release without an Eclipse plugin, it is really cool to see some improvements being worked on for the plugin build. As a user of google-java-format, I would like to say thanks!

Is there a chance that we get an official build of the plugin with google-java-format 1.8 after this gets merged?

tobous · 2020-05-05T16:48:10Z

Reworked the PR and updated the PR message to match the new content.

reworked the build logic
dropped the changes to the general version logic
dropped the addition of the Java 11 README section
dropped the change to always build the Eclipse plugin

I force-pushed the changes to clean up the commits of this PR. I hope this is not an issue.

The new build logic now works independently of the core build. The dependencies are pulled from maven instead of being provided by the core build. Local core snapshots can also be used for the build.

The change to always also build the Eclipse plugin as part of the core build has also been reverted as you did not seem to like it. This, however, also means that the new build logic is not covered by the CI.

tobous · 2020-05-14T14:31:40Z

@sormuras @tbroyer Any feedback on the updated PR?

tobous · 2020-06-01T15:33:58Z

@plumpy Are there issues with the PR? Is there something I can do to help it get reviewed? I would really appreciate any feedback.

jan-z · 2020-06-07T07:53:47Z

The build works fine for me. Using the plugin on Eclipse 2020-03 without problems.

tobous · 2020-07-30T18:58:05Z

It's been a while since anything happened on this PR. I still don't see any open issues with it but haven't gotten any further feedback. As far as I can tell, there also aren't any merge/compatibility issues caused by other changes that were merged in the meantime.

Is there a fundamental issue with the PR? The way I understand it from reading the issues regarding the Eclipse plugin, the plugin was not released for newer versions of the formatter as you did not have the time to update the build scripts. But, from the muted response/feedback I have gotten on this PR, it makes me wonder whether you have made the internal decision to drop the Eclipse plugin.

This would be sad to here as I was hoping to keep using the google java formatter for both IntelliJ IDEA and Eclipse, but it is still your decision to make. It would, however, be nice to get any response/an official statement on the Eclipse plugin situation to avoid people (pointlessly) investing time providing PRs or reporting bugs for it or asking questions about its status.

Updates the Eclipse plugin build logic. The logic is now completely separate from the main google java format build. Instead of relying on the build artifacts of the core build, the needed google-java-format and guava build artifacts used for the build are now pulled from maven. Updates google-java-format version used for the build to '1.8', the build JDK to 11, and removes old dependencies which are no longer used. Updates tycho version to 1.7.0 in order to build with JDK > 8. Updates the build guide in the README. Co-authored-by: Kelvin Glaß <kelvin.glass@fu-berlin.de> Co-authored-by: Tobias Bouschen <tobias.bouschen@googlemail.com>

Removes the remnants of the old Eclipse plugin build logic from the core pom as they are no longer needed. Co-authored-by: Kelvin Glaß <kelvin.glass@fu-berlin.de> Co-authored-by: Tobias Bouschen <tobias.bouschen@googlemail.com>

Moves the Google copyright header below the XML version declaration in the Eclipse plugin.xml. Otherwise, reading the XML file fails with newer Java versions (e.g. Java 14).

tobous · 2020-10-27T15:03:29Z

Rebased onto current master and moved build to use GJF 1.9.

oliviercailloux · 2020-12-03T14:39:15Z

Please anyone at Google (@tbroyer?), can you provide at least some statement about an intent to consider or not consider this PR? It would be good to know if this path has to be considered hopeless for us users of Eclipse, and time invested to search for another solution for formatting code systematically, or if we can keep some hope that this PR will be merged in the foreseeable future.

av1m · 2020-12-03T15:20:35Z

@cushon I see you are the most active... You have no idea on the merge of this PR? Or could you just give us a feedback on the evolution of the eclipse plugin?

cristatus · 2020-12-18T09:25:21Z

For those looking for a working plugin for the latest eclipse release, here is the one I built from @tobous's branch.

google-java-format-eclipse-plugin-1.9.0.zip

Extract the zip file and put the plugin jar inside the dropins folder.

gmasil · 2021-01-04T18:34:14Z

I tried your fix and it works like a charm. Thank you very much!
Can be found here with GitHub release, if someone is interested.

To the google guys: Please value his work with as little as clicking the merge button...

ParanoidUser · 2021-02-17T23:24:24Z

As far as I can tell, the javadoc run is configured in the pom.xml files. As mentioned, I tried copying the configuration section of the core pom to the pom of the Eclipse build but it did not fix the build.
(Updating to the latest maven-javadoc-plugin release 3.2.0 did not help either.)

Would simply skipping the maven javadoc build for the eclipse plugin be an acceptable solution?

This can be easily done by adding <maven.javadoc.skip>true</maven.javadoc.skip> to the properties section of the Eclipse build pom.

Sorry for the late reply, I just joined. Looking at suggestions from @tobous and @sormuras - should we enforce a minimal JDK version that contains the Javadoc fix?

<plugin>
   <artifactId>maven-enforcer-plugin</artifactId>
   <executions>
      <execution>
         <id>enforce-versions</id>
         <goals>
            <goal>enforce</goal>
         </goals>
         <configuration>
            <rules>
               <requireJavaVersion>
                  <version>11.0.3</version> <!-- https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8212233 -->
               </requireJavaVersion>
            </rules>
         </configuration>
      </execution>
   </executions>
</plugin>

tobous · 2021-02-18T11:51:15Z

As far as I can tell, the javadoc run is configured in the pom.xml files. As mentioned, I tried copying the configuration section of the core pom to the pom of the Eclipse build but it did not fix the build.
(Updating to the latest maven-javadoc-plugin release 3.2.0 did not help either.)
Would simply skipping the maven javadoc build for the eclipse plugin be an acceptable solution?
This can be easily done by adding <maven.javadoc.skip>true</maven.javadoc.skip> to the properties section of the Eclipse build pom.

Sorry for the late reply, I just joined. Looking at suggestions from @tobous and @sormuras - should we enforce a minimal JDK version that contains the Javadoc fix?
<plugin>
   <artifactId>maven-enforcer-plugin</artifactId>
   <executions>
      <execution>
         <id>enforce-versions</id>
         <goals>
            <goal>enforce</goal>
         </goals>
         <configuration>
            <rules>
               <requireJavaVersion>
                  <version>11.0.3</version> 
               </requireJavaVersion>
            </rules>
         </configuration>
      </execution>
   </executions>
</plugin>

I separated the Eclipse plugin build from the main build a while ago. As a result, the javadoc job is no longer inherited from the main job, thereby avoiding the issue.

oliviercailloux · 2021-03-18T07:55:42Z

For those interested in a solution to let Eclipse format code according to the Google style guide AND let Maven fail the build when the formatting is wrong, see here for a solution based on Checkstyle. Please report issues there if something goes wrong (I have only tested this on my install).

I hope it is not considered inappropriate to hijack this thread to suggest a workaround that has nothing to do with the Eclipse plugin to the Google formatting library. I believe that this comment may nonetheless be useful because I suspect that many readers of this page are, as I was, eager to solve the concrete code formatting problem, whether by this specific plugin or not.

cristatus · 2021-04-07T10:23:31Z

I have just built the latest plugin for eclipse from @tobous's branch and rebased on official repo,

google-java-format-eclipse-plugin-1.10.0.jar

Put the plugin jar inside the dropins folder.

timgesekus · 2021-07-28T19:00:07Z

Does anybody know, why this isn't merged?

cristatus · 2021-07-30T06:17:53Z

Thanks for merging it. However, it required some updates for 1.10.0 release. I will make a pull request soon.

cristatus · 2021-07-30T10:05:16Z

I just submitted a new pull request #635.

cushon · 2021-07-30T16:38:27Z

Now that this and #635 is merged, I have uploaded a build of the Eclipse plugin to the releases page: https://github.com/google/google-java-format/releases/tag/v1.11.0

Sorry for the silence here. We're not using Eclipse as much these days, and I have limited expertise to support the plugin, but I will try to make sure that PRs to keep it working are merged in a more timely manner.

Upgrade from version 1.6 to version 1.11 (according #465) Fixes #639 COPYBARA_INTEGRATE_REVIEW=#639 from av1m:patch-1 2c5c2b0 PiperOrigin-RevId: 388249150

googlebot added the cla: no label Apr 29, 2020

googlebot added cla: yes and removed cla: no labels Apr 29, 2020

tbroyer suggested changes Apr 29, 2020

View reviewed changes

tobous commented Apr 29, 2020

View reviewed changes

av1m mentioned this pull request May 23, 2020

Please provide Eclipse plugin in version 1.7 #331

Closed

m273d15 and others added 4 commits October 27, 2020 14:04

Remove Eclipse plugin build logic from core build

6ec0716

Removes the remnants of the old Eclipse plugin build logic from the core pom as they are no longer needed. Co-authored-by: Kelvin Glaß <kelvin.glass@fu-berlin.de> Co-authored-by: Tobias Bouschen <tobias.bouschen@googlemail.com>

Move copyright header below XML version declaration for plugin.xml

ab5074b

Moves the Google copyright header below the XML version declaration in the Eclipse plugin.xml. Otherwise, reading the XML file fails with newer Java versions (e.g. Java 14).

Move Eclipse plugin to GJF version 1.9

e80a72b

cushon approved these changes Jul 29, 2021

View reviewed changes

copybara-service bot closed this in 93ee8fe Jul 29, 2021

av1m mentioned this pull request Aug 2, 2021

Updates the eclipse plugin link #639

Closed

copybara-service bot pushed a commit that referenced this pull request Aug 2, 2021

Updates the eclipse plugin link

f2eabaa

Upgrade from version 1.6 to version 1.11 (according #465) Fixes #639 COPYBARA_INTEGRATE_REVIEW=#639 from av1m:patch-1 2c5c2b0 PiperOrigin-RevId: 388249150

Fix Eclipse plugin build #465

Fix Eclipse plugin build #465

Conversation

tobous commented Apr 29, 2020 • edited Loading

googlebot commented Apr 29, 2020

tobous commented Apr 29, 2020

sormuras commented Apr 29, 2020

tobous commented Apr 29, 2020

googlebot commented Apr 29, 2020

m273d15 commented Apr 29, 2020

googlebot commented Apr 29, 2020

tobous commented Apr 29, 2020

sormuras commented Apr 29, 2020

tobous commented Apr 29, 2020 • edited Loading

tbroyer left a comment

Choose a reason for hiding this comment

tbroyer Apr 29, 2020

Choose a reason for hiding this comment

tobous Apr 29, 2020

Choose a reason for hiding this comment

tobous left a comment

Choose a reason for hiding this comment

tobous Apr 29, 2020

Choose a reason for hiding this comment

tobous commented Apr 29, 2020

m273d15 commented Apr 29, 2020

PhilippWendler commented May 4, 2020

tobous commented May 5, 2020 • edited Loading

tobous commented May 14, 2020

tobous commented Jun 1, 2020

jan-z commented Jun 7, 2020

tobous commented Jul 30, 2020

tobous commented Oct 27, 2020

oliviercailloux commented Dec 3, 2020

av1m commented Dec 3, 2020

cristatus commented Dec 18, 2020

gmasil commented Jan 4, 2021

ParanoidUser commented Feb 17, 2021

tobous commented Feb 18, 2021

oliviercailloux commented Mar 18, 2021 • edited Loading

cristatus commented Apr 7, 2021 • edited Loading

timgesekus commented Jul 28, 2021

cristatus commented Jul 30, 2021

cristatus commented Jul 30, 2021

cushon commented Jul 30, 2021

tobous commented Apr 29, 2020 •

edited

Loading

tobous commented Apr 29, 2020 •

edited

Loading

tobous commented May 5, 2020 •

edited

Loading

oliviercailloux commented Mar 18, 2021 •

edited

Loading

cristatus commented Apr 7, 2021 •

edited

Loading