| Submitter | Mathieu Cantin |
|---|---|
| Date | Feb. 18, 2014, 11:08 p.m. |
| Message ID | <e6277671f69e4f3e9e6be90a1a1a9308@VSVR-MX-STORE04.corp.coveo.com> |
| Download | mbox | patch |
| Permalink | /patch/3701/ |
| State | Deferred |
| Headers | show |
Comments
On Tue, 2014-02-18 at 23:08 +0000, Mathieu Cantin wrote: > # HG changeset patch > # User Mathieu Cantin <mcantin@coveo.com> > # Date 1392764497 18000 > # Tue Feb 18 18:01:37 2014 -0500 > # Node ID bf485635a586a487a732ba27e122531235062da7 > # Parent 0e2877f8605dcaf4fdf2ab7e0046f1f6f80161dd > purge: delete invalid junction point > > When you have 2 folders linked to each other by a junction point. For exemple > "A" is a folder and "B" is a junction point to "A". "A" is deleted, then > os.listdir throws an error on "B" because the target of "B" doesn't exist > anymore. Does this issue only exist with purge? > + def hasfilesindir(path): > + try: > + return len(os.listdir(path)) > 0 > + except OSError: > + # It's a junction point with an invalid target Is this really the only way os.listdir can get an OSError?
>Does this issue only exist with purge? Yes, I think. Our company has been using the fix for 1 or 2 weeks and I don't have others problems with junction points, but I always purge all repositories before pulling or updating (in the context of continuous integration). >Is this really the only way os.listdir can get an OSError? Yes, the only one other solution would be to call the win32api in order to know if the directory is a junction. Mathieu
On Wed, 2014-02-19 at 22:43 +0000, Mathieu Cantin wrote: > >Does this issue only exist with purge? > Yes, I think. Our company has been using the fix for 1 or 2 weeks and > I don't have others problems with junction points, but I always purge > all repositories before pulling or updating (in the context of > continuous integration). My point is purge isn't doing anything that doesn't happen elsewhere in Mercurial (deleting files and directories), so this bug is almost certainly more generic and thus wants a less specific fix. > >Is this really the only way os.listdir can get an OSError? > Yes, the only one other solution would be to call the win32api in order to know if the directory is a junction. There are actually lots of ways listdir can get an OSError that have nothing to do with junctions.. and should probably continue to stop Mercurial with an error. Your patch breaks all those cases because it doesn't try to distinguish between types of error.
Patch
diff -r 0e2877f8605d -r bf485635a586 hgext/purge.py --- a/hgext/purge.py Sat Feb 15 22:09:32 2014 -0600 +++ b/hgext/purge.py Tue Feb 18 18:01:37 2014 -0500 @@ -95,6 +95,13 @@ os.chmod(path, stat.S_IMODE(s.st_mode) | stat.S_IWRITE) os.remove(path) + def hasfilesindir(path): + try: + return len(os.listdir(path)) > 0 + except OSError: + # It's a junction point with an invalid target + return False + directories = [] match = scmutil.match(repo[None], dirs, opts) match.explicitdir = match.traversedir = directories.append @@ -105,6 +112,6 @@ remove(removefile, f) for f in sorted(directories, reverse=True): - if match(f) and not os.listdir(repo.wjoin(f)): + if match(f) and not hasfilesindir(repo.wjoin(f)): ui.note(_('removing directory %s\n') % f) - remove(os.rmdir, f) + remove(os.rmdir, f)