search

r-lib / zip

Platform independent zip compression via miniz
https://r-lib.github.io/zip/
Other
83 stars 19 forks source link

Incorrect behavior when adding files with cherry pick. Base directory is present when it should not. #94

Closed stefanoborini closed 1 year ago

stefanoborini commented 1 year ago

Imagine having a directory xxx, containing files foo and bar.

invoking

setwd("xxx")
zip::zip("../out.zip", ".", mode="cherry-pick", include_directories = FALSE)

is supposed to produce a file containing the files in the base of the zip file, but this is not the case. The xxx prefix is left there.

unzip -t out.zip 
Archive:  out.zip
    testing: xxx/bar                  OK
    testing: xxx/foo                  OK
No errors detected in compressed data of out.zip.

This means that the above and the following exhibit the same behaviour, when they are not supposed to:

> zip::zip("../foo.zip", "xxx", mode="cherry-pick", include_directories = FALSE)
> zip::zip("../foo.zip", ".", mode="cherry-pick", include_directories = FALSE, root = "xxx")

The only way to create these packages is to list the files one by one, or to take and hide the warning by not using cherry pick mode. Still, the resulting file contains ./ as a prefix.

Archive:  out.zip
    testing: ./bar                    OK
    testing: ./foo                    OK

The expected behavior is that this one

> zip::zip("../foo.zip", ".", mode="cherry-pick", include_directories = FALSE, root = "xxx")

produces an archive with foo and bar, exactly like zip -r behaves from the command line

$ zip -r ../out.zip .
  adding: foo (stored 0%)
  adding: bar (stored 0%)
$ unzip -t ../out.zip 
Archive:  ../out.zip
    testing: foo                      OK
    testing: bar                      OK
No errors detected in compressed data of ../out.zip.
gaborcsardi commented 1 year ago

Seems like command line zip treats . specially, and leaves the actual . directory out of the archive. This is a workaround to get that behaviour:

zip::zip(
  "../out.zip", 
  dir(all.files=TRUE, no.. = TRUE), 
  mode="cherry-pick", 
  include_directories = FALSE
)
zip_list("../out.zip")$filename
#> [1] "bar" "foo"

It would be reasonable to change the meaning of ., at least for mode = "cherry-pick", although strictly speaking it would be a breaking change for R.

gaborcsardi commented 1 year ago

I'll need to test a bit if this breaks anything (unlikely imo), and if not then we can keep it.

gaborcsardi commented 1 year ago

Seems like it'll be OK.