Is is basically a standard practice to exclude these dependencies? I feel the maintainers of the truth should fix their project setup so that these transitive dependencies are not exposed to library users?

No action needed.

The reason I had to exclude it was because different dependencies were pulling in different versions of error_prone_annotations transitively and causing upper bound failures. Apparently, some error prone annotations have runtime scope. So, the dependency can't be provided.
@suztomo In case you have any comments on this.

Is this parent necessary? For what? The Gradle doesn't seem to use it? And we specify all the versions of the libraries in the dependency management section?

This is to help us align better with client libraries. It has some checks and general configuration that is useful.

Cool. Just wanted to make sure we don't diverge from the Bazel setup. Once we start doing that, it will be difficult to reconcile and maintain them later.

This will technically change how gax artifacts get published on maven (they are getting parents which they used to not have).

Maybe it is ok, but we should be careful here. Can it cause any breakages and/or dependency conflicts for existing users of this? Also, I'm not sure if changing a parent can be considered a breaking change for maven atrifact or not.

I don't think it's a breaking change. Besides, almost everything else in the Java SDK has it as the parent.

Oh, interesting. Were we having duplicate entries? Which files? Or, was it that you just noticed a deprecation warning and found a way to resolve it?

I get it on the gax/src/main/resources/dependencies.properties and only when running ./gradlew build publishToMavenLocal. It looks like the file created by build overlaps with sourcesJar. If there is a better way to handle it, please let me know.

Hmm... I think this is because now gax/src/main/resources/dependencies.properties is checked in as source. So I guess what's happening is that, because a final version is generated at gax/build/resources/main/dependencies.properties, now you have two dependencies.properties files to include.

I think DuplicatesStrategy.EXCLUDE could be a problem, because it doesn't deterministically guarantee which file will win in the end. However, for a source JAR, I actually start to think there may be no reason to add this properties file? Not sure if it's the standard practice to add this kind of resource files into a source JAR. OTOH, since the file is part of the source set (it's under src/main), it also makes sense to include it, perhaps the one before processing with templating.

Let me try to find a way to include the original source file.

Actually, it is correct that the error is due to the new gax/src/main/resources/dependencies.properties, but this is not because we have a processed file with the same filename under gax/build/.

  task sourcesJar(type: Jar, dependsOn: classes) {
    archiveClassifier = 'sources'

    from sourceSets.main.allSource, sourceSets.test.allSource, sourceSets.main.resources.srcDirs
    exclude('**/*Test.java')
  }

So the sources JAR is composed of main.allSource, test.allSource, and main.resources. (BTW, we could just say sourceSets.main.resources instead of sourceSets.main.resources.srcDirs.)

The thing is, allSource already includes resources. The default is a list [java, resources] as documented. So, it's redundant to specify main.resources again. Indeed, if I change it to

from sourceSets.main.allSource, sourceSets.test.allSource

I still see dependencies.properties in build/libs/gax-2.7.1-SNAPSHOT-sources.jar after running ./gradlew :gax:sourcesJar. OTOH, if I change it to from sourceSets.main.allJava, sourceSets.test.allSource, I don't see the properties file in the -sources.jar.

To sum, don't set duplicatesStrategy, but set from sourceSets.main.allSource, sourceSets.test.allSource (remove main.resources).

Thanks for the investigation. This clears things up. As a side note, I'm not sure why we are including test sources at all, yet exclude all files ending in *Test.java.

Hmm... good point. My guess is that, this repo is also generating *-testlib.jar files that I think contain some test utility classes (whose filename doesn't end with *Test.java). Maybe per the Maven Central requirements or convention, because we are publishing -testlib.jar files, we also need to include source files.

This will technically change how gax artifacts get published on maven (they are getting parents which they used to not have).

Maybe it is ok, but we should be careful here. Can it cause any breakages and/or dependency conflicts for existing users of this? Also, I'm not sure if changing a parent can be considered a breaking change for maven atrifact or not.

If you pull the bazel build part out into a sepaarate workflow file, you should be able to use GitHub Matrix feature and save some code duplication.

matrix:
  script: [build.sh, maven-build.sh]

Good point. However, the gradle build will eventually be replaced by maven build though, so not sure if it's worth it.

It's up to you. I guess removing Gradle would be easy in this setup too.

This makes inconsistent which versions are used for gradle+bazel and which for maven. I understand that this is a good first step, but for this to work properly, we need some mechanism for a single source of truth for those version numbers between different build systems (now it is dependencies.properties).

This is an interim state. The plan is for the Gradle build to go away. We can keep the versions in sync with Bazel manually. Ultimately, we need to figure out if we can just rely on a single build system. Is the Bazel build used for self-service? I'm not sure why we absolutely need it.

I agree with Mike as well that eventually we want to be on a single build system, and I checked gapic-generator-java and googleapis that the bazel stuff in gax-java are only used for loading dependencies, which I think can be replaced completely by bazel targets in gapic-generator-java. My vision is that the pom.xml in the GAPIC generator mono repo will be the source of truth for dependencies, and I don't think we can get rid of dependencies.properties completely before we move gax-java to the mono repo.

In short, this is the plan:

1. Add maven build(This PR).
2. Replicate all gradle features(CI, release etc.) in maven and remove gradle build.
3. Release with Maven dependencies. Before each gax-java release, we may have to manually sync dependency.properties to keep things in sync for the time being.
4. Move gax-java to the GAPIC mono repo so that it shares dependency management with gapic-generator-java.
5. Do some changes accordingly in the mono repo so that the client library generation depends on the root pom.xml for loading dependencies(This part is not well designed yet but I believe it can be done).

We are probably not going to change the bazel surface in gapic-generator-java(like java_gapic_library), so that all the client libraries and self service can still be generated seamlessly, but we will change how the dependencies are loaded.

@meltsufin Every single automatically generated PR that you currently getting from owlbot is created using bazel build (and that relies on gax dependencies.properties). It is also used for self-service, that is correct.

And it is also very useful for development, though it may look useless on release step. For any gapic-gen/gax development work it s extremely useful. For example, with the REST transport work, I can simply plug in gax and gapic-generator in build and generate, compile and run all unit tests for all APIs in both transport (27,000 tests in one run on one maching by simply replacing http_archive to local_repository). It is also very easy to do and repeat it after every change in the generator (and due to caching, the run is also fast). I literally have no idea how else I would test and fix this whole thing on googleapis scale without this bazel build.

I'm afraid that we are on a path of fixing problems in release workflow by creating severe problems in development workflow. The reason why the autogenerated PRs produce working and compilable code and Java, for example, does not suffer from the same problems as Python releases suffer (broken code being generated), is precisely due to gax and gapic-generator being tightly integrated into the bazel build.

We fully intend to keep dependencies.properties up to date and we have no plans to break the Bazel build as part of the migration to Maven. If we are actually breaking any of the existing workflows, please let us know. However, this PR is not doing that.

I can simply plug in gax and gapic-generator in build and generate, compile and run all unit tests for all APIs in both transport

This is very interesting, can you please share the script you are using? I can make sure we are not breaking the script or create a similar experience with maven.

The reason why the autogenerated PRs produce working and compilable code and Java, for example, does not suffer from the same problems as Python releases suffer (broken code being generated), is precisely due to gax and gapic-generator being tightly integrated into the bazel build.

Can you please elaborate the example? How bazel build can help prevent generating broken code? I thought it's mostly the code logic(like AST stuff) in the generator itself not the build system.