-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
Fixed testparamter in ZipArchiverTest #4884
Conversation
I tried setting the to |
Off-topic, you're missing a letter near: But, is this just converting a broken test into a failing test as opposed to actually getting the system to pass? (Thanks for spotting it either way... "oops" on my part.) |
This:
... probably should be a ZipOutputStream , as in:
|
I'm not really sure if the test is broken or the code is not working. Are you sure the code works? |
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.
Indeed, maybe we need to fix the feature
@@ -99,7 +99,7 @@ public void huge64bitFile() { | |||
File hugeFile = new File(tmpDir, "huge64bitFileTest.txt"); | |||
try { | |||
RandomAccessFile largeFile = new RandomAccessFile(hugeFile, "rw"); | |||
largeFile.setLength(4 * 1024 * 1024 * 1024 + 2); | |||
largeFile.setLength(4L * 1024 * 1024 * 1024 + 2); |
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.
That's right, System.out.println(4 * 1024 * 1024 * 1024 + 2);
prints 2
The file used is indeed 2 bytes length.
@oleg-nenashev maybe this is affecting here:
https://commons.apache.org/proper/commons-compress/javadocs/api-1.18/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.html#setUseZip64-org.apache.commons.compress.archivers.zip.Zip64Mode-
When setting the mode to AsNeeded, Zip64 extensions will transparently be used for those entries that require them. This mode can only be used if the uncompressed size of the ZipArchiveEntry is known when calling putArchiveEntry(org.apache.commons.compress.archivers.ArchiveEntry) or the archive is written to a seekable output (i.e. you have used the File-arg constructor) - this mode is not valid when the output stream is not seekable and the uncompressed size is unknown when putArchiveEntry(org.apache.commons.compress.archivers.ArchiveEntry) is called.
The CI failure:
SEVERE: exception driving ZipArchiver
org.apache.tools.zip.Zip64RequiredException: huge64bitFileTest.txt's size exceeds the limit of 4GByte.
at org.apache.tools.zip.ZipOutputStream.checkIfNeedsZip64(ZipOutputStream.java:659)
at org.apache.tools.zip.ZipOutputStream.handleSizesAndCrc(ZipOutputStream.java:640)
at org.apache.tools.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:553)
at hudson.util.io.ZipArchiver.visit(ZipArchiver.java:83)
at hudson.util.io.ZipArchiverTest.huge64bitFile(ZipArchiverTest.java:121)
@@ -135,7 +135,6 @@ public void huge64bitFile() { | |||
String zipEntryName = null; | |||
|
|||
try (ZipFile zipFileVerify = new ZipFile(zipFile)) { | |||
|
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.
avoid adding unrelated changes to your PR
Fwiw, here's what I have: I looked into the other side and stopped. |
I stumbled upon a minor issue I thought, but this might be a real bug in the implementation. The existing txt file was just 2 bytes big, instead of 4gb +2. As soon as I corrected this, the test fails.
The PR introducing this functionality was: #3536
Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgradeDesired reviewers
@jsoref
Maintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are correctupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate
to be considered (see query).