Skip to content

Add Option to Rewrite Existing Manifest for Maven Artifacts#2115

Merged
laeubi merged 1 commit intoeclipse-m2e:mainfrom
vi-eclipse:feat-9082949060688351234-rewriteManifests
Mar 27, 2026
Merged

Add Option to Rewrite Existing Manifest for Maven Artifacts#2115
laeubi merged 1 commit intoeclipse-m2e:mainfrom
vi-eclipse:feat-9082949060688351234-rewriteManifests

Conversation

@weemve
Copy link
Copy Markdown
Contributor

@weemve weemve commented Jan 2, 2026

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:

image

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

  • Simplifies migration of third-party libraries to Maven-based .target definitions.
  • Ensures uniform OSGi metadata across all artifacts.
  • Reduces manual work and risk of errors when modifying manifests.

This Pull Request is related to eclipse-pde/eclipse.pde#2164

Copy link
Copy Markdown
Member

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

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!

@HeikoKlare
Copy link
Copy Markdown
Contributor

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.
For us, we currently see these options:

  • Update all our dependencies and contribute to all those dependencies so that we can properly consume all dependency chains together as they are: this is something we will not achieve in the short or mid term future and we will honestly not be able to do that for every single case at all
  • Stick to outdated Orbit technology, at least for parts of the dependencies that are not up-to-date and require dependency rewrites: that will clutter our dependency management to different technologies and the Orbit scripts that we use are really something we would like to get rid of.
  • Maintain and deploy an m2e and maybe PDE fork with the necessary functionality for our tooling: definitely better than the one above as we get rid of different means of dependency management; but instead of this, we would of course like to provide that functionality upstream and consume the tooling as-is :-)

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.

@laeubi
Copy link
Copy Markdown
Member

laeubi commented Jan 7, 2026

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.

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:

  • Sticking with the legacy approach temporarily
  • Reconsidering whether the dependency is truly needed and migrating away from it
  • Creating a forked variant under a new GAV
  • Manually re-bundling the dependency and deploying it to a corporate Nexus or similar
  • Copying a single JAR into the target project and using a Directory target location

Since you mentioned this affects only a subset of dependencies, these migrations don't need to happen as a big-bang change.

Maintain and deploy an m2e and maybe PDE fork with the necessary functionality for our tooling

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.

The problem you mention regarding artifact identity and consistency...

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:

  • In P2, the id+version combination is a globally unique identifier. Once you have a problematic artifact there, it persists, which can create challenging issues.
  • When a version range exists in a package, P2 might select any compatible provider, not necessarily the one you expect.

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.

Do you see a chance that we can bring this into a direction so that we may integrate it into m2e/PDE...

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:

  1. Add a new attribute manifestOverride
  2. When enabled, custom instructions must be provided and the resulting BSN must differ from the original (validated during generation, with an error reported if they match)
  3. The dependency mode must be none

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.

@weemve
Copy link
Copy Markdown
Contributor Author

weemve commented Jan 8, 2026

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:

  1. Instead of adding a generate_rewrite option to the missingManifest attribute, we propose introducing a new attributes manifestOverride (boolean)
  2. If manifestOverride = true, then:
    2.1. The resulting bundle's Bundle-SymbolicName must differ from the original one (this is how we interpreted your second point).
    2.2. The Bundle-Version attribute must be set in the Build Instructions
    2.3. The dependency mode must be none.
    2.4. The cached JAR that is built internally will automatically receive a file name which is calculated from Bundle-SymbolicName + Bundle-Version. We can customize it to contain the current timestamp. This replaces the previous PDE changes.

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 Bundle-SymbolicName + Bundle-Version instead of mirroring the original file name, to avoid such mismatches? We could implement it, if this is a prefered solution.

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.

@laeubi
Copy link
Copy Markdown
Member

laeubi commented Jan 8, 2026

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 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.

Based on your input, we discussed the following revised approach
...
2.1. The resulting bundle's Bundle-SymbolicName must differ from the original one (this is how we interpreted your second point).

Correct

2.2. The Bundle-Version attribute must be set in the Build Instructions

I don't think this is mandatory, but maybe useful to do so

The cached JAR that is built internally will automatically receive a file name which is calculated from Bundle-SymbolicName + Bundle-Version. We can customize it to contain the current timestamp. This replaces the previous PDE changes.

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.

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

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.

Additionally, would it be sensible to name the cached JAR generically after the Bundle-SymbolicName + Bundle-Version instead of mirroring the original file name, to avoid such mismatches?

No, see above the BundleWrapper should already take care of any required action and the filename does not matter at all.

You also mentioned required changes on the Tycho side. Could you elaborate on what exactly would have to be adjusted there?

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 mvnDebug.

@andreas-schwarz-vector
Copy link
Copy Markdown

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:

  • There are two products: X and Y.
  • There is a dependency, A, in both X and Y, which is no longer being actively developed. A has a transitive dependency on B.
  • Product X has a dependency on B'.
  • Product Y has a dependency on B'', where B' and B'' are different versions.
  • This results in the need for different versions of B for X and Y.

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.
Another option would be to avoid defining the upper bound, which could, however, result in pulling in elements that are truly incompatible.

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.

@laeubi
Copy link
Copy Markdown
Member

laeubi commented Jan 16, 2026

@andreas-schwarz-vector I'm not sure I fully understand your scenario.

If B' an B'' have different versions they will have different filenames after being installed into a product.

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 <bsn>_<version>.jar so I don't see a problem here either.

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.

@andreas-schwarz-vector
Copy link
Copy Markdown

If the bundle is later installed into a P2 repository it gets a new (different) name that usually of the form <bsn>_<version>.jar so I don't see a problem here either.

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?

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 {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Manifest override should be irrelevant for source bundles

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@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.

Copy link
Copy Markdown
Member

@laeubi laeubi Feb 22, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@weemve weemve Feb 23, 2026

Choose a reason for hiding this comment

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

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?

Comment thread org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/MavenTargetBundle.java Outdated
@weemve
Copy link
Copy Markdown
Contributor Author

weemve commented Feb 17, 2026

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):

image

The following prerequisites must be fulfilled:

  • The Bundle-SymbolicName must be changed.
  • The dependency mode must be none.

Additional constraints to keep complexity low:

  • Exactly one root dependency must be present.
  • Exactly one instruction must be present.

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 …).

@weemve weemve force-pushed the feat-9082949060688351234-rewriteManifests branch 2 times, most recently from a4deca5 to 1f85840 Compare February 23, 2026 15:32
@weemve weemve force-pushed the feat-9082949060688351234-rewriteManifests branch from 1f85840 to 7e637db Compare February 23, 2026 16:00
@laeubi
Copy link
Copy Markdown
Member

laeubi commented Mar 6, 2026

@weemve thanks for working on this I hope to find time making a deeper review the next days!

@weemve
Copy link
Copy Markdown
Contributor Author

weemve commented Mar 20, 2026

Hi @laeubi,
just a quick check‑in to ask whether you might already have a rough idea when you’ll be able to review the current state again.
Also, do you perhaps have a hint whether there’s anything obvious left for me to fix regarding the failing checks?

Thanks!

Copy link
Copy Markdown
Member

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

Thanks for you patience!
I have added some review comments and think we are quite close now to complete this.

@weemve weemve force-pushed the feat-9082949060688351234-rewriteManifests branch from e1c9a7f to d9e1d9d Compare March 24, 2026 11:55
@weemve weemve requested a review from laeubi March 26, 2026 15:25
Copy link
Copy Markdown
Member

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

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.

@weemve weemve force-pushed the feat-9082949060688351234-rewriteManifests branch 2 times, most recently from 7fa793d to ec01dde Compare March 27, 2026 16:34
@weemve
Copy link
Copy Markdown
Contributor Author

weemve commented Mar 27, 2026

@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
@weemve weemve force-pushed the feat-9082949060688351234-rewriteManifests branch from ec01dde to d68ea3f Compare March 27, 2026 16:52
Copy link
Copy Markdown
Member

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

It all now looks good to me!

@github-actions
Copy link
Copy Markdown

Test Results

  336 files  + 6    336 suites  +6   1h 8m 59s ⏱️ + 4m 23s
  713 tests +19    695 ✅ +19  16 💤 ±0  1 ❌ ±0  1 🔥 ±0 
2 139 runs  +57  2 088 ✅ +56  48 💤 ±0  2 ❌ +1  1 🔥 ±0 

For more details on these failures and errors, see this check.

Results for commit d68ea3f. ± Comparison against base commit c3ff8c1.

@laeubi
Copy link
Copy Markdown
Member

laeubi commented Mar 27, 2026

The jenkins warnings just changed file numbers due to the change but are pre-existing!

@laeubi laeubi merged commit 98d9dc7 into eclipse-m2e:main Mar 27, 2026
11 of 15 checks passed
@laeubi
Copy link
Copy Markdown
Member

laeubi commented Mar 27, 2026

@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!

@weemve
Copy link
Copy Markdown
Contributor Author

weemve commented Mar 30, 2026

@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.

@weemve weemve deleted the feat-9082949060688351234-rewriteManifests branch March 30, 2026 07:24
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.

4 participants