Add Option to Rewrite Existing Manifest for Maven Artifacts#2115
Conversation
laeubi
left a comment
There was a problem hiding this comment.
Thank you for taking the time to create this pull request and for the detailed explanation of the use case! I appreciate the effort you've put into this solution.
However, after careful consideration, I don't think we can accept this change. Let me explain the reasoning behind this decision:
The Challenge with Modifying Existing OSGi Manifests
While I understand the convenience this feature would provide, allowing modifications to existing OSGi manifests goes against some core principles we try to maintain:
1. Encouraging Upstream Fixes
When an OSGi manifest contains incorrect or incomplete metadata, the right solution is to fix it at the source. Providing an easy workaround risks making these temporary fixes permanent, meaning the upstream library never gets properly corrected. This ultimately affects the entire ecosystem, not just individual projects.
2. Artifact Identity and Consistency
A fundamental principle in dependency management is that a given GAV (Group, Artifact, Version) coordinate should represent the same artifact everywhere. When we allow manifest modifications, the same GAV could exist "in the wild" with different OSGi metadata depending on where and how it was processed. This leads to:
- Hard-to-diagnose dependency conflicts
- Reproducibility issues across different environments
- Confusion when sharing artifacts or debugging issues with others
3. Hidden Technical Debt
Making it easy to modify manifests can mask underlying problems rather than solving them. What appears to be a quick fix today can become a maintenance burden that's difficult to track and understand months or years later.
What We Recommend Instead
For cases where you absolutely need to work with problematic artifacts:
- Short-term/prototyping: The current approach of wrapping artifacts without existing OSGi manifests can serve as a temporary solution while working toward proper upstream support
- Long-term solution: Consider helping the library maintainers adopt proper OSGi metadata, or if necessary, create a properly forked/wrapped version under your own GAV using tools like tycho-wrap: wrap
This way, you have full control and transparency about what's been modified, and you're not creating confusion about what the original GAV represents.
Final Thoughts
I realize this might not be the answer you were hoping for, and I genuinely appreciate the thought you put into this feature. The goal isn't to make things harder, but to maintain principles that help keep the OSGi ecosystem healthy and predictable in the long run.
If you'd like to discuss alternative approaches or have questions about the recommended solutions, I'm happy to continue the conversation!
|
Thank you for the detailed response, Christoph! To put this proposal into some more context: we are working on replacing our outdated custom Orbit with the direct usage of Maven dependencies via targlets in our target platform. As a common issue of large projects, we have the situation that for some dependencies we cannot (yet) migrate to newest versions. And some dependencies have "conflicting" transitive dependencies (such as the good old Guava dependencies) or some dependencies we do not want to have for different reasons. I fully agree with everything you mentioned regarding what would be the "right" solution, just that those approaches are universally feasible for every single dependency in large-scale projects. We are of course not talking about rewriting the manifests for a large number of dependencies, but at least a bunch of that are problematic for us. And we of course properly evaluate whether such a rewrite is appropriate and asses the risk of doing so.
The problem you mention regarding artifact identitify and consistency is definitely a relevant one, as without any further adaptations you would install Maven artifacts with modified metadata (but the same version) to your local cache and maybe even deploy them to some repository, so that you produce a "conflicting" version with what is, e.g., available at Central. To avoid that, our goal was to give those Maven artifacts with rewritten Manifest a different version by adding a timestamp. If I am not mistaken that's was the underlying goal of this PR eclipse-pde/eclipse.pde#2164, which just seems to have an unfitting description. So the overall conceptual goal was: in case you want to rewrite the Manifest (at your own risk and with knowing about the problems it may produce), you can use an according m2e extension as provided here and in that case the version of the generated Maven artifact will be different than the original one by adding/modfying a timestamp. Do you see a chance that we can bring this into a direction so that we may integrate it into m2e/PDE and avoid that we need to patch our tooling locally? Maybe by thinking about a means to enforce a modification of artifact version when doing a rewrite (to address your second concern) and by somehow making more obvious that this is a risky feature? We are of course open for any ideas to move forward in a way that the result is appropriate for upstream m2e/PDE and capable of solving our current issues. |
I've occasionally encountered similar problems and initially thought adding this option would be helpful. However, in the long run, I've found that alternative solutions work better:
Since you mentioned this affects only a subset of dependencies, these migrations don't need to happen as a big-bang change.
Just a heads-up: making it work in the IDE alone typically isn't sufficient. Whatever direction we take, Tycho will need corresponding changes as well.
These items are never deployed to Maven repositories (even local ones), and attempts to do so would fail anyway, so I'm not particularly concerned about that aspect. What we're actually dealing with is the P2/OSGi ecosystem:
This needs careful handling. The existing wrapping feature is already susceptible to these problems. While I trust you'd handle it carefully, others might not, so I'm somewhat cautious here.
I think we should avoid introducing a new type that duplicates existing functionality—as evidenced by the extensive test changes. For a feature like this, I'd expect only new tests to be added, with no modifications to existing tests. Also, the PR should minimize changes; I notice many formatting-only changes with no actual code modifications. What if we defined these constraints for this feature:
As mentioned, a similar implementation will be needed in Tycho as well, also I don't think it would require any changes to PDE at all. |
|
Thank you for the quick and detailed feedback. I apologize for the confusion regarding eclipse-pde/eclipse.pde#2164. I should indeed have provided a more precise description. Our intention behind appending a timestamp to the exported JAR file name was to ensure that the target definition is reliably reloaded rather than being served from a cached version. This was meant as a lightweight mechanism to enforce proper versioning behavior. Based on your input, we discussed the following revised approach:
Do you consider this a sound approach? Regarding point 2.4.: in our initial testing, renaming the cached file caused resolution errors because the resolver expects the file name to match the original JAR. Do you foresee structural issues with renaming in this part of the pipeline, or is there a canonical way to integrate such a timestamp-based naming variant? Additionally, would it be sensible to name the cached JAR generically after the You also mentioned required changes on the Tycho side. Could you elaborate on what exactly would have to be adjusted there? Any guidance would be greatly appreciated. |
This sound suspicious. m2e caches the jars given the hashcode of the instructions so whenever something changes the path changes. Also reloading in PDE should not depend on any filename changes, if anything is not working please create an issue with a way to reproduce the problem.
Correct
I don't think this is mandatory, but maybe useful to do so
I don't se why this should be needed the current caching already is based on the given instructions, so if we have any change there (of whatever attribute) it will get a new name, but this is more a performance optimization than a requirement to not rewrite the jar over and over again.
Can you share a stacktrace of that? The name of the file should not matter at all and many maven jars do not match that expectation as well but no problem was ever reported.
No, see above the BundleWrapper should already take care of any required action and the filename does not matter at all.
A good starting point is this: https://github.com/eclipse-tycho/tycho/blob/main/tycho-core/src/main/java/org/eclipse/tycho/core/resolver/MavenTargetDefinitionContent.java m2e+ Tycho share some code (e.g. tests) so many things can be mostly copy/paste others need some specifc consideration (e.g. xml target file parsing). The best thing is usually to provide an integration-test to demonstrate the new feature and then one can just debug any problems directly from that testcase by execute it with |
|
Hi, I agree with you that the tooling behaves correctly when resolving dependencies. We do not see any issues here. The problem we see in not changing the JAR name is that this would imply the existence of universally correct metadata for a bundle, which we believe does not exist. For example, we have a setup where we need to manage a product across different versions. These versions have varying dependencies. Let’s consider the following setup:
If the JAR does not have a unique name, I imagine this could cause issues when installing into the local P2 repository, as only one version of A can exist and conflicts may occur at the operating system level. Of course, various workarounds can be considered here, such as adding B' and B'' to the product, which we previously had with 5 different Guava versions, though adding another one felt rather unpleasant. In my opinion, the cleanest approach is to maintain changes in a central location, such as the target files. Therefore, the best solution would be to generate a JAR with a unique name, ideally composed of the symbolic name and version. |
|
@andreas-schwarz-vector I'm not sure I fully understand your scenario. If For the target platform this all does not matter unless they have a unique path as mentioned before the filename does not matter. If the bundle is later installed into a P2 repository it gets a new (different) name that usually of the form In case you are talking about having the same B (aka same GAV) and once use metadata X and then metadata Y even that would work but I can only highly discourage and is exactly this kind of problem I mentioned in the initial comment as being waiting for bad things to happen. |
Thank you for pointing this out. We were not aware of this. It appeared to us that this would not happen. This means there are two open issues: incorporate your suggested changes and check what needs to be done in Tycho. Correct? |
0667c34 to
8199bd2
Compare
| public MavenSourceBundle(BundleInfo sourceTarget, Artifact artifact, CacheManager cacheManager) throws Exception { | ||
| this.fSourceTarget = sourceTarget; | ||
| public MavenSourceBundle(BundleInfo sourceTarget, Artifact artifact, CacheManager cacheManager, | ||
| boolean manifestOverride) throws Exception { |
There was a problem hiding this comment.
Manifest override should be irrelevant for source bundles
There was a problem hiding this comment.
In our opinion, Manifest override is not irrelevant for source bundles. When regenerating artifacts in cases where no manifest exists yet, the binary bundle and its source bundle are created with symmetrical Bundle‑SymbolicNames, ensuring consistent naming from the start. If we apply manifestOverride (for already existing bundles) only to the binary bundle, we introduce a name asymmetry in the target platform: the binary receives a custom BSN while the source bundle keeps the original one. This breaks the expected 1:1 mapping. To preserve the same symmetry as in the “missing manifest: generate” scenario, the override must be mirrored for source bundles as well, ideally by deriving the source bundle’s BSN from the overridden BSN.
There was a problem hiding this comment.
I didn't say that the source bundle itself does not need to be different. But weather or not it has a rewritten manifest should not matter. The BundleInfo sourceTarget (if remembering correctly) should carry the information to what attach the source too. Also currently the flag is not considered but simply disables the caching as a whole while the cache key itself should already contain whats needed here (e.g. bsn+version).
Otherwise different target locations might compete for the same file. Also the check if a manifest is valid or not should not matter if it was generated or not.
There was a problem hiding this comment.
We looked into how the source bundle is generated and observed asymmetric behavior:
-
While the generated target bundle is placed in the .m2 directory under bnd-<instruction_key>, the source bundle is stored in a PDE Core workspace cache (.workspace).
-
The cache key for the source bundle does not appear to depend on the instructions, but instead on the bundle ID. As a result, the source bundle is not regenerated after the first creation, even if the instructions change.
-
BundleInfo#sourceTarget does contain the information linking a source bundle to its target bundle, but this no longer seems to be evaluated. Instead, if a matching header is found, the original source JAR is used; if no matching source exists, a cached file is created once, but it is not updated on subsequent changes.
Additionally, Tycho behaves differently: the generated source JAR is not placed in the bnd-<instruction_key> folder, but instead in the parent directory, alongside the original JARs.
There was a problem hiding this comment.
@laeubi would it be acceptable for you if we keep the current handling in MavenSourceBundle as a temporary solution?
We could open dedicated issues in both m2e and Tycho to analyze the source‑bundle behavior more thoroughly. Based on the outcome, the implementation could be adjusted, if the checks on manifestOverride in the MavenSourceBundle turn out to become unnecessary.
There was a problem hiding this comment.
We looked into how the source bundle is generated and observed asymmetric behavior
@weemve good catch! even though these days it should even work without the headers I think we should fix that.
would it be acceptable for you if we keep the current handling in MavenSourceBundle as a temporary solution?
I would suggest to first fix this, I started an attempt at Tycho now here:
maybe you want to came up with something similar at m2e as well it should actually be only C&P as it is in the shared area of m2e+tycho? one can even think about adapting to the Tycho variant all together to make use of caching effects between both worlds and the fx is rather trivial:
As only the BSN+Version is important for the Eclipse-Source Header we can simply cache it by using these both as the cache key and we are safe.
There was a problem hiding this comment.
Thanks for the clarification and the proposed change. I’ve incorporated your suggested update as well as the corresponding test class for MavenBundleWrapper. With this in place, propagating the manifestOverride boolean is no longer necessary, and after the export the metadata between the wrapped bundle and the source bundle now aligns correctly.
However, we noticed remaining inconsistencies:
-
due to the new logic in MavenBundleWrapper, the resulting JAR file names are now asymmetric after export.
-
The bundle JAR ends up inside the bnd- directory and keeps the original artifact's name.
-
The source bundle JAR, on the other hand, is placed next to the original artifacts, outside the bnd- directory, and adopts the + naming derived from the build instructions.
This doesn’t break resolution, but the asymmetry is unexpected and might be confusing when inspecting the exported filesystem layout. Do we have to implement something here or is this an acceptable solution for our purpose and can be done within another issue?
Also, we were unsure about the else branch in MavenSourceBundle. As the generated source bundle jar is now written in the .m2 directory, maybe the CacheManager logic is outdated now?
|
We have implemented the requested changes for overriding existing manifests. A new attribute manifestOverride has been introduced. The Maven Artifact Target Entry Dialog now looks as follows (the new option is highlighted in green):
The following prerequisites must be fulfilled:
Additional constraints to keep complexity low:
The functionality is validated through dedicated integration tests. The corresponding adjustments in Tycho have also been implemented (added capability to override existing manifest in bundles by andreas-schwarz-vector · Pull Request …). |
a4deca5 to
1f85840
Compare
1f85840 to
7e637db
Compare
|
@weemve thanks for working on this I hope to find time making a deeper review the next days! |
|
Hi @laeubi, Thanks! |
laeubi
left a comment
There was a problem hiding this comment.
Thanks for you patience!
I have added some review comments and think we are quite close now to complete this.
e1c9a7f to
d9e1d9d
Compare
laeubi
left a comment
There was a problem hiding this comment.
I think it looks good in general (except the thing with the exception), so if you can adjust this, then squash everything into one commit and rebase it on main I think we are fine.
7fa793d to
ec01dde
Compare
|
@laeubi I adjusted the error handling, rebased the feature branch on main and squashed everything into one commit. For me, it looks fine now. |
and adjusted caching of source bundles with custom bundle symbolic name for ignoreExistingMetadata option
ec01dde to
d68ea3f
Compare
Test Results 336 files + 6 336 suites +6 1h 8m 59s ⏱️ + 4m 23s For more details on these failures and errors, see this check. Results for commit d68ea3f. ± Comparison against base commit c3ff8c1. |
|
The jenkins warnings just changed file numbers due to the change but are pre-existing! |
|
@weemve thanks for your patience and responsiveness. The change should appear shortly in the snapshot builds. Please now also supply the Tycho side of the change here so we have the same support on the CI as well! |
Thank you, too! Yes, we will provide the change on the Tycho side very soon. |

Problem
Currently, the .target platform supports generating an OSGi manifest when a Maven artifact is missing one, using the option generate. However, there is no way to modify or regenerate an existing manifest for Maven-based libraries.
When migrating third-party libraries to Maven locations in .target files, many artifacts already include a manifest that does not meet our requirements (e.g., incomplete OSGi metadata). We need a mechanism to rewrite these manifests without manually editing the JARs.
Proposed Solution
Introduce a new option to rewrite a manifest file in the Maven Artifact Target Entry Dialog:
The new option generate_rewrite behaves like the current generate mode for missing manifests, but applies even when a manifest already exists. So, if generate_rewrite is enabled, the manifest will be regenerated based on the same logic used for missing manifests. This allows consistent handling of Maven artifacts during migration without manual intervention.
Benefits
This Pull Request is related to eclipse-pde/eclipse.pde#2164