This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2873

remotenames: add functionality to override -B flag of push
Needs RevisionPublic

Authored by pulkit on Mar 15 2018, 9:49 AM.

Details

Reviewers
indygreg
marmoute
Group Reviewers
hg-reviewers
Summary

This patch adds a new config option which can be used to override the -B flag
of push command. If config is set to true, changesets will be pushed to that
bookmark on the server. This is equivalent to --to flag of remotenames and
infinitepush.

After this patch if config is set to true, the behavior will be:

  • Normal push without -B flag: no behavior change
  • specifying one bookmark using -B flag: changesets will be pushed to that bookmark on the server, as explained above. Bookmark pushed will be on the head most revision of the changesets pushed.
  • specifying multiple bookmarks using -B flag: no behavior change, the same behavior as it's today without this config option
  • specifying multiple topologcial heads to push: no behvaior change

There needs some extra logic to update the remotenames state locally after the
push since we know that the bookmark will now be at a known updated location.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Mar 15 2018, 9:49 AM
pulkit added a comment.Mar 16 2018, 11:00 AM

I am not sure the functionality and config names should go to core or not, so I have introduced them in this extension. Also suggestions for better config names are welcome.

pulkit updated this revision to Diff 7103.Mar 19 2018, 4:36 AM
pulkit updated this revision to Diff 7249.Mar 22 2018, 3:35 AM
indygreg requested changes to this revision.Mar 23 2018, 2:56 PM
indygreg added a subscriber: indygreg.

I'm happy with this feature.

But the code needs work around multiple revisions/heads before it can be queued.

hgext/remotenames.py
99

What happens if we're pushing multiple heads? I suspect this will choose a head/revision arbitrarily - maybe depending on the -r arguments to hg push.

I think we need to validate that the outgoing revs are in a single DAG head and we should then pick the rev that is the DAG head.

Please add test coverage for multiple -r arguments and -r arguments that resolve to multiple heads.

101

Nit: what is --create?

287

Strictly speaking, we should probably wrap exchange._pushdiscoverybookmarks so other extensions can get involved. But I think this is fine.

This revision now requires changes to proceed.Mar 23 2018, 2:56 PM
pulkit edited the summary of this revision. (Show Details)Mar 26 2018, 7:47 AM
pulkit updated this revision to Diff 7286.
pulkit added a comment.Apr 1 2018, 12:51 PM

A gentle reminder for reviewing this series. If this config option gets in, we can use it in infinitepush extension too where for now we have overridden -B flag in a hacky way.

indygreg accepted this revision.Apr 11 2018, 1:29 PM

Looks good!

hgext/remotenames.py
113

Nit: "exist". This can be fixed in flight.

This revision is now accepted and ready to land.Apr 11 2018, 1:29 PM
smf added a subscriber: smf.Apr 11 2018, 1:32 PM
In D2873#52021, @indygreg wrote:

Looks good!

I'm very heavily against this direction. Changing the behavior of push (even in this extension) is something I've always considered outside the scope of remotenames. Having another extension that changes push behavior (e.g. bookmark-push) is where I think this should go so that remotenames is just that: keeping track of remote names.

indygreg added a comment.Apr 11 2018, 4:55 PM
In D2873#52025, @smf wrote:
In D2873#52021, @indygreg wrote:

Looks good!

I'm very heavily against this direction. Changing the behavior of push (even in this extension) is something I've always considered outside the scope of remotenames. Having another extension that changes push behavior (e.g. bookmark-push) is where I think this should go so that remotenames is just that: keeping track of remote names.

I think there's room for this feature to live outside of remotenames. But currently I think it is the best place for it, since remotenames is the closest thing we have to... tracking remote names. We can always alias the old config option in the future if we move this functionality elsewhere.

smf added a comment.Apr 11 2018, 5:39 PM
In D2873#52100, @indygreg wrote:
In D2873#52025, @smf wrote:
In D2873#52021, @indygreg wrote:

Looks good!

I'm very heavily against this direction. Changing the behavior of push (even in this extension) is something I've always considered outside the scope of remotenames. Having another extension that changes push behavior (e.g. bookmark-push) is where I think this should go so that remotenames is just that: keeping track of remote names.

I think there's room for this feature to live outside of remotenames. But currently I think it is the best place for it, since remotenames is the closest thing we have to... tracking remote names. We can always alias the old config option in the future if we move this functionality elsewhere.

One of the biggest regrets I have about the original remotenames, is the modifying of push (and some default) behavior. The --to is small enough (and I guess fine enough) for now but I absolutely and strongly believe that all the other behavior modifications should stay in an out-of-core repo for now. My goal is to split my remotenames repo to be based off of core's remotenames (basically only having the behavior changing patches there).

marmoute requested changes to this revision.Apr 22 2020, 11:57 AM
marmoute added a subscriber: marmoute.

Baymax does not catch stuff in Accepted state, but this diff is over 2 years old, so resubmit if still relevant.

This revision now requires changes to proceed.Apr 22 2020, 11:57 AM

Revision Contents
Changeset List

PathPackages
Mhgext/remotenames.py (38 lines)
Mtests/test-logexchange.t (191 lines)

Diff 7103

hgext/remotenames.py

Boolean value to enable or disable showing of remotebookmarks (default: True) Boolean value to enable or disable showing of remotebookmarks (default: True)
remotenames.branches remotenames.branches
Boolean value to enable or disable showing of remotebranches (default: True) Boolean value to enable or disable showing of remotebranches (default: True)
remotenames.hoist remotenames.hoist
Name of the peer whose remotebookmarks should be hoisted into the top-level Name of the peer whose remotebookmarks should be hoisted into the top-level
namespace (default: 'default') namespace (default: 'default')
remotenames.pushtobookmark
Boolean value to change the behavior of bookmark passed to push command using
`-B` flag. If set the true, the changesets will be pushed to that bookmark on
the server. Errors if bookmark does not exists on the server. If multiple
bookmarks are specified using `-B` flag, fallbacks to default behavior.
(default: False)
""" """
from __future__ import absolute_import from __future__ import absolute_import
from mercurial.i18n import _ from mercurial.i18n import _
from mercurial.node import ( from mercurial.node import (
bin, bin,
hex,
) )
from mercurial import ( from mercurial import (
bookmarks, bookmarks,
error,
exchange,
extensions, extensions,
logexchange, logexchange,
namespaces, namespaces,
pycompat, pycompat,
registrar, registrar,
revsetlang, revsetlang,
smartset, smartset,
templateutil, templateutil,
default=True, default=True,
) )
configitem('remotenames', 'branches', configitem('remotenames', 'branches',
default=True, default=True,
) )
configitem('remotenames', 'hoist', configitem('remotenames', 'hoist',
default='default', default='default',
) )
configitem('remotenames', 'pushtobookmark',
default=False,
)
def expushdiscoverybookmarks(pushop):
# config not set, fallback to normal push behavior
if not pushop.repo.ui.configbool('remotenames', 'pushtobookmark'):
return exchange._pushdiscoverybookmarks(pushop)
# either zero or more than one bookmarks specified, fallback to normal
# push behavior, maybe we should error out in case of multiple bookmarks
if len(pushop.bookmarks) != 1:
return exchange._pushdiscoverybookmarks(pushop)
remotemarks = pushop.remote.listkeys('bookmarks')
bookmark = pushop.bookmarks[0]
rev = pushop.revs[0]
indygregUnsubmitted
Not Done

What happens if we're pushing multiple heads? I suspect this will choose a head/revision arbitrarily - maybe depending on the -r arguments to hg push.

I think we need to validate that the outgoing revs are in a single DAG head and we should then pick the rev that is the DAG head.

Please add test coverage for multiple -r arguments and -r arguments that resolve to multiple heads.

indygreg: What happens if we're pushing multiple heads? I suspect this will choose a head/revision…
# allow new bookmark only if --create is specified
indygregUnsubmitted
Not Done

Nit: what is --create?

indygreg: Nit: what is `--create`?
old = ''
if bookmark in remotemarks:
old = remotemarks[bookmark]
else:
msg = _("bookmark '%s' does not exists on remote")
raise error.Abort(msg % bookmark)
pushop.outbookmarks.append((bookmark, old, hex(rev)))
class lazyremotenamedict(mutablemapping): class lazyremotenamedict(mutablemapping):
""" """
Read-only dict-like Class to lazily resolve remotename entries Read-only dict-like Class to lazily resolve remotename entries
indygregUnsubmitted
Not Done

Nit: "exist". This can be fixed in flight.

indygreg: Nit: "exist". This can be fixed in flight.
We are doing that because remotenames startup was slow. We are doing that because remotenames startup was slow.
We lazily read the remotenames file once to figure out the potential entries We lazily read the remotenames file once to figure out the potential entries
and store them in self.potentialentries. Then when asked to resolve an and store them in self.potentialentries. Then when asked to resolve an
entry, if it is not in self.potentialentries, then it isn't there, if it entry, if it is not in self.potentialentries, then it isn't there, if it
is in self.potentialentries we resolve it and store the result in is in self.potentialentries we resolve it and store the result in
self.cache. We cannot be lazy is when asked all the entries (keys). self.cache. We cannot be lazy is when asked all the entries (keys).
""" """
node = nodes[0] node = nodes[0]
bmarks[name] = (node, ' ', '') bmarks[name] = (node, ' ', '')
return orig(ui, repo, bmarks, **opts) return orig(ui, repo, bmarks, **opts)
def extsetup(ui): def extsetup(ui):
extensions.wrapfunction(bookmarks, '_printbookmarks', wrapprintbookmarks) extensions.wrapfunction(bookmarks, '_printbookmarks', wrapprintbookmarks)
exchange.pushdiscoverymapping['bookmarks'] = expushdiscoverybookmarks
indygregUnsubmitted
Not Done

Strictly speaking, we should probably wrap exchange._pushdiscoverybookmarks so other extensions can get involved. But I think this is fine.

indygreg: Strictly speaking, we should probably wrap `exchange._pushdiscoverybookmarks` so other…
def reposetup(ui, repo): def reposetup(ui, repo):
if not repo.local(): if not repo.local():
return return
repo._remotenames = remotenames(repo) repo._remotenames = remotenames(repo)
ns = namespaces.namespace ns = namespaces.namespace

tests/test-logexchange.t

summary: added bar summary: added bar
$ hg bookmarks $ hg bookmarks
$TESTTMP/server2/bar 6:87d6d6676308 $TESTTMP/server2/bar 6:87d6d6676308
$TESTTMP/server2/foo 3:62615734edd5 $TESTTMP/server2/foo 3:62615734edd5
default/bar 6:87d6d6676308 default/bar 6:87d6d6676308
default/foo 3:62615734edd5 default/foo 3:62615734edd5
* foo 8:3e1487808078 * foo 8:3e1487808078
Testing the remotenames.pushtobookmark config option to push to bookmark
$ hg up .
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
(leaving bookmark foo)
$ echo foo > foobar
$ hg add foobar
$ hg ci -m "added foobar"
$ hg log -G
@ changeset: 9:aa6a885086c0
| branch: wat
| tag: tip
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: added foobar
|
o changeset: 8:3e1487808078
| branch: wat
| bookmark: foo
| remote branch: $TESTTMP/server2/wat
| remote branch: default/wat
| parent: 4:aa98ab95a928
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: added bar
|
| o changeset: 7:ec2426147f0e
| | remote branch: $TESTTMP/server2/default
| | remote branch: default/default
| | user: test
| | date: Thu Jan 01 00:00:00 1970 +0000
| | summary: Added h
| |
| o changeset: 6:87d6d6676308
| | remote bookmark: $TESTTMP/server2/bar
| | remote bookmark: default/bar
| | hoistedname: bar
| | user: test
| | date: Thu Jan 01 00:00:00 1970 +0000
| | summary: Added g
| |
| o changeset: 5:825660c69f0c
|/ user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: Added f
|
o changeset: 4:aa98ab95a928
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: Added e
|
o changeset: 3:62615734edd5
| remote bookmark: $TESTTMP/server2/foo
| remote bookmark: default/foo
| hoistedname: foo
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: Added d
|
o changeset: 2:28ad74487de9
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: Added c
|
o changeset: 1:29becc82797a
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: Added b
|
o changeset: 0:18d04c59bb5d
user: test
date: Thu Jan 01 00:00:00 1970 +0000
summary: Added a
$ echo '[remotenames]' >> .hg/hgrc
$ echo 'pushtobookmark = True' >> .hg/hgrc
Trying to push a non-existing bookmark on the server
$ hg push ../server/ -B nonexistentbm
pushing to ../server/
searching for changes
abort: bookmark 'nonexistentbm' does not exists on remote
[255]
$ hg push ../server/ -B nonexistinentbm -f
pushing to ../server/
searching for changes
abort: bookmark 'nonexistinentbm' does not exists on remote
[255]
Pushing changesets to a bookmark on the remote
$ hg bookmarks -R ../server/
bar 6:87d6d6676308
foo 3:62615734edd5
$ hg push ../server/ -r . -B foo
pushing to ../server/
searching for changes
adding changesets
adding manifests
adding file changes
added 1 changesets with 1 changes to 1 files
updating bookmark foo
After this push, `foo` bookmark on the server is updated whereas the local
bookmark of that name stayed at its own place
$ hg log -G -R ../server/ -r tip
o changeset: 9:aa6a885086c0
| branch: wat
~ bookmark: foo
tag: tip
user: test
date: Thu Jan 01 00:00:00 1970 +0000
summary: added foobar
$ hg bookmarks -R ../server/
bar 6:87d6d6676308
foo 9:aa6a885086c0
XXX: remotebookmarks should have been updated after this push
$ hg log -G
@ changeset: 9:aa6a885086c0
| branch: wat
| tag: tip
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: added foobar
|
o changeset: 8:3e1487808078
| branch: wat
| bookmark: foo
| remote branch: $TESTTMP/server2/wat
| remote branch: default/wat
| parent: 4:aa98ab95a928
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: added bar
|
| o changeset: 7:ec2426147f0e
| | remote branch: $TESTTMP/server2/default
| | remote branch: default/default
| | user: test
| | date: Thu Jan 01 00:00:00 1970 +0000
| | summary: Added h
| |
| o changeset: 6:87d6d6676308
| | remote bookmark: $TESTTMP/server2/bar
| | remote bookmark: default/bar
| | hoistedname: bar
| | user: test
| | date: Thu Jan 01 00:00:00 1970 +0000
| | summary: Added g
| |
| o changeset: 5:825660c69f0c
|/ user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: Added f
|
o changeset: 4:aa98ab95a928
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: Added e
|
o changeset: 3:62615734edd5
| remote bookmark: $TESTTMP/server2/foo
| remote bookmark: default/foo
| hoistedname: foo
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: Added d
|
o changeset: 2:28ad74487de9
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: Added c
|
o changeset: 1:29becc82797a
| user: test
| date: Thu Jan 01 00:00:00 1970 +0000
| summary: Added b
|
o changeset: 0:18d04c59bb5d
user: test
date: Thu Jan 01 00:00:00 1970 +0000
summary: Added a