[PATCH RFC] repo: add an ability to hide nodes in an appropriate way
Jun Wu
quark at fb.com
Mon Mar 27 13:58:01 EDT 2017
Excerpts from Augie Fackler's message of 2017-03-27 13:24:22 -0400:
> On Mon, Mar 27, 2017 at 10:27:33AM +0200, Pierre-Yves David wrote:
> >
> >
> > On 03/27/2017 12:32 AM, Kostia Balytskyi wrote:
> > > # HG changeset patch
> > > # User Kostia Balytskyi <ikostia at fb.com>
> > > # Date 1490567500 25200
> > > # Sun Mar 26 15:31:40 2017 -0700
> > > # Node ID 43cd56f38c1ca2ad1920ed6804e1a90a5e263c0d
> > > # Parent c60091fa1426892552dd6c0dd4b9c49e3c3da045
> > > repo: add an ability to hide nodes in an appropriate way
> > >
> > > Potentially frequent usecase is to hide nodes in the most appropriate
> > > for current repo configuration way. Examples of things which use this
> > > are rebase (strip or obsolete old nodes), shelve (strip of obsolete
> > > temporary nodes) and probably others.
> > > Jun Wu had an idea of having one place in core where this functionality
> > > is implemented so that others can utilize it. This patch
> > > implements this idea.
> >
> > I do not think this is a step in the right direction, creating obsolescence
> > marker is not just about hiding changeset.
>
> But we're using them for that today, all over the place. We're also
> having pretty productive (I think!) discussions about non-obsmarker
> based mechanisms for hiding things that are implementation details of
> a sort (amend changesets that get folded immediately, shelve changes,
> etc).
>
> > It is about recording the
> > evolution of time of changesets (predecessor ← [successors]). This data is
> > used in multiple place for advance logic, exchanged across repository etc.
> > Being too heavy handed on stripping will have problematic consequence for
> > the user experiences. Having an easy interfaces to do such heavy handed
> > pruning will encourage developer do to such mistake.
>
> I'm struggling a bit to understand your argument here. I think your
> argument is this: hiding changes can be done for many reasons, and
> having a simple "just hide this I don't care how" API means people
> won't think before they do the hiding. Is that an accurate
> simplification and restating of your position here? (If no, please try
> and restate with more nuance.)
>
> > The fact this function only takes a list of nodes (instead of (pec, suc)
> > mapping) is a good example of such simplification. Series from Jun about
> > histedit is another. Obsolescence markers are not meant as a generic
> > temporary hidden mechanism for local/temporary nodes and should not be used
> > that way.
>
> In broad strokes, I agree here. Have we made any conclusive progress
> on a mechanism for archiving changes? The reason we're seeing all
> these "abuses" of obsolete markers is that they're currently the only
> append-only means by which things can be discarded, and there are (as
> you know) ugly cache implications to running strip.
>
> (I'm sort of +0 on this series, but I'd be more enthusiastic about
> having a proper hiding mechanism in place that isn't obsmarkers before
> we go too far down this road.)
There are 2 issues:
1. Hide changeset A locally, and the prune marker could be exchanged. But
we don't want to exchange it.
That is solvable by [1] today, which seems to be pretty straightfoward.
It is also solvable by root-based hidden, which won't happen soon.
2. Hide changeset A locally, and later get A from a "pull". "A" remains
unexpectedly hidden.
This is *not* solved by root-based hidden. But the "node versions"
approach will solve it. I think this is an annoying limitation of
evolve today. evolve does not solve it, but avoids it at all costs (by
using workarounds like _source and strip).
[1]: https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-March/095687.html
>
> >
> > > 'insistonstripping' is necessary for cases when caller might need
> > > to enforce or not enforce stripping depending on some configuration
> > > (like shelve needs now).
> > >
> > > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> > > --- a/mercurial/localrepo.py
> > > +++ b/mercurial/localrepo.py
> > > @@ -50,6 +50,7 @@ from . import (
> > > phases,
> > > pushkey,
> > > pycompat,
> > > + repair,
> > > repoview,
> > > revset,
> > > revsetlang,
> > > @@ -1884,6 +1885,23 @@ class localrepository(object):
> > > # tag cache retrieval" case to work.
> > > self.invalidate()
> > >
> > > + def hidenodes(self, nodes, insistonstripping=False):
> > > + '''Remove nodes from visible repoview in the most appropriate
> > > + supported way. When obsmarker creation is enabled, this
> > > + function will obsolete these nodes without successors
> > > + (unless insistonstripping is True). Otherwise, it will
> > > + strip nodes.
> > > +
> > > + In future we can add other node-hiding capabilities to this
> > > + function.'''
> > > + with self.lock():
> > > + if obsolete.isenabled(self, obsolete.createmarkersopt)\
> > > + and not insistonstripping:
> > > + relations = [(self[n], ()) for n in nodes]
> > > + obsolete.createmarkers(self, relations)
> > > + else:
> > > + repair.strip(self.ui, self, nodes, backup=False)
> > > +
> > > def walk(self, match, node=None):
> > > '''
> > > walk recursively through the directory tree or a given
> > > diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
> > > --- a/tests/test-obsolete.t
> > > +++ b/tests/test-obsolete.t
> > > @@ -1259,3 +1259,43 @@ Test the --delete option of debugobsolet
> > > 1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'user': 'test'} (re)
> > > $ cd ..
> > >
> > > +Test that repo.hidenodes respects obsolescense configs
> > > + $ hg init hidenodesrepo && cd hidenodesrepo
> > > + $ cat <<EOF > debughidenodes.py
> > > + > from __future__ import absolute_import
> > > + > from mercurial import (
> > > + > cmdutil,
> > > + > )
> > > + > cmdtable = {}
> > > + > command = cmdutil.command(cmdtable)
> > > + > @command('debughidenodes',
> > > + > [('', 'insiststrip', None, 'pass insistonstripping=True')])
> > > + > def debughidenodes(ui, repo, *args, **opts):
> > > + > nodes = [repo[n].node() for n in args]
> > > + > repo.hidenodes(nodes, insistonstripping=bool(opts.get('insiststrip')))
> > > + > EOF
> > > + $ cat <<EOF > .hg/hgrc
> > > + > [extensions]
> > > + > debughidenodes=debughidenodes.py
> > > + > EOF
> > > + $ echo a > a
> > > + $ hg add a && hg ci -m a && hg up null
> > > + 0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> > > + $ hg --config experimental.evolution=! debughidenodes 0
> > > + $ hg log -qr "all()" --hidden | wc -l # commit was actually stripped
> > > + \s*0 (re)
> > > + $ hg debugobsolete | wc -l # no markers were created
> > > + \s*0 (re)
> > > + $ echo a > a && hg add a && hg ci -m a && hg up null
> > > + 0 files updated, 0 files merged, 1 files removed, 0 files unresolved
> > > + $ hg --config experimental.evolution=createmarkers debughidenodes 0
> > > + $ hg log -qr "all()" --hidden | wc -l # commit was not stripped
> > > + \s*1 (re)
> > > + $ hg debugobsolete | wc -l # a marker was created
> > > + \s*1 (re)
> > > + $ hg --config experimental.evolution=createmarkers --hidden debughidenodes 0 --insiststrip
> > > + $ hg log -qr "all()" --hidden | wc -l # commit was actually stripped
> > > + \s*0 (re)
> > > + $ hg debugobsolete | wc -l # no new markers were created
> > > + \s*1 (re)
> > > + $ cd ..
> > > _______________________________________________
> > > Mercurial-devel mailing list
> > > Mercurial-devel at mercurial-scm.org
> > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> > >
> >
> > --
> > Pierre-Yves David
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
More information about the Mercurial-devel
mailing list