D8117: bookmarks: prevent pushes of divergent bookmarks (foo@remote)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

D8117: bookmarks: prevent pushes of divergent bookmarks (foo@remote)

mharbison72 (Matt Harbison)
valentin.gatienbaron created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Before this change, such bookmarks are write-only: a client can push
  them but not pull/read them. And because these bookmark can't be read,
  even pushes are limited (for instance trying to delete such a bookmark
  fails with a vanilla client because the client thinks the bookmark is
  neither on the local nor the remote).
 
  This change makes the server refuses such bookmarks, and for earlier
  errors, makes the client refuse to send them.
 
  I think the change of behavior is acceptable because I think this is a
  bug in push/pull, and I don't think we change the behavior of `hg
  unbundle`, because it doesn't seem that `hg bundle` ever store
  bookmarks (and even if it did, it would seem weird anyway to try to
  send divergent bookmarks).

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/bookmarks.py
  mercurial/bundle2.py
  mercurial/exchange.py
  tests/test-bookmarks-pushpull.t

CHANGE DETAILS

diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
--- a/tests/test-bookmarks-pushpull.t
+++ b/tests/test-bookmarks-pushpull.t
@@ -328,6 +328,17 @@
 
 #endif
 
+Divergent bookmark cannot be exported
+
+  $ hg book W@default
+  $ hg push -B W@default ../a
+  pushing to ../a
+  searching for changes
+  cannot push divergent bookmark W@default!
+  no changes found
+  [2]
+  $ hg book -d W@default
+
 export the active bookmark
 
   $ hg bookmark V
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -856,7 +856,11 @@
     for b, scid, dcid in addsrc:
         if b in explicit:
             explicit.remove(b)
-            pushop.outbookmarks.append((b, b'', scid))
+            if bookmod.isdivergent(b):
+                pushop.ui.warn(_(b'cannot push divergent bookmark %s!\n') % b)
+                pushop.bkresult = 2
+            else:
+                pushop.outbookmarks.append((b, b'', scid))
     # search for overwritten bookmark
     for b, scid, dcid in list(advdst) + list(diverge) + list(differ):
         if b in explicit:
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -2368,6 +2368,11 @@
                     b'prepushkey', throw=True, **pycompat.strkwargs(hookargs)
                 )
 
+        for book, node in changes:
+            if bookmarks.isdivergent(book):
+                msg = _(b'cannot accept divergent bookmark %s!') % book
+                raise error.Abort(msg)
+
         bookstore.applychanges(op.repo, op.gettransaction(), changes)
 
         if pushkeycompat:
diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -483,6 +483,8 @@
 
 
 def pushbookmark(repo, key, old, new):
+    if isdivergent(key):
+        return False
     if bookmarksinstore(repo):
         wlock = util.nullcontextmanager()
     else:



To: valentin.gatienbaron, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8117: bookmarks: prevent pushes of divergent bookmarks (foo@remote)

mharbison72 (Matt Harbison)
valentin.gatienbaron updated this revision to Diff 20246.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8117?vs=20211&id=20246

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8117/new/

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

AFFECTED FILES
  mercurial/bookmarks.py
  mercurial/bundle2.py
  mercurial/exchange.py
  relnotes/next
  tests/test-bookmarks-pushpull.t

CHANGE DETAILS

diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
--- a/tests/test-bookmarks-pushpull.t
+++ b/tests/test-bookmarks-pushpull.t
@@ -328,6 +328,17 @@
 
 #endif
 
+Divergent bookmark cannot be exported
+
+  $ hg book W@default
+  $ hg push -B W@default ../a
+  pushing to ../a
+  searching for changes
+  cannot push divergent bookmark W@default!
+  no changes found
+  [2]
+  $ hg book -d W@default
+
 export the active bookmark
 
   $ hg bookmark V
diff --git a/relnotes/next b/relnotes/next
--- a/relnotes/next
+++ b/relnotes/next
@@ -18,6 +18,7 @@
 
 == Bug Fixes  ==
 
+ * prevent pushes of divergent bookmarks (foo@remote)
 
 == Backwards Compatibility Changes ==
 
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -856,7 +856,11 @@
     for b, scid, dcid in addsrc:
         if b in explicit:
             explicit.remove(b)
-            pushop.outbookmarks.append((b, b'', scid))
+            if bookmod.isdivergent(b):
+                pushop.ui.warn(_(b'cannot push divergent bookmark %s!\n') % b)
+                pushop.bkresult = 2
+            else:
+                pushop.outbookmarks.append((b, b'', scid))
     # search for overwritten bookmark
     for b, scid, dcid in list(advdst) + list(diverge) + list(differ):
         if b in explicit:
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -2368,6 +2368,11 @@
                     b'prepushkey', throw=True, **pycompat.strkwargs(hookargs)
                 )
 
+        for book, node in changes:
+            if bookmarks.isdivergent(book):
+                msg = _(b'cannot accept divergent bookmark %s!') % book
+                raise error.Abort(msg)
+
         bookstore.applychanges(op.repo, op.gettransaction(), changes)
 
         if pushkeycompat:
diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -483,6 +483,8 @@
 
 
 def pushbookmark(repo, key, old, new):
+    if isdivergent(key):
+        return False
     if bookmarksinstore(repo):
         wlock = util.nullcontextmanager()
     else:



To: valentin.gatienbaron, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8117: bookmarks: prevent pushes of divergent bookmarks (foo@remote)

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
Closed by commit rHG8407031f195f: bookmarks: prevent pushes of divergent bookmarks (foo@remote) (authored by valentin.gatienbaron).
This revision was automatically updated to reflect the committed changes.

CHANGED PRIOR TO COMMIT
  https://phab.mercurial-scm.org/D8117?vs=20246&id=20272#toc

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8117?vs=20246&id=20272

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8117/new/

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

AFFECTED FILES
  mercurial/bookmarks.py
  mercurial/bundle2.py
  mercurial/exchange.py
  relnotes/next
  tests/test-bookmarks-pushpull.t

CHANGE DETAILS

diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
--- a/tests/test-bookmarks-pushpull.t
+++ b/tests/test-bookmarks-pushpull.t
@@ -328,6 +328,17 @@
 
 #endif
 
+Divergent bookmark cannot be exported
+
+  $ hg book W@default
+  $ hg push -B W@default ../a
+  pushing to ../a
+  searching for changes
+  cannot push divergent bookmark W@default!
+  no changes found
+  [2]
+  $ hg book -d W@default
+
 export the active bookmark
 
   $ hg bookmark V
diff --git a/relnotes/next b/relnotes/next
--- a/relnotes/next
+++ b/relnotes/next
@@ -24,6 +24,7 @@
  * Use `hg copy --forget --at-rev REV` to unmark already committed
    copies.
 
+ * prevent pushes of divergent bookmarks (foo@remote)
 
 == Bug Fixes  ==
 
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -856,7 +856,11 @@
     for b, scid, dcid in addsrc:
         if b in explicit:
             explicit.remove(b)
-            pushop.outbookmarks.append((b, b'', scid))
+            if bookmod.isdivergent(b):
+                pushop.ui.warn(_(b'cannot push divergent bookmark %s!\n') % b)
+                pushop.bkresult = 2
+            else:
+                pushop.outbookmarks.append((b, b'', scid))
     # search for overwritten bookmark
     for b, scid, dcid in list(advdst) + list(diverge) + list(differ):
         if b in explicit:
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -2368,6 +2368,11 @@
                     b'prepushkey', throw=True, **pycompat.strkwargs(hookargs)
                 )
 
+        for book, node in changes:
+            if bookmarks.isdivergent(book):
+                msg = _(b'cannot accept divergent bookmark %s!') % book
+                raise error.Abort(msg)
+
         bookstore.applychanges(op.repo, op.gettransaction(), changes)
 
         if pushkeycompat:
diff --git a/mercurial/bookmarks.py b/mercurial/bookmarks.py
--- a/mercurial/bookmarks.py
+++ b/mercurial/bookmarks.py
@@ -487,6 +487,8 @@
 
 
 def pushbookmark(repo, key, old, new):
+    if isdivergent(key):
+        return False
     if bookmarksinstore(repo):
         wlock = util.nullcontextmanager()
     else:



To: valentin.gatienbaron, #hg-reviewers, pulkit
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8117: bookmarks: prevent pushes of divergent bookmarks (foo@remote)

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
durin42 added a comment.


  I'm enthusiastic about this change, and in general a fan of declaring misfeatures in bookmarks exchange bugs. I think it's pretty clear the existing bookmarks exchange behavior isn't right to the point of being buggy.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8117/new/

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

To: valentin.gatienbaron, #hg-reviewers, pulkit
Cc: durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D8117: bookmarks: prevent pushes of divergent bookmarks (foo@remote)

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
marmoute added a comment.


  I am also +1 for this change. divergence bookmark are close to "remote" bookmark and should not be exchanged. Especially because it open the way to divergent-divergent-bookmark and other hells.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8117/new/

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

To: valentin.gatienbaron, #hg-reviewers, pulkit
Cc: marmoute, durin42, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel