[PATCH RFC] repo: add an ability to hide nodes in an appropriate way

Augie Fackler raf at durin42.com
Mon Mar 27 13:25:26 EDT 2017


On Sun, Mar 26, 2017 at 04:44:11PM -0700, Jun Wu wrote:
> I think this is a good direction. See inline comments.
>
> Not related directly to this patch, but I was thinking introducing some
> "post-transaction" mechanism for transaction. So the caller won't need to
> put the "strip" outside a transaction. Compare:
>
> Currently:
>
>     if obsmarker-enabled:
>         with repo.transaction():
>             ....
>             create commits
>             create markers for strip # ideally inside the transaction
>     else:
>         with repo.transaction():
>             create commits
>         strip # must be outside a transaction
>
> If we have "post-transaction" for repair.strip. The code could be written in
> a cleaner form without the "if", and there is only one transaction as
> expected.

This seems like a good thing to have in a world where we expect strip
to continue to be a thing we have to use, but I'd rather the
engineering energy that could go into this API instead went into a
safer append-only hiding mechanism.

>
> Excerpts from Kostia Balytskyi's message of 2017-03-26 15:32:55 -0700:
> > # 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.
> >
> > 'insistonstripping' is necessary for cases when caller might need
> > to enforce or not enforce stripping depending on some configuration
> > (like shelve needs now).
>
> That sounds against the motivation of introducing the method. Could the
> caller just call repair.strip, or disable createmarkers (using
> configoverride) if it insists to?
>
> >
> > 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):
>
> Since it may call "strip", "hide" is not accurate. Maybe just call it
> "strip".
>
> > +        '''Remove nodes from visible repoview in the most appropriate
>
> Since strip won't support pruning a commit in the middle, and root-based
> hidden won't support that either. Maybe make it clear nodes' descendants
> will also be affected.
>
> > +        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():
>
> Then select "nodes::" here to get their descendants hidden.
>
> > +            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


More information about the Mercurial-devel mailing list