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.

Sign up for GitHub

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

Jump to bottom

feat: GAPIC library BOM in monorepo_script bootstrap #7991

Merged
merged 5 commits into from
Jul 8, 2022
Merged

feat: GAPIC library BOM in monorepo_script bootstrap #7991

merged 5 commits into from
Jul 8, 2022

Conversation

suztomo
Copy link
Member

@suztomo suztomo commented Jul 8, 2022 •
edited
Loading

The bootstrap script now creates google-cloud-gapic-bom that covers the GA artifacts released from the monorepo.

@suztomo suztomo requested a review from Neenu1995 July 8, 2022 16:44
@suztomo suztomo requested a review from ddixit14 July 8, 2022 16:58
suztomo
suztomo commented Jul 8, 2022
View reviewed changes
README.md

Review the artifact name "google-cloud-gapic-bom" in the bom directory and
configure the version managed by Release Please. Ensure the BOM is part of the
entire release pipeline.
Copy link
Member Author

Choose a reason for hiding this comment

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

@ddixit14 This message is for you in near future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, will keep this in mind.

meltsufin
meltsufin reviewed Jul 8, 2022
View reviewed changes
bom.pom.xml
detail of the Libraries BOM.
</description>
<dependencyManagement>
BOM_ARTIFACT_LIST
Copy link
Member

Choose a reason for hiding this comment

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

You probably need <dependencies> before the placeholder, and a closing one after.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, the check failed with Error: Malformed POM /home/runner/work/google-cloud-java/google-cloud-java/monorepo/google-cloud-java/bom/pom.xml: Unrecognised tag: 'dependency' (position: START_TAG seen ...<dependencyManagement>\n <dependency>... @16:17) @ /home/runner/work/google-cloud-java/google-cloud-java/monorepo/google-cloud-java/bom/pom.xml, line 16, column 17.

bootstrap.sh Outdated
artifactId_line=$(grep --max-count=1 'artifactId' "${pom_file}")
version_line=$(grep --max-count=1 'x-version-update' "${pom_file}")

if [[ $version_line == *"<version>0"* ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

Another method is to read the .repo-metadata.json for each library, which specifies whether it's preview or not. It might be more reliable. Also, why not do this in the main loop above where we iterate over repos.txt?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice idea.

why not do this in the main loop above where we iterate over repos.txt?

I feel it's better to split the logic of BOM generation as separate one. No technical reason.

bootstrap.sh Outdated

mkdir bom
awk -v "dependencyManagements=$bom_lines" '{gsub(/BOM_ARTIFACT_LIST/,dependencyManagements)}1' \
../../bom.pom.xml > bom/pom.xml
Copy link
Member

Choose a reason for hiding this comment

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

The directly should match the artifact name if possible: google-cloud-gapic-bom.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

@suztomo
Copy link
Member Author

suztomo commented Jul 8, 2022 •
edited
Loading

Error: Non-resolvable import POM: Could not find artifact com.google.cloud:google-identity-accesscontextmanager-bom:pom:1.4.1-SNAPSHOT @ line 24, column 19. When I remove google-identity-accesscontextmanager-bom entry from the pom.xml, it works. What's special about google-identity-accesscontextmanager-bom?

It turned out that the bom module was not declared in the parent module (java-accesscontextmanager). Fix: googleapis/java-accesscontextmanager#359

The error from CoverageAggregator/pom.xml is:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <parent>
    <artifactId>google-cloud-java</artifactId>
    <groupId>com.google.api</groupId>
    <version>0.0.1-SNAPSHOT</version>
  </parent>
  <modelVersion>4.0.0</modelVersion>

  <artifactId>CoverageAggregator</artifactId>

  <properties>
    <maven.compiler.source>11</maven.compiler.source>
    <maven.compiler.target>11</maven.compiler.target>
  </properties>

  <dependencies>
[ERROR] [ERROR] Some problems were encountered while processing the POMs:
 @ 
[ERROR] The build could not read 1 project -> [Help 1]
[ERROR]   
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/ProjectBuildingException

  </dependencies>

</project>

@suztomo suztomo mentioned this pull request Jul 8, 2022
Merged
gcf-merge-on-green bot pushed a commit to googleapis/java-accesscontextmanager that referenced this pull request Jul 8, 2022
@suztomo
fix: the bom module was missing in the root pom (#359)
2af19cd 
The bom module was missing in the root pom. It caused build failure in another issue in google-cloud-java repository: googleapis/google-cloud-java#7991 (comment)

Other repositories declare the bom module correctly. Example: https://github.com/googleapis/java-accessapproval/blob/main/pom.xml#L116
@suztomo
Copy link
Member Author

suztomo commented Jul 8, 2022

All checks are green.

@suztomo suztomo merged commit 46654e0 into googleapis:monorepo_script Jul 8, 2022
github-actions bot pushed a commit that referenced this pull request Jul 8, 2022
@suztomo
fix: the bom module was missing in the root pom (#359)
476e618 
The bom module was missing in the root pom. It caused build failure in another issue in google-cloud-java repository: #7991 (comment)

Other repositories declare the bom module correctly. Example: https://github.com/googleapis/java-accessapproval/blob/main/pom.xml#L116
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meltsufin meltsufin meltsufin left review comments

@ddixit14 ddixit14 ddixit14 approved these changes

@Neenu1995 Neenu1995 Awaiting requested review from Neenu1995

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@suztomo @meltsufin @ddixit14