feat: add generate_library.sh without post processing

[JoeWang1127]

Fixes #1922 ☕️

[JoeWang1127]

The differences in the comparison is because the entries in gapic_metadata.json and package-info.java are in the same order of proto files in proto_library target in BUILD. I sorted the proto files with respect to file name to get a deterministic result (proto_files=$(find "$proto_path" -type f -name "*.proto" | sort)).

However, in a recent submit (cl/536794833), model_garden_service.proto is added after model_service.proto in BUILD. The generate_library.sh still generates library since model_garden_service.proto is before model_service.proto upon sorted and causes the difference.

This is not a source code difference and there are some approaches

1. change the proto files's order in google3
2. ignore the difference in gapic_metadata.json and package-info.java, though this is risky as real differences, lack of an entry for example, will be ignored.
3. refactor generate_library.sh such that list all proto files in proto_path as the same order in BUILD. This will increase the input list of generate_library.sh and seems redundant since we have proto_path as an input. Of course, here the assumption is all proto files in proto_path should be generated, but why put it there if it's not for code generation?
4. modify the generator to generate gapic_metadata.json and package-info.java with sorted proto files.

@meltsufin @blakeli0 @suztomo WDYT?
cc: @diegomarquezp

[blakeli0]

in a recent submit (cl/536794833), model_garden_service.proto is added after model_service.proto in BUILD

Can you take a look at the commit history? It might be manually added.
Either way, if the current generation process is assuming the protos are sorted, then we should add them as input as well, because we can not control the order of the protos, but the result is dependent on them so it makes the generation process not hermetic.

[JoeWang1127]

Can you take a look at the commit history? It might be manually added.

The CL's title suggested that it's auto-generated but I think the proto file is added manually.

Either way, if the current generation process is assuming the protos are sorted, then we should add them as input as well, because we can not control the order of the protos, but the result is dependent on them so it makes the generation process not hermetic.

I think by sorting the proto files, the result is deterministic and it is also hermetic as given the same proto files and dependencies, the generated code is the same. I added an approach in my previous comment that the generator can generate the two files with sorted proto files, is that possible?

If we choose to list the proto files as BUILD does, we are tied with BUILD and we need to change the list whenever a new proto files is added (or deleted) in proto_path unless I find an example such that a proto file is not generated in proto_path.

[blakeli0]

change the proto files's order in google3

This is not a sustainable approach as there could be many manual changes.

ignore the difference in gapic_metadata.json and package-info.java, though this is risky as real differences, lack of an entry for example, will be ignored.
modify the generator to generate gapic_metadata.json and package-info.java with sorted proto files.

In order for us to make a decision, we need to understand the usage of gapic_metadata.json and package-info.java. If the order of entries in these two files do not matter to customers, then we can change gapic-generator-java to always generated sorted gapic_metadata.json and package-info.java.

refactor generate_library.sh such that list all proto files in proto_path as the same order in BUILD. This will increase the input list of generate_library.sh and seems redundant since we have proto_path as an input. Of course, here the assumption is all proto files in proto_path should be generated, but why put it there if it's not for code generation?

This would be the only approach if the order matters in gapic_metadata.json and package-info.java. This does not mean that we are tied to BUILD file, the input could be a list of proto files. Similar to other parameters like transport, it is from BUILD file but it does mean we are tied to it.

[JoeWang1127]

In order for us to make a decision, we need to understand the usage of gapic_metadata.json and package-info.java. If the order of entries in these two files do not matter to customers, then we can change gapic-generator-java to always generated sorted gapic_metadata.json and package-info.java.

The package-info.java has mainly comments and I guess the difference in comments do not matter.

The gapic_metadata.json lists metadata of the grpc services. I think typically json file is parsed into map-like structure so the order of the key does not matter.

Of course there maybe other use cases and I'll do some research.

[blakeli0]

We have to copy the whole google/ folder? Or maybe only the common protos?

[JoeWang1127]

Some protos reference google/api, google/iam, google/type. I'm not sure whether we have a exhausted list of protos such that only those protos can be referenced. So copy google folder is a safe option.

[blakeli0]

Not part of this PR, but I think we should come up with an exhaustive list before we switch to use it in our generation process. As google/ folder contains protos from all services, which is a lot more than what we need, and it could easily confuse people in the future.

[JoeWang1127]

We can do that, but do we have a guidance that a proto should not reference a proto outside of a list? If not, the list is likely changing and we are back to copy google.

[blakeli0]

Theoretically, the service team can reference protos from anywhere, but cross service proto depedencies are not allowed per https://google.aip.dev/213, so the service protos should only reference common protos from a few selected folders. I think we may have two services that have cross service proto dependencies but all other services should obey it.

Now regarding common protos, we don't have a guidance on where they should be and it's possible that new common protos are added to new locations. But if it happens, I think it would be better to explicitly add it, similar to explicitly declare a new dependency vs getting a new dependency transitively from a existing dependency in Maven's world.

We can discuss more in next project meeting.

[blakeli0]

Can you please add comments or more examples regarding proto_path that all protos referenced in the path has to be in the current working directory(assuming my understanding is correct from another comment)?

[JoeWang1127]

I added an example of generating showcase client.

[sonarcloud]

[gapic-generator-java-root] Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[sonarcloud]

[java_showcase_integration_tests] Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[sonarcloud]

[java_showcase_unit_tests] Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information