-
Notifications
You must be signed in to change notification settings - Fork 125
[TP] Update to Archetype 3.3.1 and unify archetypes OSGi metadata #1942
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
base: main
Are you sure you want to change the base?
Conversation
HannesWell
commented
Feb 23, 2025
- Adapt to API changes in archetype-common
- Updating to Archetype 3.3. allows to remove the 'maven-artifact-transfer' dependency.
d34408b to
c012d58
Compare
org.eclipse.m2e.core.ui/src/org/eclipse/m2e/core/ui/internal/archetype/ArchetypePlugin.java
Outdated
Show resolved
Hide resolved
Test Results 324 files ±0 324 suites ±0 1h 5m 43s ⏱️ + 5m 46s For more details on these failures and errors, see this check. Results for commit 37124de. ± Comparison against base commit e11227a. ♻️ This comment has been updated with latest results. |
c012d58 to
48b1fb4
Compare
|
@HannesWell I have checked this now and the problem ist that with the move to sisu now all components are tried to be looked up, and we have removed some dependencies intentionally. I'll check if we can use some bnd magic or if I can somehow intercept the loading of unwanted parts. |
|
Another issue seem that at least locally I can't see that the fragment approach works her all archetype artifacts are now plain bundles. |
|
With the proposed changes it works for me using plexus here. the main problem was that the fragments where not generated due to missing |
|
By the way we probably should contribute to the archetype-plugin and ask if they can use constructor injection instead of field injection, then we can avoid the whole DI container thing and just create a default instance what should be enough for our case. |
48b1fb4 to
fb76184
Compare
Yes, then the implementation of the interface is found, however unfortunately the fields are still not injected. I have no clue why :/
Yes that would be great. |
laeubi
left a comment
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.
With the proposed changes I finally was able to use the injector instead of plexus and getting a proper injected instance.
Main problems here:
javax.injectwas not imported and so guice was not finding injectable fields / constructors- We need a module that supplies the
RepositorySystemrequired by the downloader
org.eclipse.m2e.core.ui/src/org/eclipse/m2e/core/ui/internal/archetype/ArchetypePlugin.java
Show resolved
Hide resolved
org.eclipse.m2e.core.ui/src/org/eclipse/m2e/core/ui/internal/archetype/ArchetypePlugin.java
Outdated
Show resolved
Hide resolved
org.eclipse.m2e.core.ui/src/org/eclipse/m2e/core/ui/internal/archetype/ArchetypePlugin.java
Outdated
Show resolved
Hide resolved
org.eclipse.m2e.core.ui/src/org/eclipse/m2e/core/ui/internal/archetype/ArchetypePlugin.java
Outdated
Show resolved
Hide resolved
|
@HannesWell I have pushed my current state and it looks like everything is injected correctly here: |
fb76184 to
b7c8de4
Compare
Yes, thanks for finding them. Since explicitly listing the required imports seems to be very error-prone I not went with the opposite approach and excluded everything that we want to strip of. In the future we can simply inspect the generated Manifest and see what doesn't resolve and can then decide if we want to add it or exclude it. I have also continued based on your changes and combined the two modules supplied by us and also found a way with that we can continue to use the |
0478cd7 to
eb681cb
Compare
eb681cb to
65022cf
Compare
|
All tests except those for Windows succeed now but since I'm not sure if this a flaw in the test or an actual problem I'm postponing this to the next release cycle as I have to create the release for the 2025-03 now and have no more time to investigate this now. |
5392eb8 to
8314b18
Compare
8314b18 to
658dfc6
Compare
658dfc6 to
b34fb4b
Compare
e6828a4 to
aa422dc
Compare
aa422dc to
b10c80f
Compare
|
@HannesWell Do you plan to revise this one? It would be nice to not have to extract dependency updates as done in #2019 . |
|
I'll try to find the reasons for the failing tests and to compete this soon. |
- Adapt to API changes in archetype-common - Updating to Archetype 3.3. allows to remove the 'maven-artifact-transfer' dependency. Co-authored-by: Christoph Läubrich <[email protected]>
2da1df3 to
78da599
Compare
to replace plexus container. Co-authored-by: Hannes Wellmann <[email protected]>
78da599 to
039304e
Compare
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |