D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

Closed by commit rHG3332bde53714: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159) (authored by navaneeth.suresh). This revision was automatically updated to reflect the committed changes. CHANGED PRIOR TO COMMIT

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

pulkit added a comment. Queued the patches for stable, many thanks! REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6731/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6731 To: navaneeth.suresh, #hg-reviewers, pulkit Cc: pulkit,

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

This revision is now accepted and ready to land. pulkit added inline comments. pulkit accepted this revision. INLINE COMMENTS > test-bookmarks-pushpull.t:1347 > Pushing the bookmark "foo" now fails as it contains a secret changeset > #if b2-pushkey >$ hg push -r foo removing the casing in

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh added a comment. In D6731#99192 , @pulkit wrote: > Looks like you forgot to update tests. My bad! Sorry about that. Updated it now. Kindly review. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh updated this revision to Diff 16305. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D6731?vs=16299=16305 CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6731/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6731

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

pulkit added a comment. Looks like you forgot to update tests. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6731/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6731 To: navaneeth.suresh, #hg-reviewers Cc: pulkit, mercurial-devel

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh updated this revision to Diff 16299. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D6731?vs=16291=16299 CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6731/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6731

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh added inline comments. navaneeth.suresh marked 2 inline comments as done. INLINE COMMENTS > navaneeth.suresh wrote in exchange.py:1044 > `ctx = pushop.repo[node]` is needed to check the phase of the ctx. also, > iiuc, if i write `if a and b: pass` in python, both a and b are

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh updated this revision to Diff 16291. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D6731?vs=16267=16291 CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6731/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6731

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh added inline comments. INLINE COMMENTS > pulkit wrote in exchange.py:1044 > You won't need `ctx = pushop.repo[node]` anymore then. > > In general, I suggest understanding the error as why are they are happening > and you will find a fix. `ctx = pushop.repo[node]` is needed to

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

pulkit added inline comments. INLINE COMMENTS > navaneeth.suresh wrote in exchange.py:1044 > I'm getting the following error with the code snippet that you've suggested: > > + Traceback (most recent call last): > +File "/tmp/hgtests.nS3TJv/install/bin/hg", line 43, in > +

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh added inline comments. navaneeth.suresh marked an inline comment as done. INLINE COMMENTS > pulkit wrote in exchange.py:1044 > If the node is None, then `if node and ` will be false and the second > condition won't be executed. I'm getting the following error with the code

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

pulkit added inline comments. INLINE COMMENTS > navaneeth.suresh wrote in exchange.py:1044 > updated to get rid of one return. if node is `None`, then hg will throw an > error there. If the node is None, then `if node and ` will be false and the second condition won't be executed.

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh added inline comments. navaneeth.suresh marked 2 inline comments as done. INLINE COMMENTS > pulkit wrote in exchange.py:1044 > What does the return value mean here? I think we can get rid of this. > > Something like this: > > if node and pushop.repo[node].phase() ==

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh updated this revision to Diff 16267. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D6731?vs=16258=16267 CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6731/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6731

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

pulkit added a comment. Can you also add a test where we are pushing multiple heads/bookmarks and one of the bookmark is problematic? INLINE COMMENTS > exchange.py:1043 > +if ctx.phase() == phases.secret: > +raise error.Abort(_('bookmark %s points to a secret changeset') % b) >

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh added a comment. In D6731#98871 , @pulkit wrote: > In such cases, I like the idea of having fix as two patches, first which demonstrates the bug and the second which fixes the bug. What do you think? Yeah, sounds good to

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh updated this revision to Diff 16258. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D6731?vs=16225=16258 CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6731/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6731

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

pulkit added a comment. In such cases, I like the idea of having fix as two patches, first which demonstrates the bug and the second which fixes the bug. What do you think? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6731/new/ REVISION DETAIL

D6731: exchange: abort on pushing bookmarks pointing to secret changesets (issue6159)

navaneeth.suresh created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION SUMMARY Until now, if there is a bookmark points to a changeset which is in secret phase, hg will push the bookmark, but not the changeset referenced by that