[PATCH V2] revert: add support for reverting subrepos

[Angel Ezquerra]

On Thu, Oct 6, 2011 at 5:48 PM, Angel Ezquerra <angel.ezquerra at gmail.com> wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra at gmail.com>
> # Date 1317712886 -7200
> # Node ID 48f9111208bfbbe831bb514bc133b854c3838d2d
> # Parent  6dc67dced8c122f6139ae20ccdc03a6b11e8b765
> revert: add support for reverting subrepos
>
> Reverting a subrepo is done by updating it to the revision that is selected on
> the parent repo .hgsubstate file.
>
> * ISSUES/TODO: - I've refactored the revert() function in commands.py to make
> this change less intrusive. I've moved most of the original revert function
> into a function called revertfiles(), except the option checking part of the
> code which is left in the revert() function, which will also call revertfiles()
> and a new function named revertsubrepos(). These and other helps functions
> should be moved to some other file? If so, where?
>
> - This version of this patch only allows reverting a subrepo if the --no-backup
> flag is used, since no backups are performed on the contents of the subrepo. It
> could be possible to add support for backing up the subrepo contents by first
> performing a "revert --all" on the subrepo, and then updating the subrepo to
> the proper revision.
>
> - The behavior of the --all flag has not been changed: The --all flag will not
> revert the state of the subrepos. This could be changed as well, but it is left
> for a later patch (if considered appropriate).
>
> - I'm calling update() on the subrepos while a wlock is active. I don't know if
> this is correct.
>
> - I've used ui.status() to show a message while the subrepos are being
> reverted. However TortoiseHg does not properly capture this message (it shows a
> dialog box rather than showing the message on its console).
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -4419,11 +4419,31 @@
>         raise util.Abort(_('uncommitted merge with no revision specified'),
>                          hint=_('use "hg update" or see "hg help revert"'))
>
> +    # Get the context of the target revision
>     ctx = scmutil.revsingle(repo, opts.get('rev'))
> -    node = ctx.node()
> -
> -    if not pats and not opts.get('all'):
> -        msg = _("no files or directories specified")
> +
> +    # reverting files and subrepos is a very different operation
> +    # separate subrepos from regular files from the pats list
> +    [filepats, subpats] = separatefilesandsubs(pats, ctx)
> +
> +    if subpats and not opts.get('no_backup'):
> +        # we cannot revert subrepos unless the no_backup flag is set!
> +        msg = _("cannot revert subrepos unless the no-backup flag is set")
> +        hint = _("there are subrepos on the revert list, "
> +            "use --no-backup to revert them")
> +        raise util.Abort(msg, hint=hint)
> +
> +    # We currently do not support reverting non mercurial subrepos, so check
> +    # that all subrepos on the subrepo list are mercurial subrepos
> +    for sname in subpats:
> +        stype = ctx.substate[sname][2]
> +        if stype != "hg":
> +            msg = _("cannot revert %s subrepo '%s'") % (stype, sname)
> +            hint = _("reverting %s repositories is not supported") % stype
> +            raise util.Abort(msg, hint=hint)
> +
> +    if not pats and not substate and not opts.get('all'):
> +        msg = _("no files, directories or subrepos specified")
>         if p2 != nullid:
>             hint = _("uncommitted merge, use --all to discard all changes,"
>                      " or 'hg update -C .' to abort the merge")
> @@ -4442,6 +4462,71 @@
>             hint = _("use --all to revert all files")
>         raise util.Abort(msg, hint=hint)
>
> +    # now we are ready to revert the files and subrepos on the list
> +    revertfiles(ui, repo, ctx, filepats, opts)
> +    revertsubrepos(ui, repo, ctx, subpats, opts)
> +
> +def separatefilesandsubs(pats, ctx):
> +    """
> +    Get a list of files, folders and subrepo names and a repository context and
> +    return two lists:
> +        - a list of files and folder names
> +        - a list of subrepo names
> +    """
> +
> +    if not ctx.substate:
> +        return pats, []
> +
> +    filepats = []
> +    subpats = []
> +    for p in pats:
> +        if p in ctx.substate:
> +            subpats.append(p)
> +        else:
> +            filepats.append(p)
> +
> +    return (filepats, subpats)
> +
> +def revertsubrepos(ui, repo, ctx, subs, opts):
> +    """
> +    Revert a list of subrepos to the revision given by a particular context
> +
> +    This function assumes that all the items on the subs list are valid
> +    mercurial subrepos on the target context
> +    """
> +    # we ignore the 'all' option when reverting subrepos
> +    # so we don't check it here
> +    if not subs:
> +        return
> +
> +    print dir(ctx)
> +    wlock = repo.wlock()
> +    try:
> +        # Revert the subrepos on the revert list
> +        # reverting a subrepo is done by updating it to the revision specified
> +        # in the corresponding substate dictionary
> +        for sname in subs:
> +            ui.status(_('s reverting suprepo %s\n') % sname)
> +            if not opts.get('dry_run'):
> +                substate = ctx.substate[sname]
> +                srepo = ctx.sub(sname)._repo
> +                update(ui, srepo,
> +                    node=ctx.substate[sname][1], clean=True)
> +    finally:
> +        wlock.release()
> +
> +def revertfiles(ui, repo, ctx, pats, opts):
> +    """
> +    Revert a list of files to the revision given by a particular context
> +
> +    Assume that the revert options are valid.
> +    """
> +    if not pats and not opts.get('all'):
> +        return
> +
> +    parent, p2 = repo.dirstate.parents()
> +    node = ctx.node()
> +
>     mf = ctx.manifest()
>     if node == parent:
>         pmf = mf
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>

In this second version of this patch I tried to follow Greg's advice
and made the patch easier to read (I hope!). The functionality is the
same as in the previous version.

One question that I have (and which I stated on the commit message) is
whether it would be best to move the revertfiles(), revertsubrepos()
and separatefilesandsubs() helper functions into some other file
(perhaps into hg.py, as Greg suggested)?

Cheers,

Angel