Re: [PATCH 4 of 8] template: add minimal obsfate template function

[Yuya Nishihara]

On Thu, 17 Aug 2017 16:43:11 +0200, Boris Feld wrote:
> On Wed, 2017-08-09 at 22:42 +0900, Yuya Nishihara wrote:
> > In which case, {successorsset} could populate markers by makemap(),
> > 
> >   successorssets % "{successorset} {obsmarkers}"
> >    
> >    no idea how this should be rendered,
> > but
> >    let's revisit it later.
> 
> I tried modifying successorssets to smuggle the markers via makemap but
> it has one limitation.
> 
> In the case of pruned commits, we would have an empty successorsset but
> at least one marker (the marker for pruning). Adding markers via
> makemap would make the following condition: if(succsandmarkers, ...)
> false for pruned markers.
> 
> I propose to keep succsandmarkers but hex all node ids so we would
> never see binary information.

Good point. I have no better idea right now then, though I think
{succsandmarkers} exposes the implementation detail which a typical user
shouldn't have to understand.

> > and perhaps obsfate could take the pair explicitly or implicitly.
> > 
> >   successorsets % "{obsfate(successorset, obsmarkers)}"  # a function
> >   successorsets % "{obsfate}"  # a keyword available only in
> > successorsets
> > 
> > obsfate could be even split to verb/users/mindate/maxdate.
> > 
> >   successorsets % "{obsverb} {obsusers} ..."
> >    
> >  these keyword functions may be populated by
> > makemap(), too.
> 
> I tried splitting obsfate into several functions and it looks nicer. I
> prefer functions instead of keywords because keywords feels too magic,

Perhaps that depends on how useful the function interface will be. Can we
pass another pair of successorset and obsmarkers which isn't generated by
{succsandmarkers} ?

> do we have already some keywords that are similar (not taking context
> as inputs) in core?

There are a few examples of not depending on the current context (e.g.
termwidth), but they aren't similar to the obsfate stuff.

> I have a rough series splitting obsfate(succsandmarkers) into
> obsfateverb(successorset, markers), obsfateusers(successorset,
> markers), ... Should I finish it and send a V2?

Well, since we don't seem to have any better idea, and these features are
experimental, it's probably a good move to start with that.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 4 of 8] template: add minimal obsfate template function

[Boris Feld]

On Wed, 2017-08-09 at 22:42 +0900, Yuya Nishihara wrote:
> On Tue, 08 Aug 2017 18:48:08 +0200, Boris Feld wrote:
> > On Wed, 2017-08-09 at 00:19 +0900, Yuya Nishihara wrote:
> > > On Mon, 07 Aug 2017 16:56:23 +0200, Boris Feld wrote:
> > > > +@templatekeyword("succsandmarkers")
> > > > +def showsuccsandmarkers(repo, ctx, **args):
> > > > +"""Returns a list of dict for each final successor of ctx.
> > > > +
> > > > +The dict contains successors node id in "successors" keys
> > > > and
> > > > the list of
> > > > +obs-markers from ctx to the set of successors in "markers"
> > > > +"""
> > > > +
> > > > +values = obsutil.successorsandmarkers(repo, ctx)
> > > > +
> > > > +if values is None:
> > > > +values = []
> > > > +
> > > > +return showlist('succsandmarkers', values, args)
> > > 
> > > I think returning a list of successor nodes is more natural.
> > > 
> > > Can we theoretically gather the relevant markers from successor
> > > nodes?
> > > If the "markers" field serves just as a cache, it could be stored
> > > in
> > > revcache[] and passed to obsfate() under the hood.
> > 
> > The current algorithm for computing the successors sets of a
> > changeset
> > is this one:
> > 
> > Walk the obs-marker graph starting at a given revision. Follow
> > successors of each revisions walked until revisions has no more
> > successors, they are stable and they are the tip-most successor of
> > the
> > initial revision.
> > 
> > Having the sets of successors doesn't helps because a successor
> > could
> > be successor for more than one obsolete revision (fold for
> > example).
> 
> Thanks for clarifying that. So markers are tied to each successorset?
> 
>   successorssets(rev) -> [(succs0, markers0), (succs1, markers1),
> ...]

Yes markers are tied to each successorset.

> 
> In which case, {successorsset} could populate markers by makemap(),
> 
>   successorssets % "{successorset} {obsmarkers}"
>    
>    no idea how this should be rendered,
> but
>    let's revisit it later.

I tried modifying successorssets to smuggle the markers via makemap but
it has one limitation.

In the case of pruned commits, we would have an empty successorsset but
at least one marker (the marker for pruning). Adding markers via
makemap would make the following condition: if(succsandmarkers, ...)
false for pruned markers.

I propose to keep succsandmarkers but hex all node ids so we would
never see binary information.

> 
> and perhaps obsfate could take the pair explicitly or implicitly.
> 
>   successorsets % "{obsfate(successorset, obsmarkers)}"  # a function
>   successorsets % "{obsfate}"  # a keyword available only in
> successorsets
> 
> obsfate could be even split to verb/users/mindate/maxdate.
> 
>   successorsets % "{obsverb} {obsusers} ..."
>    
>  these keyword functions may be populated by
> makemap(), too.

I tried splitting obsfate into several functions and it looks nicer. I
prefer functions instead of keywords because keywords feels too magic,
do we have already some keywords that are similar (not taking context
as inputs) in core?

I have a rough series splitting obsfate(succsandmarkers) into
obsfateverb(successorset, markers), obsfateusers(successorset,
markers), ... Should I finish it and send a V2?
> 
> > The list of markers between a revisions and its successors could be
> > cached, I'm not sure how to use revcache for that. Did you meant
> > returning a hybrid object like this one, with a non-null revcache
> > parameter: https://www.mercurial-scm.org/repo/hg/file/tip/mercurial
> > /tem
> > platekw.py#l641?
> 
> Something like showlatesttag() or showfile*() was in mind, but
> "revcache"
> seems not the right tool as the markers aren't bound to the current
> rev.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 4 of 8] template: add minimal obsfate template function

[Yuya Nishihara]

On Tue, 08 Aug 2017 18:48:08 +0200, Boris Feld wrote:
> On Wed, 2017-08-09 at 00:19 +0900, Yuya Nishihara wrote:
> > On Mon, 07 Aug 2017 16:56:23 +0200, Boris Feld wrote:
> > > +@templatekeyword("succsandmarkers")
> > > +def showsuccsandmarkers(repo, ctx, **args):
> > > +"""Returns a list of dict for each final successor of ctx.
> > > +
> > > +The dict contains successors node id in "successors" keys and
> > > the list of
> > > +obs-markers from ctx to the set of successors in "markers"
> > > +"""
> > > +
> > > +values = obsutil.successorsandmarkers(repo, ctx)
> > > +
> > > +if values is None:
> > > +values = []
> > > +
> > > +return showlist('succsandmarkers', values, args)
> > 
> > I think returning a list of successor nodes is more natural.
> > 
> > Can we theoretically gather the relevant markers from successor
> > nodes?
> > If the "markers" field serves just as a cache, it could be stored in
> > revcache[] and passed to obsfate() under the hood.
> 
> The current algorithm for computing the successors sets of a changeset
> is this one:
> 
> Walk the obs-marker graph starting at a given revision. Follow
> successors of each revisions walked until revisions has no more
> successors, they are stable and they are the tip-most successor of the
> initial revision.
> 
> Having the sets of successors doesn't helps because a successor could
> be successor for more than one obsolete revision (fold for example).

Thanks for clarifying that. So markers are tied to each successorset?

  successorssets(rev) -> [(succs0, markers0), (succs1, markers1), ...]

In which case, {successorsset} could populate markers by makemap(),

  successorssets % "{successorset} {obsmarkers}"
   
   no idea how this should be rendered, but
   let's revisit it later.

and perhaps obsfate could take the pair explicitly or implicitly.

  successorsets % "{obsfate(successorset, obsmarkers)}"  # a function
  successorsets % "{obsfate}"  # a keyword available only in successorsets

obsfate could be even split to verb/users/mindate/maxdate.

  successorsets % "{obsverb} {obsusers} ..."
   
 these keyword functions may be populated by makemap(), too.

> The list of markers between a revisions and its successors could be
> cached, I'm not sure how to use revcache for that. Did you meant
> returning a hybrid object like this one, with a non-null revcache
> parameter: https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/tem
> platekw.py#l641?

Something like showlatesttag() or showfile*() was in mind, but "revcache"
seems not the right tool as the markers aren't bound to the current rev.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 4 of 8] template: add minimal obsfate template function

[Boris Feld]

On Wed, 2017-08-09 at 00:19 +0900, Yuya Nishihara wrote:
> On Mon, 07 Aug 2017 16:56:23 +0200, Boris Feld wrote:
> > # HG changeset patch
> > # User Boris Feld 
> > # Date 1501850856 -7200
> > #  Fri Aug 04 14:47:36 2017 +0200
> > # Node ID a96edc5bcdc8790719e003eefff91a4f656cc559
> > # Parent  e3f0339b83553039dcd87b62dc1dfbdf98548792
> > # EXP-Topic obsfatetemplate
> > template: add minimal obsfate template function
> 
> (Only reviewed the template stuff, since I haven't got so involved in
> obsolete
> naming, the operation metadata, etc.)
> 
> > +def successorsandmarkers(repo, ctx):
> > +"""compute the raw data needed for computing obsfate
> > +Returns a list of dict, one dict per successors set
> > +"""
> > +if not ctx.obsolete():
> > +return None
> > +
> > +ssets = successorssets(repo, ctx.node(), closest=True)
> > +
> > +values = []
> > +for sset in ssets:
> > +values.append({'successors': sset, 'markers':
> > sset.markers})
> > +
> > +return values
> > diff -r e3f0339b8355 -r a96edc5bcdc8 mercurial/templatekw.py
> > --- a/mercurial/templatekw.py   Mon Jul 03 03:27:58 2017 +0200
> > +++ b/mercurial/templatekw.py   Fri Aug 04 14:47:36 2017 +0200
> > @@ -655,6 +655,21 @@
> >  return _hybrid(gen(data), data, lambda x: {'successorset': x},
> > lambda d: d["successorset"])
> >  
> > +@templatekeyword("succsandmarkers")
> > +def showsuccsandmarkers(repo, ctx, **args):
> > +"""Returns a list of dict for each final successor of ctx.
> > +
> > +The dict contains successors node id in "successors" keys and
> > the list of
> > +obs-markers from ctx to the set of successors in "markers"
> > +"""
> > +
> > +values = obsutil.successorsandmarkers(repo, ctx)
> > +
> > +if values is None:
> > +values = []
> > +
> > +return showlist('succsandmarkers', values, args)
> 
> I think returning a list of successor nodes is more natural.
> 
> Can we theoretically gather the relevant markers from successor
> nodes?
> If the "markers" field serves just as a cache, it could be stored in
> revcache[] and passed to obsfate() under the hood.

The current algorithm for computing the successors sets of a changeset
is this one:

Walk the obs-marker graph starting at a given revision. Follow
successors of each revisions walked until revisions has no more
successors, they are stable and they are the tip-most successor of the
initial revision.

Having the sets of successors doesn't helps because a successor could
be successor for more than one obsolete revision (fold for example).

The list of markers between a revisions and its successors could be
cached, I'm not sure how to use revcache for that. Did you meant
returning a hybrid object like this one, with a non-null revcache
parameter: https://www.mercurial-scm.org/repo/hg/file/tip/mercurial/tem
platekw.py#l641?

> 
> > +@templatefunc('obsfate(succsandmarkers)')
> > +def obsfate(context, mapping, args):
> > +""" Compute obsfate related information based on successors
> > and markers
> > +"""
> > +data = args[0][0](context, mapping, args[0][1])
> > +data = obsutil.computeobsfate(data['successors'],
> > data['markers'])
> 
> It has to check the number and types of the arguments. And please use
> evalfuncarg().

Thank you, I was looking for a cleaner way to do it. I will send a V2
after I fixed the return value of showsuccsandmarkers. 

I will send a separate series to clean join, I used it as example.

> 
> > +_hybrid = templatekw._hybrid
> > +
> > +# Format the successors for displaying
> > +succs = _hybrid(None, data['successors'],
> > +lambda x: {'ctx': mapping['repo'][x],
> > 'revcache': {}},
> > +lambda d: templatekw._formatrevnode(d['ctx']))
> > +data['successors'] = succs
> > +
> > +return _hybrid(None, [data], lambda x: x, ', ')
> 
> [...]
> 
> > +  |/ Obsfate: [{"markers":
> > [["\udca4h\u071b63\udc8b\u0014\udcfd\udcb7\udc82_U\udcce=\udcf4\udc
> > e7\u0015\u0017\udcad"
> 
> Ugh, binary in JSON. This is one reason why I think raw "markers"
> shouldn't
> be exposed.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 4 of 8] template: add minimal obsfate template function

[Yuya Nishihara]

On Mon, 07 Aug 2017 16:56:23 +0200, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld 
> # Date 1501850856 -7200
> #  Fri Aug 04 14:47:36 2017 +0200
> # Node ID a96edc5bcdc8790719e003eefff91a4f656cc559
> # Parent  e3f0339b83553039dcd87b62dc1dfbdf98548792
> # EXP-Topic obsfatetemplate
> template: add minimal obsfate template function

(Only reviewed the template stuff, since I haven't got so involved in obsolete
naming, the operation metadata, etc.)

> +def successorsandmarkers(repo, ctx):
> +"""compute the raw data needed for computing obsfate
> +Returns a list of dict, one dict per successors set
> +"""
> +if not ctx.obsolete():
> +return None
> +
> +ssets = successorssets(repo, ctx.node(), closest=True)
> +
> +values = []
> +for sset in ssets:
> +values.append({'successors': sset, 'markers': sset.markers})
> +
> +return values
> diff -r e3f0339b8355 -r a96edc5bcdc8 mercurial/templatekw.py
> --- a/mercurial/templatekw.py Mon Jul 03 03:27:58 2017 +0200
> +++ b/mercurial/templatekw.py Fri Aug 04 14:47:36 2017 +0200
> @@ -655,6 +655,21 @@
>  return _hybrid(gen(data), data, lambda x: {'successorset': x},
> lambda d: d["successorset"])
>  
> +@templatekeyword("succsandmarkers")
> +def showsuccsandmarkers(repo, ctx, **args):
> +"""Returns a list of dict for each final successor of ctx.
> +
> +The dict contains successors node id in "successors" keys and the list of
> +obs-markers from ctx to the set of successors in "markers"
> +"""
> +
> +values = obsutil.successorsandmarkers(repo, ctx)
> +
> +if values is None:
> +values = []
> +
> +return showlist('succsandmarkers', values, args)

I think returning a list of successor nodes is more natural.

Can we theoretically gather the relevant markers from successor nodes?
If the "markers" field serves just as a cache, it could be stored in
revcache[] and passed to obsfate() under the hood.

> +@templatefunc('obsfate(succsandmarkers)')
> +def obsfate(context, mapping, args):
> +""" Compute obsfate related information based on successors and markers
> +"""
> +data = args[0][0](context, mapping, args[0][1])
> +data = obsutil.computeobsfate(data['successors'], data['markers'])

It has to check the number and types of the arguments. And please use
evalfuncarg().

> +_hybrid = templatekw._hybrid
> +
> +# Format the successors for displaying
> +succs = _hybrid(None, data['successors'],
> +lambda x: {'ctx': mapping['repo'][x], 'revcache': {}},
> +lambda d: templatekw._formatrevnode(d['ctx']))
> +data['successors'] = succs
> +
> +return _hybrid(None, [data], lambda x: x, ', ')

[...]

> +  |/ Obsfate: [{"markers": 
> [["\udca4h\u071b63\udc8b\u0014\udcfd\udcb7\udc82_U\udcce=\udcf4\udce7\u0015\u0017\udcad"

Ugh, binary in JSON. This is one reason why I think raw "markers" shouldn't
be exposed.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 4 of 8] template: add minimal obsfate template function

[Boris Feld]

# HG changeset patch
# User Boris Feld 
# Date 1501850856 -7200
#  Fri Aug 04 14:47:36 2017 +0200
# Node ID a96edc5bcdc8790719e003eefff91a4f656cc559
# Parent  e3f0339b83553039dcd87b62dc1dfbdf98548792
# EXP-Topic obsfatetemplate
template: add minimal obsfate template function

The goal of this series is to have templates capable of displaying the
evolution of each changeset in a clean and human-readable way.

Add the succsandmarkers template return successors and markers so it can be
used isolated like:

> {succsandmarkers % "{get(succsandmarkers, "markers")|json};"}

Also add a template function obsfate that takes succsandmarkers as input and
compute various fields from the list of markers and successors. Additional
fields will be added in following patches.

diff -r e3f0339b8355 -r a96edc5bcdc8 mercurial/obsutil.py
--- a/mercurial/obsutil.py  Mon Jul 03 03:27:58 2017 +0200
+++ b/mercurial/obsutil.py  Fri Aug 04 14:47:36 2017 +0200
@@ -8,6 +8,7 @@
 from __future__ import absolute_import
 
 from . import (
+node as nodemod,
 phases,
 )
 
@@ -551,3 +552,45 @@
 final.reverse() # put small successors set first
 cache[current] = final
 return cache[initialnode]
+
+def computeobsfate(successorset, rawmarkers):
+""" For a successor set, get all related markers and convert every nodeid
+into its hexadecimal form.
+"""
+hex = nodemod.hex
+
+successorset = [hex(n) for n in successorset]
+
+# hex the binary nodes in the markers
+markers = []
+for m in rawmarkers:
+hexprec = hex(m[0])
+hexsucs = tuple(hex(n) for n in m[1])
+hexparents = None
+if m[5] is not None:
+hexparents = tuple(hex(n) for n in m[5])
+newmarker = (hexprec, hexsucs) + m[2:5] + (hexparents,) + m[6:]
+markers.append(newmarker)
+
+# Format basic data
+data = {
+"successors": sorted(successorset),
+"markers": sorted(markers)
+}
+
+return data
+
+def successorsandmarkers(repo, ctx):
+"""compute the raw data needed for computing obsfate
+Returns a list of dict, one dict per successors set
+"""
+if not ctx.obsolete():
+return None
+
+ssets = successorssets(repo, ctx.node(), closest=True)
+
+values = []
+for sset in ssets:
+values.append({'successors': sset, 'markers': sset.markers})
+
+return values
diff -r e3f0339b8355 -r a96edc5bcdc8 mercurial/templatekw.py
--- a/mercurial/templatekw.py   Mon Jul 03 03:27:58 2017 +0200
+++ b/mercurial/templatekw.py   Fri Aug 04 14:47:36 2017 +0200
@@ -655,6 +655,21 @@
 return _hybrid(gen(data), data, lambda x: {'successorset': x},
lambda d: d["successorset"])
 
+@templatekeyword("succsandmarkers")
+def showsuccsandmarkers(repo, ctx, **args):
+"""Returns a list of dict for each final successor of ctx.
+
+The dict contains successors node id in "successors" keys and the list of
+obs-markers from ctx to the set of successors in "markers"
+"""
+
+values = obsutil.successorsandmarkers(repo, ctx)
+
+if values is None:
+values = []
+
+return showlist('succsandmarkers', values, args)
+
 @templatekeyword('p1rev')
 def showp1rev(repo, ctx, templ, **args):
 """Integer. The repository-local revision number of the changeset's
diff -r e3f0339b8355 -r a96edc5bcdc8 mercurial/templater.py
--- a/mercurial/templater.pyMon Jul 03 03:27:58 2017 +0200
+++ b/mercurial/templater.pyFri Aug 04 14:47:36 2017 +0200
@@ -18,6 +18,7 @@
 encoding,
 error,
 minirst,
+obsutil,
 parser,
 pycompat,
 registrar,
@@ -849,6 +850,23 @@
 func = lambda a, b: a % b
 return runarithmetic(context, mapping, (func, args[0], args[1]))
 
+@templatefunc('obsfate(succsandmarkers)')
+def obsfate(context, mapping, args):
+""" Compute obsfate related information based on successors and markers
+"""
+data = args[0][0](context, mapping, args[0][1])
+data = obsutil.computeobsfate(data['successors'], data['markers'])
+
+_hybrid = templatekw._hybrid
+
+# Format the successors for displaying
+succs = _hybrid(None, data['successors'],
+lambda x: {'ctx': mapping['repo'][x], 'revcache': {}},
+lambda d: templatekw._formatrevnode(d['ctx']))
+data['successors'] = succs
+
+return _hybrid(None, [data], lambda x: x, ', ')
+
 @templatefunc('relpath(path)')
 def relpath(context, mapping, args):
 """Convert a repository-absolute path into a filesystem path relative to
diff -r e3f0339b8355 -r a96edc5bcdc8 tests/test-obsmarker-template.t
--- a/tests/test-obsmarker-template.t   Mon Jul 03 03:27:58 2017 +0200
+++ b/tests/test-obsmarker-template.t   Fri Aug 04 14:47:36 2017 +0200
@@ -20,6 +20,8 @@
   > {if(successorssets, "\n  Successors: {successorssets}")}\
   > {if(successorssets, "\n  multi-line: {join(successorssets, "\n  
multi-line: ")}")}\
   >