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

Fix pom file errors caused by the extra "$" #12177

Merged
merged 1 commit into from
Mar 8, 2023

Conversation

deannagarcia
Copy link
Member

Fixes #12170

@deannagarcia deannagarcia added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Mar 8, 2023
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Mar 8, 2023
@deannagarcia deannagarcia requested a review from perezd March 8, 2023 17:39
@deannagarcia deannagarcia merged commit be7f6b0 into protocolbuffers:22.x Mar 8, 2023
@@ -32,7 +32,7 @@
<Automatic-Module-Name>com.google.protobuf.util</Automatic-Module-Name> <!-- Java9+ Jigsaw module name -->
<Bundle-DocURL>https://developers.google.com/protocol-buffers/</Bundle-DocURL>
<Bundle-SymbolicName>com.google.protobuf.util</Bundle-SymbolicName>
<Export-Package>com.google.protobuf.util;version=${project.version}</Export-Package>
<Export-Package>com.google.protobuf.util;version={project.version}</Export-Package>
Copy link

Choose a reason for hiding this comment

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

since we see com.google.protobuf.util;version=${project.version} in the generated pom of 3.22.1

i am thinking this needs to use {version} instead (see line 7)

if {project.version} would be available, the generated pom would have looked like this:

com.google.protobuf.util;version=$3.22.1

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm trying to match https://repo1.maven.org/maven2/com/google/protobuf/protobuf-java-util/3.21.12/protobuf-java-util-3.21.12.pom which uses pom replacements (instead of the bazel replacements) for project.version so I think the original in 22.1 was correct. I'm reverting that part of this change now.

Does that seem right to you?

Copy link

Choose a reason for hiding this comment

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

i think both variants work, the plugin configuration probably does not even need to be in the published pom

what is important is that the version can be evaluated correctly when the build is generating the bundle, so that in the jar we see the right values in the META-INF/MANIFEST.MF file (snippet from protobuf-java-util-3.21.12.jar):

...
Bundle-SymbolicName: com.google.protobuf.util
Bundle-Version: 3.21.12
Created-By: Apache Maven Bundle Plugin
Export-Package: com.google.protobuf.util;version="3.21.12";uses:="com.go
 ogle.protobuf,javax.annotation"
...

this can be achieved by "hardcoding" the version during bazel replacement or by letting the bundle maven plugin evaluate ${project.version} correctly (as done in the follow-up PR where $ was added back)

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

Successfully merging this pull request may close these issues.

None yet

3 participants