Patchwork [4,of,4,RFC,path,options] ui: add a "pushnames" path option

# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1423550715 28800
#      Mon Feb 09 22:45:15 2015 -0800
# Node ID 96836adc7bfd1a1b41befc2cbbcc135fc35b7948
# Parent  4da24a2a9e7b82e5cd0082e4d69255c0cd5b805b
ui: add a "pushnames" path option

"pushrevset" will not push bookmarks because revsets expand to
revisions, even if bookmarks are queried for. So, we need an
additional mechanism to specify names to push.

We introduce the "pushnames" path option for this purpose.

The intent of this option is to resolve all names. This includes
bookmarks, branches, and any other namespaces that may be installed
through extensions. However, supporting all names is rather
difficult, so support hasn't yet been added for things that aren't
bookmarks. See "hg.addbranchrevs" for an example of the complexity
this involves.

Given that bookmarks (and other names for that matter) are special,
it might be worth abandoning a generic API and going for name-specific
options. This would map to command line argument usage (-B, --branch).

Another thought that came up when implementing this patch is that it
might be worth creating a unified "what to push" API that takes
a "path", a remote, and a set of options and returns a data structure
describing what to push. This is because the logic for figuring this
out is somewhat complicated and it might warrant living in a
more core API, such as exchange.push. But I'm hard pressed to name
any benefits besides the ability to make this code unit testable,
as I don't think there are other code paths that require knowledge
on how to resolve what to push. Then again, once this API exists,
perhaps extensions will invent a need for it.

On Sun, Mar 01, 2015 at 03:23:48PM -0800, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1423550715 28800
> #      Mon Feb 09 22:45:15 2015 -0800
> # Node ID 96836adc7bfd1a1b41befc2cbbcc135fc35b7948
> # Parent  4da24a2a9e7b82e5cd0082e4d69255c0cd5b805b
> ui: add a "pushnames" path option

I think I'm +0 on this in principle, but we should probably coordinate
between this and the workflow ideas smf and Ryan have around
remotenames. I haven't done enough thinking to have strong convictions yet.

>
> "pushrevset" will not push bookmarks because revsets expand to
> revisions, even if bookmarks are queried for. So, we need an
> additional mechanism to specify names to push.
>
> We introduce the "pushnames" path option for this purpose.
>
> The intent of this option is to resolve all names. This includes
> bookmarks, branches, and any other namespaces that may be installed
> through extensions. However, supporting all names is rather
> difficult, so support hasn't yet been added for things that aren't
> bookmarks. See "hg.addbranchrevs" for an example of the complexity
> this involves.
>
> Given that bookmarks (and other names for that matter) are special,
> it might be worth abandoning a generic API and going for name-specific
> options. This would map to command line argument usage (-B, --branch).
>
> Another thought that came up when implementing this patch is that it
> might be worth creating a unified "what to push" API that takes
> a "path", a remote, and a set of options and returns a data structure
> describing what to push. This is because the logic for figuring this
> out is somewhat complicated and it might warrant living in a
> more core API, such as exchange.push. But I'm hard pressed to name
> any benefits besides the ability to make this code unit testable,
> as I don't think there are other code paths that require knowledge
> on how to resolve what to push. Then again, once this API exists,
> perhaps extensions will invent a need for it.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -5071,8 +5071,20 @@ def push(ui, repo, dest=None, **opts):
>      URLs. If DESTINATION is omitted, a default path will be used.
>
>      Returns 0 if push was successful, 1 if nothing to push.
>      """
> +    allowpathrevs = not opts.get('bookmark') and not opts.get('rev') \
> +                    and not opts.get('branch')
> +
> +    path = ui.paths.getpath(dest, default='push', require=True)
> +
> +    if path.pushnames and allowpathrevs:
> +        for name in path.pushnames:
> +            if name in repo._bookmarks:
> +                opts.setdefault('bookmark', []).append(name)
> +            else:
> +                raise util.Abort('non-bookmarks not currently supported '
> +                                 'by pushnames path option')
>
>      if opts.get('bookmark'):
>          ui.setconfig('bookmarks', 'pushing', opts['bookmark'], 'push')
>          for b in opts['bookmark']:
> @@ -5083,9 +5095,8 @@ def push(ui, repo, dest=None, **opts):
>                  # if we try to push a deleted bookmark, translate it to null
>                  # this lets simultaneous -r, -b options continue working
>                  opts.setdefault('rev', []).append("null")
>
> -    path = ui.paths.getpath(dest, default='push', require=True)
>      url = path.pushurl
>      branches = [path.rev, opts.get('branch', [])]
>      ui.status(_('pushing to %s\n') % util.hidepassword(url))
>      revs, checkout = hg.addbranchrevs(repo, repo, branches, opts.get('rev'))
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1115,9 +1115,23 @@ The following per-path options are recog
>      working copy is based upon.
>
>      Note: Bookmarks will not be pushed if this option is set, even if the
>      revset specifies bookmarks to push. This is because revsets evaluate
> -    to changesets, not names.
> +    to changesets, not names. To push a set of bookmarks by default, use
> +    ``pushnames``.
> +
> +    This option is mutually exclusive with ``pushnames``.
> +
> +``pushnames``
> +    A list of comma or space delimited names to push to this path by default.
> +
> +    This option is similar to ``pushrevset`` except it operates on names,
> +    not revisions.
> +
> +    This option is mutually exclusive with ``pushrevset``.
> +
> +    Note: Bookmarks are names. Use this option to specify which bookmarks
> +    to push by default.
>
>  ``phases``
>  ----------
>
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -960,9 +960,10 @@ class paths(object):
>                  continue
>
>              yield path(name, url=loc,
>                         pushurl=ui.config('paths', '%s.pushurl' % name),
> -                       pushrevset=ui.config('paths', '%s.pushrevset' % name))
> +                       pushrevset=ui.config('paths', '%s.pushrevset' % name),
> +                       pushnames=ui.configlist('paths', '%s.pushnames' % name))
>
>      def __getitem__(self, key):
>          for path in self:
>              if path.name == key:
> @@ -1024,9 +1025,9 @@ class paths(object):
>  class path(object):
>      """Represents an individual path and its configuration."""
>
>      def __init__(self, name, url=None, local=None, pushurl=None,
> -                 pushrevset=None):
> +                 pushrevset=None, pushnames=None):
>          """Construct a path from its config options.
>
>          The primary URL for the path is defined as either a URL via ``url``
>          (preferred) or from a local, relative filesystem path (``local``).
> @@ -1048,5 +1049,11 @@ class path(object):
>              raise util.Abort(_('pushurl may not have #revision fragment: %s') %
>                                 pushurl)
>
>          self.pushurl = pushurl or url
> +
> +        if pushrevset and pushnames:
> +            raise util.Abort(_('only 1 of pushrevset and pushnames may be '
> +                               'defined on path: %s') % name)
> +
>          self.pushrevset = pushrevset
> +        self.pushnames = pushnames
> diff --git a/tests/test-push-path-config.t b/tests/test-push-path-config.t
> --- a/tests/test-push-path-config.t
> +++ b/tests/test-push-path-config.t
> @@ -131,4 +131,60 @@ A pushrevset with multiple branches conf
>    remote: adding changesets
>    remote: adding manifests
>    remote: adding file changes
>    remote: added 2 changesets with 2 changes to 1 files
> +
> +pushrevset and pushnames are mutually exclusive
> +
> +  $ hg --config paths.pushrevset1.pushnames=foo push pushrevset1
> +  abort: only 1 of pushrevset and pushnames may be defined on path: pushrevset1
> +  [255]
> +
> +adding bookmarks to pushnames will push bookmarks
> +
> +  $ cat >> .hg/hgrc << EOF
> +  > pushbookmark = ssh://user@dummy/$TESTTMP/server
> +  > pushbookmark.pushnames = bookmark1
> +  > EOF
> +
> +  $ hg -q up -r 0
> +  $ echo bm1_1 > foo
> +  $ hg commit -m 'bookmark 1 commit 1'
> +  created new head
> +  $ echo bm1_2 > foo
> +  $ hg commit -m 'bookmark 1 commit 2'
> +  $ hg bookmark -r ce2028409ac8 bookmark1
> +  $ hg -q up -r 0
> +  $ echo bm2 > foo
> +  $ hg commit -m 'bookmark 2'
> +  created new head
> +  $ hg bookmark bookmark2
> +
> +  $ hg push pushbookmark
> +  pushing to ssh://user@dummy/$TESTTMP/server
> +  searching for changes
> +  remote: adding changesets
> +  remote: adding manifests
> +  remote: adding file changes
> +  remote: added 1 changesets with 1 changes to 1 files (+1 heads)
> +  exporting bookmark bookmark1
> +
> +Specifying -B overrides pushnames
> +
> +  $ hg bookmark -r a41d7b167990 bookmark1
> +  moving bookmark 'bookmark1' forward from ce2028409ac8
> +
> +  $ hg push -B bookmark2 pushbookmark
> +  pushing to ssh://user@dummy/$TESTTMP/server
> +  searching for changes
> +  remote: adding changesets
> +  remote: adding manifests
> +  remote: adding file changes
> +  remote: added 1 changesets with 1 changes to 1 files (+1 heads)
> +  exporting bookmark bookmark2
> +
> +Specifying an unknown name is currently unsupported
> +
> +  $ hg --config paths.pushbookmark.pushnames=unknown push pushbookmark
> +  abort: non-bookmarks not currently supported by pushnames path option
> +  [255]
> +
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

On 03/01/2015 03:23 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1423550715 28800
> #      Mon Feb 09 22:45:15 2015 -0800
> # Node ID 96836adc7bfd1a1b41befc2cbbcc135fc35b7948
> # Parent  4da24a2a9e7b82e5cd0082e4d69255c0cd5b805b
> ui: add a "pushnames" path option

For the record, these path option idea makes sens to me and I think we 
should pursus in this direction.

I would be fan of an "alias" entry too to have multiple name for the 
same entry.

On 3/3/2015 11:39 AM, Augie Fackler wrote:
> On Sun, Mar 01, 2015 at 03:23:48PM -0800, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc@gmail.com>
>> # Date 1423550715 28800
>> #      Mon Feb 09 22:45:15 2015 -0800
>> # Node ID 96836adc7bfd1a1b41befc2cbbcc135fc35b7948
>> # Parent  4da24a2a9e7b82e5cd0082e4d69255c0cd5b805b
>> ui: add a "pushnames" path option
> I think I'm +0 on this in principle, but we should probably coordinate
> between this and the workflow ideas smf and Ryan have around
> remotenames. I haven't done enough thinking to have strong convictions yet.

Gregory, have you played around with the latest remotenames extension? 
There's this interesting/controversial/etc "feature" called tracking, 
inspired by git, where a bookmark can "track" another name (usually a 
remote bookmark, but could also be local, or other names exposed via the 
namespace API), so that a few commands just "do the right thing" when 
called without parameters:

"hg rebase" rebases with an implicit "--dest <tracking name>"
"hg push" pushes with an implicit "dest" path and updates an implicit 
"--to" bookmark

Today, while chatting with smf, I realized we can go one step further 
and allow the push tracking to be different from the rebase tracking. 
This idea was inspired by the workflow Sean and I have adopted:

sean's laptop ---(push)---> smf.io ---(pull)--> ryan's devserver 
---(push)---> "nexus" (hg.silverfir.net) ---(pull)---> sean's laptop ...

In this world, on my devserver, I want to rebase onto the changes from 
smf.io's @ bookmark, but I want to push to nexus and the @ bookmark 
there. If we have two tracking targets, we can do this. I'm not 
convinced this is a good idea yet, since it seems too complicated, but 
we're in exploratory mode here so I might implement this at some point.

Anyway, back to this patch: I think it's useful -- it's essentially 
moving command-line options into config to avoid repeated typing and 
potential mistakes, which makes me think this is good, but it also isn't 
a fundamentally different workflow, like remotenames is attempting to 
unlock (eg the --to flag on push allows pushing to a remote bookmark 
without the same bookmark -- or any bookmark for that matter -- being 
present in the local repo for that rev).

I'd call it 'pushbookmark' and restrict to to the equivalent of passing 
-B <name> to hg push. The other stuff can come later when it becomes 
more clear what direction we want to go.

>
>> "pushrevset" will not push bookmarks because revsets expand to
>> revisions, even if bookmarks are queried for. So, we need an
>> additional mechanism to specify names to push.
>>
>> We introduce the "pushnames" path option for this purpose.
>>
>> The intent of this option is to resolve all names. This includes
>> bookmarks, branches, and any other namespaces that may be installed
>> through extensions. However, supporting all names is rather
>> difficult, so support hasn't yet been added for things that aren't
>> bookmarks. See "hg.addbranchrevs" for an example of the complexity
>> this involves.
>>
>> Given that bookmarks (and other names for that matter) are special,
>> it might be worth abandoning a generic API and going for name-specific
>> options. This would map to command line argument usage (-B, --branch).
>>
>> Another thought that came up when implementing this patch is that it
>> might be worth creating a unified "what to push" API that takes
>> a "path", a remote, and a set of options and returns a data structure
>> describing what to push. This is because the logic for figuring this
>> out is somewhat complicated and it might warrant living in a
>> more core API, such as exchange.push. But I'm hard pressed to name
>> any benefits besides the ability to make this code unit testable,
>> as I don't think there are other code paths that require knowledge
>> on how to resolve what to push. Then again, once this API exists,
>> perhaps extensions will invent a need for it.
>>
>> diff --git a/mercurial/commands.py b/mercurial/commands.py
>> --- a/mercurial/commands.py
>> +++ b/mercurial/commands.py
>> @@ -5071,8 +5071,20 @@ def push(ui, repo, dest=None, **opts):
>>       URLs. If DESTINATION is omitted, a default path will be used.
>>
>>       Returns 0 if push was successful, 1 if nothing to push.
>>       """
>> +    allowpathrevs = not opts.get('bookmark') and not opts.get('rev') \
>> +                    and not opts.get('branch')
>> +
>> +    path = ui.paths.getpath(dest, default='push', require=True)
>> +
>> +    if path.pushnames and allowpathrevs:
>> +        for name in path.pushnames:
>> +            if name in repo._bookmarks:
>> +                opts.setdefault('bookmark', []).append(name)
>> +            else:
>> +                raise util.Abort('non-bookmarks not currently supported '
>> +                                 'by pushnames path option')
>>
>>       if opts.get('bookmark'):
>>           ui.setconfig('bookmarks', 'pushing', opts['bookmark'], 'push')
>>           for b in opts['bookmark']:
>> @@ -5083,9 +5095,8 @@ def push(ui, repo, dest=None, **opts):
>>                   # if we try to push a deleted bookmark, translate it to null
>>                   # this lets simultaneous -r, -b options continue working
>>                   opts.setdefault('rev', []).append("null")
>>
>> -    path = ui.paths.getpath(dest, default='push', require=True)
>>       url = path.pushurl
>>       branches = [path.rev, opts.get('branch', [])]
>>       ui.status(_('pushing to %s\n') % util.hidepassword(url))
>>       revs, checkout = hg.addbranchrevs(repo, repo, branches, opts.get('rev'))
>> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
>> --- a/mercurial/help/config.txt
>> +++ b/mercurial/help/config.txt
>> @@ -1115,9 +1115,23 @@ The following per-path options are recog
>>       working copy is based upon.
>>
>>       Note: Bookmarks will not be pushed if this option is set, even if the
>>       revset specifies bookmarks to push. This is because revsets evaluate
>> -    to changesets, not names.
>> +    to changesets, not names. To push a set of bookmarks by default, use
>> +    ``pushnames``.
>> +
>> +    This option is mutually exclusive with ``pushnames``.
>> +
>> +``pushnames``
>> +    A list of comma or space delimited names to push to this path by default.
>> +
>> +    This option is similar to ``pushrevset`` except it operates on names,
>> +    not revisions.
>> +
>> +    This option is mutually exclusive with ``pushrevset``.
>> +
>> +    Note: Bookmarks are names. Use this option to specify which bookmarks
>> +    to push by default.
>>
>>   ``phases``
>>   ----------
>>
>> diff --git a/mercurial/ui.py b/mercurial/ui.py
>> --- a/mercurial/ui.py
>> +++ b/mercurial/ui.py
>> @@ -960,9 +960,10 @@ class paths(object):
>>                   continue
>>
>>               yield path(name, url=loc,
>>                          pushurl=ui.config('paths', '%s.pushurl' % name),
>> -                       pushrevset=ui.config('paths', '%s.pushrevset' % name))
>> +                       pushrevset=ui.config('paths', '%s.pushrevset' % name),
>> +                       pushnames=ui.configlist('paths', '%s.pushnames' % name))
>>
>>       def __getitem__(self, key):
>>           for path in self:
>>               if path.name == key:
>> @@ -1024,9 +1025,9 @@ class paths(object):
>>   class path(object):
>>       """Represents an individual path and its configuration."""
>>
>>       def __init__(self, name, url=None, local=None, pushurl=None,
>> -                 pushrevset=None):
>> +                 pushrevset=None, pushnames=None):
>>           """Construct a path from its config options.
>>
>>           The primary URL for the path is defined as either a URL via ``url``
>>           (preferred) or from a local, relative filesystem path (``local``).
>> @@ -1048,5 +1049,11 @@ class path(object):
>>               raise util.Abort(_('pushurl may not have #revision fragment: %s') %
>>                                  pushurl)
>>
>>           self.pushurl = pushurl or url
>> +
>> +        if pushrevset and pushnames:
>> +            raise util.Abort(_('only 1 of pushrevset and pushnames may be '
>> +                               'defined on path: %s') % name)
>> +
>>           self.pushrevset = pushrevset
>> +        self.pushnames = pushnames
>> diff --git a/tests/test-push-path-config.t b/tests/test-push-path-config.t
>> --- a/tests/test-push-path-config.t
>> +++ b/tests/test-push-path-config.t
>> @@ -131,4 +131,60 @@ A pushrevset with multiple branches conf
>>     remote: adding changesets
>>     remote: adding manifests
>>     remote: adding file changes
>>     remote: added 2 changesets with 2 changes to 1 files
>> +
>> +pushrevset and pushnames are mutually exclusive
>> +
>> +  $ hg --config paths.pushrevset1.pushnames=foo push pushrevset1
>> +  abort: only 1 of pushrevset and pushnames may be defined on path: pushrevset1
>> +  [255]
>> +
>> +adding bookmarks to pushnames will push bookmarks
>> +
>> +  $ cat >> .hg/hgrc << EOF
>> +  > pushbookmark = ssh://user@dummy/$TESTTMP/server
>> +  > pushbookmark.pushnames = bookmark1
>> +  > EOF
>> +
>> +  $ hg -q up -r 0
>> +  $ echo bm1_1 > foo
>> +  $ hg commit -m 'bookmark 1 commit 1'
>> +  created new head
>> +  $ echo bm1_2 > foo
>> +  $ hg commit -m 'bookmark 1 commit 2'
>> +  $ hg bookmark -r ce2028409ac8 bookmark1
>> +  $ hg -q up -r 0
>> +  $ echo bm2 > foo
>> +  $ hg commit -m 'bookmark 2'
>> +  created new head
>> +  $ hg bookmark bookmark2
>> +
>> +  $ hg push pushbookmark
>> +  pushing to ssh://user@dummy/$TESTTMP/server
>> +  searching for changes
>> +  remote: adding changesets
>> +  remote: adding manifests
>> +  remote: adding file changes
>> +  remote: added 1 changesets with 1 changes to 1 files (+1 heads)
>> +  exporting bookmark bookmark1
>> +
>> +Specifying -B overrides pushnames
>> +
>> +  $ hg bookmark -r a41d7b167990 bookmark1
>> +  moving bookmark 'bookmark1' forward from ce2028409ac8
>> +
>> +  $ hg push -B bookmark2 pushbookmark
>> +  pushing to ssh://user@dummy/$TESTTMP/server
>> +  searching for changes
>> +  remote: adding changesets
>> +  remote: adding manifests
>> +  remote: adding file changes
>> +  remote: added 1 changesets with 1 changes to 1 files (+1 heads)
>> +  exporting bookmark bookmark2
>> +
>> +Specifying an unknown name is currently unsupported
>> +
>> +  $ hg --config paths.pushbookmark.pushnames=unknown push pushbookmark
>> +  abort: non-bookmarks not currently supported by pushnames path option
>> +  [255]
>> +
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

On Sat, Mar 14, 2015 at 12:38 AM, Ryan McElroy <rm@fb.com> wrote:

> On 3/3/2015 11:39 AM, Augie Fackler wrote:
>
>> On Sun, Mar 01, 2015 at 03:23:48PM -0800, Gregory Szorc wrote:
>>
>>> # HG changeset patch
>>> # User Gregory Szorc <gregory.szorc@gmail.com>
>>> # Date 1423550715 28800
>>> #      Mon Feb 09 22:45:15 2015 -0800
>>> # Node ID 96836adc7bfd1a1b41befc2cbbcc135fc35b7948
>>> # Parent  4da24a2a9e7b82e5cd0082e4d69255c0cd5b805b
>>> ui: add a "pushnames" path option
>>>
>> I think I'm +0 on this in principle, but we should probably coordinate
>> between this and the workflow ideas smf and Ryan have around
>> remotenames. I haven't done enough thinking to have strong convictions
>> yet.
>>
>
> Gregory, have you played around with the latest remotenames extension?
> There's this interesting/controversial/etc "feature" called tracking,
> inspired by git, where a bookmark can "track" another name (usually a
> remote bookmark, but could also be local, or other names exposed via the
> namespace API), so that a few commands just "do the right thing" when
> called without parameters:
>
> "hg rebase" rebases with an implicit "--dest <tracking name>"
> "hg push" pushes with an implicit "dest" path and updates an implicit
> "--to" bookmark
>
> Today, while chatting with smf, I realized we can go one step further and
> allow the push tracking to be different from the rebase tracking. This idea
> was inspired by the workflow Sean and I have adopted:
>
> sean's laptop ---(push)---> smf.io ---(pull)--> ryan's devserver
> ---(push)---> "nexus" (hg.silverfir.net) ---(pull)---> sean's laptop ...
>
> In this world, on my devserver, I want to rebase onto the changes from
> smf.io's @ bookmark, but I want to push to nexus and the @ bookmark
> there. If we have two tracking targets, we can do this. I'm not convinced
> this is a good idea yet, since it seems too complicated, but we're in
> exploratory mode here so I might implement this at some point.
>
> Anyway, back to this patch: I think it's useful -- it's essentially moving
> command-line options into config to avoid repeated typing and potential
> mistakes, which makes me think this is good, but it also isn't a
> fundamentally different workflow, like remotenames is attempting to unlock
> (eg the --to flag on push allows pushing to a remote bookmark without the
> same bookmark -- or any bookmark for that matter -- being present in the
> local repo for that rev).
>
> I'd call it 'pushbookmark' and restrict to to the equivalent of passing -B
> <name> to hg push. The other stuff can come later when it becomes more
> clear what direction we want to go.
>

Yes, I've been following the remotenames extension and discussions with
extreme interest. I'm even a user!

I think I already said that I would drop this patch from the eventual
non-RFC series because it is unclear how it will interact with remotenames
and it would be best to have a better story before adding this feature to
core.