r-lib / zip

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

Handle long file names on Windows #63

Closed gaborcsardi closed 3 years ago

gaborcsardi commented 4 years ago
i Packaging devtools 2.3.1.9000                                            
x Failed to uncompress devtools 2.3.1.9000 258ms                           
(############ ) | [BLD]  76/77 | 1 | [INS]  76/77     | building devtools
Error: Failed to uncompress 'devtools' from 'C:\\Users\\csard\\AppData\\Local\\Temp\\Rtmp8gyfqS\\filee5685c3a14d3/src/contrib/devtools_2.3.1.9000_df619ce29b722f34e5d5f83e336881f12ab3ed05.tar.gz-tree'.

Standard output:
O> 

Standard error:
E> zip error: `Cannot extract file `r-lib-devtools-df619ce/tests/testthat/shallowRepo/objects/pack/pack-c4e0f1d1d68408f260cbbf0a533ad5f6bfd5524e.idx`` in file `zip.c:211`

See `.Last.error.trace` for a stack trace.

> .Last.error.trace

 Stack trace:

 1. pp$install()
 2. pkgdepends:::install_package_plan(plan, lib = private$library,  ...
 3. base:::withCallingHandlers({ ...
 4. pkgdepends:::handle_events(state, events)
 5. pkgdepends:::handle_event(state, i)
 6. pkgdepends:::stop_task(state, worker)
 7. pkgdepends:::stop_task_package(state, worker)
 8. pkgdepends:::stop_task_package_uncompress(state, worker)
 9. base:::throw(new_pkg_uncompress_error("Failed to uncompress {pkg} from {state$plan$file[[pk ...

 x Failed to uncompress 'devtools' from 'C:\\Users\\csard\\AppData\\Local\\Temp\\Rtmp8gyfqS\\filee5685c3a14d3/src/contrib/devtools_2.3.1.9000_df619ce29b722f34e5d5f83e336881f12ab3ed05.tar.gz-tree'. 

Possibly something in the zip package?

jimhester commented 4 years ago

Looking at the error on stderr gives a bit more information. My guess is file path limitations

> .Last.error$parent$data$stderr
[1] "zip error: `Cannot extract file `r-lib-devtools-df619ce/tests/testthat/shallowRepo/objects/pack/pack-c4e0f1d1d68408f260cbbf0a533ad5f6bfd5524e.idx`` in file `zip.c:211`"
jimhester commented 4 years ago

Hmm maybe not the length, that fie path is only 112 characters long...

jimhester commented 4 years ago

We could consider downloading the tarball instead of the zip file I guess, we switched to that in remotes a while ago, though there were some issues when people have weird tar versions on their PATH.

jimhester commented 4 years ago

The error seems to be opening the filehandle (https://github.com/r-lib/zip/blob/6efe5c6c40a7aed8e849bdb511726512fb2b7cda/src/zip.c#L203-L211), so I think it must be some sort of file path issue, not a data extraction issue from the archive.

jimhester commented 4 years ago

Is this supposed to be a zip file or a gzip file? I ask because the error message is coming from zip, but the filename is /devtools_2.3.1.9000_df619ce29b722f34e5d5f83e336881f12ab3ed05.tar.gz-tree...

gaborcsardi commented 4 years ago

The .tar.gz is the file we should build, from -tree, which is an extracted directory I think.

gaborcsardi commented 4 years ago

So I think the error message is from zip trying to extract the downloaded package into that directory.

jimhester commented 4 years ago

Yeah, I can reproduce it now, it is just related to the length of the file we are trying to extract, e.g. output directory + path within the zip. With a shorter output directory everything is good, but once it gets too long we get this error.

d <- tempfile()
dir.create(d)
out_file <- file.path(d, "master.zip")

download.file("https://github.com/r-lib/devtools/archive/master.zip", out_file)
out_dir <- file.path(tempdir(), paste0(collapse = "", rep("x", 50)))
dir.create(out_dir)
nchar(out_dir)
#> [1] 98
zip::unzip(out_file, exdir = out_dir)

out_dir <- file.path(tempdir(), paste0(collapse = "", rep("x", 100)))
dir.create(out_dir)
nchar(out_dir)
#> [1] 148
zip::unzip(out_file, exdir = out_dir)
#> Error in zip::unzip(out_file, exdir = out_dir): zip error: `Cannot extract file `devtools-master/tests/testthat/shallowRepo/objects/pack/pack-c4e0f1d1d68408f260cbbf0a533ad5f6bfd5524e.pack`` in file `zip.c:211`

Created on 2020-08-31 by the reprex package (v0.3.0)

jimhester commented 4 years ago

I think we need to add a "\\?\ prefix to the buffer used in wfopen in https://github.com/r-lib/zip/blob/6efe5c6c40a7aed8e849bdb511726512fb2b7cda/src/zip.c#L203

gaborcsardi commented 4 years ago

Cool, yeah! We can just move this to zip then. I think it'll need this fix at a couple of places, for every wfopen() basically.

I'll move it over there.

jimhester commented 3 years ago

This should be fixed by https://github.com/r-lib/zip/pull/64 BTW, I seem not to have tagged it in that PR.

gaborcsardi commented 3 years ago

zip still has an issue with long path names:

> zip::unzip("thematic_0.1.1.9000_03b8c7b8e762a3a21c6aacbe71ea183433240182.tar$
Error in zip::unzip("thematic_0.1.1.9000_03b8c7b8e762a3a21c6aacbe71ea183433240182.tar.gz-tree",  :
  zip error: `Cannot extract entry `rstudio-thematic-03b8c7b/tests/testthat/auto_theme_rmd/darkly_rmd/tests/shinytest/mytest-expected-linux/` from archive `C:\Users\csard\AppData\Local\Temp\Rtmp67xw4E\file7f302f93a24\src\contrib\thematic_0.1.1.9000_03b8c7b8e762a3a21c6aacbe71ea183433240182.tar.gz-tree`` in file `zip.c:183`

Line 183 is for zip_mkdirp: https://github.com/r-lib/zip/blob/7646fb5d18a312a85846d225afa4d6aa70738905/src/zip.c#L179