Skip to content

Fix ModelloCli broken after moving from Plexus to JSR330 #435

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

Closed
wants to merge 7 commits into from
Closed

Conversation

fridrich
Copy link

This is my attempt at least to make the ModelloCli.java at least start and run. It still throws an exception on simple models. But at least the guice bindings look correct and the thing is at least starting and running.

@fridrich
Copy link
Author

#434

@fridrich
Copy link
Author

This version is now working for me and from my side, it is ready to get integrated. I don't know which kind of test to write for a command-line tool so that it does not regress any more. Please, advise.
Btw, the guice path I tried before is largely convoluted, and complicated. After I studied the commit 3c05d7a, I understood the problem.

@fridrich
Copy link
Author

fridrich commented Apr 13, 2024

The latest version actually restores the possibility to instantiate directly the Modello class, since I see now that some Fedora tools in javapackages-bootstrap do it. So this version partially reverts commit 8804652 concerning the two files and adds the configuration including the autowiring and index scanning. Like that, stuff out there that was using Modello() constructor directly, will be able to use it from this commit on again.

@fridrich fridrich changed the title Fix the ModelloCli to use Guice, since it is not Plexus component anymore Fix ModelloCli broken after moving from Plexus to JSR330 Apr 13, 2024
@fridrich
Copy link
Author

It would be good to have some test for this. Minimum that it can generate simple java files from a model. But for that, one needs basically the whole classpath including the plugins.

@fridrich
Copy link
Author

fridrich commented May 3, 2024

This is running in openSUSE since and generates on command-line the correct Java files. Any chance to have it reviewed?

@slawekjaranowski slawekjaranowski linked an issue May 4, 2024 that may be closed by this pull request
@fridrich
Copy link
Author

Where is this standing?

@fridrich
Copy link
Author

ping?

@cstamas
Copy link
Member

cstamas commented May 29, 2024

@fridrich do you have any needed change in here, or goal was really just to make it work (again)? As if latter, I'd like to have this PR superseded by #452 as it does more correct approach IMO (using shade plugin that is able to process Sisu index and Plexus XMLs).

@fridrich
Copy link
Author

I would really love to have at least this in 90352d3
The reasons are 1) normally, we in distributions (SUSE for me) unshade the things anyway. 2) then javapackages-bootstrap in Fedora, as I saw call the new Modello() construtor. Also this change is in line with what was done in the tests when moving from plexus-containers to jsr330. After, I don't mind any other solution that works and that works also unshaded.
I don't care about the commit that adds the exception. But I would really love to have it working as it was before. Just adding cdi-api for something that can be done with existing dependencies...
Now, I am not the one who decides. I would still prefer this, since the tests use exactly the same.

@fridrich
Copy link
Author

or if I missunderstood something, I am fine with whichever that makes this work. In SUSE we anyway use jpackage_script to create the classpath for the modello script.

@fridrich
Copy link
Author

I am just wondering whether one cannot just simply apply that one commit and then maybe shade artifacts, which means that I can unshade it in the package without having to patch the logic that relied on shade plugin?

@cstamas
Copy link
Member

cstamas commented May 29, 2024

Yes, will see about that. Am not at desk, so you can do it as well (pr against branchin this pr), will see tomorrow.

Or just wait until tomorrow and i will sort it out.

@fridrich
Copy link
Author

@fridrich
Copy link
Author

Closing in favour of #452

@fridrich fridrich closed this May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ModelloCli is not working in 2.3.0
3 participants