Ensure that the manifest of the generated jar is the first entry

[geoand]

Fixes: #5399

[cescoffier]

no backport?

[geoand]

We could add it for sure, but I don't think it's terribly critical. I'll let others decide if we want to backport it or not.

[geoand]

There might a windows issue with this... I restarted the tests to see if it was just a flake

[geoand]

I just ran the entire build and tests on my Linux machine and it worked. I'll restart the Windows stuff and in case it's a flake. Otherwise I might need some help from someone with a Windows machine.

[stuartwdouglas]

The windows failure looks related.

[geoand]

The Windows test does seem to fail consistently... Unfortunately there are no logs nor do I have access to a Windows machine.

@jaikiran IIRC you did look into some related Windows things in the past? Mind taking a look at this one please?
If you like I can give you access to my fork so you can push straight to my branch.
Thanks!

[jaikiran]

Hello @geoand, sorry was away and am just catching up with the messages.

Mind taking a look at this one please?

Certainly, I'll take a look at this one along with the other Windows issue that I was planning to look into. I expect to get access to a Windows system tomorrow, so should be able to see what's going on.

If you like I can give you access to my fork so you can push straight to my branch.

For now, I'll just pull in your commit locally during testing.

[geoand]

Thanks a ton @jaikiran!

[jaikiran]

Hello @geoand, I was able to look into this and figure out what's going on. It turns out an issue within the test case itself. Can you give me write permissions on this PR/branch(?) so that I can push a commit which will fix it and retrigger the CI?

[geoand]

@jaikiran thanks a lot! You should have received an invitation to collaborate on my fork

[jaikiran]

Thank you @geoand.

So the reason why this PR was failing on Windows was due to the following exception (which wasn't accessible easily in the CI infrastructure)

Caused by: java.nio.file.FileSystemException: C:\quarkus\integration-tests\maven\target\test-classes\projects\project-uberjar-true\target\acme-1.0-SNAPSHOT-runner.jar: The process cannot access the file because it is being used by another process.

	at sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:86)
	at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:97)
	at sun.nio.fs.WindowsException.rethrowAsIOException(WindowsException.java:102)
	at sun.nio.fs.WindowsFileSystemProvider.implDelete(WindowsFileSystemProvider.java:269)
	at sun.nio.fs.AbstractFileSystemProvider.deleteIfExists(AbstractFileSystemProvider.java:108)
	at java.nio.file.Files.deleteIfExists(Files.java:1165)
	at io.quarkus.deployment.pkg.steps.JarResultBuildStep.buildUberJar(JarResultBuildStep.java:169)
	at io.quarkus.deployment.pkg.steps.JarResultBuildStep.buildRunnerJar(JarResultBuildStep.java:138)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at io.quarkus.deployment.ExtensionLoader$1.execute(ExtensionLoader.java:948)
	... 7 more

This was because the newly added method ensureManifestOfJarIsReadableByJarInputStream method wasn't closing the streams it was opening. I've updated this PR to fix that. This change passed locally on a Windows setup. So let's see how it goes now on our CI.

As for the changes in this PR itself, they look good to me.

[geoand]

@jaikiran thanks for getting to the bottom of this!
Silly mistake, I should have spotted in. Next time with the logs you added it will be much easier 😎.

Also feel free to add yourself as a co-author of the commit if you haven't done so already.

[geoand]

@stuartwdouglas does this look good to you?

[geoand]

@jaikiran seems to still be failing in Windows

[jaikiran]

Let me just rerun this whole branch against a Windows OS locally. Will get the results in a few minutes.

[jaikiran]

@jaikiran seems to still be failing in Windows

@geoand Looks like this branch lost the change I had committed. The try-with-resources change I had added to ensureManifestOfJarIsReadableByJarInputStream no longer exists. Perhaps an accidental overwrite?

[geoand]

@jaikiran hm... I deleted my branch locally and then pulled your change.
However maybe I made a mistake, let me update it.

[geoand]

Pushed the correct change. Thanks again @jaikiran

[geoand]

All tests pass and this was verified also by @jaikiran (which I thank for his Windows fix), so I'll merge it