r-lib / archive

R bindings to libarchive, supporting a large variety of archive formats
https://archive.r-lib.org/
Other
145 stars 17 forks source link

Fix archive_write_files on Windows (add zip extract test) #80

Closed cielavenir closed 8 months ago

cielavenir commented 1 year ago

This temporal pull request simply adds zip version of "can strip components if desired" test, but it raises "4821 in central directory, 4972 in local header" on Windows. It is not related to password, so super-strange.

Could you take a look? @gaborcsardi

cielavenir commented 1 year ago

calling archive_write_close is not sufficient

gaborcsardi commented 1 year ago

@cielavenir Thanks, I can take a look later today.

cielavenir commented 1 year ago

The thing is, is my library usage is wrong or is there potential problem on this library?

cielavenir commented 1 year ago

Hello @gaborcsardi , it turned out that open() argument requires O_BINARY on Windows. I have not checked in detail but it is possible that on Windows archive_write_files would not be working from the beginning.

I added integrity check to avoid this kind of accident further.

cielavenir commented 1 year ago

Simply adding integrity check to tar failed as https://github.com/r-lib/archive/actions/runs/4074516161 This means tar content was corrupted.

cielavenir commented 1 year ago

now it is ready.

cielavenir commented 1 year ago

hi, is cf0b568 good or still bad

gaborcsardi commented 1 year ago

I think it is good.

cielavenir commented 1 year ago

I don't think there are remaining issue here

cielavenir commented 1 year ago

are there still anything interfering merging?

cielavenir commented 1 year ago

This one should be ready, could you merge or are there any issues?

cielavenir commented 8 months ago

I'm tired, converted to issue https://github.com/r-lib/archive/issues/98

cielavenir commented 8 months ago

alternatively we can use #ifndef O_BINARY