On irc dww was talking about #1006238: Add a weight limit for project_release_handler_field_files. As I tested zip last year and it failed, I was curious if it worked today. It didn't...
This doesn't seem a big issue as every project is packed as .tar.gz on drupal.org. But apparantly we are going to add the possibility to download the project as a zip file. Every sane windows user will download a zip and not the tar.gz. Those people will complain cause the installation fails which leads to dissapointed (maybe new?) drupal users.
We have to fix this before drupal gets released, BUT I agree it's not critical.
Here is the error message as a starting point:
UpdaterException: Unable to determine the type of the source directory. in Updater::factory() (line 100 of ...\includes\updater.inc).
Comment | File | Size | Author |
---|---|---|---|
#8 | 1006302-8.patch | 1.31 KB | bfroehle |
#5 | 1006302-5.patch | 522 bytes | bfroehle |
#5 | php-zip-stat-warnings-patch-to-5.3.4.txt | 847 bytes | bfroehle |
Comments
Comment #1
dwwRe component: see #990366: Rename core's "update system" component to "database update system" ... ;)
Yeah, that sucks. We tried to make this work. I can confirm it doesn't work, even with a "properly" packaged .zip file on d.o itself:
http://drupal.org/dww/apachesolr-7.x-1.x-dev.zip
I don't have time to debug this now, but it's a real bug, and we should *really* try to fix this before 7.0, since .zip is hopefully going to be live by then.
Comment #2
aspilicious CreditAttribution: aspilicious commentedThis is the problem funtion:
Solution:
Replace
$archiver->extract($directory);
with
$archiver->extract($directory, $files);
Zip archiver need the array of files, tar archiver don't :)
There are some flickering errors but it installs ok!
Comment #3
aspilicious CreditAttribution: aspilicious commented*ignore this*
Comment #4
aspilicious CreditAttribution: aspilicious commentedI got the following warnings. http://awesomescreenshot.com/08552aef1
*I think* the problem is that the files have forward slashes in it, that's something the stat function doesn't like.
But I don't understand the "no active batch".
Comment #5
bfroehle CreditAttribution: bfroehle commentedHere's a patch which should address the original issue. The idea for fixing it was copied from the similar treatment in the tar implementation.
I spent a good amount of time tracking down the warnings which appear in #4. The backtrace for each warning shows that it is coming from ZipArchive::extractTo. In each case, stat is being called to determine if a directory exists. (In each case, the directory does not exist). What I think is happening is that the PHP Zip library is failing to set STREAM_URL_STAT_QUIET internally. I think the attached patch for PHP 5.3.4 (!!) would probably fix the problem. I don't have time to test it at the moment. I'll file a bug report with PHP if it turns out we think this is the cause of the problem.
Edit: PHP bug report filed at http://bugs.php.net/bug.php?id=53603.
Comment #6
aspilicious CreditAttribution: aspilicious commentedAs you can see I was wrong, it doesn't need $files :) (looked at some examples)
==> Learned a lot :)
Thnx for looking into this!
Comment #7
aspilicious CreditAttribution: aspilicious commentedThose php dudes answered:
Comment #8
bfroehle CreditAttribution: bfroehle commentedThis is #5 + fixing url_stat to not emit messages when a file doesn't exist.
Comment #10
hass CreditAttribution: hass commented+
Comment #11
bfroehle CreditAttribution: bfroehle commentedThe patch in #8 allows me to install from zip files and doesn't flash a screen of error messages. Anybody else care to comment?
Comment #12
aspilicious CreditAttribution: aspilicious commentedI installed 10 modules with this patch, works perfect, only affects zip archive.
Same code as tar archive which is working perfectly and this code can't break anything.
More details for webchick:
-------------------------
- Without this patch the installation of zip archived modules stops somewhere in the middle.
- This is caused by and empty file array that we are passing to the extracter
- Patch copies the workflow of the tar extracter.
- The other change fixes url_stat to not emit messages when a file doesn't exist. This has to be done due to a phpbug.
(see bug report http://bugs.php.net/bug.php?id=53603)
Result: working installer + no anoying warnings/errors
Comment #13
webchickGreat work on this, folks.
Though this could ideally use dww's sign-off, this is a pretty nasty bug. I think it's better to fix this before release and then go back to clean it up if necessary. aspilicious has done ample manual testing to ensure it's working properly.
Committed to HEAD. Thanks!
Comment #14
webchickOh and an EXTRA bfroehle++ for fixing upstream PHP bugs as part of Drupal patches! :D Awesome!!
Comment #15
dwwThanks for fixing this, everyone! Sorry I haven't been able to comment. Not only have I been slammed with tons of work, and a nasty sore throat, the net where I'm currently staying utterly sucks and I can only seem to be online for about 10 minutes at a time... :( But yeah, patch looks fine, and yes, way to go finding and patching upstream PHP bugs while we're at it. ;)
Cheers,
-Derek