[PATCH 2 of 2 V2] strip: take branch into account when selecting update target (issue5540)

Yuya Nishihara yuya at tcha.org
Mon Oct 9 09:40:37 EDT 2017


On Mon, 9 Oct 2017 13:08:29 +0200, Paul Morelle wrote:
> On 10/08/2017 08:02 AM, Yuya Nishihara wrote:
> >>> +    current_branch = repo[None].branch()
> >>>      if (util.safehasattr(repo, 'mq') and p2 != nullid
> >>>          and p2 in [x.node for x in repo.mq.applied]):
> >>>          urev = p2
> >>> +    elif current_branch != repo[urev].branch():
> >>> +        revset = "(parents(%ln::parents(wdir())) -  
> >>> %ln::parents(wdir()))" \
> >>> +                 + " and branch(%s)"
> >>> +        branch_targets = repo.revs(revset, revs, revs, current_branch)
> >>> +        if branch_targets:
> >>> +            cl = repo.changelog
> >>> +            urev = min(cl.node(r) for r in branch_targets)
> >> Should this be max() instead of min(), to get the node closest to wdir?
> > or just 'max(parents(%ln::...)'. Sorting sha1 hash doesn't make sense.
> I agree with the fact that sorting on sha1 hashes doesn't really make sense.
> However, this reproduces the same (mis-)behavior as the replaced code
> https://www.mercurial-scm.org/repo/hg/file/tip/hgext/strip.py#l199,
> which takes the first item of the sorted array, hence the min.
> > Perhaps, destutil.destupdate() will give more hints about tricky cases. I
> > don't know how "hg prune" resolve the update destination, but I think that
> > would be similar to what "hg strip" should do.
> destutil.destupdate() is about updating to descendants while here we are
> updating to ancestors, so we can't reuse it.

I meant destupdate() could provide hints to determine the desired behavior,
and some corner cases which we might have to take care. IIUC, we roughly need
a destupdate() equivalent which doesn't see the revisions to be stripped.

> "hg prune" does nothing smart about branches, and has the same issue as
> the current strip code (and we should probably reuse the new strip code
> for prune too).
> 
> Since using the highest revision number seems more appropriate, does
> everyone agree to use a max in a V3?

+1, and Mads said it might be more useful, in ff2c89ebf5d4.


More information about the Mercurial-devel mailing list