Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

ci: installing google-play-services artifact for Windows #1706

Merged
merged 12 commits into from Feb 12, 2021

Conversation

suztomo
Copy link
Member

@suztomo suztomo commented Feb 11, 2021

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #1705 ☕️

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 11, 2021
@suztomo
Copy link
Member Author

suztomo commented Feb 11, 2021

92a180b failed:

wget: D:\a\_temp\e1fca20a-80ef-4a04-9b5e-b66495e66e06.ps1:4
Line |
   4 |  wget https://dl.google.com/dl/android/maven2/com/google/android/gms/p …
     |  ~~~~
     | The term 'wget' is not recognized as a name of a cmdlet, function, script file, or executable program.
     | Check the spelling of the name, or if a path was included, verify that the path is correct and try
     | again.

@codecov
Copy link

codecov bot commented Feb 11, 2021

Codecov Report

Merging #1706 (f5f016c) into master (922af40) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@      Coverage Diff       @@
##   master   #1706   +/-   ##
==============================
==============================

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 922af40...472d361. Read the comment docs.

@suztomo
Copy link
Member Author

suztomo commented Feb 11, 2021

4af8b2c fails:

Error:  The goal you specified requires a project to execute but there is no POM in this directory (D:\a\google-api-java-
client\google-api-java-client\play-services). Please verify you invoked Maven from the correct directory. -> [Help 1]

@suztomo
Copy link
Member Author

suztomo commented Feb 11, 2021

"shell: bash" seems working for windows-latest.

@suztomo
Copy link
Member Author

suztomo commented Feb 11, 2021

000d56d failed with checkstyle violation.

...
[INFO] --- maven-checkstyle-plugin:3.1.2:check (default) @ google-api-client-servlet ---
Warning:  Old version of checkstyle detected. Consider updating to >= v8.30
Warning:  For more information see: https://maven.apache.org/plugins/maven-checkstyle-plugin/examples/upgrading-checkstyle.html
...
Error:  D:\a\google-api-java-client\google-api-java-client\google-api-client-servlet\src\main\java\com\google\api\client\googleapis\extensions\servlet\notifications\NotificationServlet.java:55: Line is longer than 100 characters (found 113). [LineLength]
Error:  D:\a\google-api-java-client\google-api-java-client\google-api-client-servlet\src\main\java\com\google\api\client\googleapis\extensions\servlet\notifications\NotificationServlet.java:72: Line is longer than 100 characters (found 162). [LineLength]
Audit done.
[INFO] There are 2 errors reported by Checkstyle 8.23 with checkstyle.xml ruleset.
Error:  src\main\java\com\google\api\client\googleapis\extensions\servlet\notifications\NotificationServlet.java:[55] (sizes) LineLength: Line is longer than 100 characters (found 113).
Error:  src\main\java\com\google\api\client\googleapis\extensions\servlet\notifications\NotificationServlet.java:[72] (sizes) LineLength: Line is longer than 100 characters (found 162).
...

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Parent for the Google API Client Library for Java 1.31.2:
[INFO] 
[INFO] Parent for the Google API Client Library for Java .. SUCCESS [  3.553 s]
[INFO] Google APIs Client Library for Java ................ SUCCESS [ 14.844 s]
[INFO] Servlet and JDO extensions to the Google API Client Library for Java. FAILURE [  1.824 s]
[INFO] Android Platform Extensions to the Google APIs Client Library for Java. SUCCESS [  5.274 s]
[INFO] Google App Engine extensions to the Google API Client Library for Java. SKIPPED
[INFO] GSON extensions to the Google APIs Client Library for Java SUCCESS [  1.872 s]
[INFO] Jackson 2 extensions to the Google APIs Client Library for Java SKIPPED
[INFO] Java 6 (and higher) Extensions to the Google API Client Library for Java. SKIPPED
[INFO] Protocol Buffer extensions to the Google APIs Client Library for Java SKIPPED
[INFO] XML extensions to the Google APIs Client Library for Java SUCCESS [  4.409 s]
[INFO] Assembly for the Google APIs Client Library for Java SKIPPED
[INFO] Google API Client Library for Java BOM ............. SUCCESS [  0.300 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  21.799 s (Wall Clock)
[INFO] Finished at: 2021-02-11T19:20:34Z
[INFO] ------------------------------------------------------------------------
Error:  Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.2:check (default) on project google-api-client-servlet: You have 2 Checkstyle violations. -> [Help 1]

The file has long lines in <pre> element:

Screen Shot 2021-02-11 at 15 52 55

Why does this fail only in Windows?

@suztomo
Copy link
Member Author

suztomo commented Feb 11, 2021

The multi-line with bash (10a5b9a) works. Working on fixing the checkstyle problem.

@suztomo
Copy link
Member Author

suztomo commented Feb 11, 2021

checkstyle-suppressions.xml includes com/google/api/client/googleapis/extensions/servlet/notifications/NotificationServlet.java. Doesn't this work for Windows build?

@suztomo
Copy link
Member Author

suztomo commented Feb 11, 2021

5d112be failed with

Error:  Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.2:check (default) on project google-api-
client-parent: Failed during checkstyle configuration: cannot initialize module TreeWalker - cannot initialize module 
JavadocMethod - Property 'allowUndeclaredRTE' does not exist, please check the documentation -> [Help 1]

To allow documented java.lang.RuntimeExceptions that are not declared, set property allowUndeclaredRTE to true.

https://checkstyle.sourceforge.io/version/4.4/5.x/config_javadoc.html

Other errors on properties that no longer exist

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.2:check (default) on project google-
api-client-parent: Failed during checkstyle configuration: cannot initialize module TreeWalker - cannot initialize module 
JavadocMethod - Property 'allowThrowsTagsForSubclasses' does not exist, please check the documentation -> [Help 1]

The configuration file structure also changed:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.2:check (default) on project google-
api-client-parent: Failed during checkstyle configuration: cannot initialize module TreeWalker - TreeWalker is not allowed as a
 parent of LineLength Please review 'Parent Module' section for this Check in web documentation if Check is standard. -> [Help 1]

https://checkstyle.org/config_sizes.html#LineLength_Parent_Module says the parent is "Check" module.

@suztomo
Copy link
Member Author

suztomo commented Feb 11, 2021

43d9e56 failed with checkstyle error:

[INFO] --- maven-checkstyle-plugin:3.1.2:check (default) @ google-api-client ---
[INFO] Starting audit...
Error:  D:\a\google-api-java-client\google-api-java-client\google-api-client\src\main\java\com\google\api
\client\googleapis\apache\GoogleApacheHttpTransport.java:1: Expected line ending for file
 is LF(\n), but CRLF(\r\n) is detected. [NewlineAtEndOfFile]
Error:  D:\a\google-api-java-client\google-api-java-client\google-api-client\src\main\java\com\google\api
\client\googleapis\apache\package-info.java:1: Expected line ending for file is LF(\n), but
 CRLF(\r\n) is detected. [NewlineAtEndOfFile]
Error:  D:\a\google-api-java-client\google-api-java-client\google-api-client\src\main\java\com\google\api
\client\googleapis\apache\v2\GoogleApacheHttpTransport.java:1: Expected line ending for file is LF(\n), but CRLF(\r\n) is detected. [NewlineAtEndOfFile]

My IntelliJ shows the file uses LF. It seems that there's some process that converts LF to CRLF for windows.

@suztomo
Copy link
Member Author

suztomo commented Feb 11, 2021

It passed!

Screen Shot 2021-02-11 at 17 42 03

Comment on lines +30 to +33
- name: git configuration to avoid automatic CRLF conversion
run: |
git config --global core.autocrlf false
git config --global core.eol lf
Copy link
Member Author

Choose a reason for hiding this comment

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

This avoids the error below:

[INFO] --- maven-checkstyle-plugin:3.1.2:check (default) @ google-api-client ---
[INFO] Starting audit...
Error:  D:\a\google-api-java-client\google-api-java-client\google-api-client\src\main\java\com\google\api
\client\googleapis\apache\GoogleApacheHttpTransport.java:1: Expected line ending for file
 is LF(\n), but CRLF(\r\n) is detected. [NewlineAtEndOfFile]
Error:  D:\a\google-api-java-client\google-api-java-client\google-api-client\src\main\java\com\google\api
\client\googleapis\apache\package-info.java:1: Expected line ending for file is LF(\n), but
 CRLF(\r\n) is detected. [NewlineAtEndOfFile]
Error:  D:\a\google-api-java-client\google-api-java-client\google-api-client\src\main\java\com\google\api
\client\googleapis\apache\v2\GoogleApacheHttpTransport.java:1: Expected line ending for file is LF(\n), but CRLF(\r\n) is detected. [NewlineAtEndOfFile]

Comment on lines +40 to +53
shell: bash
run: |
mkdir play-services
cd play-services
curl --output play-services-basement-8.3.0.aar https://dl.google.com/dl/android/maven2/com/google/android/gms/play-services-basement/8.3.0/play-services-basement-8.3.0.aar
unzip play-services-basement-8.3.0.aar
mvn install:install-file \
-Dfile=classes.jar \
-DgroupId=com.google.android.google-play-services \
-DartifactId=google-play-services \
-Dversion=1 \
-Dpackaging=jar
- run: .kokoro/build.sh
shell: bash
Copy link
Member Author

Choose a reason for hiding this comment

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

Windows has bash.

Comment on lines -8 to -13
<suppress checks="LineLength" files="com/google/api/client/googleapis/apache/GoogleApacheHttpTransport.java"/>
<suppress checks="LineLength" files="com/google/api/client/googleapis/extensions/appengine/notifications/AppEngineNotificationServlet.java"/>
<suppress checks="LineLength" files="com/google/api/client/googleapis/extensions/appengine/subscriptions/package-info.java"/>
<suppress checks="LineLength" files="com/google/api/client/googleapis/extensions/servlet/notifications/NotificationServlet.java"/>
<suppress checks="LineLength" files="com/google/api/client/googleapis/javanet/GoogleNetHttpTransport.java"/>
<suppress checks="LineLength" files="com/google/api/client/googleapis/subscriptions/NotificationHeaders.java"/>
Copy link
Member Author

Choose a reason for hiding this comment

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

Files that no longer violates the line length have been removed.

Comment on lines +7 to +8
<suppress checks="LineLength" files="com[/\\]google[/\\]api[/\\]client[/\\]googleapis[/\\]extensions[/\\]appengine[/\\]notifications[/\\]AppEngineNotificationServlet.java"/>
<suppress checks="LineLength" files="com[/\\]google[/\\]api[/\\]client[/\\]googleapis[/\\]extensions[/\\]servlet[/\\]notifications[/\\]NotificationServlet.java"/>
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to add forward slash and back slash to make it work for both Windows and Unix filesystem.

Comment on lines -75 to 77
<property name="allowMissingJavadoc" value="true"/>
<property name="allowMissingParamTags" value="true"/>
<property name="allowMissingReturnTag" value="true"/>
<property name="allowMissingThrowsTags" value="true"/>
<property name="allowThrowsTagsForSubclasses" value="true"/>
<property name="allowUndeclaredRTE" value="true"/>
</module>
Copy link
Member Author

Choose a reason for hiding this comment

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

The removed properties are no longer supported in newer version of checkstyle.

@@ -183,22 +179,6 @@ page at http://checkstyle.sourceforge.net/config.html -->

-->

<module name="LineLength">
Copy link
Member Author

Choose a reason for hiding this comment

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

The LineLength's parent has to be "Check" element in the newer checkstyle.

@@ -561,7 +561,7 @@
<dependency>
<groupId>com.puppycrawl.tools</groupId>
<artifactId>checkstyle</artifactId>
<version>8.23</version>
<version>8.39</version>
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the version google-cloud-shared-config 0.10.0 uses.

@suztomo suztomo marked this pull request as ready for review February 11, 2021 22:47
@suztomo suztomo requested a review from a team as a code owner February 11, 2021 22:47
@suztomo suztomo merged commit 60cce2a into googleapis:master Feb 12, 2021
@suztomo suztomo deleted the windows_build branch February 12, 2021 14:17
gcf-owl-bot bot added a commit that referenced this pull request Nov 16, 2022
* chore(java): pom generation to look at root versions.txt

* not to include irrelevant modules in monorepo

Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>

Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>
Source-Link: googleapis/synthtool@909f3c8
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:a57d2ea6d1a77aa96c17ad0850b779ec6295f88b6c1da3d214b2095d140a2066
diegomarquezp pushed a commit that referenced this pull request Nov 16, 2022
* chore(java): pom generation to look at root versions.txt

* not to include irrelevant modules in monorepo

Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>

Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>
Source-Link: googleapis/synthtool@909f3c8
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:a57d2ea6d1a77aa96c17ad0850b779ec6295f88b6c1da3d214b2095d140a2066

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>
blakeli0 pushed a commit that referenced this pull request Dec 16, 2022
* chore(java): pom generation to look at root versions.txt

* not to include irrelevant modules in monorepo

Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>

Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>
Source-Link: googleapis/synthtool@909f3c8
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:a57d2ea6d1a77aa96c17ad0850b779ec6295f88b6c1da3d214b2095d140a2066

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>
blakeli0 pushed a commit that referenced this pull request Dec 16, 2022
* chore(java): pom generation to look at root versions.txt

* not to include irrelevant modules in monorepo

Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>

Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>
Source-Link: googleapis/synthtool@909f3c8
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:a57d2ea6d1a77aa96c17ad0850b779ec6295f88b6c1da3d214b2095d140a2066

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Burke Davison <40617934+burkedavison@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix "ci / windows" GitHub check
2 participants