You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/02/22 08:27:37 UTC
[GitHub] [commons-compress] PeterAlfredLee opened a new pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
PeterAlfredLee opened a new pull request #169:
URL: https://github.com/apache/commons-compress/pull/169
Add a new AlwaysWithComestibles in Zip64Mode, this is a compromise for some libraries including 7z and Expand-Archive Powershell utility(and likely Excel).
With `Zip64Mode.AlwaysWithComestibles`, the following fields in Central File Header will be set as needed:
1. disk number offset in split zip
2. relative offset of LFH
3. relative header offset in zip64 extra
4. disk number start in zip64 extra
These changes are consistent with commit `2b7281`.
It would be good if the generated file in test could tested by Expand-Archive or 7z, but I didn't find a way to detect them in Java.
It's welcome for suggestions about the name `Zip64Mode.AlwaysWithComestibles` :)
See [COMPRESS-565](https://issues.apache.org/jira/projects/COMPRESS/issues/COMPRESS-565?filter=allissues&orderby=created+DESC%2C+priority+DESC%2C+updated+DESC) for more detailed information.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] coveralls commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-783198259
[](https://coveralls.io/builds/37318931)
Coverage increased (+0.002%) to 87.357% when pulling **67aa5ee6bb1182fc74819f8074270512e27c9d12 on PeterAlfredLee:COMPRESS-565** into **1b7528fbd6295a3958daf1b1114621ee5e40e83c on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] PeterAlfredLee commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-787585567
And I made a typo in the name - I was thinking about the name `AlwaysWithCompatibility`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] garydgregory commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-835823337
Surely we can come up with a better name than a "food item" (comestible) or is that term used in the zip spec itself?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] coveralls edited a comment on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-783198259
[](https://coveralls.io/builds/37515557)
Coverage increased (+0.002%) to 87.363% when pulling **76cea5ed721c37b9964076f868d048caffd9ab11 on PeterAlfredLee:COMPRESS-565** into **65cbce6c54e11d7633483093e6b7e4d6c92dde64 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] PeterAlfredLee commented on pull request #169: COMPRESS-565 : add a new AlwaysWithCompatibility in Zip64Mode
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-836032619
> you may want to change the name of the test method.
Ah, yes. I forgot to change the name of the tests. :-(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] bodewig commented on pull request #169: COMPRESS-565 : add a new AlwaysWithCompatibility in Zip64Mode
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-836147157
looks good to me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] missingdays edited a comment on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
missingdays edited a comment on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-788828622
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] bodewig commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-835823881
you may want to change the name of the test method. Other than that, please just go ahead.
@garydgregory "comestible" is almost only present in the PR's title. Peter has fixed the occurrences in code already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] bodewig commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-787053225
I like the code change.
It is good you always include a central directory zip64 when there has been one in LFH, something I likely would have overlooked. Thanks. The javadoc of the new option is not completely accurate, as it will always add zip64 entries to the central directory, but not necessarily encode the disk number and relative offset.
Oh, I believe we are overlooking something. We must ensure we add the relative LFH offset to the Zip64 extended information entry inside the central directory even if it is not too big in the case where the disk number is too big - or we'd create a gap inside of the extra field. This is currently wrong inside master as well, unless I'm wrong.
I don't like the name of the new option, but I'm the last one who should suggest a different one - I've got a long track record of picking bad names. Let's discuss this on the dev list.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] PeterAlfredLee commented on pull request #169: COMPRESS-565 : add a new AlwaysWithCompatibility in Zip64Mode
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-836140258
The Github Actions is weird - I can successfully compile and test the class `Zip64SupportIT`, but Actions is reporting compilation failure, which is caused by the missing of `FileOutputStream` and `FileInputStream`. These 2 classes are already imported but Actions seems missed them.
Maybe this is a bug of Github Actions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] PeterAlfredLee merged pull request #169: COMPRESS-565 : add a new AlwaysWithCompatibility in Zip64Mode
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee merged pull request #169:
URL: https://github.com/apache/commons-compress/pull/169
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] bodewig edited a comment on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
bodewig edited a comment on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-788865243
> This means we should encode the relative offset and disk number at the same time - even through only eithor of them is too big. Am I right about it?
No, this is not what I meant.
In the layout of the ZIP64 extended information extra field you have the two sizes - which must always be present - then the LFH offset, then the disk number. If you don't need to encode the LFH offset but have to encode the disk number the disk number would appear directly after the sizes. I'm afraid there might be parsers that try to read the LFH offset anyway if the size of the extra field is bigger than the two size fields. So I'd encode both, even if only the disk number needs to be encoded - and would be willing to encode the LFH offset (when needed) without encoding the disk number.
I may be overcautious (the parser should consult the corresponding field inside of the central directory actually contains FFs and it should see the size is only 20 bytes not 24 or even 28) but prefer to stay on the safe side. Also, this is an edge case of an edge case - I don't expect us to ever create archives with more than 64k parts.
`AlwaysWithCompatibility` sounds a lot better, thanks :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] bodewig commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-835824973
@PeterAlfredLee I had another look and I don't think you've addressed my comment from https://github.com/apache/commons-compress/pull/169#issuecomment-788865243 - but we can deal with that after the PR has been merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] PeterAlfredLee commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-787572402
> The javadoc of the new option is not completely accurate, as it will always add zip64 entries to the central directory, but not necessarily encode the disk number and relative offset.
Yes, it is. I pushed a new commit with only the javadoc of the new option changed.
> We must ensure we add the relative LFH offset to the Zip64 extended information entry inside the central directory even if it is not too big in the case where the disk number is too big - or we'd create a gap inside of the extra field. This is currently wrong inside master as well, unless I'm wrong.
This means we should encode the relative offset and disk number at the same time - even through only eithor of them is too big. Am I right about it?
> Let's discuss this on the dev list.
+1.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] missingdays commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
missingdays commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-788828622
Hi!
Any change this fix will be releases in the following couple of days? We have a release soon, and if this fix won't be released, I'll have to use custom jar with this fix, which will lead to some problems for us.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] missingdays commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
missingdays commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-790681610
> If you want to release the software on your own, and especially if you plan on pushing it out to the world through Maven Central or elsewhere, you must change the Maven Group and Artifact IDs.
I mean bundle the jar to our release, not release the library anywhere.
Is this branch stable enough for the production usage?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] bodewig commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-790729399
I'd say it depends on which parts of Compress you use. If you are only using code that hasn't changed or has only seen bugfixes, you should be pretty safe. Usually we do some extra code reviews when preparing a release and this may not have happened fully for new code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] PeterAlfredLee commented on pull request #169: COMPRESS-565 : add a new AlwaysWithCompatibility in Zip64Mode
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-836132501
I pushed a commit that fixes name of tests, and also fixes the [comment](https://github.com/apache/commons-compress/pull/169#issuecomment-788865243).
WDYT? @bodewig @garydgregory
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] garydgregory commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-790678150
> > I'm sorry but the master branch isn't quite ready for a new release
>
> Ok, but if I build the jar from this branch, is this ok to use in release?
If you want to release the software on your own, and especially if you plan on pushing it out to the world through Maven Central or elsewhere, you must change the Maven Group and Artifact IDs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] PeterAlfredLee commented on pull request #169: COMPRESS-565 : add a new AlwaysWithCompatibility in Zip64Mode
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-836027490
> I had another look and I don't think you've addressed my comment from #169 (comment)
I must have missed that. Sorry about that.
I will fix this today and push a commit in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] PeterAlfredLee commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-836017333
> Surely we can come up with a better name than a "food item" (comestible) or is that term used in the zip spec itself?
That was a typo and I'm using `Compatibility` now. But I forgot to change the title of this PR, my bad.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] missingdays commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
missingdays commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-790675280
> I'm sorry but the master branch isn't quite ready for a new release
Ok, but if I build the jar from this branch, is this ok to use in release?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] PeterAlfredLee commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-835802713
I'm thinking about merge this PR.
Are there anything to be modified about this PR? @bodewig
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] PeterAlfredLee edited a comment on pull request #169: COMPRESS-565 : add a new AlwaysWithCompatibility in Zip64Mode
Posted by GitBox <gi...@apache.org>.
PeterAlfredLee edited a comment on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-836140258
The Github Actions is weird - I can successfully compile and test the class `Zip64SupportIT`, but Actions is reporting compilation failure, which is caused by the missing of `FileOutputStream` and `FileInputStream`. These 2 classes are already imported but Actions seems missed them.
Maybe this is a bug of Github Actions?
P.S. The build is successfully finished in my [forked repository](https://github.com/PeterAlfredLee/commons-compress/runs/2542092389). So I believe this is a problem of GH Actions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] bodewig commented on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
bodewig commented on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-788860729
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-compress] coveralls edited a comment on pull request #169: COMPRESS-565 : add a new AlwaysWithComestibles in Zip64Mode
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #169:
URL: https://github.com/apache/commons-compress/pull/169#issuecomment-783198259
[](https://coveralls.io/builds/37516405)
Coverage increased (+0.02%) to 87.384% when pulling **535dd98fbbc5a26066d16a4aae462af0dd5e27b8 on PeterAlfredLee:COMPRESS-565** into **65cbce6c54e11d7633483093e6b7e4d6c92dde64 on apache:master**.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org