-
-
Notifications
You must be signed in to change notification settings - Fork 260
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
Retain comments / licenses in service provider files when merging jars #1123
Retain comments / licenses in service provider files when merging jars #1123
Conversation
add tests, remove un-needed lines add comments
Thank you for the PR. Keeping the license may or may not be necessary, but any approach we take that requires user intervention is a problem: people forget, and that's a pain. One question: is the header you're adding for your code, or something you're preserving? If we're attempting to preserve comments and license information already present in the file, we may want to do something like ordering the inputs deterministically, and then concatenating service files. Assuming that the creation of the service files is deterministic (oh boy), this should be enough to ensure a repeatable build. If it's the case that you're adding the header yourself, that seems pretty unusual, and it may be worth gently pushing back on that requirement. If it's inescapable, creating the service file yourself and then using the above solution may work. |
I am trying to preserve the header that already exists in the service file
This could work. Are you opposed to just concatenating the input service files and not doing any sorting of the input lines?
If so, I'd prefer this too, because managing the additional property will likely lead to errors in the future, like you said. |
Concatenating in a deterministic order is what I'm suggesting. |
Updated with @shs96c 's suggestions. It now concatenates the service provider files, retaining the comments/licenses. It should be in a deterministic order given that |
@shs96c I think this change should be uncontroversial now. |
@shs96c could you take a look? Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for your patience with this review.
I have a situation where I need a license to be on the
META-INF/services
files in the final jar.The current behavior sorts the lines of the service file, including lines that are just comments, resulting in a jumbled output.
The desired behavior is to preserve the comments/licenses
Examples below and in this repo: https://github.com/vinnybod/bazel_java_example/tree/service-file-issues
For an input of:
The current output is: