Patchwork [STABLE] extdiff: abort if an empty revision is given with --patch

login
register
mail settings
Submitter Matt Harbison
Date Nov. 1, 2015, 1:53 a.m.
Message ID <c735abc328fefe3d0695.1446342812@Envy>
Download mbox | patch
Permalink /patch/11254/
State Changes Requested
Headers show

Comments

Matt Harbison - Nov. 1, 2015, 1:53 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1446342346 14400
#      Sat Oct 31 21:45:46 2015 -0400
# Branch stable
# Node ID c735abc328fefe3d0695a8d82b68ee0030f0e5bc
# Parent  6474b64045fb83733d6f7b774bbc8b2869bd1fed
extdiff: abort if an empty revision is given with --patch

I ran into this when I forgot --hidden to make a precursor visible, but it will
occur any time one --rev evaluates to an empty set.  The error message is
probably slightly confusing since it can now be seen when "--rev" is actually
specified twice, but this seems better than the previous behavior of comparing
the first --rev argument against itself.  Since the issue isn't necessarily a
missing --hidden, I'm hesitant to abort this separately and mention the --hidden
flag.

The reason forgetting --hidden causes the revision to be duplicated is that
scmutil.revpair() will calculate a revrange that looks like this:

    $ hg extdiff --patch --rev 'precursors(29204)' --rev 29204
    <addset <baseset+ []>, <baseset [29204]>>

... which has 29204 as its first and last entry.  Since multiple --rev args are
provided, this causes None to _not_ be returned as the second item in the tuple.

The extdiff functionality without --patch probably could use similar logic, but
I've only ever run into this with --patch when using it to compare with a
precursor.  Given that nobody has ever complained about it, I'm hesitant to
change that this close to a release.

Patch

diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -150,7 +150,7 @@ 
     if opts.get('patch'):
         if subrepos:
             raise error.Abort(_('--patch cannot be used with --subrepos'))
-        if node2 is None:
+        if node2 is None or node1a == node2:
             raise error.Abort(_('--patch requires two revisions'))
     else:
         mod_a, add_a, rem_a = map(set, repo.status(node1a, node2, matcher,
diff --git a/tests/test-extdiff.t b/tests/test-extdiff.t
--- a/tests/test-extdiff.t
+++ b/tests/test-extdiff.t
@@ -70,6 +70,12 @@ 
   [1]
 #endif
 
+Specifying an empty revision to --patch should abort.
+
+  $ hg extdiff --patch --rev 'ancestor()' --rev 1
+  abort: --patch requires two revisions
+  [255]
+
 Test diff during merge:
 
   $ hg update -C 0