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

Remove Linkage Monitor check #8977

Closed
suztomo opened this issue Sep 14, 2021 · 7 comments
Closed

Remove Linkage Monitor check #8977

suztomo opened this issue Sep 14, 2021 · 7 comments

Comments

@suztomo
Copy link
Contributor

suztomo commented Sep 14, 2021

Let's remove Linkage Monitor check.

Screen Shot 2021-09-14 at 09 46 15

What language does this apply to?

It's CI for protobuf Java.

If it's a proto syntax change, is it for proto2 or proto3? -> N/A
If it's about generated code change, what programming language? -> N/A

Describe the problem you are trying to solve.

Linkage Monitor check (#6318) is no longer needed.

https://github.com/protocolbuffers/protobuf/blob/master/tests.sh#L248

Describe the solution you'd like

I will remove the linkage monitor setup from this repository.

https://github.com/protocolbuffers/protobuf/blob/master/tests.sh#L248

Describe alternatives you've considered

Additional context

I'll create a pull request.

Background

When we installed Linkage Monitor, we were choosing the protobuf-java version for the Libraries BOM independently from the Google Cloud client libraries. Now we choose the protobuf Java version that is used by the Google Cloud client libraries. When there's any linkage errors between them, it would manifest as compilation errors. Therefore, we no longer need to run Linkage Monitor for this repository.

@elharo
Copy link
Contributor

elharo commented Sep 14, 2021

This allows us to ship a release that introduces a new linkage error and that could not be pulled into libraries-bom. I'm not sure we want to do that. Just yesterday protobuf-team was talking about how to more strictly guarantee that our latest release works with the latest grpc release.

@suztomo
Copy link
Contributor Author

suztomo commented Sep 14, 2021

Protobuf guaranteeing with compatibility with gRPC is great.

I see similarity between protobuf to gRPC relationship and java-shared-dependencies to Google Cloud client libraries. The shared dependencies BOM has GitHub checks to run the client libraries' builds and tests using the pull request's version (This is configuration file in the java-shared-dependencies.). Note that gRPC is built with Gradle, thus you cannot use xmllint to replace a version.

Screen Shot 2021-09-14 at 13 18 48

This kind of checks are stronger assertion than Linkage Monitor, in that it not just checks API compatibility, but also the behavioral compatibility. Linkage Monitor may miss a change not shown in APIs.

What do you think?

@elharo
Copy link
Contributor

elharo commented Sep 14, 2021

I think that check doesn't happen until protobuf has already shipped a release with the breakage. The linkage monitor catches the mistake at the PR phase in this repo before we've merged, much less shipped.

@suztomo
Copy link
Contributor Author

suztomo commented Sep 14, 2021

The downstream dependencies checks do catch mistakes at the PR phase. What it does is the followings:

@elharo
Copy link
Contributor

elharo commented Sep 14, 2021

So this requires installing the downstream dependencies check in this repo?

@suztomo
Copy link
Contributor Author

suztomo commented Sep 16, 2021

Yes, to achieve "more strictly guarantee that our latest release works with the latest grpc release", a new check similar to Yoshi Java's downstream dependency check needs to be installed into this repo.

@deannagarcia
Copy link
Member

I don't think we want to start adding downstream dependencies checks. Because of this, I think we are stuck with the Linkage Monitor check until we find a better test solution.

I'm going to close this bug since we won't be removing the check. However, if you come up with a way to better test these linkages feel free to open it back up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants