Patchwork subrepo: append subrepo path to subrepo RepoError and urllib2.HTTPError messages

login
register
mail settings
Submitter Angel Ezquerra
Date March 20, 2013, 10:53 p.m.
Message ID <e0463027f57ec460d3f8.1363819986@Angel-PC.localdomain>
Download mbox | patch
Permalink /patch/1145/
State Deferred, archived
Headers show

Comments

Angel Ezquerra - March 20, 2013, 10:53 p.m.
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1363819941 -3600
#      Wed Mar 20 23:52:21 2013 +0100
# Node ID e0463027f57ec460d3f8784044954e6e0e0fdec8
# Parent  84f24bbe0c3e97e4e1753c894e3c963d1e6c6d63
subrepo: append subrepo path to subrepo RepoError and urllib2.HTTPError messages

These errors can happen when a local or remote subrepo source is missing.
I wonder if we should not intercept all exceptions generated withint subrepos,
rather than just intercepting some of them (currently Abort, RepoError and
HTTPError).
Matt Harbison - March 29, 2013, 2:56 a.m.
On Wed, 20 Mar 2013 23:53:06 +0100, Angel Ezquerra wrote:

> # HG changeset patch # User Angel Ezquerra <angel.ezquerra@gmail.com>
> # Date 1363819941 -3600 #      Wed Mar 20 23:52:21 2013 +0100 # Node ID
> e0463027f57ec460d3f8784044954e6e0e0fdec8 # Parent 
> 84f24bbe0c3e97e4e1753c894e3c963d1e6c6d63 subrepo: append subrepo path to
> subrepo RepoError and urllib2.HTTPError messages
> 
> These errors can happen when a local or remote subrepo source is
> missing.
> I wonder if we should not intercept all exceptions generated withint
> subrepos, rather than just intercepting some of them (currently Abort,
> RepoError and HTTPError).
> 
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py ---
> a/mercurial/subrepo.py +++ b/mercurial/subrepo.py @@ -6,7 +6,7 @@
>  # GNU General Public License version 2 or any later version.
>  
>  import errno, os, re, xml.dom.minidom, shutil, posixpath
> -import stat, subprocess, tarfile +import stat, subprocess, tarfile,
> urllib2
>  from i18n import _
>  import config, scmutil, util, node, error, cmdutil, bookmarks, match as
>  matchmod hg = None
> @@ -27,11 +27,15 @@
>          except SubrepoAbort, ex:
>              # This exception has already been handled raise ex
> -        except error.Abort, ex:
> +        except (error.Abort, error.RepoError, urllib2.HTTPError), ex:
>              subrepo = subrelpath(self)
>              errormsg = str(ex) + ' ' + _('(in subrepo %s)') % subrepo
> +            if hasattr(ex, 'hint'):
> +                hint = ex.hint
> +            else:
> +                hint = None
>              # avoid handling this exception by raising a SubrepoAbort
>              exception
> -            raise SubrepoAbort(errormsg, hint=ex.hint, subrepo=subrepo)
> +            raise SubrepoAbort(errormsg, hint=hint, subrepo=subrepo)
>          return res
>      return decoratedmethod

Won't this have unexpected side effects because e.g. a RepoError is 
essentially being turned into an Abort, which will then sail past the 
various except clauses for RepoError and HTTPError once rethrown?  I can't 
point to a specific failure now, just observing that these errors are 
caught and acted upon.

One SubRepo subclass per exception is obviously overkill, so is it 
possible with some python magic to alter the superclass to match what was 
caught before rethrowing?  (and without breaking the except SubrepoAbort 
clause)  A quick google search doesn't yield anything promising, but maybe 
there's an alternate way to effect the same thing?

(I didn't forget about the other subrepo stuff you submitted, I've just 
been too busy the past few weeks.)

--Matt

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -6,7 +6,7 @@ 
 # GNU General Public License version 2 or any later version.
 
 import errno, os, re, xml.dom.minidom, shutil, posixpath
-import stat, subprocess, tarfile
+import stat, subprocess, tarfile, urllib2
 from i18n import _
 import config, scmutil, util, node, error, cmdutil, bookmarks, match as matchmod
 hg = None
@@ -27,11 +27,15 @@ 
         except SubrepoAbort, ex:
             # This exception has already been handled
             raise ex
-        except error.Abort, ex:
+        except (error.Abort, error.RepoError, urllib2.HTTPError), ex:
             subrepo = subrelpath(self)
             errormsg = str(ex) + ' ' + _('(in subrepo %s)') % subrepo
+            if hasattr(ex, 'hint'):
+                hint = ex.hint
+            else:
+                hint = None
             # avoid handling this exception by raising a SubrepoAbort exception
-            raise SubrepoAbort(errormsg, hint=ex.hint, subrepo=subrepo)
+            raise SubrepoAbort(errormsg, hint=hint, subrepo=subrepo)
         return res
     return decoratedmethod