Skip to content

Conversation

HannesWell
Copy link
Contributor

  • Adapt to API changes in archetype-common
  • Updating to Archetype 3.3. allows to remove the 'maven-artifact-transfer' dependency.

Copy link

github-actions bot commented Feb 23, 2025

Test Results

  324 files  ±0    324 suites  ±0   57m 32s ⏱️ - 1m 33s
  689 tests ±0    661 ✅  -  4  21 💤 ±0   4 ❌ +2  3 🔥 +2 
2 067 runs  ±0  1 991 ✅  - 10  63 💤 ±0  10 ❌ +8  3 🔥 +2 

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

Results for commit b10c80f. ± Comparison against base commit 0f862e5.

♻️ This comment has been updated with latest results.

@laeubi
Copy link
Member

laeubi commented Feb 26, 2025

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

@laeubi
Copy link
Member

laeubi commented Feb 26, 2025

Another issue seem that at least locally I can't see that the fragment approach works her all archetype artifacts are now plain bundles.

@laeubi
Copy link
Member

laeubi commented Feb 26, 2025

With the proposed changes it works for me using plexus here. the main problem was that the fragments where not generated due to missing ${...} in if clause, so it used the literal what always evaluated to "true"

@laeubi
Copy link
Member

laeubi commented Feb 26, 2025

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.

@HannesWell
Copy link
Contributor Author

With the proposed changes it works for me using plexus here. the main problem was that the fragments where not generated due to missing ${...} in if clause, so it used the literal what always evaluated to "true"

Yes, then the implementation of the interface is found, however unfortunately the fields are still not injected. I have no clue why :/

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.

Yes that would be great.

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

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.inject was not imported and so guice was not finding injectable fields / constructors
  • We need a module that supplies the RepositorySystem required by the downloader

@laeubi
Copy link
Member

laeubi commented Mar 2, 2025

@HannesWell I have pushed my current state and it looks like everything is injected correctly here:

@HannesWell
Copy link
Contributor Author

Main problems here:

* `javax.inject` was not imported and so guice was not finding injectable fields / constructors

* We need a module that supplies the `RepositorySystem` required by the downloader

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

@HannesWell
Copy link
Contributor Author

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.

@HannesWell HannesWell force-pushed the update-archetype branch 2 times, most recently from 5392eb8 to 8314b18 Compare March 3, 2025 21:36
@HannesWell HannesWell force-pushed the update-archetype branch 2 times, most recently from e6828a4 to aa422dc Compare June 20, 2025 06:50
HannesWell and others added 2 commits June 20, 2025 09:28
- 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]>
to replace plexus container.

Co-authored-by: Hannes Wellmann <[email protected]>
@akurtakov
Copy link
Contributor

@HannesWell Do you plan to revise this one? It would be nice to not have to extract dependency updates as done in #2019 .

@HannesWell
Copy link
Contributor Author

I'll try to find the reasons for the failing tests and to compete this soon.

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.

3 participants