[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