D1074: branch: add a --rev flag to change branch name of given revisions

[martinvonz (Martin von Zweigbergk)]

martinvonz added inline comments.

INLINE COMMENTS

> cmdutil.py:720
> +    with repo.wlock(), repo.lock(), repo.transaction('branches'):
> +        # abort incase in uncommitted merger of dirty wdir
> +        bailifchanged(repo)

"in case of uncommited merge or dirty wdir"? (note that there are 4 fixes there)

> cmdutil.py:722
> +        bailifchanged(repo)
> +        revs = scmutil.revrange(repo, revs)
> +        roots = repo.revs('roots(%ld)', revs)

may want to check if revs is empty (e.g. `hg branch -r 'draft() - all()' foo`)

> cmdutil.py:728
> +        root = repo[roots.first()]
> +        if root.phase() <= phases.public:
> +            raise error.Abort(_("cannot change branch of public revisions"))

maybe `if not root.mutable():`

> cmdutil.py:737-741
> +        for r in heads:
> +            if not repo[r].children():
> +                headcount += 1
> +        if headcount < 1:
> +            raise error.Abort(_("cannot change branch in middle of a stack"))

children() can be expensive because it iterates over all later revisions (in revlog order). So when you want to call children() for many revisions (especially old revisions), it can be better to do a single scan. This *might* be better:

  if repo.revs('(%ld::head()) - %ld', heads, heads):
    raise error.Abort(_("cannot change branch in middle of a stack"))

I'm not sure it will be, or if it's even correct, so please test (or just tell me that this command will be used rarely enough that no one cares).

> commands.py:1051-1056
>              if not opts.get('force') and label in repo.branchmap():
>                  if label not in [p.branch() for p in repo[None].parents()]:
>                      raise error.Abort(_('a branch of the same name already'
>                                         ' exists'),
>                                       # i18n: "it" refers to an existing branch
>                                       hint=_("use 'hg update' to switch to it"))

Does this need to be modified to work with --rev? I've never used `hg branch`, but it sounds like `hg branch foo` (without --force) is supposed to fail if `foo` is already taken unless one of the working directory parents already has that name. Should we do the corresponding check for --rev? If not, it seems like we should at least *not* do the check (in its current form) when --rev is given. If I'm doing `hg branch --rev deadbeef foo`, it doesn't seem relevant what the branch names in my working directory parents are. Would be good to add a test case or two for this too.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1074

To: pulkit, #hg-reviewers, dlax, ryanmce
Cc: martinvonz, durin42, krbullock, yuja, indygreg, lothiraldan, ryanmce, dlax, mercurial-devel