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

ZipMisc exception with existing directories #84

Closed
hacki11 opened this issue Oct 4, 2018 · 4 comments
Closed

ZipMisc exception with existing directories #84

hacki11 opened this issue Oct 4, 2018 · 4 comments
Labels

Comments

@hacki11
Copy link
Contributor

hacki11 commented Oct 4, 2018

I have a module where ZipMisc has a problem saying a path would be duplicate.

there are some resources within my module like:

model/fileA.xyz
model/fileB.xyz

During the BndManifestPlugin ZipMisc.modify(jarTask.getArchivePath(), toModify, Predicates.alwaysFalse()); it wants to add the files in the following order:

model/fileA.xyz
model/fileB.xyz
model/

The last entry is the problem here.

Stacktrace
Execution failed for task ':module:p2'.
> java.util.zip.ZipException: duplicate entry: model/

* Try:
Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Exception is:
org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':module:p2'.
	at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:110)
	at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:77)
	at org.gradle.api.internal.tasks.execution.OutputDirectoryCreatingTaskExecuter.execute(OutputDirectoryCreatingTaskExecuter.java:51)
	at org.gradle.api.internal.tasks.execution.SkipUpToDateTaskExecuter.execute(SkipUpToDateTaskExecuter.java:59)
	at org.gradle.api.internal.tasks.execution.ResolveTaskOutputCachingStateExecuter.execute(ResolveTaskOutputCachingStateExecuter.java:54)
	at org.gradle.api.internal.tasks.execution.ValidatingTaskExecuter.execute(ValidatingTaskExecuter.java:59)
	at org.gradle.api.internal.tasks.execution.SkipEmptySourceFilesTaskExecuter.execute(SkipEmptySourceFilesTaskExecuter.java:101)
	at org.gradle.api.internal.tasks.execution.FinalizeInputFilePropertiesTaskExecuter.execute(FinalizeInputFilePropertiesTaskExecuter.java:44)
	at org.gradle.api.internal.tasks.execution.CleanupStaleOutputsExecuter.execute(CleanupStaleOutputsExecuter.java:91)
	at org.gradle.api.internal.tasks.execution.ResolveTaskArtifactStateTaskExecuter.execute(ResolveTaskArtifactStateTaskExecuter.java:62)
	at org.gradle.api.internal.tasks.execution.SkipTaskWithNoActionsExecuter.execute(SkipTaskWithNoActionsExecuter.java:59)
	at org.gradle.api.internal.tasks.execution.SkipOnlyIfTaskExecuter.execute(SkipOnlyIfTaskExecuter.java:54)
	at org.gradle.api.internal.tasks.execution.ExecuteAtMostOnceTaskExecuter.execute(ExecuteAtMostOnceTaskExecuter.java:43)
	at org.gradle.api.internal.tasks.execution.CatchExceptionTaskExecuter.execute(CatchExceptionTaskExecuter.java:34)
	at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.run(EventFiringTaskExecuter.java:51)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:317)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:309)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:185)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:97)
	at org.gradle.internal.operations.DelegatingBuildOperationExecutor.run(DelegatingBuildOperationExecutor.java:31)
	at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter.execute(EventFiringTaskExecuter.java:46)
	at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$ExecuteTaskAction.execute(DefaultTaskExecutionGraph.java:262)
	at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$ExecuteTaskAction.execute(DefaultTaskExecutionGraph.java:246)
	at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker$1.execute(DefaultTaskPlanExecutor.java:136)
	at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker$1.execute(DefaultTaskPlanExecutor.java:130)
	at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker.execute(DefaultTaskPlanExecutor.java:201)
	at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker.executeWithTask(DefaultTaskPlanExecutor.java:192)
	at org.gradle.execution.taskgraph.DefaultTaskPlanExecutor$TaskExecutorWorker.run(DefaultTaskPlanExecutor.java:130)
	at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:63)
	at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:46)
	at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:55)
Caused by: com.diffplug.common.base.Errors$WrappedAsRuntimeException: java.util.zip.ZipException: duplicate entry: model/
	at com.diffplug.common.base.Errors.asRuntime(Errors.java:408)
	at com.diffplug.common.base.Errors.rethrowErrorAndWrapOthersAsRuntime(Errors.java:132)
	at com.diffplug.common.base.Errors$Rethrowing.lambda$new$10(Errors.java:327)
	at com.diffplug.common.base.Errors.lambda$wrap$5(Errors.java:220)
	at com.diffplug.common.base.Errors.run(Errors.java:210)
	at com.diffplug.gradle.osgi.BndManifestPlugin.lambda$null$3(BndManifestPlugin.java:113)
	at org.gradle.api.internal.AbstractTask$TaskActionWrapper.execute(AbstractTask.java:794)
	at org.gradle.api.internal.AbstractTask$TaskActionWrapper.execute(AbstractTask.java:761)
	at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter$1.run(ExecuteActionsTaskExecuter.java:131)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:317)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:309)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:185)
	at org.gradle.internal.operations.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:97)
	at org.gradle.internal.operations.DelegatingBuildOperationExecutor.run(DelegatingBuildOperationExecutor.java:31)
	at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeAction(ExecuteActionsTaskExecuter.java:120)
	at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:99)
	... 30 more
Caused by: java.util.zip.ZipException: duplicate entry: model/
	at com.diffplug.gradle.ZipMisc.modify(ZipMisc.java:129)
	at com.diffplug.gradle.ZipMisc.modify(ZipMisc.java:145)
	at com.diffplug.gradle.osgi.BndManifestPlugin.lambda$null$2(BndManifestPlugin.java:117)
	at com.diffplug.common.base.Errors.lambda$wrap$5(Errors.java:218)
	... 42 more

My fix would be to check if the entry already exists in ZipMisc:

} else if (!toOmit.test(entry.getName())) {
    if(!names.contains(entry.getName())) {
        names.add(entry.getName());
        // if it isn't being modified, just copy the file stream straight-up
        ZipEntry newEntry = new ZipEntry(entry);
        newEntry.setCompressedSize(-1);
        zipOutput.putNextEntry(newEntry);
        copy(zipInput, zipOutput);
        }
    }
@nedtwigg
Copy link
Member

nedtwigg commented Oct 5, 2018

Hmm... It's always been a little unclear to me whether model/ belonged in the zip file, or if it would just be implicitly created because it's a prerequisite for model/fileA.xyz. The ZipMisc model ignores parent directories entirely - it creates them if needed when extracting, and doesn't worry about them when encoding.

The nice thing about ZipMisc, as it is, is that it tries to modify the zip file as little as possible - it just copies entries over wholesale. To make the "names" work, we'll have to first read the whole file upfront to populate names.

It looks to me like model/ is being implicitly added to the zip file when we add model/fileA.xyz, which is what causes explicitly adding model/ to fail. Seems that ZipMisc's previous input has always been either this:

model/fileA.xyz
model/fileB.xyz

or

model/
model/fileA.xyz
model/fileB.xyz

and now, for whatever reason, for the first time it's seeing

model/fileA.xyz
model/fileB.xyz
model/

I could be wrong in my diagnosis, but if I'm correct, it seems to me that the easiest fix might be this:

} else if (!toOmit.test(entry.getName())) {
  // if it isn't being modified, just copy the file stream straight-up
  try {
    ZipEntry newEntry = new ZipEntry(entry);
    newEntry.setCompressedSize(-1);
    zipOutput.putNextEntry(newEntry);
    copy(zipInput, zipOutput);
  } catch (java.util.zip.ZipException e) {
    if (e.getMessage().startsWith("duplicate entry")) {
      // no worries, sometimes input zips have strange orders, see https://github.com/diffplug/goomph/issues/84
    } else {
      throw e;
    }
}

@hacki11
Copy link
Contributor Author

hacki11 commented Oct 5, 2018

further investigation had shown that the order is not the problem. there are indeed duplicate entries within the zip file (ZipOutputStream provided by gradle does support this).
One can set the behavior at jar { duplicatesStrategy = DuplicatesStrategy.INCLUDE/WARN/EXCLUDE }.

My duplicates arise due the following setting:

if (project.convention.findPlugin(JavaPluginConvention)) {
        // Change the output directory for the main and test source sets back to the old path
        sourceSets.main.output.classesDir = new File(buildDir, "classes/main")
        sourceSets.main.output.resourcesDir = new File(buildDir, "classes/main")
        sourceSets.test.output.classesDir = new File(buildDir, "classes/test")
        sourceSets.test.output.resourcesDir = new File(buildDir, "classes/test")
    }

I did this because IntelliJ does not support the different folders for languages (java, groovy, etc.) like gradle needs. Because i want to use the platform testrunner instead of gradle i need this setting.

Though not directly an issue with ZipMisc here, but for me it would be nice to have.
It is your decision what to do - with your go i will provide a pr of course.

@nedtwigg
Copy link
Member

nedtwigg commented Oct 7, 2018

Sounds like your zip file is gonna have duplicate entries for every file? What's the platform testrunner? The eclipse built-in one? Making this work seems less productive than making it so that you don't need your zips to have duplicate entries.

@nedtwigg
Copy link
Member

nedtwigg commented Feb 4, 2019

Closing due to inactivity. Feel free to reopen.

@nedtwigg nedtwigg closed this as completed Feb 4, 2019
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