# HG changeset patch
# User Manuel Jacob <me@manueljacob.de>
# Date 1589115046 -7200
#      Sun May 10 14:50:46 2020 +0200
# Node ID 3483dccd45a514a0a7cf11465727512300fa8436
# Parent  2e68868b65e40e91dc05cee9bfc44e5f6cf8653b
hggit: fix safebranchrevs() for the case when the remote repo is not a gitrepo

(folded with intermediate revision with new message by Georges Racinet)

The previous conditional was wrong: in current Mercurial, 'co' is in principle
not an integer. The condition on integer has been introduced for the port to
py3, but does not change the fact that changelog.__contains__ is for integers.

The original intent in a90fe3e8a8c3 was probably to exclude local repositories,
but this had the side effect of setting `co=None` in all cases, hence affecting
pulls having nothing to do with hg-git, as the new test demontstrates.

What we really want is to force `co=None` only for remote Git repositories,
for the reasons explained in the comment.

Previous intermediate commit was

  cleanup: eliminate if statement with condition that is always true

  'co' is never an integer. It’s always None or of type bytes. Apparently 'co'
  was of type int some years ago.

  A proper fix would require to fix the lookup() remote command or change hg to
  not require it e.g. when pulling with -r. For now, let’s remove the broken
  code and be honest about that we give up.

diff --git a/hggit/__init__.py b/hggit/__init__.py
--- a/hggit/__init__.py
+++ b/hggit/__init__.py
@@ -185,18 +185,15 @@
 # defend against tracebacks if we specify -r in 'hg pull'
 def safebranchrevs(orig, lrepo, otherrepo, branches, revs):
     revs, co = orig(lrepo, otherrepo, branches, revs)
-    if (
-        not isinstance(co, int)
-        or hgutil.safehasattr(lrepo, b'changelog')
-        and co not in lrepo.changelog
-    ):
+    if isinstance(otherrepo, gitrepo.gitrepo):
         # FIXME: Unless it's None, the 'co' result is passed to the lookup()
         # remote command. Since our implementation of the lookup() remote
         # command is incorrect, we set it to None to avoid a crash later when
         # the incorect result of the lookup() remote command would otherwise be
         # used. This can, in undocumented corner-cases, result in that a
         # different revision is updated to when passing both -u and -r to
-        # 'hg pull'.
+        # 'hg pull'. An example of such case is in tests/test-addbranchrevs.t
+        # (for the non-hg-git case).
         co = None
     return revs, co
 extensions.wrapfunction(hg, b'addbranchrevs', safebranchrevs)
diff --git a/tests/test-addbranchrevs.t b/tests/test-addbranchrevs.t
new file mode 100644
--- /dev/null
+++ b/tests/test-addbranchrevs.t
@@ -0,0 +1,20 @@
+Load commonly used test logic
+  $ . "$TESTDIR/testutil"
+
+This test doesn’t test any git-related functionality. It checks that a previous
+bug is not present where mercurial.hg.addbranchrevs() was erroneously
+monkey-patched such that the 'checkout' return value was always None. This
+caused the pull to not update to the passed revision.
+
+  $ hg init orig
+  $ cd orig
+  $ echo a > a; hg add a; hg ci -m a
+  $ hg branch foo -q
+  $ echo b > b; hg add b; hg ci -m b
+
+  $ cd ..
+  $ hg clone orig clone -r 0 -q
+  $ cd clone
+  $ hg pull -u -r 1 -q
+  $ hg id -n
+  1