Patchwork D9355: errors: raise ConfigError on failure to parse config file

login
register
mail settings
Submitter phabricator
Date Nov. 21, 2020, 12:29 a.m.
Message ID <differential-rev-PHID-DREV-vf2sbxepl3jisd4dgicu-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/47633/
State Superseded
Headers show

Comments

phabricator - Nov. 21, 2020, 12:29 a.m.
martinvonz created this revision.
Herald added a reviewer: durin42.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  This replaces two raises of `ParseError` by `ConfigError`, which makes
  it so we get the desired exit code when `ui.detailed-exit-code` is
  enabled. Because the exceptions include a location, I had to add that
  to `ConfigError` as well. I considered making `ConfigError` a subclass
  of `ParseError`, but it doesn't feel like it quite passes the "is-a"
  test.
  
  I used "config error: " as prefix for these errors instead of the
  previous "hg: parse error: ", which seems a little less accurate now
  (and, as I've said before, I don't know what the "hg: " part is
  supposed to signify anyway). I can easily be convinced to change the
  prefix to something else (including "abort: ").

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D9355

AFFECTED FILES
  hgext/eol.py
  mercurial/config.py
  mercurial/error.py
  mercurial/ui.py
  tests/test-add.t
  tests/test-audit-subrepo.t
  tests/test-config.t
  tests/test-convert-git.t
  tests/test-default-push.t
  tests/test-dispatch.t
  tests/test-hgrc.t
  tests/test-histedit-arguments.t
  tests/test-lfs.t
  tests/test-merge1.t
  tests/test-phases.t
  tests/test-repo-compengines.t
  tests/test-trusted.py
  tests/test-trusted.py.out

CHANGE DETAILS




To: martinvonz, durin42, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-trusted.py.out b/tests/test-trusted.py.out
--- a/tests/test-trusted.py.out
+++ b/tests/test-trusted.py.out
@@ -176,7 +176,7 @@ 
 not trusting file .hg/hgrc from untrusted user abc, group def
 ignored .hg/hgrc:1: foo
 # same user, same group
-hg: parse error at .hg/hgrc:1: foo
+config error at .hg/hgrc:1: foo
 
 
 # access typed information
diff --git a/tests/test-trusted.py b/tests/test-trusted.py
--- a/tests/test-trusted.py
+++ b/tests/test-trusted.py
@@ -256,12 +256,12 @@ 
 
 try:
     testui(user=b'abc', group=b'def', silent=True)
-except error.ParseError as inst:
+except error.ConfigError as inst:
     bprint(inst.format())
 
 try:
     testui(debug=True, silent=True)
-except error.ParseError as inst:
+except error.ConfigError as inst:
     bprint(inst.format())
 
 print()
diff --git a/tests/test-repo-compengines.t b/tests/test-repo-compengines.t
--- a/tests/test-repo-compengines.t
+++ b/tests/test-repo-compengines.t
@@ -129,8 +129,8 @@ 
   > revlog.zlib.level=foobar
   > EOF
   $ commitone zlib-level-invalid
-  abort: storage.revlog.zlib.level is not a valid integer ('foobar')
-  abort: storage.revlog.zlib.level is not a valid integer ('foobar')
+  config error: storage.revlog.zlib.level is not a valid integer ('foobar')
+  config error: storage.revlog.zlib.level is not a valid integer ('foobar')
   [30]
 
   $ hg init zlib-level-out-of-range
@@ -186,8 +186,8 @@ 
   > revlog.zstd.level=foobar
   > EOF
   $ commitone zstd-level-invalid
-  abort: storage.revlog.zstd.level is not a valid integer ('foobar')
-  abort: storage.revlog.zstd.level is not a valid integer ('foobar')
+  config error: storage.revlog.zstd.level is not a valid integer ('foobar')
+  config error: storage.revlog.zstd.level is not a valid integer ('foobar')
   [30]
 
   $ hg init zstd-level-out-of-range --config format.revlog-compression=zstd
diff --git a/tests/test-phases.t b/tests/test-phases.t
--- a/tests/test-phases.t
+++ b/tests/test-phases.t
@@ -512,7 +512,7 @@ 
   $ mkcommit I --config phases.new-commit='babar'
   transaction abort!
   rollback completed
-  abort: phases.new-commit: not a valid phase name ('babar')
+  config error: phases.new-commit: not a valid phase name ('babar')
   [30]
 Test phase command
 ===================
diff --git a/tests/test-merge1.t b/tests/test-merge1.t
--- a/tests/test-merge1.t
+++ b/tests/test-merge1.t
@@ -138,7 +138,7 @@ 
 
 bad config
   $ hg merge 1 --config merge.checkunknown=x
-  abort: merge.checkunknown not valid ('x' is none of 'abort', 'ignore', 'warn')
+  config error: merge.checkunknown not valid ('x' is none of 'abort', 'ignore', 'warn')
   [30]
 this merge should fail
   $ hg merge 1 --config merge.checkunknown=abort
diff --git a/tests/test-lfs.t b/tests/test-lfs.t
--- a/tests/test-lfs.t
+++ b/tests/test-lfs.t
@@ -1118,8 +1118,8 @@ 
 
   $ echo x > file.txt
   $ hg ci -Aqm 'should fail'
-  hg: parse error at .hglfs:3: bad file ... no commit
-  [255]
+  config error at .hglfs:3: bad file ... no commit
+  [30]
 
   $ cat > .hglfs << EOF
   > [track]
diff --git a/tests/test-histedit-arguments.t b/tests/test-histedit-arguments.t
--- a/tests/test-histedit-arguments.t
+++ b/tests/test-histedit-arguments.t
@@ -126,7 +126,7 @@ 
 ---------------------------
 
   $ hg histedit --config "histedit.defaultrev="
-  abort: config option histedit.defaultrev can't be empty
+  config error: config option histedit.defaultrev can't be empty
   [30]
 
 Run on a revision not descendants of the initial parent
diff --git a/tests/test-hgrc.t b/tests/test-hgrc.t
--- a/tests/test-hgrc.t
+++ b/tests/test-hgrc.t
@@ -16,7 +16,7 @@ 
 
   $ echo "invalid" > $HGRC
   $ hg version
-  hg: parse error at $TESTTMP/hgrc:1: invalid
+  config error at $TESTTMP/hgrc:1: invalid
   [255]
   $ echo "" > $HGRC
 
@@ -59,7 +59,7 @@ 
 #if unix-permissions no-root
   $ chmod u-r $TESTTMP/included
   $ hg showconfig section
-  hg: parse error at $TESTTMP/hgrc:2: cannot include $TESTTMP/included (Permission denied)
+  config error at $TESTTMP/hgrc:2: cannot include $TESTTMP/included (Permission denied)
   [255]
 #endif
 
@@ -68,7 +68,7 @@ 
   $ echo '[foo]' > $HGRC
   $ echo '  x = y' >> $HGRC
   $ hg version
-  hg: parse error at $TESTTMP/hgrc:2: unexpected leading whitespace:   x = y
+  config error at $TESTTMP/hgrc:2: unexpected leading whitespace:   x = y
   [255]
 
   $ "$PYTHON" -c "from __future__ import print_function; print('[foo]\nbar = a\n b\n c \n  de\n fg \nbaz = bif cb \n')" \
@@ -275,7 +275,7 @@ 
   > EOF
 
   $ hg path
-  hg: parse error at $TESTTMP/.hg/hgrc:3: [broken
+  config error at $TESTTMP/.hg/hgrc:3: [broken
   [255]
   $ HGRCSKIPREPO=1 hg path
   foo = $TESTTMP/bar
@@ -283,7 +283,7 @@ 
 Check that hgweb respect HGRCSKIPREPO=1
 
   $ hg serve -n test -p $HGPORT -d --pid-file=hg.pid -A access.log -E errors.log
-  hg: parse error at $TESTTMP/.hg/hgrc:3: [broken
+  config error at $TESTTMP/.hg/hgrc:3: [broken
   [255]
   $ test -f hg.pid && (cat hg.pid >> $DAEMON_PIDS)
   [1]
@@ -302,7 +302,7 @@ 
 Check that zeroconf respect HGRCSKIPREPO=1
 
   $ hg paths --config extensions.zeroconf=
-  hg: parse error at $TESTTMP/.hg/hgrc:3: [broken
+  config error at $TESTTMP/.hg/hgrc:3: [broken
   [255]
   $ HGRCSKIPREPO=1 hg paths --config extensions.zeroconf=
   foo = $TESTTMP/bar
diff --git a/tests/test-dispatch.t b/tests/test-dispatch.t
--- a/tests/test-dispatch.t
+++ b/tests/test-dispatch.t
@@ -91,8 +91,8 @@ 
   $ mkdir -p badrepo/.hg
   $ echo 'invalid-syntax' > badrepo/.hg/hgrc
   $ hg log -b -Rbadrepo default
-  hg: parse error at badrepo/.hg/hgrc:1: invalid-syntax
-  [255]
+  config error at badrepo/.hg/hgrc:1: invalid-syntax
+  [30]
 
   $ hg log -b --cwd=inexistent default
   abort: $ENOENT$: 'inexistent'
diff --git a/tests/test-default-push.t b/tests/test-default-push.t
--- a/tests/test-default-push.t
+++ b/tests/test-default-push.t
@@ -18,7 +18,7 @@ 
 Push should provide a hint when both 'default' and 'default-push' not set:
   $ cd c
   $ hg push --config paths.default=
-  abort: default repository not configured!
+  config error: default repository not configured!
   (see 'hg help config.paths')
   [30]
 
diff --git a/tests/test-convert-git.t b/tests/test-convert-git.t
--- a/tests/test-convert-git.t
+++ b/tests/test-convert-git.t
@@ -332,7 +332,7 @@ 
 
 input validation
   $ hg convert --config convert.git.similarity=foo --datesort git-repo2 fullrepo
-  abort: convert.git.similarity is not a valid integer ('foo')
+  config error: convert.git.similarity is not a valid integer ('foo')
   [30]
   $ hg convert --config convert.git.similarity=-1 --datesort git-repo2 fullrepo
   abort: similarity must be between 0 and 100
diff --git a/tests/test-config.t b/tests/test-config.t
--- a/tests/test-config.t
+++ b/tests/test-config.t
@@ -7,8 +7,8 @@ 
   > novaluekey
   > EOF
   $ hg showconfig
-  hg: parse error at $TESTTMP/.hg/hgrc:1: novaluekey
-  [255]
+  config error at $TESTTMP/.hg/hgrc:1: novaluekey
+  [30]
 
 Invalid syntax: no key
 
@@ -16,8 +16,8 @@ 
   > =nokeyvalue
   > EOF
   $ hg showconfig
-  hg: parse error at $TESTTMP/.hg/hgrc:1: =nokeyvalue
-  [255]
+  config error at $TESTTMP/.hg/hgrc:1: =nokeyvalue
+  [30]
 
 Test hint about invalid syntax from leading white space
 
@@ -25,16 +25,16 @@ 
   >  key=value
   > EOF
   $ hg showconfig
-  hg: parse error at $TESTTMP/.hg/hgrc:1: unexpected leading whitespace:  key=value
-  [255]
+  config error at $TESTTMP/.hg/hgrc:1: unexpected leading whitespace:  key=value
+  [30]
 
   $ cat > .hg/hgrc << EOF
   >  [section]
   > key=value
   > EOF
   $ hg showconfig
-  hg: parse error at $TESTTMP/.hg/hgrc:1: unexpected leading whitespace:  [section]
-  [255]
+  config error at $TESTTMP/.hg/hgrc:1: unexpected leading whitespace:  [section]
+  [30]
 
 Reset hgrc
 
diff --git a/tests/test-audit-subrepo.t b/tests/test-audit-subrepo.t
--- a/tests/test-audit-subrepo.t
+++ b/tests/test-audit-subrepo.t
@@ -120,8 +120,8 @@ 
   $ hg init sub
   $ echo '= sub' >> .hgsub
   $ hg ci -qAm 'add subrepo ""'
-  hg: parse error at .hgsub:1: = sub
-  [255]
+  config error at .hgsub:1: = sub
+  [30]
 
 prepare tampered repo (including the commit above):
 
@@ -144,8 +144,8 @@ 
 on clone (and update):
 
   $ hg clone -q emptypath emptypath2
-  hg: parse error at .hgsub:1: = sub
-  [255]
+  config error at .hgsub:1: = sub
+  [30]
 
 Test current path
 -----------------
diff --git a/tests/test-add.t b/tests/test-add.t
--- a/tests/test-add.t
+++ b/tests/test-add.t
@@ -44,7 +44,7 @@ 
 #if no-windows
   $ echo foo > con.xml
   $ hg --config ui.portablefilenames=jump add con.xml
-  abort: ui.portablefilenames value is invalid ('jump')
+  config error: ui.portablefilenames value is invalid ('jump')
   [30]
   $ hg --config ui.portablefilenames=abort add con.xml
   abort: filename contains 'con', which is reserved on Windows: con.xml
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -466,7 +466,7 @@ 
 
             try:
                 cfg.read(filename, fp, sections=sections, remap=remap)
-            except error.ParseError as inst:
+            except error.ConfigError as inst:
                 if trusted:
                     raise
                 self.warn(
diff --git a/mercurial/error.py b/mercurial/error.py
--- a/mercurial/error.py
+++ b/mercurial/error.py
@@ -227,6 +227,24 @@ 
 class ConfigError(Abort):
     """Exception raised when parsing config files"""
 
+    def __init__(self, message, location=None, hint=None):
+        super(ConfigError, self).__init__(message, hint=hint)
+        self.location = location
+
+    def format(self):
+        from .i18n import _
+
+        if self.location is not None:
+            message = _(b"config error at %s: %s\n") % (
+                pycompat.bytestr(self.location),
+                self.message,
+            )
+        else:
+            message = _(b"config error: %s\n") % self.message
+        if self.hint:
+            message += _(b"(%s)\n") % self.hint
+        return message
+
 
 class UpdateAbort(Abort):
     """Raised when an update is aborted for destination issue"""
diff --git a/mercurial/config.py b/mercurial/config.py
--- a/mercurial/config.py
+++ b/mercurial/config.py
@@ -165,7 +165,7 @@ 
                     include(expanded, remap=remap, sections=sections)
                 except IOError as inst:
                     if inst.errno != errno.ENOENT:
-                        raise error.ParseError(
+                        raise error.ConfigError(
                             _(b"cannot include %s (%s)")
                             % (expanded, encoding.strtolocal(inst.strerror)),
                             b"%s:%d" % (src, line),
@@ -203,7 +203,7 @@ 
             message = l.rstrip()
             if l.startswith(b' '):
                 message = b"unexpected leading whitespace: %s" % message
-            raise error.ParseError(message, (b"%s:%d" % (src, line)))
+            raise error.ConfigError(message, (b"%s:%d" % (src, line)))
 
     def read(self, path, fp=None, sections=None, remap=None):
         if not fp:
diff --git a/hgext/eol.py b/hgext/eol.py
--- a/hgext/eol.py
+++ b/hgext/eol.py
@@ -274,7 +274,7 @@ 
                 return eolfile(ui, repo.root, data)
             except (IOError, LookupError):
                 pass
-    except errormod.ParseError as inst:
+    except errormod.ConfigError as inst:
         ui.warn(
             _(
                 b"warning: ignoring .hgeol file due to parse error "