Skip to content
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

The module descriptor does not see "packages" from resources, when using 'run' task #22

Open
Vampire opened this issue Mar 14, 2018 · 7 comments
Labels

Comments

@Vampire
Copy link

Vampire commented Mar 14, 2018

Source Files:

  • root-project/application-project/src/main/java/my/package/with/Classes.java
  • root-project/application-project/src/main/java/module-info.java
  • root-project/application-project/src/main/resources/images/logo.png

Result Files:

  • root-project/application-project/build/classes/java/main/my/package/with/Classes.java
  • root-project/application-project/build/classes/java/main/module-info.class
  • root-project/application-project/build/resources/main/images/logo.png

Inside module-info.java (central framework class resolves and loads images):
opens images

When this is packaged as a JAR it works fine, as the images package then is present at runtime.
If there would be an images folder with a java file in root-project/application-project/src/main/java/ this would also work fine, as you patch the module with the resources folder.
But unfortunately the module-info.class seems to be evaluated before the --patch-module flag is effective and thus fails as there is no images package at that point.
Due to that, execution fails with java.lang.module.InvalidModuleDescriptorException: Package images not found in module as it indeed is not in there in this situation.

I guess you should either

  • replace the exploded directories and the --patch-module flag by adding the built artifact to the module path instead, or
  • need to copy both directories into a new directory and put that on the module path instead;
  • actually it also works if I create an empty file inside root-project/application-project/classes/java/main/images/, but you shouldn't do that generally as it makes the compileJava task out-of-date to do this, so you would need to sync the dir to some other place (e. g. the temporary directory for the run task) and then add the files, but then you can also simply copy both folders to that place.
@Vampire
Copy link
Author

Vampire commented Mar 14, 2018

Workaround: Create a package-info.java file for the package in question and annotate the package with @Deprecated. Without an annotation the package-info.java file is not compiled as it would not provide any added value at runtime without annotations.

@zyxist
Copy link
Owner

zyxist commented Mar 14, 2018

The reason why it fails is that Gradle keeps resources separately from the compiled classes in the build directory. I haven't dug enough in the application plugin to be 100% sure, but as of now I doubt it is easily fixable. One would have to change the way Java plugins in Gradle handle compiled files and resources...

I'll keep the issue open, and for now, I will document it as a known limitation with the description of the workaround you have found.

@zyxist zyxist changed the title invalid module descriptor when using the run task The module descriptor does not see "packages" from resources, when using 'run' task Mar 14, 2018
@zyxist zyxist added the bug label Mar 14, 2018
@Vampire
Copy link
Author

Vampire commented Mar 14, 2018

You don't like either of my two suggested solutions?
Gradle must keep those files separate, otherwise the up-to-date checks would not work properly.
The application plugin just uses sourceSets.main.runtimeClasspath to set the classpath of the run task.

@zyxist
Copy link
Owner

zyxist commented Mar 14, 2018

I'm not 100% sure, but I see some potential technical issues:

  1. Gradle run task does not depend on jar by default, which means that there might be no JAR archive to run,
  2. making a repackaged copy in another directory - I remember some issues I found while migrating projects from Maven to Gradle, related to resource files. I must investigate this topic a bit more before I start combining them. I assume that Gradle devs had a reason to keep those files separate and run them from that directory.

Generally, I'm a bit afraid of changing too much internal logic of certain plugins :), and I need to take a closer look why application plugin works in this way. If you have some free time, you can also check your concept (I would prefer the solution number #2).

@Vampire
Copy link
Author

Vampire commented Mar 14, 2018

You can add task dependencies at any time, so not problem with 1. Doing tasks.run.dependsOn jar is easy, even in a plugin.

Regarding 2. I already told you what the reason is. The incremental build feature aka up-to-date checks. Gradle checksums the inputs and outputs before and the outputs after the task is run. If the inputs or outputs change between two runs, the task is executed again. If the task inputs are the same as the task inputs before the last run and the task outputs are the same as the task outputs after the last run, the task is considered up-to-date.

If two tasks write files to the same directory, it is much harder to determine which output file belongs to which task and also files that were added spuriously and thus can be considered a modification of outputs if the output is the directory, not the individual files, would not be detectable. Thus it is much cleaner and easier to clearly separate the outputs of various tasks.

For executing a JavaExec task it is simply not necessary usually to build a JAR first, so it is more efficient to just put the two task output directories onto the classpath instead of building the JAR.

There should be absolutely no problem to either use the built JAR, or to copy the two output directories into the tasks temporary directory after deleting its contents and adding that to the module path instead of the individual directories.

Another option that I used for a different project where the execution of the tool depended on additional files that were put to the distribution, but were of course not present like that to the default run task is to replace the default run task completely by a task that depends on installDist and then runs the assembled distribution. In a build script this looks like

task run(type: JavaExec, dependsOn: installDist, overwrite: true) {
   description 'Runs this project as a JVM application'
   group 'application'
   main mainClassName
   classpath fileTree("$installDist.destinationDir/lib")
}

In a plugin this has to be adapted to get modifications the using build did to the run task, or instead of replacing the task, the exsting one should maybe just be reconfigured to replace the classpath with the installDist result and an added dependency to the installDist task.

@zyxist
Copy link
Owner

zyxist commented Mar 15, 2018

Actually, run task is not incremental but your mention of the incremental build led me to the conclusion, why Gradle does things in this way:

run task is not incremental, but earlier tasks are. Java code and resources are processed by two distinct tasks, so they must write to separate directories. This is also the answer to my issue I ran in the past, why resources placed in /src/main/java are not visible in Gradle, but visible in Maven, even if this is not a valid layout.

run task uses this layout to speed up application launching. In this way, Gradle does not have to package everything into JARs, or do anything with the filesystem in order to run the app.

I think I will ask Gradle developers for an advice, what the best option to implement that would be. In addition, they will fail into the same issue during their implementation of native Gradle support for Jigsaw, so it is even more important to let them know and find a common solution.

@Vampire
Copy link
Author

Vampire commented Mar 15, 2018

I never said run is incremental. Your conlusion is exactly what I described and what could be work-arounded by either using the JAR or by combining the directories into one like:

run {
   doFirst {
      def args = jvmArgs
      def modulePathIndex = args.indexOf('--module-path') + 1
      def ps = Pattern.quote(System.properties.'path.separator')
      def jd = Pattern.quote(file("$buildDir/classes/java/main") as String)
      def rd = Pattern.quote(file("$buildDir/resources/main") as String)
      args[modulePathIndex] = args[modulePathIndex]
            .replaceFirst(/(?:$ps|^)$rd/, '')
            .replaceFirst(/(?:$ps|^)$jd/, Matcher.quoteReplacement(temporaryDir as String))
      jvmArgs = args
      project.sync {
         from compileJava
         from processResources
         into temporaryDir
      }
   }
}

This code works and must be done in doFirst, as your plugin uses doFirst to set the jvmArgs at execution time instead of configuration time. Better would be of course to do it at confiugration time.

Btw. while looking into this I observed

private String stripResources(String classpath, File resourceOutputDir) {
	String outPath = ":" + resourceOutputDir.getAbsolutePath();
	if (classpath.contains(outPath)) {
		return classpath.replace(outPath, "");
	}
	return classpath;
}

This cannot work correctly, as : is no the path separator generally, it is system dependent what the path separator is. This code only works on *nix, not on Windows. You have to use System.properties.'path.separator' instead of ":".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants