566950 - test-file.testRmdirNonempty fails on Windows

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[:Felipe Gomes (needinfo for replies!)]

I get the following error:

error: TEST FAILED: test-file.testRmdirNonempty (failure)
error: fail: rmdir on non-empty directory should raise error ("Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsILocalFile.remove]" doesn't match /^The directory is not empty: .+$/)


It looks like nsILocalFile doesn't raise the right error on Windows? NS_ERROR_FAILURE instead of NS_ERROR_FILE_DIR_NOT_EMPTY

Comment 1

[Drew Willcoxon :adw]

Attachment: patch

Grr, thanks Felipe.  Could you try this patch?

Comment 2

[:Felipe Gomes (needinfo for replies!)]

It works!

Comment 3

[Drew Willcoxon :adw]

Comment on attachment 446306 [details] [diff] [review]
patch

Probably it would be safest for rmdir to ensure that the directory is actually empty rather than relying on Gecko to throw any exception at all.

But, nsIFile.remove's comment says "The 'recursive' flag must be PR_TRUE to delete directories which are not empty" (it's false) [1], and I checked Gecko's various file implementations.  The OS/2, OS X, and Unix implementations all use rmdir [2].  On POSIX systems it's an error to call rmdir on a nonempty directory.  The Windows implementation [3] calls RemoveDirectoryW, which removes an empty directory [4].

What do you think Atul?

[1]: http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsIFile.idl#199
[2]: nsLocalFile*.cpp at http://mxr.mozilla.org/mozilla-central/source/xpcom/io/
[3]: http://mxr.mozilla.org/mozilla-central/source/xpcom/io/nsLocalFileWin.cpp#1805
[4]: http://msdn.microsoft.com/en-us/library/aa365488%28VS.85%29.aspx

Comment 4

[Atul Varma [:atul]]

Comment on attachment 446306 [details] [diff] [review]
patch

Uh, we have an OS/2 implementation of nsIFile?

This looks fine to me. Only nit is that you might want to add a blurb within the catch clause that points to this bug for details on why we're not catching a specific exception.

Thanks!

Comment 5

[Drew Willcoxon :adw]

Attachment: patch v2

Thanks Atul.  This adds the blurb.

Comment 6

[Drew Willcoxon :adw]

Myk, requesting permission to land, and also to block 0.4 final release on this.  Without this fix, which is very small, the file unit test fails on Windows (7).

Comment 7

[Myk Melez [:myk] [@mykmelez]]

a=myk, as that test should pass on Windows, and this is a good, low-risk fix, although I wouldn't absolutely block on it.

Comment 8

[Drew Willcoxon :adw]

http://hg.mozilla.org/labs/jetpack-sdk/rev/c9feffc46e02

Comment 9

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".