The current implementation of SkipDotsRecursiveDirectoryIterator doesn't work on systems where dotfiles are included in the list, and the first entry is a dotfile. Next is only called when iterating to the next element, rewind is triggered to reset the iterator to the first element.

I triggered this bug on a php 5.4.11 system.

The fix is simple, skip the dot files either on next or on rewind calls.

Files: 
CommentFileSizeAuthor
#10 SkipDotsRecursiveDirectoryIterator-skipping-all-dot-files-1915088-2.patch662 bytesbfroehle
PASSED: [[SimpleTest]]: [MySQL] 39,957 pass(es).
[ View ]
#2 SkipDotsRecursiveDirectoryIterator-skipping-all-dot-files-1915088.patch597 bytesopenminds
PASSED: [[SimpleTest]]: [MySQL] 39,760 pass(es).
[ View ]

Comments

Version:7.9» 7.19

StatusFileSize
new597 bytes
PASSED: [[SimpleTest]]: [MySQL] 39,760 pass(es).
[ View ]

attached patch

Version:7.19» 7.x-dev
Status:Active» Needs review

I assume you meant that it needs review.

I don't know - it works for me, so it should somehow work its way through all the drupal-how-to-get-a-patch-in-core...

Okay yes. This looks good. Do we need to advance in __construct() too? Or does that call rewind?

According to the PHP documentation, rewind() is automatically called when an Iterator instance is being iterated, as demonstrated in this example. Combined to the patch proposed, this behavior would guarantee that dots are always skipped while iterating.

Status:Needs review» Needs work

@Krizalys: Yes, but I just read through the PHP source and their implementation (at the C level) would end up calling the equivalent of our skipdots in __construct, next, and rewind.

It's a little pedantic, but I think we should also call skipdots in __construct. This would fix the result of creating an instance and immediately calling current.

Just applied this to a 7.20 as well (un PHP 5.4.11) - confirmed it fixed my modules-upgrade issue

Priority:Normal» Major

I'm raising the priority of this issue since Drupal 7 will face the same bug (not being able to upgrade some modules, among other things, will be one consequence) in PHP 5.3.23, the next version to be released. Ideally, this patch should be applied to Drupal 7.21.

Status:Needs work» Needs review
StatusFileSize
new662 bytes
PASSED: [[SimpleTest]]: [MySQL] 39,957 pass(es).
[ View ]

Thank you for the patch it worked perfectly and kept me from going bald by pulling my hair out.

Issue tags:+Needs tests

Awesome, thanks!

Since this is not needed for D8 the patch is probably good to go. Is there a good way to catch the problem in an automated test?

Status:Needs review» Reviewed & tested by the community

The patch in comment #10 is good. RTBC-ing.

I applied the patch in comment #10, and it didn't resolve the module update problem in a clean install (Drupal 7.21 and PHP 5.4.11). I'd be happy try any other proposed solutions or alternate patches, and report the results.

Are you 110% sure that you did the test correctly - cleared cache and all that before testing? What exactly was the error message you got?

Status:Needs review» Reviewed & tested by the community
Issue tags:-Needs manual testing

Yes, I verified that the patch changes were applied to /includes/filetransfer/filetransfer.inc, and cleared the cache. The error message is the type listed at that issue I linked to. For instance, for the Token module:

    Error installing / updating
    File Transfer failed, reason: Cannot remove directory /sites/all/modules/token\..

OK, I don't have any other suggestions.

PS! Why is the slash backward in stead of forward. On what OS are you testing.

Windows 7. I haven't examined the Drupal code in detail, but presumably Drupal uses the standard "/" in the directory path, and when it seeks the parent directory, Windows returns "\..".

Downgrading the version of PHP might be a final option, but I would much rather avoid that.

Status:Reviewed & tested by the community» Needs review
Issue tags:+Needs manual testing

It sounds like this needs more manual testing, especially if we can't write automated tests for it (and if there are no existing ones)? Especially to make sure it doesn't cause any regressions in the module install/update functionality.

The code looks pretty reasonable though.

So basically, if people can report that they've tested actual Drupal functionality that uses this code, and make sure it continues to work correctly, we should be able to get it committed.

#10 is working for me too. D 7.21 installed.

@yazzy008 (and others who commented earlier): It would be useful if you could summarize in more detail what steps you actually took to test the patch :) Thanks!

I encountered an error running module updates from within Drupal.

This comment summarises the iterations I went through to fix the issue: http://drupal.org/node/1900580#comment-7209694

Essentially, I'm running PHP 5.4 and Drupal 7.21; the patch referred to in #10 resolved the issue I was experiencing with module updates. I have not tested it any more than that.

Status:Reviewed & tested by the community» Needs review
Issue tags:+Needs manual testing

I repeated the module update process, and it worked this time. It is baffling that different results occurred even though no changes were made to the files. The only apparent change from yesterday vs. today is the system reboot (this morning), which of course means Apache was restarted; but I can't imagine how that would affect the results. Anyway, the patch seems fine. Thank you to everyone who worked on it.

I ran into this bug today and applying the patch at #10 fixed that module update error on 7.21. I did not try downgrading PHP (currently 5.4). Thanks a lot!

Ran into the module update problem on a fresh install of Drupal 7.21 (PHP 5.4), and #10 fixed my issue - thanks much! Will report back if I have any further issues once I configure the site.

Patch at #10 also fixed the problem for me (drupal 7.19 + PHP 5.3.23)

Patch at #10 worked for me Drupal 7.21 + PHP 5.3.23 on a Linux server

Status:Needs review» Reviewed & tested by the community
Issue tags:-Needs manual testing

I think we have gotten enough manually tests now. The only negative report was positive on the second try.

Status:Reviewed & tested by the community» Fixed
Issue tags:+7.22 release blocker

Thanks for the extra testing, and the patch!

Committed to 7.x. http://drupalcode.org/project/drupal.git/commit/d9cf661

This issue still has the "needs tests" tag... if anyone can figure out how to write automated tests for it, it would be great to reopen to do so.

Oops, wrong issue tag.

This patch fixed the update problem for me. Xammp on Windows. Drupal 7.21 PHP 5.4.7

Automatically closed -- issue fixed for 2 weeks with no activity.

Hello,

how can I apply this patch? I have the issue "File Transfer failed, reason: Cannot remove directory".

Best regards,

Toni