GoogleCloudPlatform / artifact-registry-maven-tools

Apache License 2.0
24 stars 22 forks source link

Make compatible with Gradle Config Cache #92

Open bjoernmayer opened 6 months ago

bjoernmayer commented 6 months ago

Fixes #85

This is based upon @Naitbit solution.

I introduce CommandExecutor in order to abstract the command execution of gcloud.

The implementation of Maven uses the same strategy as before. The implementation for Gradle makes uses the providerFactory.

google-cla[bot] commented 6 months ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

TSampley commented 4 months ago

@bjoernmayer have you already signed the CLA? https://github.com/GoogleCloudPlatform/artifact-registry-maven-tools/pull/92/checks?check_run_id=23209124250

I'm trying to figure out why Google hasn't merged this yet when it affects so many modern developers.

bjoernmayer commented 4 months ago

@bjoernmayer have you already signed the CLA? https://github.com/GoogleCloudPlatform/artifact-registry-maven-tools/pull/92/checks?check_run_id=23209124250

I'm trying to figure out why Google hasn't merged this yet when it affects so many modern developers.

All signed. See the build

I even mentioned the Devs in this PR. No reaction whatsoever. That's why I decided to setup my own Plugin.

yihanzhen commented 3 months ago

/gcbrun

yihanzhen commented 3 months ago

@bjoernmayer very sorry I might have ruined this PR by updating the branch (looks like you had more commits to your branch) and now the build failed. Any chance you can fix this?

bjoernmayer commented 3 months ago

@bjoernmayer very sorry I might have ruined this PR by updating the branch (looks like you had more commits to your branch) and now the build failed. Any chance you can fix this?

Sure thing. Unfortunately, I do not know what is broken as I cannot see the build output.

yihanzhen commented 3 months ago

@bjoernmayer very sorry I might have ruined this PR by updating the branch (looks like you had more commits to your branch) and now the build failed. Any chance you can fix this?

Sure thing. Unfortunately, I do not know what is broken as I cannot see the build output.

So it seems a lot of changes were not originally in this PR. After pulling in the new changes, the error is:

/workspace/artifactregistry-gradle-plugin/src/main/java/com/google/cloud/artifactregistry/gradle/plugin/ProviderFactoryCommandExecutor.java:6: error: cannot find symbol
import org.gradle.process.ExecOutput;
                         ^
  symbol:   class ExecOutput
  location: package org.gradle.process
/workspace/artifactregistry-gradle-plugin/src/main/java/com/google/cloud/artifactregistry/gradle/plugin/ProviderFactoryCommandExecutor.java:27: error: cannot find symbol
        ExecOutput execOutput;
        ^
  symbol:   class ExecOutput
  location: class ProviderFactoryCommandExecutor
/workspace/artifactregistry-gradle-plugin/src/main/java/com/google/cloud/artifactregistry/gradle/plugin/ProviderFactoryCommandExecutor.java:29: error: cannot find symbol
            execOutput = providerFactory.exec(execSpec -> {
                                        ^
  symbol:   method exec((execSpec)[...]t); })
  location: variable providerFactory of type ProviderFactory
/workspace/artifactregistry-gradle-plugin/src/main/java/com/google/cloud/artifactregistry/gradle/plugin/ArtifactRegistryGradlePlugin.java:99: error: cannot find symbol
        providerFactory = ((Settings) o).getProviders();
                                        ^
  symbol:   method getProviders()
  location: interface Settings
4 errors

I think it might still be better to stick to the just the gradle update in the PR, if it still possible for you :)

bjoernmayer commented 3 months ago

@yihanzhen are we able to merge https://github.com/GoogleCloudPlatform/artifact-registry-maven-tools/pull/93?

I could merge master then, which brings this PR down to the config cache changes

yihanzhen commented 3 months ago

Yep, just merged #93

bjoernmayer commented 3 months ago

Hey @yihanzhen , I just opened https://github.com/GoogleCloudPlatform/artifact-registry-maven-tools/pull/98 which - when merged - should fix the build in this PR

bjoernmayer commented 3 months ago

@yihanzhen please retrigger the build :)

yihanzhen commented 2 months ago

/gcbrun