Patchwork [STABLE] backout 306fef8440af and e291c039d8ec

# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1396381841 25200
#      Tue Apr 01 12:50:41 2014 -0700
# Node ID 50eb1290a52c238a5a83b7cf9f3cefaad5d12a3c
# Parent  8a6a86c9a5b58ccc020de1ff0429e72dfa5599fc
backout 306fef8440af and e291c039d8ec

Those two changesets lets "bookmark" spill out of the local repo during pull and
push.

This is strong regression that costed hundreds of ingeneer hours.

I recommend urgent backout and unchedule bugfix release.

pierre-yves.david@ens-lyon.org writes:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1396381841 25200
> #      Tue Apr 01 12:50:41 2014 -0700
> # Node ID 50eb1290a52c238a5a83b7cf9f3cefaad5d12a3c
> # Parent  8a6a86c9a5b58ccc020de1ff0429e72dfa5599fc
> backout 306fef8440af and e291c039d8ec
>
> Those two changesets lets "bookmark" spill out of the local repo during pull and
> push.
>
> This is strong regression that costed hundreds of ingeneer hours.
>
> I recommend urgent backout and unchedule bugfix release.

This is a pretty awful regression, we might want to consider a hotfix
release.

On 04/01/2014 12:51 PM, pierre-yves.david@ens-lyon.org wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1396381841 25200
> #      Tue Apr 01 12:50:41 2014 -0700
> # Node ID 50eb1290a52c238a5a83b7cf9f3cefaad5d12a3c
> # Parent  8a6a86c9a5b58ccc020de1ff0429e72dfa5599fc
> backout 306fef8440af and e291c039d8ec
>
> Those two changesets lets "bookmark" spill out of the local repo during pull and
> push.
>
> This is strong regression that costed hundreds of ingeneer hours.

LGTM

>
> I recommend urgent backout and unchedule bugfix release.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -1227,13 +1227,10 @@ def clone(ui, source, dest=None, **opts)
>       their ancestors. These options (or 'clone src#rev dest') imply
>       --pull, even for local source repositories. Note that specifying a
>       tag will include the tagged changeset but not the changeset
>       containing the tag.
>   
> -    If the source repository has a bookmark called '@' set, that
> -    revision will be checked out in the new repository by default.
> -
>       To check out a particular version, use -u/--update, or
>       -U/--noupdate to create a clone with no working directory.
>   
>       .. container:: verbose
>   
> @@ -1265,11 +1262,10 @@ def clone(ui, source, dest=None, **opts)
>         c) the changeset specified with -u (if a branch name, this means the
>            latest head of that branch)
>         d) the changeset specified with -r
>         e) the tipmost head specified with -b
>         f) the tipmost head specified with the url#branch source syntax
> -      g) the revision marked with the '@' bookmark, if present
>         h) the tipmost head of the default branch
>         i) tip
>   
>         Examples:
>   
> @@ -4635,11 +4631,10 @@ def pull(ui, repo, source="default", **o
>       try:
>           ui.status(_('pulling from %s\n') % util.hidepassword(source))
>           revs, checkout = hg.addbranchrevs(repo, other, branches,
>                                             opts.get('rev'))
>   
> -        remotebookmarks = other.listkeys('bookmarks')
>   
>           if opts.get('bookmark'):
>               if not revs:
>                   revs = []
>               for b in opts['bookmark']:
> @@ -4654,38 +4649,28 @@ def pull(ui, repo, source="default", **o
>                   err = _("other repository doesn't support revision lookup, "
>                           "so a rev cannot be specified.")
>                   raise util.Abort(err)
>   
>           modheads = repo.pull(other, heads=revs, force=opts.get('force'))
> -        bookmarks.updatefromremote(ui, repo, remotebookmarks, source)
>           if checkout:
>               checkout = str(repo.changelog.rev(other.lookup(checkout)))
>           repo._subtoppath = source
>           try:
>               ret = postincoming(ui, repo, modheads, opts.get('update'), checkout)
>   
>           finally:
>               del repo._subtoppath
>   
> -        # update specified bookmarks
> -        if opts.get('bookmark'):
> -            marks = repo._bookmarks
> -            for b in opts['bookmark']:
> -                # explicit pull overrides local bookmark if any
> -                ui.status(_("importing bookmark %s\n") % b)
> -                marks[b] = repo[remotebookmarks[b]].node()
> -            marks.write()
>       finally:
>           other.close()
>       return ret
>   
>   @command('^push',
>       [('f', 'force', None, _('force push')),
>       ('r', 'rev', [],
>        _('a changeset intended to be included in the destination'),
>        _('REV')),
> -    ('B', 'bookmark', [], _("bookmark to push"), _('BOOKMARK')),
>       ('b', 'branch', [],
>        _('a specific branch you would like to push'), _('BRANCH')),
>       ('', 'new-branch', False, _('allow pushing a new branch')),
>       ] + remoteopts,
>       _('[-f] [-r REV]... [-e CMD] [--remotecmd CMD] [DEST]'))
> @@ -4714,30 +4699,16 @@ def push(ui, repo, dest=None, **opts):
>         almost always cause confusion for collaborators.
>   
>       If -r/--rev is used, the specified revision and all its ancestors
>       will be pushed to the remote repository.
>   
> -    If -B/--bookmark is used, the specified bookmarked revision, its
> -    ancestors, and the bookmark will be pushed to the remote
> -    repository.
> -
>       Please see :hg:`help urls` for important details about ``ssh://``
>       URLs. If DESTINATION is omitted, a default path will be used.
>   
>       Returns 0 if push was successful, 1 if nothing to push.
>       """
>   
> -    if opts.get('bookmark'):
> -        ui.setconfig('bookmarks', 'pushing', opts['bookmark'], 'push')
> -        for b in opts['bookmark']:
> -            # translate -B options to -r so changesets get pushed
> -            if b in repo._bookmarks:
> -                opts.setdefault('rev', []).append(b)
> -            else:
> -                # 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")
>   
>       dest = ui.expandpath(dest or 'default-push', dest or 'default')
>       dest, branches = hg.parseurl(dest, opts.get('branch'))
>       ui.status(_('pushing to %s\n') % util.hidepassword(dest))
>       revs, checkout = hg.addbranchrevs(repo, repo, branches, opts.get('rev'))
> @@ -4766,17 +4737,10 @@ def push(ui, repo, dest=None, **opts):
>       result = repo.push(other, opts.get('force'), revs=revs,
>                          newbranch=opts.get('new_branch'))
>   
>       result = not result
>   
> -    if opts.get('bookmark'):
> -        bresult = bookmarks.pushtoremote(ui, repo, other, opts['bookmark'])
> -        if bresult == 2:
> -            return 2
> -        if not result and bresult:
> -            result = 2
> -
>       return result
>   
>   @command('recover', [])
>   def recover(ui, repo):
>       """roll back an interrupted transaction
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -7,11 +7,11 @@
>   
>   from i18n import _
>   from node import hex, nullid
>   import errno
>   import util, scmutil, changegroup, base85
> -import discovery, phases, obsolete, bookmarks
> +import discovery, phases, obsolete
>   
>   
>   class pushoperation(object):
>       """A object that represent a single push operation
>   
> @@ -112,11 +112,10 @@ def push(repo, remote, force=False, revs
>                   lock.release()
>       finally:
>           if locallock is not None:
>               locallock.release()
>   
> -    _pushbookmark(pushop)
>       return pushop.ret
>   
>   def _pushdiscovery(pushop):
>       # discovery
>       unfi = pushop.repo.unfiltered()
> @@ -159,11 +158,10 @@ def _pushcheckoutgoing(pushop):
>                       raise util.Abort(mso % ctx)
>                   elif ctx.troubled():
>                       raise util.Abort(_(mst)
>                                        % (ctx.troubles()[0],
>                                           ctx))
> -        newbm = pushop.ui.configlist('bookmarks', 'pushing')
>           discovery.checkheads(unfi, pushop.remote, outgoing,
>                                pushop.remoteheads,
>                                pushop.newbranch,
>                                bool(pushop.incoming),
>                                newbm)
> @@ -351,30 +349,10 @@ def _pushobsolete(pushop):
>               rslts.append(remote.pushkey('obsolete', key, '', data))
>           if [r for r in rslts if not r]:
>               msg = _('failed to push some obsolete markers!\n')
>               repo.ui.warn(msg)
>   
> -def _pushbookmark(pushop):
> -    """Update bookmark position on remote"""
> -    ui = pushop.ui
> -    repo = pushop.repo.unfiltered()
> -    remote = pushop.remote
> -    ui.debug("checking for updated bookmarks\n")
> -    revnums = map(repo.changelog.rev, pushop.revs or [])
> -    ancestors = [a for a in repo.changelog.ancestors(revnums, inclusive=True)]
> -    (addsrc, adddst, advsrc, advdst, diverge, differ, invalid
> -     ) = bookmarks.compare(repo, repo._bookmarks, remote.listkeys('bookmarks'),
> -                           srchex=hex)
> -
> -    for b, scid, dcid in advsrc:
> -        if ancestors and repo[scid].rev() not in ancestors:
> -            continue
> -        if remote.pushkey('bookmarks', b, dcid, scid):
> -            ui.status(_("updating bookmark %s\n") % b)
> -        else:
> -            ui.warn(_('updating bookmark %s failed!\n') % b)
> -
>   class pulloperation(object):
>       """A object that represent a single pull operation
>   
>       It purpose is to carry push related state and very common operation.
>   
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel