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.

Sign up for GitHub

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

Jump to bottom

Add an exclusion rule for VisibilityProto.getDescriptor #2048

Merged
merged 2 commits into from May 10, 2021
Merged

Add an exclusion rule for VisibilityProto.getDescriptor #2048

merged 2 commits into from May 10, 2021

Conversation

suztomo
Copy link
Contributor

@suztomo suztomo commented May 7, 2021

Fixes #2045

@google-cla google-cla bot added the cla: yes label May 7, 2021
@suztomo
Copy link
Contributor Author

suztomo commented May 7, 2021

How can we test this change against the pull request in question? googleapis/java-common-protos#148 (comment)

@suztomo
Copy link
Contributor Author

suztomo commented May 7, 2021

@elharo This is the implementation of "Temporary Solution" I wrote in #2045. What do you think?

@elharo
Copy link
Contributor

elharo commented May 7, 2021

This is probably OK, but I don't think I can approve it until I understand the root cause.

@elharo elharo changed the title To add an exclusion rule for VisibilityProto.getDescriptor Add an exclusion rule for VisibilityProto.getDescriptor May 10, 2021
elharo
elharo approved these changes May 10, 2021
View reviewed changes
dependencies/src/main/resources/linkage-checker-exclusion-default.xml Outdated
<Package name="com.google.appengine.repackaged.com.google" />
</Source>
<Reason>
The appengine-api-1.0-sdk shades certain classes, creating method references that does not
Copy link
Contributor

Choose a reason for hiding this comment

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

that do not match

@suztomo suztomo mentioned this pull request May 10, 2021
Closed
4 tasks
@suztomo
Copy link
Contributor Author

suztomo commented May 10, 2021

I confirmed it works to resolve the problem.

I locally installed the branch googleapis/java-common-protos#148 to my local Maven repository. I ran Linkage Monitor with working directory at ~/java-common-protos. First time without the new exclusion rule, then it failed. Second time it with the exclusion rule and it succeeded:

May 10, 2021 3:57:14 PM com.google.cloud.tools.dependencies.linkagemonitor.LinkageMonitor run
INFO: BOM Coordinates: com.google.cloud:libraries-bom:20.3.0
May 10, 2021 3:59:07 PM com.google.cloud.tools.dependencies.linkagemonitor.LinkageMonitor run
INFO: The following problems in the baseline no longer appear in the snapshot:
  Class com.google.api.VisibilityProto is not found
  Class com.google.api.VisibilityProto is not found
  Class com.google.api.VisibilityProto is not found
  Class com.google.api.VisibilityProto is not found
  Class com.google.api.VisibilityProto is not found
  Class com.google.api.VisibilityProto is not found

May 10, 2021 3:59:07 PM com.google.cloud.tools.dependencies.linkagemonitor.LinkageMonitor main
INFO: No new problem found

@suztomo suztomo merged commit e8b9944 into master May 10, 2021
@suztomo suztomo deleted the appengine_exclusion_rule branch May 10, 2021 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elharo elharo elharo approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Linkage Monitor detects a genuine error between proto-google-common-protos and appengine-api-1.0-sdk
2 participants
@suztomo @elharo