Hi, > Looking quickly the drop of the conditional read of the manifest seems the > issue for tests.
Yes it was. Must have been late when I wrote that ... I have fixed this silly error of mine and now it all works. > In terms of impl my 2cts would be that we shouldnt have had jdependency in > this plugin IMHO so better to stay off it (in particular cause there it is > really not needed, probably saner to read the manifest first then if mjar > is enabled list META-INF/$digit/.* and enable to copy them at need but > otherwise impl looks at the right location for me. The current implementation of shade already uses this dependency. I just added a lacking feature which makes all of the JEP238 classes available in the list of classes. Niels On Sat, Nov 11, 2023 at 7:47 PM Romain Manni-Bucau <[email protected]> wrote: > Hi Niels, > > Looking quickly the drop of the conditional read of the manifest seems the > issue for tests. > > In terms of impl my 2cts would be that we shouldnt have had jdependency in > this plugin IMHO so better to stay off it (in particular cause there it is > really not needed, probably saner to read the manifest first then if mjar > is enabled list META-INF/$digit/.* and enable to copy them at need but > otherwise impl looks at the right location for me. > > Romain Manni-Bucau > @rmannibucau <https://twitter.com/rmannibucau> | Blog > <https://rmannibucau.metawerx.net/> | Old Blog > <http://rmannibucau.wordpress.com> | Github < > https://github.com/rmannibucau> | > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book > < > https://www.packtpub.com/application-development/java-ee-8-high-performance > > > > > Le sam. 11 nov. 2023 à 19:29, Niels Basjes <[email protected]> a écrit : > > > Hi, > > > > I've been working on trying to make the shade plugin support jars that > > are Multi Release Jars (JEP-238) > > https://issues.apache.org/jira/browse/MSHADE-406 > > A Multi Release Jar simply means you can have a single class in a single > > jar with multiple implementations (and thus multiple class files in the > > jar) which are used depending on the runtime Java version. > > > > Right now I have made some changes in > > https://github.com/tcurdt/jdependency/pull/209 and > > https://github.com/apache/maven-shade-plugin/pull/202 > > > > Right now the m-shade-p build fails over a few tests that are related to > > the META-INF/MANIFEST.MF file. > > > > I have trouble figuring out what is going wrong. > > > > *I would really appreciate it if someone can have a peek at my efforts > and > > provide feedback.* > > > > My key questions: > > > > - Is this a correct implementation direction? All the m-shade-p code > > assumes the path of the class file to be the same (if you replace '.' > > and > > '/') as the packagename+classname. I have tried to keep the impact > > limited. > > Perhaps a much more impactful change is needed where for all files a > > record > > is introduced which contains a set of properties (like classname and > > filename separately). > > - What did I do to break the META-INF/MANIFEST.MF related tests? I did > > add the extra flag to indicate it is a multi release jar because that > is > > needed by the java runtime, but other than that I've tried to not > touch > > it > > (and apparently failed). > > > > Thanks for the feedback > > > > -- > > Best regards / Met vriendelijke groeten, > > > > Niels Basjes > > > -- Best regards / Met vriendelijke groeten, Niels Basjes
