-
Notifications
You must be signed in to change notification settings - Fork 132
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
Package from single platform to multiple platforms #248
Conversation
…anged when modifying or adding new stuff; iconFile was not present in gradle PackageTask before, added that; renamed appName to name (gradle); renamed appDescription to description (gradle);
…ePackagerSettings
Please, don't push to master. Use devel instead. |
fuck |
Don't worry! I can rebase before merging. I was referring to future PRs |
Ok was scared that there would be a lot of changes already that interfere with mine in the devel branch... |
@fvarrui are there tests somewhere, so I can make sure everything works as intended? |
No offense, but adding new settings is a mess and painful. |
Btw, dunno why there are so many whitespace changes, even though I copied the stuff directly from the devel branch... |
You don't offend me, don't worry about it! 😄 The best way to grow is listening to the opinions of others, so thank you for your comments (and your contributions). Of course, there is a lot to improve, sometimes I spend some time refactoring, but I always prioritize resolving issues and adding new features. It started as a small personal project, but it has been growing over time, it's getting harder and harder to keep it up to date, but I keep trying as much as possible 😅 |
Sorry! I didn't catch this point ... are you referring to whitespaces/tabs in the source code? (indentation) |
No, sorry. I would like it, but there's not unit tests at the moment. I use two projects for testing, depending on the buiding tool: |
.generateInstaller(extension.getGenerateInstaller()) | ||
.jdkPath(extension.getJdkPath()) | ||
.jdkVersion(extension.getJdkVersion()) | ||
.jdkVendor(extension.getJdkVendor()) |
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.
Idk why this file has so many diffs, maybe because it was completely different before, but
the only thing changed are the two lines above.
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.
No idea 😅 ... Git diff algorithm? Your IDE replacing tabs with whitespaces or new lines style?
@@ -16,44 +16,44 @@ | |||
import io.github.fvarrui.javapackager.packagers.PackagerSettings; |
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.
This file has only whitespace changes...
@@ -26,70 +26,70 @@ | |||
import io.github.fvarrui.javapackager.packagers.PackagerFactory; | |||
|
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.
Also tons of whitespace idk why.
But some additions further below.
jdkPath = new File(System.getProperty("java.home")); | ||
TaskJavaUpdater taskJavaUpdater = new TaskJavaUpdater(platform); | ||
taskJavaUpdater.execute(jdkVersion, jdkVendor); | ||
jdkPath = taskJavaUpdater.jdkPath; | ||
} |
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.
Not sure if this is the right place to the the updating/downloading...
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.
Also not sure if the default jdkVersion and jdkVendor can be null here.
That should not happen!
@fvarrui Now im stuck with maven problems. Do you have any idea what this means: Show exception[INFO] --- javapackager:1.7.0:package (default) @ HelloWorldMaven ---
[WARNING] The POM for io.github.fvarrui:launch4j:jar:2.5.2 is invalid, transitive dependencies (if any) will not be available, enable debug logging for more details
[WARNING] ClassRealm[plugin>io.github.fvarrui:javapackager:1.7.0, parent: jdk.internal.loader.ClassLoaders$AppClassLoader@4e0e2f2a]
com.google.inject.CreationException: Unable to create injector, see the following errors:
1) Scope org.apache.maven.execution.scope.internal.MojoExecutionScope@6ad59d92 is already bound to org.apache.maven.execution.scope.MojoExecutionScoped at org.apache.maven.execution.scope.internal.MojoExecutionScopeModule.configure(MojoExecutionScopeModule.java:52) (via modules: org.eclipse.sisu.wire.WireModule -> org.apache.maven.execution.scope.internal.MojoExecutionScopeModule).
Cannot bind org.apache.maven.execution.scope.internal.MojoExecutionScope@1d226f27.
at ClassRealm[plugin>io.github.fvarrui:javapackager:1.7.0, parent: jdk.internal.loader.ClassLoaders$AppClassLoader@4e0e2f2a] (via modules: org.eclipse.sisu.wire.WireModule -> org.eclipse.sisu.plexus.PlexusBindingModule -> org.apache.maven.execution.scope.internal.MojoExecutionScopeCoreModule)
1 error
at com.google.inject.internal.Errors.throwCreationExceptionIfErrorsExist (Errors.java:543)
at com.google.inject.internal.InternalInjectorCreator.initializeStatically (InternalInjectorCreator.java:159)
at com.google.inject.internal.InternalInjectorCreator.build (InternalInjectorCreator.java:106)
at com.google.inject.Guice.createInjector (Guice.java:87)
at com.google.inject.Guice.createInjector (Guice.java:69)
at com.google.inject.Guice.createInjector (Guice.java:59)
at org.codehaus.plexus.DefaultPlexusContainer.addPlexusInjector (DefaultPlexusContainer.java:481)
at org.codehaus.plexus.DefaultPlexusContainer.discoverComponents (DefaultPlexusContainer.java:460)
at org.apache.maven.plugin.internal.DefaultMavenPluginManager.discoverPluginComponents (DefaultMavenPluginManager.java:436)
at org.apache.maven.plugin.internal.DefaultMavenPluginManager.createPluginRealm (DefaultMavenPluginManager.java:415)
at org.apache.maven.plugin.internal.DefaultMavenPluginManager.setupPluginRealm (DefaultMavenPluginManager.java:374)
at org.apache.maven.plugin.DefaultBuildPluginManager.getPluginRealm (DefaultBuildPluginManager.java:234)
at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:105)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:210)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:156)
at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:148)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117)
at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:81)
at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:56)
at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:128)
at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:305)
at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192)
at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105)
at org.apache.maven.cli.MavenCli.execute (MavenCli.java:957)
at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:289)
at org.apache.maven.cli.MavenCli.main (MavenCli.java:193)
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:64)
at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke (Method.java:564)
at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282)
at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225)
at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406)
at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347) |
Hi @Osiris-Team! Sorry for being missing in action, but I'm back 😄 I'm going to merge your PR in a new branch and test it. I'll tell you something ASAP. Thanks so much for your hard work |
Np. Also noticed that shading breaks the gradle plugin somehow. |
Hi @Osiris-Team! |
All errors are in
|
@fvarrui the plugin compiles fine, guess your errors are related to some additional commits (dc88aa4) you merged into my PR. |
On the other hand, I've been looking at the changes, and I don't like the idea of coupling the plugin to specific JDKs. Wouldn't it cover more possibilities if the programmer only indicated the URL of the JDK? |
What do you mean by URL? The file path? Or the download URL of the JDK? |
I mean the download URL
Yes, you are right, but the plugin will depend on external URLs, which can be broken in the future. |
By the way, what would you think if I add you as a collaborator? You have contributed a lot to the project lately... no more PRs, just push. I need help like yours to be able to keep this plugin up to date. |
yeah why not ^^ |
well to be exact only the downloading and updating of the packaging JDK will be dependent on external URLs, aka the REST API provided by adoptium, the rest of the plugin will work fine. |
Great!! 😄 I just send you an invitation |
Ok. Do you think it is a good idea to add a task/mojo which shows all available JDK versions? |
I mean that wouldnt hurt, it would just be extra work. |
Resolved conflicts give it a try ;) |
Seems like the test dependencies have also been removed -_- |
Adds feature #247
I also included an updating feature, which means that if the installed jdk in the temp directory is outdated it gets updated before being used further.
Also integrated maven and gradle hello world projects into unit test:
Also was able to merge everything into
PackageTask
(merged maven and gradle package tasks + plugin extension into one with all the settings included). This makes everythig much simpler and easier to modify settings or add new ones.Maven is having issues with the new config I think. Doing research on that.