Use OS.File to remove directories recursively

RESOLVED FIXED in mozilla28

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mossop, Assigned: mossop)

Tracking

unspecified
mozilla28
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Now the dependant bugs are fixed we can remove a bunch of unnecessary code.
(Assignee)

Comment 1

4 years ago
Created attachment 827709 [details] [diff] [review]
patch rev 1

Having different OS.File methods depending on whether we're removing a directory is a little annoying and I'd rather test and call the right one rather than assume since if this goes wrong all sorts of things break.
Attachment #827709 - Flags: review?(bmcbride)
Comment on attachment 827709 [details] [diff] [review]
patch rev 1

Review of attachment 827709 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +1387,2 @@
>      }
>      catch (e if e instanceof OS.File.Error && e.becauseNoSuchFile) {

Don't need this try/catch anymore either, as both methods support an ignoreAbsent flag.

AFAIK that flag is meant to default to true, but it currently defaults to false for removeDir() - filed bug 935792.
Attachment #827709 - Flags: review?(bmcbride) → review-
(Assignee)

Comment 3

4 years ago
Comment on attachment 827709 [details] [diff] [review]
patch rev 1

(In reply to Blair McBride [:Unfocused] from comment #2)
> Comment on attachment 827709 [details] [diff] [review]
> patch rev 1
> 
> Review of attachment 827709 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/mozapps/extensions/XPIProvider.jsm
> @@ +1387,2 @@
> >      }
> >      catch (e if e instanceof OS.File.Error && e.becauseNoSuchFile) {
> 
> Don't need this try/catch anymore either, as both methods support an
> ignoreAbsent flag.
> 
> AFAIK that flag is meant to default to true, but it currently defaults to
> false for removeDir() - filed bug 935792.

It's still needed for the OS.File.stat call I can pull the remove calls outside of the try...catch block and set ignoreAbsent on them if you want though
Attachment #827709 - Flags: review- → review?(bmcbride)
Comment on attachment 827709 [details] [diff] [review]
patch rev 1

Review of attachment 827709 [details] [diff] [review]:
-----------------------------------------------------------------

Oh, duh.
Attachment #827709 - Flags: review?(bmcbride) → review+
Status: NEW → ASSIGNED
(Assignee)

Comment 5

4 years ago
https://hg.mozilla.org/integration/fx-team/rev/7cefaf69392d
https://hg.mozilla.org/mozilla-central/rev/7cefaf69392d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28

Updated

4 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.