[PATCH] subrepo: better error messages in _ensuregit

Mason Malone mason.malone at gmail.com
Sun Jan 17 22:01:01 CST 2016


On Sun, 17 Jan 2016 21:00:28 -0600
Matt Mackall <mpm at selenic.com> wrote:

> This patch looks ok, but unfortunately you've arrived on the evening of the 3.7
> code freeze so this might have to wait until February.

No problem! 

> Users are extremely prone to turn off their reading comprehension faculty if
> presented with a multi-line error. Even getting them to read two lines like the
> above is often a stretch.

That makes sense. Here's a revised patch that uses terser messages and the "hint" argument (and fixes the standards violation with "generic_error"):

# HG changeset patch
# User Mason Malone <mason.malone at gmail.com>
# Date 1453089237 18000
#      Sun Jan 17 22:53:57 2016 -0500
# Node ID 8d4adcb40b3dd06775123bcaabe113221f553358
# Parent  27f2f5c1d499ae7af51af88a1a8e618fb64c4ab4
subrepo: better error messages in _ensuregit

This patch improves the error messages raised when an OSError occurs, since
simply re-raising the exception can be both confusing and misleading. For
example, if "hg identify" is run inside a repository that contains a Git
subrepository and the git binary could not be found, it'll exit with the message
"abort: No such file or directory". That implies "identify" has a problem
reading the repository itself. There's no way for the user to know what the
real problem is unless they dive into the Mercurial source, which is what I
ended up doing after spending hours debugging errors while provisioning a VM
with Ansible (turns out I forgot to install Git on it).

Descriptive errors are especially important on Windows, since it's common for
Windows users to forget to set the "Path" system variable after installing Git.

diff -r 27f2f5c1d499 -r 8d4adcb40b3d mercurial/subrepo.py
--- a/mercurial/subrepo.py	Sun Jan 17 19:29:27 2016 +0100
+++ b/mercurial/subrepo.py	Sun Jan 17 22:53:57 2016 -0500
@@ -1294,10 +1294,25 @@
             self._gitexecutable = 'git'
             out, err = self._gitnodir(['--version'])
         except OSError as e:
-            if e.errno != 2 or os.name != 'nt':
-                raise
-            self._gitexecutable = 'git.cmd'
-            out, err = self._gitnodir(['--version'])
+            genericerror = _("error executing git for subrepo '%s': %s")
+            notfoundhint = _("check git is installed and in your PATH")
+            if e.errno != errno.ENOENT:
+                raise error.Abort(genericerror % (self._path, e.strerror))
+            elif os.name == 'nt':
+                try:
+                    self._gitexecutable = 'git.cmd'
+                    out, err = self._gitnodir(['--version'])
+                except OSError as e2:
+                    if e2.errno == errno.ENOENT:
+                        raise error.Abort(_("couldn't find 'git' or 'git.cmd'"
+                            " for subrepo '%s'") % self._path,
+                            hint=notfoundhint)
+                    else:
+                        raise error.Abort(genericerror % (self._path,
+                            e2.strerror))
+            else:
+                raise error.Abort(_("couldn't find git for subrepo '%s'")
+                    % self._path, hint=notfoundhint)
         versionstatus = self._checkversion(out)
         if versionstatus == 'unknown':
             self.ui.warn(_('cannot retrieve git version\n'))


More information about the Mercurial-devel mailing list