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

fix: fixed the profile for Artifact Registry deployment #950

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

suztomo
Copy link
Member

@suztomo suztomo commented Dec 10, 2024

Defining the profile to publish artifacts to Artifact Registry.

The previous (reverted) attempt in #936 was wrong in that it missed this point when I used <activeByDefault>true</activeByDefault>:

This profile will automatically be active for all builds unless another profile in the same POM is activated using one of the previously described methods.

from https://maven.apache.org/guides/introduction/introduction-to-profiles.html.

This pull request now turns on the release-sonatype profile unless the artifact-registry-url property is defined. The property artifact-registry-url is only used for Artifact Registry publication.

Experiment

The java-shared-config repository was able to publish the artifacts to my Artifact Registry:

mvn deploy -P=release-gcp-artifact-registry -P=-release-sonatype
-P=-release-staging-repository -Dartifact-registry-url=artifactregistry://us-maven.pkg.dev/...

http://gpaste/5275049054175232

The java-shared-config repository was able to publish the
artifacts to my Artifact Registry:

mvn deploy -P=release-gcp-artifact-registry -P=-release-sonatype \
  -P=-release-staging-repository -Dartifact-registry-url=artifactregistry://us-maven.pkg.dev/...
@suztomo suztomo requested a review from a team as a code owner December 10, 2024 15:14
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Dec 10, 2024
<!-- Only when we use the release-gcp-artifact-registry profile,
which comes with artifact-registry-url property, this profile is
turned off. -->
<name>!artifact-registry-url</name>
Copy link
Member

@burkedavison burkedavison Dec 10, 2024

Choose a reason for hiding this comment

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

I don't think it's possible to chain profile activations/deactivations like this.

https://stackoverflow.com/a/2248552

Are you able to verify this works?

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 not chaining profile activation (this is not relying on the line 145).

This activation relies on the fact that when we publish artifacts to GCP Artifact Registry using release-gcp-artifact-registry profile, -Dartifact-registry-url=... is always passed to the mvn command.

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'll verify it works in the Kokoro release job.

Copy link
Member

Choose a reason for hiding this comment

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

Why have line 145, then?

Copy link
Member Author

Choose a reason for hiding this comment

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

Line 145 is dummy value that tells the property is undefined.

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'll verify it works in the Kokoro release job.

The release of this repository and google-auth-library-java (googleapis/google-auth-library-java#1593) succeeded.

@suztomo suztomo merged commit 8a496d3 into googleapis:main Dec 10, 2024
57 checks passed
@suztomo suztomo deleted the artifactregistry-profile branch December 10, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants