[PATCH 4 of 7] tags: make argument 'tagtype' optional in '_updatetags'

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490679550 -7200
#  Tue Mar 28 07:39:10 2017 +0200
# Node ID e49ee337ec51b64e440585d44e6c7df736164e98
# Parent  f0c93dd8d018c9f6828c97be8ccb80dbfca694b8
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r e49ee337ec51
tags: make argument 'tagtype' optional in '_updatetags'

This is the next step from the previous changesets, we are now ready to use this
function in a simpler way.

diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -223,15 +223,20 @@ def _readtags(ui, repo, lines, fn, recod
 newtags[tag] = (taghist[-1], taghist[:-1])
 return newtags
 
-def _updatetags(filetags, alltags, tagtype, tagtypes):
+def _updatetags(filetags, alltags, tagtype=None, tagtypes=None):
 '''Incorporate the tag info read from one file into the two
 dictionaries, alltags and tagtypes, that contain all tag
-info (global across all heads plus local).'''
+info (global across all heads plus local).
+
+The second dictionnary is optional.'''
+if tagtype is None:
+assert tagtypes is None
 
 for name, nodehist in filetags.iteritems():
 if name not in alltags:
 alltags[name] = nodehist
-tagtypes[name] = tagtype
+if tagtype is not None:
+tagtypes[name] = tagtype
 continue
 
 # we prefer alltags[name] if:
@@ -243,7 +248,7 @@ def _updatetags(filetags, alltags, tagty
 if (bnode != anode and anode in bhist and
 (bnode not in ahist or len(bhist) > len(ahist))):
 anode = bnode
-else:
+elif tagtype is not None:
 tagtypes[name] = tagtype
 ahist.extend([n for n in bhist if n not in ahist])
 alltags[name] = anode, ahist
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 7 of 7] tags: extract filenode filtering in it own function

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490675008 -7200
#  Tue Mar 28 06:23:28 2017 +0200
# Node ID dd6a04d26b611ac8d192f868daba734622e97528
# Parent  3d8a09214760799868b472b97e964e88f7ec8fd5
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r dd6a04d26b61
tags: extract filenode filtering in it own function

We'll also need to reuse this logic so we extract it in its own function. We
document some of the logic in the process.

diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -95,17 +95,10 @@ def findglobaltags(ui, repo):
 _updatetags(cachetags, alltags)
 return alltags
 
-seen = set()  # set of fnode
-fnodes = []
 for head in reversed(heads):  # oldest to newest
 assert head in repo.changelog.nodemap, \
"tag cache returned bogus head %s" % short(head)
-
-fnode = tagfnode.get(head)
-if fnode and fnode not in seen:
-seen.add(fnode)
-fnodes.append(fnode)
-
+fnodes = _filterfnodes(tagfnode, reversed(heads))
 alltags = _tagsfromfnodes(ui, repo, fnodes)
 
 # and update the cache (if necessary)
@@ -113,6 +106,20 @@ def findglobaltags(ui, repo):
 _writetagcache(ui, repo, valid, alltags)
 return alltags
 
+def _filterfnodes(tagfnode, nodes):
+"""return a list of unique fnodes
+
+The order of that list match the order "nodes". Preserving this order is
+important as reading tags in different order provides different results."""
+seen = set()  # set of fnode
+fnodes = []
+for no in nodes:  # oldest to newest
+fnode = tagfnode.get(no)
+if fnode and fnode not in seen:
+seen.add(fnode)
+fnodes.append(fnode)
+return fnodes
+
 def _tagsfromfnodes(ui, repo, fnodes):
 """return a tagsmap from a list of file-node
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 2 of 7] tags: do not feed dictionaries to 'findglobaltags'

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490674429 -7200
#  Tue Mar 28 06:13:49 2017 +0200
# Node ID 147b98bfa4afbaf608d9e1f5227a48a46e386ea4
# Parent  1e77e505e7bacf59d1200714dc92e827523d7301
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 147b98bfa4af
tags: do not feed dictionaries to 'findglobaltags'

The code assert that these dictionnary are empty anyway. So we can as well be
more explicit and have the function returns the dictionaries directly.

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -707,10 +707,11 @@ class localrepository(object):
 # be one tagtype for all such "virtual" tags?  Or is the status
 # quo fine?
 
-alltags = {}# map tag name to (node, hist)
-tagtypes = {}
 
-tagsmod.findglobaltags(self.ui, self, alltags, tagtypes)
+globaldata = tagsmod.findglobaltags(self.ui, self)
+alltags = globaldata[0]   # map tag name to (node, hist)
+tagtypes = globaldata[1]  # map tag name to tag type
+
 tagsmod.readlocaltags(self.ui, self, alltags, tagtypes)
 
 # Build the return dicts.  Have to re-encode tag names because
diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -78,23 +78,18 @@ from . import (
 # The most recent changeset (in terms of revlog ordering for the head
 # setting it) for each tag is last.
 
-def findglobaltags(ui, repo, alltags, tagtypes):
-'''Find global tags in a repo.
+def findglobaltags(ui, repo):
+'''Find global tags in a repo: return (alltags, tagtypes)
 
 "alltags" maps tag name to (node, hist) 2-tuples.
 
 "tagtypes" maps tag name to tag type. Global tags always have the
 "global" tag type.
 
-The "alltags" and "tagtypes" dicts are updated in place. Empty dicts
-should be passed in.
-
 The tags cache is read and updated as a side-effect of calling.
 '''
-# This is so we can be lazy and assume alltags contains only global
-# tags when we pass it to _writetagcache().
-assert len(alltags) == len(tagtypes) == 0, \
-   "findglobaltags() should be called first"
+alltags = {}
+tagtypes = {}
 
 (heads, tagfnode, valid, cachetags, shouldwrite) = _readtagcache(ui, repo)
 if cachetags is not None:
@@ -103,7 +98,7 @@ def findglobaltags(ui, repo, alltags, ta
 # cases where a global tag should outrank a local tag but won't,
 # because cachetags does not contain rank info?
 _updatetags(cachetags, 'global', alltags, tagtypes)
-return
+return alltags, tagtypes
 
 seen = set()  # set of fnode
 fctx = None
@@ -125,6 +120,7 @@ def findglobaltags(ui, repo, alltags, ta
 # and update the cache (if necessary)
 if shouldwrite:
 _writetagcache(ui, repo, valid, alltags)
+return alltags, tagtypes
 
 def readlocaltags(ui, repo, alltags, tagtypes):
 '''Read local tags in repo. Update alltags and tagtypes.'''
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 6 of 7] tags: extract tags computation from fnodes in its own function

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490674092 -7200
#  Tue Mar 28 06:08:12 2017 +0200
# Node ID 3d8a09214760799868b472b97e964e88f7ec8fd5
# Parent  d5c70d5f7de740d3fa946318998f0f0c1204e4eb
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3d8a09214760
tags: extract tags computation from fnodes in its own function

I'm about to introduce code that needs to perform such computation on
"arbitrary" node. The logic is extract in its own function for reuse.

diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -85,19 +85,18 @@ def findglobaltags(ui, repo):
 
 The tags cache is read and updated as a side-effect of calling.
 '''
-alltags = {}
-
 (heads, tagfnode, valid, cachetags, shouldwrite) = _readtagcache(ui, repo)
 if cachetags is not None:
 assert not shouldwrite
 # XXX is this really 100% correct?  are there oddball special
 # cases where a global tag should outrank a local tag but won't,
 # because cachetags does not contain rank info?
+alltags = {}
 _updatetags(cachetags, alltags)
 return alltags
 
 seen = set()  # set of fnode
-fctx = None
+fnodes = []
 for head in reversed(heads):  # oldest to newest
 assert head in repo.changelog.nodemap, \
"tag cache returned bogus head %s" % short(head)
@@ -105,19 +104,32 @@ def findglobaltags(ui, repo):
 fnode = tagfnode.get(head)
 if fnode and fnode not in seen:
 seen.add(fnode)
-if not fctx:
-fctx = repo.filectx('.hgtags', fileid=fnode)
-else:
-fctx = fctx.filectx(fnode)
+fnodes.append(fnode)
 
-filetags = _readtags(ui, repo, fctx.data().splitlines(), fctx)
-_updatetags(filetags, alltags)
+alltags = _tagsfromfnodes(ui, repo, fnodes)
 
 # and update the cache (if necessary)
 if shouldwrite:
 _writetagcache(ui, repo, valid, alltags)
 return alltags
 
+def _tagsfromfnodes(ui, repo, fnodes):
+"""return a tagsmap from a list of file-node
+
+tagsmap: tag name to (node, hist) 2-tuples.
+
+The order of the list matters."""
+alltags = {}
+fctx = None
+for fnode in fnodes:
+if fctx is None:
+fctx = repo.filectx('.hgtags', fileid=fnode)
+else:
+fctx = fctx.filectx(fnode)
+filetags = _readtags(ui, repo, fctx.data().splitlines(), fctx)
+_updatetags(filetags, alltags)
+return alltags
+
 def readlocaltags(ui, repo, alltags, tagtypes):
 '''Read local tags in repo. Update alltags and tagtypes.'''
 try:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 5 of 7] tags: only return 'alltags' in 'findglobaltags'

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490679683 -7200
#  Tue Mar 28 07:41:23 2017 +0200
# Node ID d5c70d5f7de740d3fa946318998f0f0c1204e4eb
# Parent  e49ee337ec51b64e440585d44e6c7df736164e98
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r d5c70d5f7de7
tags: only return 'alltags' in 'findglobaltags'

This is minor update along the way. We simplify the 'findglobaltags' function to
only return the tags. Since no existing data are reused, we know that all tags
are returned are global and we can let the caller get that information if it
cares about it.

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -708,9 +708,10 @@ class localrepository(object):
 # quo fine?
 
 
-globaldata = tagsmod.findglobaltags(self.ui, self)
-alltags = globaldata[0]   # map tag name to (node, hist)
-tagtypes = globaldata[1]  # map tag name to tag type
+# map tag name to (node, hist)
+alltags = tagsmod.findglobaltags(self.ui, self)
+# map tag name to tag type
+tagtypes = dict((tag, 'global') for tag in alltags)
 
 tagsmod.readlocaltags(self.ui, self, alltags, tagtypes)
 
diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -79,17 +79,13 @@ from . import (
 # setting it) for each tag is last.
 
 def findglobaltags(ui, repo):
-'''Find global tags in a repo: return (alltags, tagtypes)
+'''Find global tags in a repo: return a tagsmap
 
-"alltags" maps tag name to (node, hist) 2-tuples.
-
-"tagtypes" maps tag name to tag type. Global tags always have the
-"global" tag type.
+tagsmap: tag name to (node, hist) 2-tuples.
 
 The tags cache is read and updated as a side-effect of calling.
 '''
 alltags = {}
-tagtypes = {}
 
 (heads, tagfnode, valid, cachetags, shouldwrite) = _readtagcache(ui, repo)
 if cachetags is not None:
@@ -97,8 +93,8 @@ def findglobaltags(ui, repo):
 # XXX is this really 100% correct?  are there oddball special
 # cases where a global tag should outrank a local tag but won't,
 # because cachetags does not contain rank info?
-_updatetags(cachetags, alltags, 'global', tagtypes)
-return alltags, tagtypes
+_updatetags(cachetags, alltags)
+return alltags
 
 seen = set()  # set of fnode
 fctx = None
@@ -115,12 +111,12 @@ def findglobaltags(ui, repo):
 fctx = fctx.filectx(fnode)
 
 filetags = _readtags(ui, repo, fctx.data().splitlines(), fctx)
-_updatetags(filetags, alltags, 'global', tagtypes)
+_updatetags(filetags, alltags)
 
 # and update the cache (if necessary)
 if shouldwrite:
 _writetagcache(ui, repo, valid, alltags)
-return alltags, tagtypes
+return alltags
 
 def readlocaltags(ui, repo, alltags, tagtypes):
 '''Read local tags in repo. Update alltags and tagtypes.'''
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 3 of 7] tags: reorder argument of '_updatetags'

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490679490 -7200
#  Tue Mar 28 07:38:10 2017 +0200
# Node ID f0c93dd8d018c9f6828c97be8ccb80dbfca694b8
# Parent  147b98bfa4afbaf608d9e1f5227a48a46e386ea4
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r f0c93dd8d018
tags: reorder argument of '_updatetags'

We move all arguments related to tagtype at the end, together. This will allow
us to make these argument optional. This will be useful to reuse this logic for
caller that do not care about the tag types.

diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -97,7 +97,7 @@ def findglobaltags(ui, repo):
 # XXX is this really 100% correct?  are there oddball special
 # cases where a global tag should outrank a local tag but won't,
 # because cachetags does not contain rank info?
-_updatetags(cachetags, 'global', alltags, tagtypes)
+_updatetags(cachetags, alltags, 'global', tagtypes)
 return alltags, tagtypes
 
 seen = set()  # set of fnode
@@ -115,7 +115,7 @@ def findglobaltags(ui, repo):
 fctx = fctx.filectx(fnode)
 
 filetags = _readtags(ui, repo, fctx.data().splitlines(), fctx)
-_updatetags(filetags, 'global', alltags, tagtypes)
+_updatetags(filetags, alltags, 'global', tagtypes)
 
 # and update the cache (if necessary)
 if shouldwrite:
@@ -145,7 +145,7 @@ def readlocaltags(ui, repo, alltags, tag
 except (LookupError, ValueError):
 del filetags[t]
 
-_updatetags(filetags, "local", alltags, tagtypes)
+_updatetags(filetags, alltags, 'local', tagtypes)
 
 def _readtaghist(ui, repo, lines, fn, recode=None, calcnodelines=False):
 '''Read tag definitions from a file (or any source of lines).
@@ -223,7 +223,7 @@ def _readtags(ui, repo, lines, fn, recod
 newtags[tag] = (taghist[-1], taghist[:-1])
 return newtags
 
-def _updatetags(filetags, tagtype, alltags, tagtypes):
+def _updatetags(filetags, alltags, tagtype, tagtypes):
 '''Incorporate the tag info read from one file into the two
 dictionaries, alltags and tagtypes, that contain all tag
 info (global across all heads plus local).'''
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 1 of 7] tags: extract fnode retrieval in its own function

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490673691 -7200
#  Tue Mar 28 06:01:31 2017 +0200
# Node ID 1e77e505e7bacf59d1200714dc92e827523d7301
# Parent  e6fd7930cf0b37710e379de37b7d87d5c1ea5dc9
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 1e77e505e7ba
tags: extract fnode retrieval in its own function

My main goal here is to be able to reuse this logic easily. As a side effect
this important logic is now insulated and the code is clearer.

diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -341,15 +341,27 @@ def _readtagcache(ui, repo):
 # potentially expensive search.
 return ([], {}, valid, None, True)
 
-starttime = util.timer()
 
 # Now we have to lookup the .hgtags filenode for every new head.
 # This is the most expensive part of finding tags, so performance
 # depends primarily on the size of newheads.  Worst case: no cache
 # file, so newheads == repoheads.
+cachefnode = _getfnodes(ui, repo, repoheads)
+
+# Caller has to iterate over all heads, but can use the filenodes in
+# cachefnode to get to each .hgtags revision quickly.
+return (repoheads, cachefnode, valid, None, True)
+
+def _getfnodes(ui, repo, nodes):
+"""return fnode for .hgtags in a list of node
+
+return value is a {node: fnode} mapping. their will be no entry for node
+without a '.hgtags' file.
+"""
+starttime = util.timer()
 fnodescache = hgtagsfnodescache(repo.unfiltered())
 cachefnode = {}
-for head in reversed(repoheads):
+for head in reversed(nodes):
 fnode = fnodescache.getfnode(head)
 if fnode != nullid:
 cachefnode[head] = fnode
@@ -361,10 +373,7 @@ def _readtagcache(ui, repo):
'%d/%d cache hits/lookups in %0.4f '
'seconds\n',
fnodescache.hitcount, fnodescache.lookupcount, duration)
-
-# Caller has to iterate over all heads, but can use the filenodes in
-# cachefnode to get to each .hgtags revision quickly.
-return (repoheads, cachefnode, valid, None, True)
+return cachefnode
 
 def _writetagcache(ui, repo, valid, cachetags):
 filename = _filename(repo)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 7 of 7 V5] hgweb: expose a followlines UI in filerevision view (RFC)

2017-03-27 Thread Gregory Szorc
On Mon, Mar 27, 2017 at 9:40 AM, Augie Fackler  wrote:

> On Sat, Mar 25, 2017 at 12:34:47PM -0700, Gregory Szorc wrote:
> > On Sat, Mar 25, 2017 at 2:21 AM, Denis Laxalde 
> wrote:
> >
> > > # HG changeset patch
> > > # User Denis Laxalde 
> > > # Date 1489594320 -3600
> > > #  Wed Mar 15 17:12:00 2017 +0100
> > > # Node ID ec77aa4ff2993057b604bdffb449d2d179525a9f
> > > # Parent  2f75006a7f31c97d29fd6dd9b72fa7cc9e7ab840
> > > # Available At https://hg.logilab.org/users/dlaxalde/hg
> > > #  hg pull https://hg.logilab.org/users/dlaxalde/hg -r
> > > ec77aa4ff299
> > > # EXP-Topic linerange-log/hgweb-filelog
> > > hgweb: expose a followlines UI in filerevision view (RFC)
> > >
> > > In filerevision view (/file//) we add some event listeners
> on
> > > mouse selection of  elements in the 
> block.
> > > Those listeners will capture the range of mouse-selected lines and,
> upon
> > > mouse
> > > release, a box inviting to follow the history of selected lines will
> show
> > > up.
> > > This action is advertised by a :after pseudo-element on file lines that
> > > shows
> > > up on hover and invite to "select a block of lines to follow its
> history".
> > >
> > > All JavaScript code lives in a new "linerangelog.js" file, sourced in
> > > filerevision template (only in "paper" style for now).
> > >
> > > This is proposal implementation, comments welcome on any aspects.
> > >
> >
> > This feature is frigging awesome!!! I can pretty much guarantee that some
> > engineers at Mozilla will go completely crazy for this feature.
> >
> > As I'm playing with it locally, I found a few paper cuts.
>
> I've queued patches 1-6, which lay all the groundwork for this. Greg,
> can I have you drive the review of this last patch and let me know
> when I should take a look at it?
>

OK.

I'm not sure how others feel about landing this in its current form. If
another reviewer is OK with it, I'll give it a thorough review. Otherwise,
I'll wait for UI enhancements.

Also, I'd encourage others to comment on the UI.


>
> Thanks!
>
> (Very excited for this cool feature)
>
> >
> > First, on the initial revision introducing lines (last revision as
> rendered
> > in hgweb), the diff is often (always?) empty. I can also see empty diffs
> in
> > the intermediate changesets. For example, run this on
> mercurial/exchange.py
> > and ask it for the line range of the _pushdiscovery(pushop) function. For
> > this function, I get 2 empty diffs (revisions 233623d58c9a and
> > ddb56e7e1b92). Highlighting _pushdiscoverychangeset() yields only an
> empty
> > initial diff.
> >
> > Second, the highlighting UI is a bit confusing. It took me a few seconds
> to
> > realize I had to actually hold down the mouse button and highlight the
> > lines. I don't think users are accustomed to do this on text in web pages
> > unless they are copying text. It was definitely surprising to me that
> > highlighting lines of text resulted in a special pop-up appearing. I'm no
> > web design expert, but how about this design.
> >
> > As you mouseover lines, you see a graphic cursor somewhere on that line.
> > There is likely a label next to it saying "click anywhere to follow from
> > this line" or something like that. This is similar to the floating text
> > label you have now. When you mouse click, that floating cursor is dropped
> > and locked into place on that line. On subsequent mouseovers, another
> > cursor+label appears. The lines between the initial cursor and the
> current
> > mouse location are highlighted somehow (possibly adding a different
> > background color). The label says "click anywhere to follow lines XX to
> YY"
> > or something. When the user clicks to drop the 2nd cursor, either they're
> > taken directly to the filelog view or we get an inline box with links
> (like
> > the line number mouseovers on the annotate page). If the user
> accidentally
> > clicks to drop a cursor, they can clear the cursor be clicking the cursor
> > or an "x" next to it.
> >
> > Another minor nitpick with the highlighting UI is the "follow lines" box
> > may not appear next to the mouse cursor. I think we want it to appear
> near
> > the cursor so it is easier to find/use.
> >
> > The UX can obviously be improved as a follow-up. I don't want to detract
> > from getting the core of the feature landed. This is shaping up to be
> > pretty amazing!
> >
> >
> > >
> > > diff --git a/contrib/wix/templates.wxs b/contrib/wix/templates.wxs
> > > --- a/contrib/wix/templates.wxs
> > > +++ b/contrib/wix/templates.wxs
> > > @@ -225,6 +225,7 @@
> > >   />
> > >   Name="coal-folder.png" />
> > >  
> > > + Name="linerangelog.js" />
> > >   />
> > >  
> > >  
> > > diff --git a/mercurial/templates/paper/filerevision.tmpl
> > > b/mercurial/templates/paper/filerevision.tmpl
> > > --- a/mercurial/templates/paper/filerevision.tmpl
> > > +++ b/mercurial/templates/paper/filerevision.tmpl
> > > @@ -

[PATCH 2 of 3 V2] ui: defer setting pager related properties until the pager has spawned

2017-03-27 Thread Matt Harbison
# HG changeset patch
# User Matt Harbison 
# Date 1490490720 14400
#  Sat Mar 25 21:12:00 2017 -0400
# Node ID 1c73c5cde39463e03c3c7be10b6fe1256a6e5143
# Parent  2566b7eac73c4851edc21b73a833f86bf878285e
ui: defer setting pager related properties until the pager has spawned

When --pager=on is given, dispatch.py spawns a pager before setting up color.
If the pager failed to launch, ui.pageractive was left set to True, so color
configured itself based on 'color.pagermode'.  A typical MSYS setting would be
'color.mode=auto, color.pagermode=ansi'.  In the failure case, this would print
a warning, disable the pager, and then print the raw ANSI codes to the terminal.

Care needs to be taken, because it appears that leaving ui.pageractive=True was
the only thing that prevented an attempt at running the pager again from inside
the command.  This results in a double warning message, so pager is simply
disabled on failure.

The ui config settings didn't need to be moved to fix this, but it seemed like
the right thing to do for consistency.

diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
--- a/mercurial/chgserver.py
+++ b/mercurial/chgserver.py
@@ -193,6 +193,7 @@
 def _runpager(self, cmd):
 self._csystem(cmd, util.shellenviron(), type='pager',
   cmdtable={'attachio': attachio})
+return True
 
 return chgui(srcui)
 
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -848,15 +848,22 @@
 
 self.debug('starting pager for command %r\n' % command)
 self.flush()
-self.pageractive = True
-# Preserve the formatted-ness of the UI. This is important
-# because we mess with stdout, which might confuse
-# auto-detection of things being formatted.
-self.setconfig('ui', 'formatted', self.formatted(), 'pager')
-self.setconfig('ui', 'interactive', False, 'pager')
+
+wasformatted = self.formatted()
 if util.safehasattr(signal, "SIGPIPE"):
 signal.signal(signal.SIGPIPE, _catchterm)
-self._runpager(pagercmd)
+if self._runpager(pagercmd):
+self.pageractive = True
+# Preserve the formatted-ness of the UI. This is important
+# because we mess with stdout, which might confuse
+# auto-detection of things being formatted.
+self.setconfig('ui', 'formatted', wasformatted, 'pager')
+self.setconfig('ui', 'interactive', False, 'pager')
+else:
+# If the pager can't be spawned in dispatch when --pager=on is
+# given, don't try again when the command runs, to avoid a 
duplicate
+# warning about a missing pager command.
+self.disablepager()
 
 def _runpager(self, command):
 """Actually start the pager and set up file descriptors.
@@ -866,7 +873,7 @@
 """
 if command == 'cat':
 # Save ourselves some work.
-return
+return False
 # If the command doesn't contain any of these characters, we
 # assume it's a binary and exec it directly. This means for
 # simple pager command configurations, we can degrade
@@ -896,7 +903,7 @@
 if e.errno == errno.ENOENT and not shell:
 self.warn(_("missing pager command '%s', skipping pager\n")
   % command)
-return
+return False
 raise
 
 # back up original file descriptors
@@ -917,6 +924,8 @@
 pager.stdin.close()
 pager.wait()
 
+return True
+
 def interface(self, feature):
 """what interface to use for interactive console features?
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 1 of 3 V2] color: stop mutating the default effects map

2017-03-27 Thread Matt Harbison
# HG changeset patch
# User Matt Harbison 
# Date 1490464217 14400
#  Sat Mar 25 13:50:17 2017 -0400
# Node ID 2566b7eac73c4851edc21b73a833f86bf878285e
# Parent  e86eb75e74ce1b0803c26d86a229b9b711f6d76a
color: stop mutating the default effects map

A future change will make color.setup() callable a second time when the pager is
spawned, in order to honor the 'color.pagermode' setting.  The problem was that
when 'color.mode=auto' was resolved to 'win32' in the first pass, the default
ANSI effects were overwritten, making it impossible to honor 'pagermode=ansi'.
Also, the two separate maps didn't have the same keys.  The symmetric difference
is 'dim' and 'italic' (from ANSI), and 'bold_background' (from win32).  Thus,
the update left entries that didn't belong for the current mode.  This bled
through `hg debugcolor`, where the unsupported ANSI keys were listed in 'win32'
mode.

As an added bonus, this now correctly enables color with MSYS `less` for a
command like this, where pager is forced on:

$ hg log --config color.pagermode=ansi --pager=yes --color=auto

Previously, the output was corrupted.  The raw output, as seen through the ANSI
blind `more.com` was:

<-[-1;6mchangeset:   34840:3580d1197af9<-[-1m
...

which MSYS `less -FRX` rendered as:

1;6mchangeset:   34840:3580d1197af91m
...

(The two '<-' instances were actually an arrow character that TortoiseHg warned
couldn't be encoded, and notepad++ translated to a single '?'.)

Returning an empty map for 'ui._colormode == None' seems better that defaulting
to '_effects' (since some keys are mode dependent), and is better than None,
which blows up `hg debugcolor --color=never`.

diff --git a/mercurial/color.py b/mercurial/color.py
--- a/mercurial/color.py
+++ b/mercurial/color.py
@@ -238,7 +238,6 @@
 if not w32effects:
 modewarn()
 return None
-_effects.update(w32effects)
 elif realmode == 'ansi':
 ui._terminfoparams.clear()
 elif realmode == 'terminfo':
@@ -271,9 +270,17 @@
 % (e, status))
 ui._styles[status] = ' '.join(good)
 
+def _activeeffects(ui):
+'''Return the effects map for the color mode set on the ui.'''
+if ui._colormode == 'win32':
+return w32effects
+elif ui._colormode != None:
+return _effects
+return {}
+
 def valideffect(ui, effect):
 'Determine if the effect is valid or not.'
-return ((not ui._terminfoparams and effect in _effects)
+return ((not ui._terminfoparams and effect in _activeeffects(ui))
  or (effect in ui._terminfoparams
  or effect[:-11] in ui._terminfoparams))
 
@@ -324,9 +331,10 @@
 for effect in ['none'] + effects.split())
 stop = _effect_str(ui, 'none')
 else:
-start = [str(_effects[e]) for e in ['none'] + effects.split()]
+activeeffects = _activeeffects(ui)
+start = [str(activeeffects[e]) for e in ['none'] + effects.split()]
 start = '\033[' + ';'.join(start) + 'm'
-stop = '\033[' + str(_effects['none']) + 'm'
+stop = '\033[' + str(activeeffects['none']) + 'm'
 return _mergeeffects(text, start, stop)
 
 _ansieffectre = re.compile(br'\x1b\[[0-9;]*m')
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -360,7 +360,7 @@
 def _debugdisplaycolor(ui):
 ui = ui.copy()
 ui._styles.clear()
-for effect in color._effects.keys():
+for effect in color._activeeffects(ui).keys():
 ui._styles[effect] = effect
 if ui._terminfoparams:
 for k, v in ui.configitems('color'):
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 3 of 3 V2] ui: rerun color.setup() once the pager has spawned to honor 'color.pagermode'

2017-03-27 Thread Matt Harbison
# HG changeset patch
# User Matt Harbison 
# Date 1490483831 14400
#  Sat Mar 25 19:17:11 2017 -0400
# Node ID 3fa40378c88e3b9149054de48b9b880d93653488
# Parent  1c73c5cde39463e03c3c7be10b6fe1256a6e5143
ui: rerun color.setup() once the pager has spawned to honor 'color.pagermode'

Otherwise, ui.pageractive is False when color is setup in dispatch.py (without
--pager=on), and this config option is ignored.

diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -859,6 +859,12 @@
 # auto-detection of things being formatted.
 self.setconfig('ui', 'formatted', wasformatted, 'pager')
 self.setconfig('ui', 'interactive', False, 'pager')
+
+# If pagermode differs from color.mode, reconfigure color now that
+# pageractive is set.
+cm = self._colormode
+if cm != self.config('color', 'pagermode', cm):
+color.setup(self)
 else:
 # If the pager can't be spawned in dispatch when --pager=on is
 # given, don't try again when the command runs, to avoid a 
duplicate
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 5 of 5] statfs: make getfstype() raise OSError

2017-03-27 Thread Augie Fackler
On Sat, Mar 25, 2017 at 07:38:51PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara 
> # Date 1490430323 -32400
> #  Sat Mar 25 17:25:23 2017 +0900
> # Node ID 05dc34258f021523bc1de5c403f671fa4d1b9905
> # Parent  69a0abd9c61f0dae12d60b4f6fd4ada8288bd451
> statfs: make getfstype() raise OSError

Queued, thanks

>
> It's better for getfstype() function to not suppress an error. Callers can
> handle it as necessary. Now "hg debugfsinfo" will report OSError.
>
> diff --git a/mercurial/osutil.c b/mercurial/osutil.c
> --- a/mercurial/osutil.c
> +++ b/mercurial/osutil.c
> @@ -1090,7 +1090,7 @@ static PyObject *getfstype(PyObject *sel
>   memset(&buf, 0, sizeof(buf));
>   r = statfs(path, &buf);
>   if (r != 0)
> - Py_RETURN_NONE;
> + return PyErr_SetFromErrno(PyExc_OSError);
>   return Py_BuildValue("s", describefstype(&buf));
>  }
>  #endif /* defined(HAVE_LINUX_STATFS) || defined(HAVE_BSD_STATFS) */
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -1089,7 +1089,10 @@ def copyfile(src, dest, hardlink=False,
>  if hardlink:
>  # Hardlinks are problematic on CIFS (issue4546), do not allow 
> hardlinks
>  # unless we are confident that dest is on a whitelisted filesystem.
> -fstype = getfstype(os.path.dirname(dest))
> +try:
> +fstype = getfstype(os.path.dirname(dest))
> +except OSError:
> +fstype = None
>  if fstype not in _hardlinkfswhitelist:
>  hardlink = False
>  if hardlink:
> @@ -1372,7 +1375,7 @@ def fspath(name, root):
>  def getfstype(dirpath):
>  '''Get the filesystem type name from a directory (best-effort)
>
> -Returns None if we are unsure, or errors like ENOENT, EPERM happen.
> +Returns None if we are unsure. Raises OSError on ENOENT, EPERM, etc.
>  '''
>  return getattr(osutil, 'getfstype', lambda x: None)(dirpath)
>
> diff --git a/tests/hghave.py b/tests/hghave.py
> --- a/tests/hghave.py
> +++ b/tests/hghave.py
> @@ -349,7 +349,10 @@ def has_hardlink():
>  @check("hardlink-whitelisted", "hardlinks on whitelisted filesystems")
>  def has_hardlink_whitelisted():
>  from mercurial import util
> -fstype = util.getfstype('.')
> +try:
> +fstype = util.getfstype('.')
> +except OSError:
> +return False
>  return fstype in util._hardlinkfswhitelist
>
>  @check("rmcwd", "can remove current working directory")
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] runtests: change local IP glob pattern from "127.0.0.1" to "$LOCALIP"

2017-03-27 Thread Augie Fackler

> On Mar 26, 2017, at 22:58, Jun Wu  wrote:
> 
> # HG changeset patch
> # User Jun Wu 
> # Date 1490583437 25200
> #  Sun Mar 26 19:57:17 2017 -0700
> # Node ID 82f3dbff59306181303549600cd7a307da667d05
> # Parent  38ff33314869869535eb8f5c9cf4fa688847010e
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #  hg pull https://bitbucket.org/quark-zju/hg-draft -r 
> 82f3dbff5930
> runtests: change local IP glob pattern from "127.0.0.1" to "$LOCALIP"

Very nice, this fixes some BSD issues. Queued with delight (now that I've 
resurrected the buildbot).

> 
> This is similar to what 348b2b9da703 does. Since 636cf3f7620d has changed
> "127.0.0.1" to "$LOCALIP". The glob pattern needs update accordingly. It is
> expected to fix tests running in some BSD jails.
> 
> diff --git a/contrib/check-code.py b/contrib/check-code.py
> --- a/contrib/check-code.py
> +++ b/contrib/check-code.py
> @@ -211,6 +211,6 @@ utestpats = [
>   # warnings
>   [
> -(r'^  (?!.*127\.0\.0\.1)[^*?/\n]* \(glob\)$',
> - "glob match with no glob string (?, *, /, and 127.0.0.1)"),
> +(r'^  (?!.*\$LOCALIP)[^*?/\n]* \(glob\)$',
> + "glob match with no glob string (?, *, /, and $LOCALIP)"),
>   ]
> ]
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -1035,5 +1035,5 @@ checkcodeglobpats = [
> # Not all platforms have 127.0.0.1 as loopback (though most do),
> # so we always glob that too.
> -re.compile(br'.*127.0.0.1.*$'),
> +re.compile(br'.*\$LOCALIP.*$'),
> ]
> 
> @@ -1343,5 +1343,5 @@ class TTest(Test):
> return b'-glob'
> return True
> -el = el.replace(b'127.0.0.1', b'*')
> +el = el.replace(b'$LOCALIP', b'*')
> i, n = 0, len(el)
> res = b''
> diff --git a/tests/test-run-tests.t b/tests/test-run-tests.t
> --- a/tests/test-run-tests.t
> +++ b/tests/test-run-tests.t
> @@ -736,7 +736,9 @@ backslash on end of line with glob match
>   $ rm -f test-glob-backslash.t
> 
> -Test globbing of 127.0.0.1
> +Test globbing of local IP addresses
>   $ echo 172.16.18.1
> -  127.0.0.1 (glob)
> +  $LOCALIP (glob)
> +  $ echo dead:beef::1
> +  $LOCALIP (glob)
> 
> Test reusability for third party tools
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 2 of 2 RFC] RFC: switch to immutable configs

2017-03-27 Thread Jun Wu
# HG changeset patch
# User Jun Wu 
# Date 1490639836 25200
#  Mon Mar 27 11:37:16 2017 -0700
# Node ID 13bee3e959f04f970f2fc0a01120f0b30d725b84
# Parent  4eb7c76340791f379a34f9df4ec42e0c8b9b2a2f
RFC: switch to immutable configs

This replaces "ui" so it's based on immutable config. Tests pass.

The config hierarchy is currently like:

mergedconfig (root config)
 \_ mergedconfig (self.__class__._gcfgs, global configs across repos)
 |   \_ atomicconfig (added by setconfig(priority=3))
 |   \_ atomicconfig (added by setconfig(priority=3))
 |   \_ ...
 \_ mergedconfig (self._ocfgs)
 |   \_ atomicconfig (added by setconfig)
 |   \_ atomicconfig (added by setconfig)
 |   \_ filteredconfig (if the subconfig has [paths], needs fix)
 |   |   \_ atomicconfig (added by setconfig)
 |   \_ ...
 \_ filteredconfig (filters out HGPLAIN stuff)
 |   \_ mergedconfig (self._tcfgs, or self._ucfgs, config files)
 |   \_ filteredconfig (if the subconfig has [paths], needs xi)
 |   |   \_ fileconfig
 |   \_ fileconfig (added by readconfig)
 |   \_ fileconfig (added by readconfig)
 |   \_ ...
 \_ atomicconfig (handles ui.compat, dos not exists yet, replaceable)

In the future we may want to let rcutil.py construct part of the configs,
and make systemrc and userrc separately accessible.

As a RFC, the patch is not split intentionally, to give an overview of what
needed to be done for switching to immutable configs. The current code is
done in a relatively quick & dirty way. If we feel good about the direction,
I'll clean things up into smaller patches.

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1652,5 +1652,5 @@ def _docommit(ui, repo, *pats, **opts):
 raise error.Abort(_('cannot amend with --subrepos'))
 # Let --subrepos on the command line override config setting.
-ui.setconfig('ui', 'commitsubrepos', True, 'commit')
+ui.setconfig('ui', 'commitsubrepos', True, 'commit', priority=3)
 
 cmdutil.checkunfinished(repo, commit=True)
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -109,5 +109,6 @@ def dispatch(req):
 req.ui = uimod.ui.load()
 if '--traceback' in req.args:
-req.ui.setconfig('ui', 'traceback', 'on', '--traceback')
+req.ui.setconfig('ui', 'traceback', 'on', '--traceback',
+ priority=2)
 
 # set ui streams from the request
@@ -180,5 +181,6 @@ def _runcatch(req):
 # the repo ui
 for sec, name, val in cfgs:
-req.repo.ui.setconfig(sec, name, val, source='--config')
+req.repo.ui.setconfig(sec, name, val, source='--config',
+  priority=2)
 
 # developer config: ui.debugger
@@ -511,5 +513,5 @@ def _parseconfig(ui, config):
 if not section or not name:
 raise IndexError
-ui.setconfig(section, name, value, '--config')
+ui.setconfig(section, name, value, '--config', priority=2)
 configs.append((section, name, value))
 except (IndexError, ValueError):
@@ -683,5 +685,6 @@ def _dispatch(req):
 if '--profile' in args:
 for ui_ in uis:
-ui_.setconfig('profiling', 'enabled', 'true', '--profile')
+ui_.setconfig('profiling', 'enabled', 'true', '--profile',
+  priority=2)
 
 with profiling.maybeprofile(lui):
@@ -755,13 +758,14 @@ def _dispatch(req):
 val = val.encode('ascii')
 for ui_ in uis:
-ui_.setconfig('ui', opt, val, '--' + opt)
+ui_.setconfig('ui', opt, val, '--' + opt, priority=2)
 
 if options['traceback']:
 for ui_ in uis:
-ui_.setconfig('ui', 'traceback', 'on', '--traceback')
+ui_.setconfig('ui', 'traceback', 'on', '--traceback',
+  priority=2)
 
 if options['noninteractive']:
 for ui_ in uis:
-ui_.setconfig('ui', 'interactive', 'off', '-y')
+ui_.setconfig('ui', 'interactive', 'off', '-y', priority=2)
 
 if util.parsebool(options['pager']):
@@ -778,5 +782,5 @@ def _dispatch(req):
 for ui_ in uis:
 if coloropt:
-ui_.setconfig('ui', 'color', coloropt, '--color')
+ui_.setconfig('ui', 'color', coloropt, '--color', priority=2)
 color.setup(ui_)
 
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -130,4 +130,115 @@ def _catchterm(*args):
 raise error.SignalInterrupt
 
+def _fixpathsection(fileconfig, root):
+"""normalize paths in the [paths] section
+
+fileconfig is an immutable config objec

[PATCH 1 of 2 RFC] RFC: implement immutable config objects

2017-03-27 Thread Jun Wu
# HG changeset patch
# User Jun Wu 
# Date 1490635856 25200
#  Mon Mar 27 10:30:56 2017 -0700
# Node ID 4eb7c76340791f379a34f9df4ec42e0c8b9b2a2f
# Parent  4a8d065bbad80d3b3401010375bc80165404aa87
RFC: implement immutable config objects

The immutable config objects are basic data structures representating
configs.

This approach highlights:

  - Config layers. Previously there is little layer support, if an extension
calls "setconfig", you lose what the config was before "setconfig".
That's part of chg's compatibilities with some extensions (evolve).
With layers, new possibilities like "inserting a layer later"
(ui.compat), "detect system extensions overridden by user hgrc" will be
possible.
  - Fast cache invalidation test. The invalidation test is just to compare
object ids. It would it affordable to remove states like "debugflag",
"verbose", "trustedusers", "trustedgroups", and just use the config as
the source of truth. It also means we can get rid of "ui.fixconfig".

In general, more flexible and more confidence.

It's RFC and not splitted, to make it easier to get the whole picture.

diff --git a/mercurial/config.py b/mercurial/config.py
--- a/mercurial/config.py
+++ b/mercurial/config.py
@@ -263,2 +263,179 @@ def parselist(value):
 result = value
 return result or []
+
+class abstractimmutableconfig(object):
+"""minimal interface defined for a read-only config accessor
+
+The immutable config object should get its state set and frozen during
+__init__ and should not have any setconfig-like method.
+
+The immutable config layer knows about sections, and should probably uses
+ordered dict for each section. To simplify things, this layer does not care
+about "source", "unset" or any filesystem IO. They're up to the upper layer
+to deal with. For example, the upper layer could store "None" as "unset"s,
+and store (value, source) as a tuple together.
+"""
+
+def __init__(self, title):
+"""title: useful to identify the config"""
+self.title = title
+
+def subconfigs(self, section=None):
+"""return a list-ish of child config objects filtered by section"""
+raise NotImplementedError
+
+def get(self, section, item):
+"""return value or None"""
+return self.getsection(section).get(item)
+
+def getsection(self, section):
+"""return a dict-ish"""
+raise NotImplementedError
+
+def sections(self):
+"""return an iter-able"""
+raise NotImplementedError
+
+class atomicconfig(abstractimmutableconfig):
+"""immutable config that converted from a list-ish"""
+
+def __init__(self, title, entries=None):
+"""
+title:   a title used to identify the atomicconfig
+entries: a list-ish of (section, item, value)
+"""
+super(atomicconfig, self).__init__(title)
+self._sections = util.sortdict()
+for entry in entries:
+section, item, value = entry
+if section not in self._sections:
+self._sections[section] = util.sortdict()
+if item is not None:
+self._sections[section][item] = value
+
+def subconfigs(self, section=None):
+return ()
+
+def getsection(self, section):
+return self._sections.get(section, {})
+
+def sections(self):
+return self._sections.keys()
+
+class mergedconfig(abstractimmutableconfig):
+"""immutable config that is a merge of a list of immutable configs"""
+
+def __init__(self, title, subconfigs):
+super(mergedconfig, self).__init__(title)
+self._subconfigs = tuple(subconfigs)  # make it immutable
+self._cachedsections = {}
+
+def subconfigs(self, section=None):
+if section is None:
+return self._subconfigs
+else:
+return self._sectionconfigs.get(section, ())
+
+def sections(self):
+return self._sectionconfigs.keys()
+
+@util.propertycache
+def _sectionconfigs(self):
+"""{section: [subconfig]}"""
+sectionconfigs = {}
+for subconfig in self._subconfigs:
+for section in subconfig.sections():
+sectionconfigs.setdefault(section, []).append(subconfig)
+return sectionconfigs
+
+def getsection(self, section):
+items = self._cachedsections.get(section, None)
+if items is None:
+subconfigs = self._sectionconfigs.get(section, [])
+if len(subconfigs) == 1:
+# no need to merge configs
+items = subconfigs[0].getsection(section)
+else:
+# merge configs
+items = util.sortdict()
+for subconfig in subconfigs:
+subconfigitems = subconfig.getsection(section).items()
+for item, value in subconfigitems:
+items[item] = value
+sel

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

2017-03-27 Thread Jun Wu
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 
> > > # 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)\
> >

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

2017-03-27 Thread Augie Fackler
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 
> > # 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 < 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 < .hg/hgrc
> > +  > [extensions]
> > +  > debughidenodes

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

2017-03-27 Thread Augie Fackler
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 
> > # 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.)

>
> > '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 < debughidenodes.py
> > +  > from __future__ import absolute_import
> > +  > from mercurial import (
> > +  > cmdutil,
> > +

Re: [PATCH 5 of 5] tags: deprecated 'repo.tag'

2017-03-27 Thread Augie Fackler
On Mon, Mar 27, 2017 at 06:28:28PM +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David 
> # Date 1490623247 -7200
> #  Mon Mar 27 16:00:47 2017 +0200
> # Node ID 3ffd0c52c373af0dd3cb8ec758fd295367ed5680
> # Parent  e321a0e656e1020b3d8f57143c5ec346e14705e7
> # EXP-Topic tags
> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> #  hg pull 
> https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3ffd0c52c373
> tags: deprecated 'repo.tag'

Nice. Probably some implications for hg-git (maybe?), but a worthy cleanup. 
Queued.

>
> All user are gone. We can now celebrate the removal of some extra line from 
> the
> 'localrepo' class.
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -650,6 +650,7 @@ class localrepository(object):
>  return hook.hook(self.ui, self, name, throw, **args)
>
>  def tag(self, names, node, message, local, user, date, editor=False):
> +self.ui.deprecwarn("use 'tagsmod.tag' instead of 'repo.tag'", '4.2')
>  tagsmod.tag(self, names, node, message, local, user, date,
>  editor=editor)
>
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 7 of 7 V5] hgweb: expose a followlines UI in filerevision view (RFC)

2017-03-27 Thread Augie Fackler
On Sat, Mar 25, 2017 at 12:34:47PM -0700, Gregory Szorc wrote:
> On Sat, Mar 25, 2017 at 2:21 AM, Denis Laxalde  wrote:
>
> > # HG changeset patch
> > # User Denis Laxalde 
> > # Date 1489594320 -3600
> > #  Wed Mar 15 17:12:00 2017 +0100
> > # Node ID ec77aa4ff2993057b604bdffb449d2d179525a9f
> > # Parent  2f75006a7f31c97d29fd6dd9b72fa7cc9e7ab840
> > # Available At https://hg.logilab.org/users/dlaxalde/hg
> > #  hg pull https://hg.logilab.org/users/dlaxalde/hg -r
> > ec77aa4ff299
> > # EXP-Topic linerange-log/hgweb-filelog
> > hgweb: expose a followlines UI in filerevision view (RFC)
> >
> > In filerevision view (/file//) we add some event listeners on
> > mouse selection of  elements in the  block.
> > Those listeners will capture the range of mouse-selected lines and, upon
> > mouse
> > release, a box inviting to follow the history of selected lines will show
> > up.
> > This action is advertised by a :after pseudo-element on file lines that
> > shows
> > up on hover and invite to "select a block of lines to follow its history".
> >
> > All JavaScript code lives in a new "linerangelog.js" file, sourced in
> > filerevision template (only in "paper" style for now).
> >
> > This is proposal implementation, comments welcome on any aspects.
> >
>
> This feature is frigging awesome!!! I can pretty much guarantee that some
> engineers at Mozilla will go completely crazy for this feature.
>
> As I'm playing with it locally, I found a few paper cuts.

I've queued patches 1-6, which lay all the groundwork for this. Greg,
can I have you drive the review of this last patch and let me know
when I should take a look at it?

Thanks!

(Very excited for this cool feature)

>
> First, on the initial revision introducing lines (last revision as rendered
> in hgweb), the diff is often (always?) empty. I can also see empty diffs in
> the intermediate changesets. For example, run this on mercurial/exchange.py
> and ask it for the line range of the _pushdiscovery(pushop) function. For
> this function, I get 2 empty diffs (revisions 233623d58c9a and
> ddb56e7e1b92). Highlighting _pushdiscoverychangeset() yields only an empty
> initial diff.
>
> Second, the highlighting UI is a bit confusing. It took me a few seconds to
> realize I had to actually hold down the mouse button and highlight the
> lines. I don't think users are accustomed to do this on text in web pages
> unless they are copying text. It was definitely surprising to me that
> highlighting lines of text resulted in a special pop-up appearing. I'm no
> web design expert, but how about this design.
>
> As you mouseover lines, you see a graphic cursor somewhere on that line.
> There is likely a label next to it saying "click anywhere to follow from
> this line" or something like that. This is similar to the floating text
> label you have now. When you mouse click, that floating cursor is dropped
> and locked into place on that line. On subsequent mouseovers, another
> cursor+label appears. The lines between the initial cursor and the current
> mouse location are highlighted somehow (possibly adding a different
> background color). The label says "click anywhere to follow lines XX to YY"
> or something. When the user clicks to drop the 2nd cursor, either they're
> taken directly to the filelog view or we get an inline box with links (like
> the line number mouseovers on the annotate page). If the user accidentally
> clicks to drop a cursor, they can clear the cursor be clicking the cursor
> or an "x" next to it.
>
> Another minor nitpick with the highlighting UI is the "follow lines" box
> may not appear next to the mouse cursor. I think we want it to appear near
> the cursor so it is easier to find/use.
>
> The UX can obviously be improved as a follow-up. I don't want to detract
> from getting the core of the feature landed. This is shaping up to be
> pretty amazing!
>
>
> >
> > diff --git a/contrib/wix/templates.wxs b/contrib/wix/templates.wxs
> > --- a/contrib/wix/templates.wxs
> > +++ b/contrib/wix/templates.wxs
> > @@ -225,6 +225,7 @@
> >  
> >  
> >  
> > +
> >  
> >  
> >  
> > diff --git a/mercurial/templates/paper/filerevision.tmpl
> > b/mercurial/templates/paper/filerevision.tmpl
> > --- a/mercurial/templates/paper/filerevision.tmpl
> > +++ b/mercurial/templates/paper/filerevision.tmpl
> > @@ -71,8 +71,11 @@
> >  
> >  line wrap:  > class="linewraplink" href="javascript:toggleLinewrap()">on
> >   line source
> > -{text%fileline}
> > + > data-logurl="{url|urlescape}log/{symrev}/{file|urlescape}"
> > >{text%fileline}
> >  
> > +
> > +
> > +
> >  
> >  
> >
> > diff --git a/mercurial/templates/static/linerangelog.js
> > b/mercurial/templates/static/linerangelog.js
> > new file mode 100644
> > --- /dev/null
> > +++ b/mercurial/templates/static/linerangelog.js
> > @@ -0,0 +1,83 @@
> > +// Copyright 2017 Logilab SA 
> > +//
> > +// This software may be used and distrib

Re: [PATCH] metadataonlyctx: speed up sanity check

2017-03-27 Thread Augie Fackler
On Sun, Mar 26, 2017 at 12:32:23PM -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu 
> # Date 1490556395 25200
> #  Sun Mar 26 12:26:35 2017 -0700
> # Node ID 2793d297600133bb83a5d2f5a46bfa3fadd037ab
> # Parent  b6766d75404fb8c5d26af016caa76f44b47ce156
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #  hg pull https://bitbucket.org/quark-zju/hg-draft -r 
> 2793d2976001
> metadataonlyctx: speed up sanity check

Queued this, thanks.

>
> Previously the sanity check will construct manifestctx for both p1 and p2.
> But it only needs the "manifest node" information, which could be read from
> changelog directly.
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -2061,8 +2061,8 @@ class metadataonlyctx(committablectx):
>  # manifests of our commit parents
>  mp1, mp2 = self.manifestctx().parents
> -if p1 != nullid and p1.manifestctx().node() != mp1:
> +if p1 != nullid and p1.changeset()[0] != mp1:
>  raise RuntimeError('can\'t reuse the manifest: '
> 'its p1 doesn\'t match the new ctx p1')
> -if p2 != nullid and p2.manifestctx().node() != mp2:
> +if p2 != nullid and p2.changeset()[0] != mp2:
>  raise RuntimeError('can\'t reuse the manifest: '
> 'its p2 doesn\'t match the new ctx p2')
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH shelve-ext v2] shelve: add logic to preserve active bookmarks

2017-03-27 Thread Augie Fackler
On Sun, Mar 26, 2017 at 04:52:56PM -0700, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi 
> # Date 1490572279 25200
> #  Sun Mar 26 16:51:19 2017 -0700
> # Node ID 89739c20e31944f987de17620d76c82df6f0eda7
> # Parent  c60091fa1426892552dd6c0dd4b9c49e3c3da045
> shelve: add logic to preserve active bookmarks

Queued this, very happy to have this explicit instead of implicit.

>
> This adds an explicit active-bookmark-handling logic
> to shelve. Traditional shelve handles it by transaction aborts,
> but it is a bit ugly and having an explicit functionality
> seems better.
>
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -28,6 +28,7 @@ import itertools
>
>  from mercurial.i18n import _
>  from mercurial import (
> +bookmarks,
>  bundle2,
>  bundlerepo,
>  changegroup,
> @@ -170,6 +171,8 @@ class shelvedstate(object):
>  _filename = 'shelvedstate'
>  _keep = 'keep'
>  _nokeep = 'nokeep'
> +# colon is essential to differentiate from a real bookmark name
> +_noactivebook = ':no-active-bookmark'
>
>  @classmethod
>  def load(cls, repo):
> @@ -187,6 +190,7 @@ class shelvedstate(object):
>  nodestoprune = [nodemod.bin(h) for h in fp.readline().split()]
>  branchtorestore = fp.readline().strip()
>  keep = fp.readline().strip() == cls._keep
> +activebook = fp.readline().strip()
>  except (ValueError, TypeError) as err:
>  raise error.CorruptedState(str(err))
>  finally:
> @@ -201,6 +205,9 @@ class shelvedstate(object):
>  obj.nodestoprune = nodestoprune
>  obj.branchtorestore = branchtorestore
>  obj.keep = keep
> +obj.activebookmark = ''
> +if activebook != cls._noactivebook:
> +obj.activebookmark = activebook
>  except error.RepoLookupError as err:
>  raise error.CorruptedState(str(err))
>
> @@ -208,7 +215,7 @@ class shelvedstate(object):
>
>  @classmethod
>  def save(cls, repo, name, originalwctx, pendingctx, nodestoprune,
> - branchtorestore, keep=False):
> + branchtorestore, keep=False, activebook=''):
>  fp = repo.vfs(cls._filename, 'wb')
>  fp.write('%i\n' % cls._version)
>  fp.write('%s\n' % name)
> @@ -220,6 +227,7 @@ class shelvedstate(object):
>   ' '.join([nodemod.hex(n) for n in nodestoprune]))
>  fp.write('%s\n' % branchtorestore)
>  fp.write('%s\n' % (cls._keep if keep else cls._nokeep))
> +fp.write('%s\n' % (activebook or cls._noactivebook))
>  fp.close()
>
>  @classmethod
> @@ -244,6 +252,16 @@ def cleanupoldbackups(repo):
>  for ext in shelvefileextensions:
>  vfs.tryunlink(base + '.' + ext)
>
> +def _backupactivebookmark(repo):
> +activebookmark = repo._activebookmark
> +if activebookmark:
> +bookmarks.deactivate(repo)
> +return activebookmark
> +
> +def _restoreactivebookmark(repo, mark):
> +if mark:
> +bookmarks.activate(repo, mark)
> +
>  def _aborttransaction(repo):
>  '''Abort current transaction for shelve/unshelve, but keep dirstate
>  '''
> @@ -377,7 +395,7 @@ def _docreatecmd(ui, repo, pats, opts):
>  if not opts.get('message'):
>  opts['message'] = desc
>
> -lock = tr = None
> +lock = tr = activebookmark = None
>  try:
>  lock = repo.lock()
>
> @@ -390,6 +408,7 @@ def _docreatecmd(ui, repo, pats, opts):
>not opts.get('addremove', False))
>
>  name = getshelvename(repo, parent, opts)
> +activebookmark = _backupactivebookmark(repo)
>  extra = {}
>  if includeunknown:
>  _includeunknownfiles(repo, pats, opts, extra)
> @@ -404,7 +423,8 @@ def _docreatecmd(ui, repo, pats, opts):
>  node = cmdutil.commit(ui, repo, commitfunc, pats, opts)
>  else:
>  node = cmdutil.dorecord(ui, repo, commitfunc, None,
> -False, cmdutil.recordfilter, *pats, 
> **opts)
> +False, cmdutil.recordfilter, *pats,
> +**opts)
>  if not node:
>  _nothingtoshelvemessaging(ui, repo, pats, opts)
>  return 1
> @@ -420,6 +440,7 @@ def _docreatecmd(ui, repo, pats, opts):
>
>  _finishshelve(repo)
>  finally:
> +_restoreactivebookmark(repo, activebookmark)
>  lockmod.release(tr, lock)
>
>  def _isbareshelve(pats, opts):
> @@ -639,6 +660,7 @@ def unshelvecontinue(ui, repo, state, op
>  restorebranch(ui, repo, state.branchtorestore)
>
>  repair.strip(ui, repo, state.nodestoprune, backup=False, 
> topic='shelve')
> +_restoreactivebookmark(repo, state.activebookmark)
>  shelvedstate.clear(repo)
>  unshelvecleanup(ui, repo, state.name, opts

[PATCH 5 of 5] tags: deprecated 'repo.tag'

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490623247 -7200
#  Mon Mar 27 16:00:47 2017 +0200
# Node ID 3ffd0c52c373af0dd3cb8ec758fd295367ed5680
# Parent  e321a0e656e1020b3d8f57143c5ec346e14705e7
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3ffd0c52c373
tags: deprecated 'repo.tag'

All user are gone. We can now celebrate the removal of some extra line from the
'localrepo' class.

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -650,6 +650,7 @@ class localrepository(object):
 return hook.hook(self.ui, self, name, throw, **args)
 
 def tag(self, names, node, message, local, user, date, editor=False):
+self.ui.deprecwarn("use 'tagsmod.tag' instead of 'repo.tag'", '4.2')
 tagsmod.tag(self, names, node, message, local, user, date,
 editor=editor)
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 4 of 5] drawdag: use 'tagsmod.tag' instead of 'repo.tag'

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490630885 -7200
#  Mon Mar 27 18:08:05 2017 +0200
# Node ID e321a0e656e1020b3d8f57143c5ec346e14705e7
# Parent  7f11598171b4ac200c25b6c72fda9db2e52865e6
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r e321a0e656e1
drawdag: use 'tagsmod.tag' instead of 'repo.tag'

The former is about to be deprecated.

diff --git a/tests/drawdag.py b/tests/drawdag.py
--- a/tests/drawdag.py
+++ b/tests/drawdag.py
@@ -85,6 +85,7 @@ from mercurial import (
 error,
 node,
 scmutil,
+tags as tagsmod,
 )
 
 cmdtable = {}
@@ -308,4 +309,5 @@ def debugdrawdag(ui, repo, **opts):
 ctx = simplecommitctx(repo, name, pctxs, [name])
 n = ctx.commit()
 committed[name] = n
-repo.tag(name, n, message=None, user=None, date=None, local=True)
+tagsmod.tag(repo, name, n, message=None, user=None, date=None,
+local=True)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 2 of 5] tags: move 'repo.tag' in the 'tags' module

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490623111 -7200
#  Mon Mar 27 15:58:31 2017 +0200
# Node ID 983954c3aeebf13202b870a476671441bab9f6bf
# Parent  5ccbaa368644a534514cc79a3375848f2733d616
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 983954c3aeeb
tags: move 'repo.tag' in the 'tags' module

Similar logic, pretty much nobody use this method (that creates a tag) so we
move it into the 'tags' module were it belong.

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -650,35 +650,8 @@ class localrepository(object):
 return hook.hook(self.ui, self, name, throw, **args)
 
 def tag(self, names, node, message, local, user, date, editor=False):
-'''tag a revision with one or more symbolic names.
-
-names is a list of strings or, when adding a single tag, names may be a
-string.
-
-if local is True, the tags are stored in a per-repository file.
-otherwise, they are stored in the .hgtags file, and a new
-changeset is committed with the change.
-
-keyword arguments:
-
-local: whether to store tags in non-version-controlled file
-(default False)
-
-message: commit message to use if committing
-
-user: name of user to use if committing
-
-date: date tuple to use if committing'''
-
-if not local:
-m = matchmod.exact(self.root, '', ['.hgtags'])
-if any(self.status(match=m, unknown=True, ignored=True)):
-raise error.Abort(_('working copy of .hgtags is changed'),
- hint=_('please commit .hgtags manually'))
-
-self.tags() # instantiate the cache
-tagsmod._tag(self.unfiltered(), names, node, message, local, user, 
date,
- editor=editor)
+tagsmod.tag(self, names, node, message, local, user, date,
+editor=editor)
 
 @filteredpropertycache
 def _tagscache(self):
diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -395,6 +395,37 @@ def _writetagcache(ui, repo, valid, cach
 except (OSError, IOError):
 pass
 
+def tag(repo, names, node, message, local, user, date, editor=False):
+'''tag a revision with one or more symbolic names.
+
+names is a list of strings or, when adding a single tag, names may be a
+string.
+
+if local is True, the tags are stored in a per-repository file.
+otherwise, they are stored in the .hgtags file, and a new
+changeset is committed with the change.
+
+keyword arguments:
+
+local: whether to store tags in non-version-controlled file
+(default False)
+
+message: commit message to use if committing
+
+user: name of user to use if committing
+
+date: date tuple to use if committing'''
+
+if not local:
+m = matchmod.exact(repo.root, '', ['.hgtags'])
+if any(repo.status(match=m, unknown=True, ignored=True)):
+raise error.Abort(_('working copy of .hgtags is changed'),
+ hint=_('please commit .hgtags manually'))
+
+repo.tags() # instantiate the cache
+_tag(repo.unfiltered(), names, node, message, local, user, date,
+ editor=editor)
+
 def _tag(repo, names, node, message, local, user, date, extra=None,
  editor=False):
 if isinstance(names, str):
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 3 of 5] tags: use the 'tag' function from the 'tags' module in the 'tag' command

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490623234 -7200
#  Mon Mar 27 16:00:34 2017 +0200
# Node ID 7f11598171b4ac200c25b6c72fda9db2e52865e6
# Parent  983954c3aeebf13202b870a476671441bab9f6bf
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 7f11598171b4
tags: use the 'tag' function from the 'tags' module in the 'tag' command

There is No need to go through the 'repo' object method anymore.

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -48,6 +48,7 @@ from . import (
 server,
 sshserver,
 streamclone,
+tags as tagsmod,
 templatekw,
 ui as uimod,
 util,
@@ -5172,8 +5173,8 @@ def tag(ui, repo, name1, *names, **opts)
 scmutil.revsingle(repo, rev_).rev() == nullrev):
 raise error.Abort(_("cannot tag null revision"))
 
-repo.tag(names, r, message, opts.get('local'), opts.get('user'), date,
- editor=editor)
+tagsmod.tag(repo, names, r, message, opts.get('local'),
+opts.get('user'), date, editor=editor)
 finally:
 release(lock, wlock)
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 3] histedit: revive commits on demand

2017-03-27 Thread Jun Wu
Excerpts from Jun Wu's message of 2017-03-27 09:24:00 -0700:
> Excerpts from Pierre-Yves David's message of 2017-03-27 13:16:15 +0200:
> > 
> > On 03/27/2017 12:29 PM, Jun Wu wrote:
> > > Excerpts from Pierre-Yves David's message of 2017-03-27 09:17:59 +0200:
> > >>
> > >> On 03/26/2017 08:41 PM, Jun Wu wrote:
> > >>> # HG changeset patch
> > >>> # User Jun Wu 
> > >>> # Date 1490552007 25200
> > >>> #  Sun Mar 26 11:13:27 2017 -0700
> > >>> # Node ID b6766d75404fb8c5d26af016caa76f44b47ce156
> > >>> # Parent  336512ee2f947f07149e399a84927f9d820d2b62
> > >>> # Available At https://bitbucket.org/quark-zju/hg-draft  
> > >>> #  hg pull https://bitbucket.org/quark-zju/hg-draft-r 
> > >>> b6766d75404f
> > >>> histedit: revive commits on demand
> > >>>
> > >>> This is to address the "histedit --abort" issue mentioned in [1].
> > >>
> > >> There are still issues with that approach. For example in this
> > >> distributed case:
> > >>
> > >> 1) Alice send changesets to Bob,
> > >> 2) Bob try to histedit something in the stack, this fails
> > >> 3) Bob --abort the histedit
> > >> 4) Bob tell Alice to perform the operation herself since she know the
> > >> code better and can resolve the conflict
> > >> 5) Alice histedit with success
> > >
> > > Good example. But that's easily solvable by not writing the "parent"
> > > information when creating the markers, so the markers won't get exchanged.
> > 
> > This does not solve the issue, When Bob will pull from Alice, the issue 
> > will appears in Bob repository.
> > 
> > Please slow down.
> 
> Why? Bob won't get the marker from Alice.

It seems to be another example of what "node version" could solve.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 1 of 5] tags: move '_tags' from 'repo' to 'tags' module

2017-03-27 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1490622907 -7200
#  Mon Mar 27 15:55:07 2017 +0200
# Node ID 5ccbaa368644a534514cc79a3375848f2733d616
# Parent  59c6489c75dcf13989f4d4a343f3a18d1475e20d
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#  hg pull 
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 5ccbaa368644
tags: move '_tags' from 'repo' to 'tags' module

As far as I understand, that function do not needs to be on the local repository
class, so we extract it in the 'tags' module were it will be nice and
comfortable. We keep the '_' in the name since its only user will follow in the
next changeset.

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -649,80 +649,6 @@ class localrepository(object):
 """
 return hook.hook(self.ui, self, name, throw, **args)
 
-@unfilteredmethod
-def _tag(self, names, node, message, local, user, date, extra=None,
- editor=False):
-if isinstance(names, str):
-names = (names,)
-
-branches = self.branchmap()
-for name in names:
-self.hook('pretag', throw=True, node=hex(node), tag=name,
-  local=local)
-if name in branches:
-self.ui.warn(_("warning: tag %s conflicts with existing"
-" branch name\n") % name)
-
-def writetags(fp, names, munge, prevtags):
-fp.seek(0, 2)
-if prevtags and prevtags[-1] != '\n':
-fp.write('\n')
-for name in names:
-if munge:
-m = munge(name)
-else:
-m = name
-
-if (self._tagscache.tagtypes and
-name in self._tagscache.tagtypes):
-old = self.tags().get(name, nullid)
-fp.write('%s %s\n' % (hex(old), m))
-fp.write('%s %s\n' % (hex(node), m))
-fp.close()
-
-prevtags = ''
-if local:
-try:
-fp = self.vfs('localtags', 'r+')
-except IOError:
-fp = self.vfs('localtags', 'a')
-else:
-prevtags = fp.read()
-
-# local tags are stored in the current charset
-writetags(fp, names, None, prevtags)
-for name in names:
-self.hook('tag', node=hex(node), tag=name, local=local)
-return
-
-try:
-fp = self.wvfs('.hgtags', 'rb+')
-except IOError as e:
-if e.errno != errno.ENOENT:
-raise
-fp = self.wvfs('.hgtags', 'ab')
-else:
-prevtags = fp.read()
-
-# committed tags are stored in UTF-8
-writetags(fp, names, encoding.fromlocal, prevtags)
-
-fp.close()
-
-self.invalidatecaches()
-
-if '.hgtags' not in self.dirstate:
-self[None].add(['.hgtags'])
-
-m = matchmod.exact(self.root, '', ['.hgtags'])
-tagnode = self.commit(message, user, date, extra=extra, match=m,
-  editor=editor)
-
-for name in names:
-self.hook('tag', node=hex(node), tag=name, local=local)
-
-return tagnode
-
 def tag(self, names, node, message, local, user, date, editor=False):
 '''tag a revision with one or more symbolic names.
 
@@ -751,7 +677,8 @@ class localrepository(object):
  hint=_('please commit .hgtags manually'))
 
 self.tags() # instantiate the cache
-self._tag(names, node, message, local, user, date, editor=editor)
+tagsmod._tag(self.unfiltered(), names, node, message, local, user, 
date,
+ editor=editor)
 
 @filteredpropertycache
 def _tagscache(self):
diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -20,9 +20,11 @@ from .node import (
 nullid,
 short,
 )
+from .i18n import _
 from . import (
 encoding,
 error,
+match as matchmod,
 scmutil,
 util,
 )
@@ -393,6 +395,79 @@ def _writetagcache(ui, repo, valid, cach
 except (OSError, IOError):
 pass
 
+def _tag(repo, names, node, message, local, user, date, extra=None,
+ editor=False):
+if isinstance(names, str):
+names = (names,)
+
+branches = repo.branchmap()
+for name in names:
+repo.hook('pretag', throw=True, node=hex(node), tag=name,
+  local=local)
+if name in branches:
+repo.ui.warn(_("warning: tag %s conflicts with existing"
+" branch name\n") % name)
+
+def writetags(fp, names, munge, prevtags):
+fp.seek(0, 2)
+if prevtags and prevtags[-1] != '\n':
+fp.write('\n')
+for name in names:
+if munge:
+m = mung

Re: [PATCH 8 of 8] largefiles: add lfile argument to updatestandin() for efficiency (API)

2017-03-27 Thread Augie Fackler
On Mon, Mar 27, 2017 at 10:53:48AM +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori 
> # Date 1490575476 -32400
> #  Mon Mar 27 09:44:36 2017 +0900
> # Node ID 30343c565caf83ff165b05d2cd95f63ab50fe377
> # Parent  b80768388eb2700a619159447461063983dd5806
> largefiles: add lfile argument to updatestandin() for efficiency (API)

These are queued, thanks.

>
> Before this patch, updatestandin() takes "standin" argument, and
> applies splitstandin() on it to pick out a path to largefile (aka
> "lfile" or so) from standin.
>
> But in fact, all callers already knows "lfile". In addition to it,
> many callers knows both "standin" and "lfile".
>
> Therefore, making updatestandin() take only one of "standin" or
> "lfile" is inefficient.
>
> diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
> --- a/hgext/largefiles/lfutil.py
> +++ b/hgext/largefiles/lfutil.py
> @@ -342,8 +342,11 @@ def splitstandin(filename):
>  else:
>  return None
>
> -def updatestandin(repo, standin):
> -lfile = splitstandin(standin)
> +def updatestandin(repo, lfile, standin):
> +"""Re-calculate hash value of lfile and write it into standin
> +
> +This assumes that "lfutil.standin(lfile) == standin", for efficiency.
> +"""
>  file = repo.wjoin(lfile)
>  if repo.wvfs.exists(lfile):
>  hash = hashfile(file)
> @@ -560,7 +563,7 @@ def updatestandinsbymatch(repo, match):
>  # performed and the working copy is not updated
>  # yet.
>  if repo.wvfs.exists(lfile):
> -updatestandin(repo, fstandin)
> +updatestandin(repo, lfile, fstandin)
>
>  return match
>
> @@ -586,7 +589,7 @@ def updatestandinsbymatch(repo, match):
>  for fstandin in standins:
>  lfile = splitstandin(fstandin)
>  if lfdirstate[lfile] != 'r':
> -updatestandin(repo, fstandin)
> +updatestandin(repo, lfile, fstandin)
>
>  # Cook up a new matcher that only matches regular files or
>  # standins corresponding to the big files requested by the
> diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
> --- a/hgext/largefiles/overrides.py
> +++ b/hgext/largefiles/overrides.py
> @@ -736,7 +736,7 @@ def overriderevert(orig, ui, repo, ctx,
>  s = lfutil.lfdirstatestatus(lfdirstate, repo)
>  lfdirstate.write()
>  for lfile in s.modified:
> -lfutil.updatestandin(repo, lfutil.standin(lfile))
> +lfutil.updatestandin(repo, lfile, lfutil.standin(lfile))
>  for lfile in s.deleted:
>  fstandin = lfutil.standin(lfile)
>  if (repo.wvfs.exists(fstandin)):
> @@ -1417,7 +1417,7 @@ def mergeupdate(orig, repo, node, branch
>  # in this case, content of standin file is meaningless
>  # (in dctx, lfile is unknown, or normal file)
>  continue
> -lfutil.updatestandin(repo, fstandin)
> +lfutil.updatestandin(repo, lfile, fstandin)
>  # mark all clean largefiles as dirty, just in case the update gets
>  # interrupted before largefiles and lfdirstate are synchronized
>  for lfile in oldclean:
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 3] histedit: revive commits on demand

2017-03-27 Thread Jun Wu
Excerpts from Pierre-Yves David's message of 2017-03-27 13:16:15 +0200:
> 
> On 03/27/2017 12:29 PM, Jun Wu wrote:
> > Excerpts from Pierre-Yves David's message of 2017-03-27 09:17:59 +0200:
> >>
> >> On 03/26/2017 08:41 PM, Jun Wu wrote:
> >>> # HG changeset patch
> >>> # User Jun Wu 
> >>> # Date 1490552007 25200
> >>> #  Sun Mar 26 11:13:27 2017 -0700
> >>> # Node ID b6766d75404fb8c5d26af016caa76f44b47ce156
> >>> # Parent  336512ee2f947f07149e399a84927f9d820d2b62
> >>> # Available At https://bitbucket.org/quark-zju/hg-draft 
> >>> #  hg pull https://bitbucket.org/quark-zju/hg-draft   -r 
> >>> b6766d75404f
> >>> histedit: revive commits on demand
> >>>
> >>> This is to address the "histedit --abort" issue mentioned in [1].
> >>
> >> There are still issues with that approach. For example in this
> >> distributed case:
> >>
> >> 1) Alice send changesets to Bob,
> >> 2) Bob try to histedit something in the stack, this fails
> >> 3) Bob --abort the histedit
> >> 4) Bob tell Alice to perform the operation herself since she know the
> >> code better and can resolve the conflict
> >> 5) Alice histedit with success
> >
> > Good example. But that's easily solvable by not writing the "parent"
> > information when creating the markers, so the markers won't get exchanged.
> 
> This does not solve the issue, When Bob will pull from Alice, the issue 
> will appears in Bob repository.
> 
> Please slow down.

Why? Bob won't get the marker from Alice.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH STABLE] changegroup: retry read() when partial data returned

2017-03-27 Thread Yuya Nishihara
On Mon, 27 Mar 2017 08:25:41 -0700, Gregory Szorc wrote:
> > On Mar 27, 2017, at 08:13, Yuya Nishihara  wrote:
> >> On Sun, 26 Mar 2017 11:56:59 -0700, Gregory Szorc wrote:
> >> # HG changeset patch
> >> # User Gregory Szorc 
> >> # Date 1490554582 25200
> >> #  Sun Mar 26 11:56:22 2017 -0700
> >> # Branch stable
> >> # Node ID 414128ddd876eb33f70b0dd95d110e29a9308c93
> >> # Parent  ed5b25874d998ababb181a939dd37a16ea644435
> >> changegroup: retry read() when partial data returned

[...]

> >> def readexactly(stream, n):
> >> '''read n bytes from stream.read and abort if less was available'''
> >> -s = stream.read(n)
> >> -if len(s) < n:
> >> +# Most of the time, stream.read() returns exactly the number of bytes
> >> +# requested. Various Python APIs will perform multiple system calls
> >> +# to retrieve more bytes if the first call did not return enough. This
> >> +# includes cases where the system call is interrupted.
> >> +#
> >> +# Even with this retry logic in the Python APIs, we still see failure
> >> +# to retrieve the requested number of bytes. So, we build in our own
> >> +# retry layer here.
> >> +left = n
> >> +chunk = stream.read(n)
> >> +left -= len(chunk)
> >> +assert left >= 0
> >> +
> >> +# Common case is it just works.
> >> +if not left:
> >> +return chunk
> >> +
> >> +chunks = [chunk]
> >> +# Retry to get remaining data. In cases where the stream has errored 
> >> or
> >> +# is at EOF, this will do a bit of redundant work. But it helps 
> >> prevent
> >> +# intermittent failures and isn't common. So the overhead is 
> >> acceptable.
> >> +for i in range(3):
> >> +chunk = stream.read(left)
> >> +chunks.append(chunk)
> >> +left -= len(chunk)
> >> +assert left >= 0
> >> +
> >> +if not left:
> >> +break
> > 
> > Do you have some number that supports this change actually mitigates the
> > problem?
> 
> No. I could try to get that this week.

Many thanks.

> > Suppose underlying functions have bugs, I don't think we can always get
> > contiguous chunks with no data loss. If a few bytes lost for example, we
> > could read arbitrary part as a length field, and call readexactly() with
> > that invalid length value.
> 
> Hmm. My assumption is that any error that would result in data loss 
> effectively poisons the stream and prevents future reads.

I was thinking e.g. some intermediate layer might not handle EINTR
appropriately and EINTR might be caught by some upper layer. If it had
a temporary buffer, its content would be lost.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] metadataonlyctx: speed up sanity check

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 12:32:23 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu 
> # Date 1490556395 25200
> #  Sun Mar 26 12:26:35 2017 -0700
> # Node ID 2793d297600133bb83a5d2f5a46bfa3fadd037ab
> # Parent  b6766d75404fb8c5d26af016caa76f44b47ce156
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #  hg pull https://bitbucket.org/quark-zju/hg-draft -r 
> 2793d2976001
> metadataonlyctx: speed up sanity check
> 
> Previously the sanity check will construct manifestctx for both p1 and p2.
> But it only needs the "manifest node" information, which could be read from
> changelog directly.
> 
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -2061,8 +2061,8 @@ class metadataonlyctx(committablectx):
>  # manifests of our commit parents
>  mp1, mp2 = self.manifestctx().parents
> -if p1 != nullid and p1.manifestctx().node() != mp1:
> +if p1 != nullid and p1.changeset()[0] != mp1:
>  raise RuntimeError('can\'t reuse the manifest: '
> 'its p1 doesn\'t match the new ctx p1')
> -if p2 != nullid and p2.manifestctx().node() != mp2:
> +if p2 != nullid and p2.changeset()[0] != mp2:

Perhaps ctx.manifestnode() can be used.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH STABLE] changegroup: retry read() when partial data returned

2017-03-27 Thread Gregory Szorc


> On Mar 27, 2017, at 08:13, Yuya Nishihara  wrote:
> 
>> On Sun, 26 Mar 2017 11:56:59 -0700, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc 
>> # Date 1490554582 25200
>> #  Sun Mar 26 11:56:22 2017 -0700
>> # Branch stable
>> # Node ID 414128ddd876eb33f70b0dd95d110e29a9308c93
>> # Parent  ed5b25874d998ababb181a939dd37a16ea644435
>> changegroup: retry read() when partial data returned
>> 
>> We've seen a rash of "stream ended unexpectedly" errors in the
>> wild. This occurs when changegroup.readexactly() fails to retrieve
>> the exact number of requested bytes in a single stream.read()
>> operation.
>> 
>> There are several plausible theories that explain this behavior.
>> Many include underlying APIs not retrying automatically when
>> interrupted by a signal. While Python is supposed to do this,
>> it could be buggy. There could also be a bug in the various
>> stream reading layers between changegroup.readexactly() and the
>> underlying file descriptor.
>> 
>> Unfortunately, getting this error to reproduce has been extremely
>> difficult and no single cause has been identified.
>> 
>> Since networks (and even filesystems) are unreliable, it seems
>> wise to have some form of retry in place to mitigate this failure.
>> This patch adds that retry logic.
>> 
>> There is already basic test coverage that the exception in this
>> function is raised. The tests didn't change with this patch.
>> 
>> The only obvious negative impact to this change I can see is that
>> in cases where the read operation fails, there is some overhead
>> involved with retrying the operation. In the worst case, this is
>> pure overhead: we'd retry an operation and get the same outcome.
>> But in the best case, it avoids an intermittent/random failure.
>> My assumption is that the overhead to retry is small and that
>> the cost to avoiding a random failure is justified.
>> 
>> There are other changes I'd like to make to stream reading code
>> to help flush out this general failure of partial stream reads.
>> See issue4923 for some suggestions with regards to swallowing
>> exception and masking the underlying error. However, these
>> changes aren't suitable for the stable branch. This patch feels
>> minimally invasive and if successful would let us narrow our focus
>> for finding the underlying bug.
>> 
>> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
>> --- a/mercurial/changegroup.py
>> +++ b/mercurial/changegroup.py
>> @@ -35,12 +35,42 @@ from . import (
>> 
>> def readexactly(stream, n):
>> '''read n bytes from stream.read and abort if less was available'''
>> -s = stream.read(n)
>> -if len(s) < n:
>> +# Most of the time, stream.read() returns exactly the number of bytes
>> +# requested. Various Python APIs will perform multiple system calls
>> +# to retrieve more bytes if the first call did not return enough. This
>> +# includes cases where the system call is interrupted.
>> +#
>> +# Even with this retry logic in the Python APIs, we still see failure
>> +# to retrieve the requested number of bytes. So, we build in our own
>> +# retry layer here.
>> +left = n
>> +chunk = stream.read(n)
>> +left -= len(chunk)
>> +assert left >= 0
>> +
>> +# Common case is it just works.
>> +if not left:
>> +return chunk
>> +
>> +chunks = [chunk]
>> +# Retry to get remaining data. In cases where the stream has errored or
>> +# is at EOF, this will do a bit of redundant work. But it helps prevent
>> +# intermittent failures and isn't common. So the overhead is acceptable.
>> +for i in range(3):
>> +chunk = stream.read(left)
>> +chunks.append(chunk)
>> +left -= len(chunk)
>> +assert left >= 0
>> +
>> +if not left:
>> +break
> 
> Do you have some number that supports this change actually mitigates the
> problem?

No. I could try to get that this week.

> 
> Suppose underlying functions have bugs, I don't think we can always get
> contiguous chunks with no data loss. If a few bytes lost for example, we
> could read arbitrary part as a length field, and call readexactly() with
> that invalid length value.

Hmm. My assumption is that any error that would result in data loss effectively 
poisons the stream and prevents future reads.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH STABLE] changegroup: retry read() when partial data returned

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 11:56:59 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc 
> # Date 1490554582 25200
> #  Sun Mar 26 11:56:22 2017 -0700
> # Branch stable
> # Node ID 414128ddd876eb33f70b0dd95d110e29a9308c93
> # Parent  ed5b25874d998ababb181a939dd37a16ea644435
> changegroup: retry read() when partial data returned
> 
> We've seen a rash of "stream ended unexpectedly" errors in the
> wild. This occurs when changegroup.readexactly() fails to retrieve
> the exact number of requested bytes in a single stream.read()
> operation.
> 
> There are several plausible theories that explain this behavior.
> Many include underlying APIs not retrying automatically when
> interrupted by a signal. While Python is supposed to do this,
> it could be buggy. There could also be a bug in the various
> stream reading layers between changegroup.readexactly() and the
> underlying file descriptor.
> 
> Unfortunately, getting this error to reproduce has been extremely
> difficult and no single cause has been identified.
> 
> Since networks (and even filesystems) are unreliable, it seems
> wise to have some form of retry in place to mitigate this failure.
> This patch adds that retry logic.
> 
> There is already basic test coverage that the exception in this
> function is raised. The tests didn't change with this patch.
> 
> The only obvious negative impact to this change I can see is that
> in cases where the read operation fails, there is some overhead
> involved with retrying the operation. In the worst case, this is
> pure overhead: we'd retry an operation and get the same outcome.
> But in the best case, it avoids an intermittent/random failure.
> My assumption is that the overhead to retry is small and that
> the cost to avoiding a random failure is justified.
> 
> There are other changes I'd like to make to stream reading code
> to help flush out this general failure of partial stream reads.
> See issue4923 for some suggestions with regards to swallowing
> exception and masking the underlying error. However, these
> changes aren't suitable for the stable branch. This patch feels
> minimally invasive and if successful would let us narrow our focus
> for finding the underlying bug.
> 
> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
> --- a/mercurial/changegroup.py
> +++ b/mercurial/changegroup.py
> @@ -35,12 +35,42 @@ from . import (
>  
>  def readexactly(stream, n):
>  '''read n bytes from stream.read and abort if less was available'''
> -s = stream.read(n)
> -if len(s) < n:
> +# Most of the time, stream.read() returns exactly the number of bytes
> +# requested. Various Python APIs will perform multiple system calls
> +# to retrieve more bytes if the first call did not return enough. This
> +# includes cases where the system call is interrupted.
> +#
> +# Even with this retry logic in the Python APIs, we still see failure
> +# to retrieve the requested number of bytes. So, we build in our own
> +# retry layer here.
> +left = n
> +chunk = stream.read(n)
> +left -= len(chunk)
> +assert left >= 0
> +
> +# Common case is it just works.
> +if not left:
> +return chunk
> +
> +chunks = [chunk]
> +# Retry to get remaining data. In cases where the stream has errored or
> +# is at EOF, this will do a bit of redundant work. But it helps prevent
> +# intermittent failures and isn't common. So the overhead is acceptable.
> +for i in range(3):
> +chunk = stream.read(left)
> +chunks.append(chunk)
> +left -= len(chunk)
> +assert left >= 0
> +
> +if not left:
> +break

Do you have some number that supports this change actually mitigates the
problem?

Suppose underlying functions have bugs, I don't think we can always get
contiguous chunks with no data loss. If a few bytes lost for example, we
could read arbitrary part as a length field, and call readexactly() with
that invalid length value.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 2 of 4] color: stop mutating the default effects map

2017-03-27 Thread Yuya Nishihara
On Mon, 27 Mar 2017 14:41:46 +0200, Pierre-Yves David wrote:
> On 03/26/2017 04:35 PM, Yuya Nishihara wrote:
> > On Sun, 26 Mar 2017 00:41:05 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison 
> >> # Date 1490464217 14400
> >> #  Sat Mar 25 13:50:17 2017 -0400
> >> # Node ID a263702b064a5a3ce1ca74b227e8e624e4b05874
> >> # Parent  22533c3af63d5a67d9210596eafd4b99ab9c7904
> >> color: stop mutating the default effects map
> >
> > I re-read the code, and noticed that both _effects and w32effects are mostly
> > constants. Perhaps we can simply switch them by ui._colormode.
> >
> >   if ui._colormode == 'win32':
> >   activeeffects = w32neffects
> >   else:
> >   activeeffects = _effects
> >   ... embed escape sequences by using activeeffects ...
> 
> Ultimately, yes we want to be there, but I remember other things muding
> with the effects dict so someone would have to give a close look to the 
> current magic before moving forward.

Maybe that is debugcolor (and w32effects.)

% grep _effects  **/*.py
mercurial/color.py:_effects = {
mercurial/color.py:_effects.update(w32effects)
mercurial/color.py:return ((not ui._terminfoparams and effect in _effects)
mercurial/color.py:'''Helper function for render_effects().'''
mercurial/color.py:def _render_effects(ui, text, effects):
mercurial/color.py:start = [str(_effects[e]) for e in ['none'] + 
effects.split()]
mercurial/color.py:stop = '\033[' + str(_effects['none']) + 'm'
mercurial/color.py:msg = '\n'.join([_render_effects(ui, line, 
effects)
mercurial/debugcommands.py:for effect in color._effects.keys():
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 4 of 4] summary: display obsolete state of parents

2017-03-27 Thread Yuya Nishihara
On Mon, 27 Mar 2017 10:00:34 +0200, Denis Laxalde wrote:
> Yuya Nishihara a écrit :
> > On Sat, 25 Mar 2017 14:09:19 +0100, Denis Laxalde wrote:
> >> # HG changeset patch
> >> # User Denis Laxalde 
> >> # Date 1490437808 -3600
> >> #  Sat Mar 25 11:30:08 2017 +0100
> >> # Node ID af7190c42c1cd6ba16a9bd83ad54208d89f343fe
> >> # Parent  94a70894c394268b845f427f7643738a70e336bb
> >> # Available At https://bitbucket.org/dlax/hg-work
> >> #  hg pull https://bitbucket.org/dlax/hg-work -r af7190c42c1c
> >> # EXP-Topic obsolete-ui
> >> summary: display obsolete state of parents
> >>
> >> Extend the "parent: " lines in summary to display "(obsolete)" when the 
> >> parent
> >> is obsolete.
> >
> > The code looks good, but I have no idea how the bulk bikeshedding about 
> > evolve
> > naming ended.
> 
> Are you referring to https://www.mercurial-scm.org/wiki/CEDVocabulary ?
> It seems to me that the "obsolete" term is accepted (nor really
> discussed in this page at least).

Ah, thanks, I hadn't read it.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] metadataonlyctx: speed up sanity check

2017-03-27 Thread Ryan McElroy

(cc Mateusz)

On 3/26/17 8:32 PM, Jun Wu wrote:

# HG changeset patch
# User Jun Wu 
# Date 1490556395 25200
#  Sun Mar 26 12:26:35 2017 -0700
# Node ID 2793d297600133bb83a5d2f5a46bfa3fadd037ab
# Parent  b6766d75404fb8c5d26af016caa76f44b47ce156
metadataonlyctx: speed up sanity check

Previously the sanity check will construct manifestctx for both p1 and p2.
But it only needs the "manifest node" information, which could be read from
changelog directly.

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -2061,8 +2061,8 @@ class metadataonlyctx(committablectx):
  # manifests of our commit parents
  mp1, mp2 = self.manifestctx().parents
-if p1 != nullid and p1.manifestctx().node() != mp1:
+if p1 != nullid and p1.changeset()[0] != mp1:
  raise RuntimeError('can\'t reuse the manifest: '
 'its p1 doesn\'t match the new ctx p1')
-if p2 != nullid and p2.manifestctx().node() != mp2:
+if p2 != nullid and p2.changeset()[0] != mp2:
  raise RuntimeError('can\'t reuse the manifest: '
 'its p2 doesn\'t match the new ctx p2')



This change looks good to me, thanks! Marked as pre-reviewed.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 07 of 11] revlog: fix many file handle opens to explicitly use bytes mode

2017-03-27 Thread Augie Fackler
On Mon, Mar 27, 2017 at 10:54:08PM +0900, Yuya Nishihara wrote:
> On Sun, 26 Mar 2017 18:36:41 -0400, Augie Fackler wrote:
> > # HG changeset patch
> > # User Augie Fackler 
> > # Date 1490567324 14400
> > #  Sun Mar 26 18:28:44 2017 -0400
> > # Node ID e7fe3ab60132647a9c3b86b2960b385e61b9dcf0
> > # Parent  48144fe2d912b7d9fc300955d0c881aceead6930
> > revlog: fix many file handle opens to explicitly use bytes mode
> >
> > This broke on Python 3, but we've probably been getting lucky that
> > this hasn't caused EOL corruption on Windows or something for
> > years. :(
> >
> > diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> > --- a/mercurial/revlog.py
> > +++ b/mercurial/revlog.py
> > @@ -1467,8 +1467,8 @@ class revlog(object):
> >
> >  dfh = None
> >  if not self._inline:
> > -dfh = self.opener(self.datafile, "a+")
> > -ifh = self.opener(self.indexfile, "a+", 
> > checkambig=self._checkambig)
> > +dfh = self.opener(self.datafile, "a+b")
> > +ifh = self.opener(self.indexfile, "a+b", 
> > checkambig=self._checkambig)
>
> 'b' shouldn't be needed since vfs adds 'b' unless text=True is explicitly
> specified.

Huh. I swear this fixed some problems for me. I'll keep it around in
reserve in case the problem crops up again.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 5 of 7] py3: fix manifestdict.fastdelta() to be compatible with memoryview

2017-03-27 Thread Augie Fackler
On Sun, Mar 26, 2017 at 08:59:48PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara 
> # Date 1490522808 -32400
> #  Sun Mar 26 19:06:48 2017 +0900
> # Node ID 5e8e48cb4f83a583515b4caaf97045b86747c499
> # Parent  435eb9e60147c1cfac43b204b637b8ac8d4830c0
> py3: fix manifestdict.fastdelta() to be compatible with memoryview

Queued these last two, many thanks.

>
> This doesn't look nice, but a straightforward way to support Python 3.
> bytes(m[start:end]) is needed because a memoryview doesn't support ordering
> operations. On Python 2, m[start:end] returns a bytes object even if m is
> a buffer, so calling bytes() should involve no additional copy.
>
> I'm tired of trying cleaner alternatives, including:
>
>  a. extend memoryview to be compatible with buffer type
> => memoryview is not an acceptable base type
>  b. wrap memoryview by buffer-like class
> => zlib complains it isn't bytes-like
>
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -650,10 +650,10 @@ def _msearch(m, s, lo=0, hi=None):
>  that string.  If start == end the string was not found and
>  they indicate the proper sorted insertion point.
>
> -m should be a buffer or a string
> -s is a string'''
> +m should be a buffer, a memoryview or a byte string.
> +s is a byte string'''
>  def advance(i, c):
> -while i < lenm and m[i] != c:
> +while i < lenm and m[i:i + 1] != c:
>  i += 1
>  return i
>  if not s:
> @@ -664,10 +664,10 @@ def _msearch(m, s, lo=0, hi=None):
>  while lo < hi:
>  mid = (lo + hi) // 2
>  start = mid
> -while start > 0 and m[start - 1] != '\n':
> +while start > 0 and m[start - 1:start] != '\n':
>  start -= 1
>  end = advance(start, '\0')
> -if m[start:end] < s:
> +if bytes(m[start:end]) < s:
>  # we know that after the null there are 40 bytes of sha1
>  # this translates to the bisect lo = mid + 1
>  lo = advance(end + 40, '\n') + 1
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 07 of 11] revlog: fix many file handle opens to explicitly use bytes mode

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 18:36:41 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler 
> # Date 1490567324 14400
> #  Sun Mar 26 18:28:44 2017 -0400
> # Node ID e7fe3ab60132647a9c3b86b2960b385e61b9dcf0
> # Parent  48144fe2d912b7d9fc300955d0c881aceead6930
> revlog: fix many file handle opens to explicitly use bytes mode
> 
> This broke on Python 3, but we've probably been getting lucky that
> this hasn't caused EOL corruption on Windows or something for
> years. :(
> 
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -1467,8 +1467,8 @@ class revlog(object):
>  
>  dfh = None
>  if not self._inline:
> -dfh = self.opener(self.datafile, "a+")
> -ifh = self.opener(self.indexfile, "a+", checkambig=self._checkambig)
> +dfh = self.opener(self.datafile, "a+b")
> +ifh = self.opener(self.indexfile, "a+b", checkambig=self._checkambig)

'b' shouldn't be needed since vfs adds 'b' unless text=True is explicitly
specified.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 02 of 11] pycompat: forward encoding kwarg on open()

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 18:36:36 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler 
> # Date 1490562287 14400
> #  Sun Mar 26 17:04:47 2017 -0400
> # Node ID 28149aad82cb12522f1ba50b0bb184d1f960a9c9
> # Parent  1250b7d77ba41c7ce1500d4c8e5fef921a14f683
> pycompat: forward encoding kwarg on open()
> 
> diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
> --- a/mercurial/pycompat.py
> +++ b/mercurial/pycompat.py
> @@ -163,8 +163,8 @@ if ispy3:
>  setattr = _wrapattrfunc(builtins.setattr)
>  xrange = builtins.range
>  
> -def open(name, mode='r', buffering=-1):
> -return builtins.open(name, sysstr(mode), buffering)
> +def open(name, mode='r', buffering=-1, encoding=None):
> +return builtins.open(name, sysstr(mode), buffering, encoding)

I have mixed feeling about this. open() should work on both Python 2 and 3,
but encoding isn't supported on Python 2. However, we would have to use
builtins.open() without this change.

I'll play with this and the fat-bytes patch to see if they can be factored
out to utility functions.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 01 of 11] revsetlang: fix _quote on int on python3

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 18:36:35 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler 
> # Date 1490561309 14400
> #  Sun Mar 26 16:48:29 2017 -0400
> # Node ID 1250b7d77ba41c7ce1500d4c8e5fef921a14f683
> # Parent  e86eb75e74ce1b0803c26d86a229b9b711f6d76a
> revsetlang: fix _quote on int on python3

Queued this one, thanks.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 05 of 11] manifest: coerce _msearch argument to bytestr at method start

2017-03-27 Thread Yuya Nishihara
On Mon, 27 Mar 2017 11:49:45 +0530, Pulkit Goyal wrote:
> Yuya had a different approach on this
> https://patchwork.mercurial-scm.org/patch/19682/

[...]

> On Mon, Mar 27, 2017 at 4:06 AM, Augie Fackler  wrote:
> > # HG changeset patch
> > # User Augie Fackler 
> > # Date 1490567242 14400
> > #  Sun Mar 26 18:27:22 2017 -0400
> > # Node ID d5dcfa6b2e20183ba2d6e439a23f5f2f4bf7981e
> > # Parent  54631fab906cb662e370ce313a0395292f7dfa15
> > manifest: coerce _msearch argument to bytestr at method start
> >
> > This potentially incurs some extra copies, but on Python 3 you can't
> > do comparisons between memoryview and bytes sequences. I have a
> > sneaking suspicion there are potential bugs in Python 2 lurking that
> > this might fix (but I have no way of proving this theory), so I'm not
> > going to sweat the risk of extra overhead here.

We use buffer() on Python 2, which returns bytes for slicing operation.
That's why 'm[start:end] < s' works on Python 2. OTOH, memoryview() can't
be used in that way.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] runtests: unset editor and pager related environment variables

2017-03-27 Thread Augie Fackler
On Sun, Mar 26, 2017 at 08:59:52PM -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu 
> # Date 1490587134 25200
> #  Sun Mar 26 20:58:54 2017 -0700
> # Node ID 65f76087bc84162bdf0735f156d403e873e5c4e8
> # Parent  e86eb75e74ce1b0803c26d86a229b9b711f6d76a
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #  hg pull https://bitbucket.org/quark-zju/hg-draft -r 
> 65f76087bc84
> runtests: unset editor and pager related environment variables

Sure, queued. Thanks.

>
> Those environment variables could affect some configuration and future
> tests. Drop them to avoid issues.
>
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -907,5 +907,5 @@ class Test(unittest.TestCase):
>
>  for k in ('HG HGPROF CDPATH GREP_OPTIONS http_proxy no_proxy ' +
> -  'HGPLAIN HGPLAINEXCEPT ' +
> +  'HGPLAIN HGPLAINEXCEPT EDITOR VISUAL PAGER ' +
>'NO_PROXY CHGDEBUG').split():
>  if k in env:
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 1 of 5] repair: use ProgrammingError

2017-03-27 Thread Augie Fackler
On Mon, Mar 27, 2017 at 09:18:55AM +0200, Pierre-Yves David wrote:
> Sure, that seems like a good idea. Can you see to it ?

It's a good idea, but I feel like the name "ProgrammingError" alone
provides some clue as to what's going on, and the migration is good.

Queued, I wouldn't mind a followup to make the hint generic to all
ProgrammingError instances, but I'm not willing to block on it either.

> (I just replied on repair because this is the first changeset in the series)
>
> On 03/27/2017 08:57 AM, Jun Wu wrote:
> > I think ProgrammingError should all have that hint. So this is not repair.py
> > specific.
> >
> > Excerpts from Pierre-Yves David's message of 2017-03-27 08:27:51 +0200:
> > >
> > > On 03/27/2017 02:14 AM, Jun Wu wrote:
> > > > # HG changeset patch
> > > > # User Jun Wu 
> > > > # Date 1490572408 25200
> > > > #  Sun Mar 26 16:53:28 2017 -0700
> > > > # Node ID 92d7eff3f4c7e44e8aab62b486bda78a37b73b83
> > > > # Parent  e86eb75e74ce1b0803c26d86a229b9b711f6d76a
> > > > # Available At https://bitbucket.org/quark-zju/hg-draft
> > > > #  hg pull https://bitbucket.org/quark-zju/hg-draft  -r 
> > > > 92d7eff3f4c7
> > > > repair: use ProgrammingError
> > > >
> > > > diff --git a/mercurial/repair.py b/mercurial/repair.py
> > > > --- a/mercurial/repair.py
> > > > +++ b/mercurial/repair.py
> > > > @@ -164,6 +164,5 @@ def strip(ui, repo, nodelist, backup=Tru
> > > >  if curtr is not None:
> > > >  del curtr  # avoid carrying reference to transaction for 
> > > > nothing
> > > > -msg = _('programming error: cannot strip from inside a 
> > > > transaction')
> > > > -raise error.Abort(msg, hint=_('contact your extension 
> > > > maintainer'))
> > > > +raise error.ProgrammingError('cannot strip from inside a 
> > > > transaction')
> > >
> > > AS much as I like the idea of moving to Programming error, this patches
> > > series drops an important hint. Can we have a version that preserve the
> > > hint ?
> > >
>
> --
> Pierre-Yves David
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] runtests: unset editor and pager related environment variables

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 20:59:52 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu 
> # Date 1490587134 25200
> #  Sun Mar 26 20:58:54 2017 -0700
> # Node ID 65f76087bc84162bdf0735f156d403e873e5c4e8
> # Parent  e86eb75e74ce1b0803c26d86a229b9b711f6d76a
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #  hg pull https://bitbucket.org/quark-zju/hg-draft -r 
> 65f76087bc84
> runtests: unset editor and pager related environment variables

Sure. Queued, thanks.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 1 of 2] debugfsinfo: show fstype for given path

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 18:33:38 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu 
> # Date 1490574577 25200
> #  Sun Mar 26 17:29:37 2017 -0700
> # Node ID 0b8f4bab3597b1621c07bfc82b9c16a77459a9b1
> # Parent  e86eb75e74ce1b0803c26d86a229b9b711f6d76a
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #  hg pull https://bitbucket.org/quark-zju/hg-draft -r 
> 0b8f4bab3597
> debugfsinfo: show fstype for given path

You beat me. Queued, thanks.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] repair: use context manager for lock management

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 15:28:02 -0400, Matt Harbison wrote:
> On Sun, 26 Mar 2017 13:34:10 -0400, Gregory Szorc  
>  wrote:
> > On Sun, Mar 26, 2017 at 4:20 AM, Yuya Nishihara  wrote:
> >> On Fri, 24 Mar 2017 23:34:28 -0400, Matt Harbison wrote:
> >> > # HG changeset patch
> >> > # User Matt Harbison 
> >> > # Date 1490327243 14400
> >> > #  Thu Mar 23 23:47:23 2017 -0400
> >> > # Node ID c053dc8a24afad24872397e5cd3f57411fc7d172
> >> > # Parent  d0c2db2d9f13dca534c598de050eb1919ef79059
> >> > repair: use context manager for lock management
> >>
> >> Sure. Queued this, thanks.
> >>
> >> > I found several other instances of acquiring the lock inside of the
> >> 'try', but
> >> > those finally blocks handle None references.  I also started switching
> >> some
> >> > trivial try/finally blocks to context managers, but didn't get them  
> >> all
> >> because
> >> > indenting over 3x for lock, wlock and transaction would have spilled
> >> over 80
> >> > characters.  That got me wondering if there should be a repo.rwlock(),
> >> to handle
> >> > locking and unlocking in the proper order.

Just nitpicking. The name "rwlock" would be misleading since both locks are
for writing.

> >> We have lockmod.release() helper. We also have util.ctxmanager(), but  
> >> IMHO
> >> it
> >> doesn't improve code readability that much.
> >>
> >> > It also looks like py27 supports supports multiple context managers  
> >> for
> >> a single
> >> > 'with' statement.  Should I hold off on the rest until py26 is  
> >> dropped?
> >>
> >
> > I think there is room for a helper context manager (like repo.rwlock())
> > that obtains multiple locks and/or a transaction. This would cut down on  
> > a
> > lot of excessive indentation while simultaneously ensuring we're doing  
> > the
> > correct thing with regards to locking and transaction semantics.
> 
> The excessive indentation is why I raised the issue.  I went looking for  
> how to use util.ctxmanager, and can't find a single usage instance in the  
> entire history.  I went back to the ML, and it seems like mpm ruled out  
> using it [1].  The referenced context manager series he mentioned taking  
> 80% of seems to be something else [2].

IIRC, that's because the scope of lock, wlock, txn, etc. is sometimes
different.

> I'm not sure if we want to revisit that decision, and even if so, I'm not  
> sure if we want to start using a crutch for py26 if we are close to  
> dumping it.  I only got off on this tangent when I noticed the  
> first-lock-inside-try construct while trying to figure out bookmark  
> pruning test failures on Windows.  So I don't have a strong opinion either  
> way.

I'm -1 for excessive indentation of large code block. I generally use 'with'
only when it improves readability.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 6 of 7] py3: abuse r'' to preserve str-ness of literals passed to __setattr__()

2017-03-27 Thread Augie Fackler
On Sun, Mar 26, 2017 at 08:59:49PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara 
> # Date 1490513592 -32400
> #  Sun Mar 26 16:33:12 2017 +0900
> # Node ID c00753c42a0b04dc72ed831a87945f1eb41c84c1
> # Parent  5e8e48cb4f83a583515b4caaf97045b86747c499
> py3: abuse r'' to preserve str-ness of literals passed to __setattr__()

I've taken patches 1-4 and 6. Looking at 5 now and going to compare to
my version (I did less work, it looks lik you might have managed to
save some perf)

>
> diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
> --- a/mercurial/demandimport.py
> +++ b/mercurial/demandimport.py
> @@ -76,9 +76,9 @@ class _demandmod(object):
>  else:
>  head = name
>  after = []
> -object.__setattr__(self, "_data",
> +object.__setattr__(self, r"_data",
> (head, globals, locals, after, level, set()))
> -object.__setattr__(self, "_module", None)
> +object.__setattr__(self, r"_module", None)
>  def _extend(self, name):
>  """add to the list of submodules to load"""
>  self._data[3].append(name)
> @@ -138,7 +138,7 @@ class _demandmod(object):
>  if modref and getattr(modref, head, None) == self:
>  setattr(modref, head, mod)
>
> -object.__setattr__(self, "_module", mod)
> +object.__setattr__(self, r"_module", mod)
>
>  def __repr__(self):
>  if self._module:
> diff --git a/mercurial/pure/osutil.py b/mercurial/pure/osutil.py
> --- a/mercurial/pure/osutil.py
> +++ b/mercurial/pure/osutil.py
> @@ -342,8 +342,8 @@ else:
>  # unfortunately, f.name is '' at this point -- so we 
> store
>  # the name on this wrapper. We cannot just assign to f.name,
>  # because that attribute is read-only.
> -object.__setattr__(self, 'name', name)
> -object.__setattr__(self, '_file', f)
> +object.__setattr__(self, r'name', name)
> +object.__setattr__(self, r'_file', f)
>
>  def __iter__(self):
>  return self._file
> diff --git a/mercurial/vfs.py b/mercurial/vfs.py
> --- a/mercurial/vfs.py
> +++ b/mercurial/vfs.py
> @@ -480,7 +480,7 @@ class closewrapbase(object):
>  Do not instantiate outside of the vfs layer.
>  """
>  def __init__(self, fh):
> -object.__setattr__(self, '_origfh', fh)
> +object.__setattr__(self, r'_origfh', fh)
>
>  def __getattr__(self, attr):
>  return getattr(self._origfh, attr)
> @@ -507,7 +507,7 @@ class delayclosedfile(closewrapbase):
>  """
>  def __init__(self, fh, closer):
>  super(delayclosedfile, self).__init__(fh)
> -object.__setattr__(self, '_closer', closer)
> +object.__setattr__(self, r'_closer', closer)
>
>  def __exit__(self, exc_type, exc_value, exc_tb):
>  self._closer.close(self._origfh)
> @@ -618,7 +618,7 @@ class checkambigatclosing(closewrapbase)
>  """
>  def __init__(self, fh):
>  super(checkambigatclosing, self).__init__(fh)
> -object.__setattr__(self, '_oldstat', util.filestat(fh.name))
> +object.__setattr__(self, r'_oldstat', util.filestat(fh.name))
>
>  def _checkambig(self):
>  oldstat = self._oldstat
> diff --git a/mercurial/windows.py b/mercurial/windows.py
> --- a/mercurial/windows.py
> +++ b/mercurial/windows.py
> @@ -61,8 +61,8 @@ class mixedfilemodewrapper(object):
>  OPWRITE = 2
>
>  def __init__(self, fp):
> -object.__setattr__(self, '_fp', fp)
> -object.__setattr__(self, '_lastop', 0)
> +object.__setattr__(self, r'_fp', fp)
> +object.__setattr__(self, r'_lastop', 0)
>
>  def __getattr__(self, name):
>  return getattr(self._fp, name)
> @@ -74,42 +74,42 @@ class mixedfilemodewrapper(object):
>  self._fp.seek(0, os.SEEK_CUR)
>
>  def seek(self, *args, **kwargs):
> -object.__setattr__(self, '_lastop', self.OPNONE)
> +object.__setattr__(self, r'_lastop', self.OPNONE)
>  return self._fp.seek(*args, **kwargs)
>
>  def write(self, d):
>  if self._lastop == self.OPREAD:
>  self._noopseek()
>
> -object.__setattr__(self, '_lastop', self.OPWRITE)
> +object.__setattr__(self, r'_lastop', self.OPWRITE)
>  return self._fp.write(d)
>
>  def writelines(self, *args, **kwargs):
>  if self._lastop == self.OPREAD:
>  self._noopeseek()
>
> -object.__setattr__(self, '_lastop', self.OPWRITE)
> +object.__setattr__(self, r'_lastop', self.OPWRITE)
>  return self._fp.writelines(*args, **kwargs)
>
>  def read(self, *args, **kwargs):
>  if self._lastop == self.OPWRITE:
>  self._noopseek()
>
> -object.__setattr__(self, '_lastop', self.OPREAD)
> +object.__setattr__(self, r'_lastop', self.OPREAD)
>  return self._fp.read(*arg

Re: [PATCH 2 of 4] color: stop mutating the default effects map

2017-03-27 Thread Pierre-Yves David

On 03/26/2017 04:35 PM, Yuya Nishihara wrote:

On Sun, 26 Mar 2017 00:41:05 -0400, Matt Harbison wrote:

# HG changeset patch
# User Matt Harbison 
# Date 1490464217 14400
#  Sat Mar 25 13:50:17 2017 -0400
# Node ID a263702b064a5a3ce1ca74b227e8e624e4b05874
# Parent  22533c3af63d5a67d9210596eafd4b99ab9c7904
color: stop mutating the default effects map


I re-read the code, and noticed that both _effects and w32effects are mostly
constants. Perhaps we can simply switch them by ui._colormode.

  if ui._colormode == 'win32':
  activeeffects = w32neffects
  else:
  activeeffects = _effects
  ... embed escape sequences by using activeeffects ...


Ultimately, yes we want to be there, but I remember other things muding 
with the effects dict so someone would have to give a close look to the 
current magic before moving forward.



--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 5 of 6 py3] test-check-py3-commands: cleanup tests related to `hg status`

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 21:16:44 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pul...@gmail.com>
> # Date 1490542825 -19800
> #  Sun Mar 26 21:10:25 2017 +0530
> # Node ID d0daada30518fb8fb12948c61e1eabb6494148cf
> # Parent  ad78dfda4e5669df04ea6441740f60658c2668a7
> test-check-py3-commands: cleanup tests related to `hg status`

Queued 1, 3, 4, and 5, thanks.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 2 of 6 py3] diff: slice over bytes to make sure conditions work normally

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 21:16:41 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pul...@gmail.com>
> # Date 1490541771 -19800
> #  Sun Mar 26 20:52:51 2017 +0530
> # Node ID 989c6385adcc3385ab984a718e09fa5d694563c4
> # Parent  2632df096fc0ac7582382b1f94ea4b9ad0bce8f2
> diff: slice over bytes to make sure conditions work normally
> 
> Both of this are part of generating `hg diff` on python 3.
> 
> diff -r 2632df096fc0 -r 989c6385adcc mercurial/mdiff.py
> --- a/mercurial/mdiff.py  Sun Mar 26 20:49:18 2017 +0530
> +++ b/mercurial/mdiff.py  Sun Mar 26 20:52:51 2017 +0530
> @@ -227,7 +227,7 @@
>  
>  def checknonewline(lines):
>  for text in lines:
> -if text[-1] != '\n':
> +if text[-1:len(text)] != '\n':
>  text += "\n\ No newline at end of file\n"
>  yield text
>  
> diff -r 2632df096fc0 -r 989c6385adcc mercurial/patch.py
> --- a/mercurial/patch.py  Sun Mar 26 20:49:18 2017 +0530
> +++ b/mercurial/patch.py  Sun Mar 26 20:52:51 2017 +0530
> @@ -737,7 +737,7 @@
>  for x in self.rej:
>  for l in x.hunk:
>  lines.append(l)
> -if l[-1] != '\n':
> +if l[-1:len(l)] != '\n':

len(l) isn't needed: l[-1:]
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 2 of 4] color: stop mutating the default effects map

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 13:27:57 -0400, Matt Harbison wrote:
> On Sun, 26 Mar 2017 10:35:34 -0400, Yuya Nishihara  wrote:
> > On Sun, 26 Mar 2017 00:41:05 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison 
> >> # Date 1490464217 14400
> >> #  Sat Mar 25 13:50:17 2017 -0400
> >> # Node ID a263702b064a5a3ce1ca74b227e8e624e4b05874
> >> # Parent  22533c3af63d5a67d9210596eafd4b99ab9c7904
> >> color: stop mutating the default effects map
> >
> > I re-read the code, and noticed that both _effects and w32effects are  
> > mostly
> > constants. Perhaps we can simply switch them by ui._colormode.
> >
> >   if ui._colormode == 'win32':
> >   activeeffects = w32neffects
> >   else:
> >   activeeffects = _effects
> >   ... embed escape sequences by using activeeffects ...
> 
> Will we get into trouble with each ui instance having a _colormode, but  
> the activeeffects map effectively being global?

Yeah, maybe. But we can select either w32effects or _effects only by
ui._colormode. Unlike ui._styles, effects table is not user configurable,
which probably means we don't have to store a copy of effects in ui.

>  I don't fully understand  
> the purpose of all the separate ui objects.

That wouldn't be practically important for color, but it isn't nice to
mutate globals depending on ui.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 4 of 4] ui: rerun color.setup() once the pager has spawned to honor 'color.pagermode'

2017-03-27 Thread Yuya Nishihara
On Sun, 26 Mar 2017 14:05:35 -0400, Matt Harbison wrote:
> On Sun, 26 Mar 2017 10:44:01 -0400, Yuya Nishihara  wrote:
> 
> > On Sun, 26 Mar 2017 00:41:07 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison 
> >> # Date 1490483831 14400
> >> #  Sat Mar 25 19:17:11 2017 -0400
> >> # Node ID 4713a38672f2d0790477b6c22180bd453f61851d
> >> # Parent  84bda5db69dbe3d550f45ccd6d6eda2aad760ee4
> >> ui: rerun color.setup() once the pager has spawned to honor  
> >> 'color.pagermode'
> >>
> >> Otherwise, ui.pageractive is False when color is setup in dispatch.py  
> >> (without
> >> --pager=on), and this config option is ignored.
> >>
> >> diff --git a/mercurial/ui.py b/mercurial/ui.py
> >> --- a/mercurial/ui.py
> >> +++ b/mercurial/ui.py
> >> @@ -861,6 +861,12 @@
> >>  # auto-detection of things being formatted.
> >>  self.setconfig('ui', 'formatted', wasformatted, 'pager')
> >>  self.setconfig('ui', 'interactive', False, 'pager')
> >> +
> >> +# If pagermode differs from color.mode, reconfigure color  
> >> now that
> >> +# pageractive is set.
> >> +cm = self._colormode
> >> +if cm != self.config('color', 'pagermode', cm):
> >> +color.setup(self)
> >
> > This also looks good. Maybe we can refactor color._modesetup() further  
> > so that
> > we can do something like
> >
> >   newmode = color.mode(self)
> >   if self._colormode != newmode:
> >   color.setup(self, newmode)
> 
> Is a follow up OK?

Sure. It will be far from trivial.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 3] histedit: test that an aborted histedit can be rerun (with obsolete)

2017-03-27 Thread Augie Fackler
On Mon, Mar 27, 2017 at 02:04:44PM +0200, Pierre-Yves David wrote:
>
>
> On 03/27/2017 12:31 PM, Jun Wu wrote:
> > Excerpts from Pierre-Yves David's message of 2017-03-27 09:19:52 +0200:
> > >
> > > On 03/26/2017 08:42 PM, Jun Wu wrote:
> > > > Excerpts from Jun Wu's message of 2017-03-26 09:59:43 -0700:
> > > > > I'm -1 on this series.
> > > > >
> > > > > If the goal is to workaround obsolete cycles, the code could do 
> > > > > additional
> > > > > checks "if the destination is obsoleted" and do "hg touch"-like thing
> > > > > automatically.
> > > >
> > > > FYI I have sent the series doing the above.
> > >
> > > I've commented on that series too, your new behavior still introduce
> > > issue in distributed case.
> > >
> > > I think we should backout for now (since this is a regression, that
> > > exist on default right now).
> > > In the mean time we take a step back and think about the situation cold
> > > headed.
> > >
> > > See comment on the other series for details,
> > >
> > > Cheers,
> > >
> >
> > I have resolved the issue you commented at [1].
>
> Actually no there are still issues. Let us channel the discussion here,
> spreading the discussion across multiple threads is getting confusing.
> Especially when series rely on other unlanded series.
>
> > I'm still not convinced that we have to go back to use strip.
>
> So far, we have a regression at hand and no known path forward fully solving
> the issue. I would like you to slow down and discuss possible alternative to
> backout in this thread. Please to not rush to writing code and do not send a
> new series until we reached a consensus here on the problem space and a
> viable solution.

Pierre-Yves is right: this is a regression, and needs to be fixed
before 4.2 is final. Given that the freeze is in three weeks, I'm
going to take this series and if we can work out a more elegant fix in
the next couple of weeks I'll be very happy.

In general, regressions are The Worst. We've got a few open regression
bugs in the BTS that have been open unacceptably long, and I'm
inclined to be aggressive about avoiding any more cases like that.

(Jun: don't take my acceptance of these changes as a rejection of your
ideas! It's just temporary damage control until we get a better
solution in place.)

>
> Cheers,
>
> --
> Pierre-Yves David
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 3] histedit: test that an aborted histedit can be rerun (with obsolete)

2017-03-27 Thread Pierre-Yves David



On 03/27/2017 12:31 PM, Jun Wu wrote:

Excerpts from Pierre-Yves David's message of 2017-03-27 09:19:52 +0200:


On 03/26/2017 08:42 PM, Jun Wu wrote:

Excerpts from Jun Wu's message of 2017-03-26 09:59:43 -0700:

I'm -1 on this series.

If the goal is to workaround obsolete cycles, the code could do additional
checks "if the destination is obsoleted" and do "hg touch"-like thing
automatically.


FYI I have sent the series doing the above.


I've commented on that series too, your new behavior still introduce
issue in distributed case.

I think we should backout for now (since this is a regression, that
exist on default right now).
In the mean time we take a step back and think about the situation cold
headed.

See comment on the other series for details,

Cheers,



I have resolved the issue you commented at [1].


Actually no there are still issues. Let us channel the discussion here, 
spreading the discussion across multiple threads is getting confusing. 
Especially when series rely on other unlanded series.



I'm still not convinced that we have to go back to use strip.


So far, we have a regression at hand and no known path forward fully 
solving the issue. I would like you to slow down and discuss possible 
alternative to backout in this thread. Please to not rush to writing 
code and do not send a new series until we reached a consensus here on 
the problem space and a viable solution.


Cheers,

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 3] histedit: revive commits on demand

2017-03-27 Thread Pierre-Yves David



On 03/27/2017 12:29 PM, Jun Wu wrote:

Excerpts from Pierre-Yves David's message of 2017-03-27 09:17:59 +0200:


On 03/26/2017 08:41 PM, Jun Wu wrote:

# HG changeset patch
# User Jun Wu 
# Date 1490552007 25200
#  Sun Mar 26 11:13:27 2017 -0700
# Node ID b6766d75404fb8c5d26af016caa76f44b47ce156
# Parent  336512ee2f947f07149e399a84927f9d820d2b62
# Available At https://bitbucket.org/quark-zju/hg-draft
#  hg pull https://bitbucket.org/quark-zju/hg-draft  -r b6766d75404f
histedit: revive commits on demand

This is to address the "histedit --abort" issue mentioned in [1].


There are still issues with that approach. For example in this
distributed case:

1) Alice send changesets to Bob,
2) Bob try to histedit something in the stack, this fails
3) Bob --abort the histedit
4) Bob tell Alice to perform the operation herself since she know the
code better and can resolve the conflict
5) Alice histedit with success


Good example. But that's easily solvable by not writing the "parent"
information when creating the markers, so the markers won't get exchanged.


This does not solve the issue, When Bob will pull from Alice, the issue 
will appears in Bob repository.


Please slow down.

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 3] histedit: test that an aborted histedit can be rerun (with obsolete)

2017-03-27 Thread Jun Wu
Excerpts from Pierre-Yves David's message of 2017-03-27 09:19:52 +0200:
> 
> On 03/26/2017 08:42 PM, Jun Wu wrote:
> > Excerpts from Jun Wu's message of 2017-03-26 09:59:43 -0700:
> >> I'm -1 on this series.
> >>
> >> If the goal is to workaround obsolete cycles, the code could do additional
> >> checks "if the destination is obsoleted" and do "hg touch"-like thing
> >> automatically.
> >
> > FYI I have sent the series doing the above.
> 
> I've commented on that series too, your new behavior still introduce 
> issue in distributed case.
> 
> I think we should backout for now (since this is a regression, that 
> exist on default right now).
> In the mean time we take a step back and think about the situation cold 
> headed.
> 
> See comment on the other series for details,
> 
> Cheers,
> 

I have resolved the issue you commented at [1]. I'm still not convinced that
we have to go back to use strip.

[1]: 
https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-March/095688.html
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 3] histedit: revive commits on demand

2017-03-27 Thread Jun Wu
Excerpts from Pierre-Yves David's message of 2017-03-27 09:17:59 +0200:
> 
> On 03/26/2017 08:41 PM, Jun Wu wrote:
> > # HG changeset patch
> > # User Jun Wu 
> > # Date 1490552007 25200
> > #  Sun Mar 26 11:13:27 2017 -0700
> > # Node ID b6766d75404fb8c5d26af016caa76f44b47ce156
> > # Parent  336512ee2f947f07149e399a84927f9d820d2b62
> > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > #  hg pull https://bitbucket.org/quark-zju/hg-draft  -r 
> > b6766d75404f
> > histedit: revive commits on demand
> >
> > This is to address the "histedit --abort" issue mentioned in [1].
> 
> There are still issues with that approach. For example in this 
> distributed case:
> 
> 1) Alice send changesets to Bob,
> 2) Bob try to histedit something in the stack, this fails
> 3) Bob --abort the histedit
> 4) Bob tell Alice to perform the operation herself since she know the 
> code better and can resolve the conflict
> 5) Alice histedit with success

Good example. But that's easily solvable by not writing the "parent"
information when creating the markers, so the markers won't get exchanged.

> If step (3) created markers for the histedit partial result, these 
> markers will apply to the result of (5) and create confusion/change 
> "loss" when the two states are gathered.
> 
> This is a non-trivial question and there is good reason why rebase and 
> histedit do not use obsolescence marker to restore the repository to its 
> former state.
> 
> note: this is a case were a local-light-weight "archiving" mechanism 
> could be handy.
> 
> > This solution is safer than what [1] proposed because it does not use unsafe
> > history writing ("strip"). The "strip" has caused several repo corruptions
> > in production.
> 
> I'm sure upstream would be happy to have some bug report about this and 
> maybe some investigation about this corruption. Strip corrupting 
> repository would happily rank a "critical".
> 
> > It's similar to what "hg touch" in mutable-history does [3], without using
> > random number. So it could be more reliably tested.
> 
> The reason why 'touch' use random number is to ensure operation in one 
> repository will not -unexpectedly- affect the one in another repo (kind 
> of like here). We could use sometimes else than random but it cannot be 
> a simple sequential number.
> 
> Note that using random will not solve the issue here since (5) does not 
> even know yet that the markers in (3) exists.
> 
> > Note that if we have obsolete marker versions, and obsoleted revisions are
> > revived automatically, as I proposed in [2], the "revive" hack will be
> > longer be necessary.
> 
> Please see my (fresh) reply to that other series.
> 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 1 of 2] obsolete: allow creating local prune markers that is not exchanged

2017-03-27 Thread Jun Wu
# HG changeset patch
# User Jun Wu 
# Date 1490609835 25200
#  Mon Mar 27 03:17:15 2017 -0700
# Node ID a3e835b04db41230b02233d5c7c3f4dee49407d3
# Parent  e86eb75e74ce1b0803c26d86a229b9b711f6d76a
# Available At https://bitbucket.org/quark-zju/hg-draft
#  hg pull https://bitbucket.org/quark-zju/hg-draft -r a3e835b04db4
obsolete: allow creating local prune markers that is not exchanged

This allows us to do "safe strip" without worrying about the markers being
incorrectly exchanged to others.

This is supposed to be transitional. Once we have root-based hidden, we
could switch to writing hidden roots.

diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
--- a/mercurial/obsolete.py
+++ b/mercurial/obsolete.py
@@ -1213,4 +1213,7 @@ def createmarkers(repo, relations, flag=
 metadata specified through the `metadata` argument of this function,
 
+The (, ...) part could also be None, in that case, a local prune
+marker will be created and it won't be exchanged.
+
 Trying to obsolete a public changeset will raise an exception.
 
@@ -1241,8 +1244,12 @@ def createmarkers(repo, relations, flag=
  hint="see 'hg help phases' for details")
 nprec = prec.node()
-nsucs = tuple(s.node() for s in sucs)
 npare = None
-if not nsucs:
-npare = tuple(p.node() for p in prec.parents())
+if sucs is None:
+# a local "prune" marker that will not be exchanged
+nsucs = ()
+else:
+nsucs = tuple(s.node() for s in sucs)
+if not nsucs:
+npare = tuple(p.node() for p in prec.parents())
 if nprec in nsucs:
 raise error.Abort(_("changeset %s cannot obsolete itself")
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 2 of 2] histedit: make prune markers local

2017-03-27 Thread Jun Wu
# HG changeset patch
# User Jun Wu 
# Date 1490610065 25200
#  Mon Mar 27 03:21:05 2017 -0700
# Node ID 293c829c83b02501a44a45bc2ed9794cec00e023
# Parent  a3e835b04db41230b02233d5c7c3f4dee49407d3
# Available At https://bitbucket.org/quark-zju/hg-draft
#  hg pull https://bitbucket.org/quark-zju/hg-draft -r 293c829c83b0
histedit: make prune markers local

This address the issue mentioned at [1].

[1]: 
https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-March/095678.html

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -1599,5 +1599,5 @@ def safecleanupnode(ui, repo, name, node
 # nodes is a set-like
 def getmarker(prec):
-return (repo[prec], ())
+return (repo[prec], None)
 # sort by revision number because it sound "right"
 sortednodes = sorted([n for n in nodes if n in repo],
diff --git a/tests/test-histedit-obsolete.t b/tests/test-histedit-obsolete.t
--- a/tests/test-histedit-obsolete.t
+++ b/tests/test-histedit-obsolete.t
@@ -171,6 +171,6 @@ Base setup for the rest of the testing
   
   $ hg debugobsolete
-  96e494a2d553dd05902ba1cee1d94d4cb7b8faed 0 
{b346ab9a313db8537ecf96fca3ca3ca984ef3bd7} (*) {'user': 'test'} (glob)
-  b558abc46d09c30f57ac31e85a8a3d64d2e906e4 0 
{96e494a2d553dd05902ba1cee1d94d4cb7b8faed} (*) {'user': 'test'} (glob)
+  96e494a2d553dd05902ba1cee1d94d4cb7b8faed 0 (*) {'user': 'test'} (glob)
+  b558abc46d09c30f57ac31e85a8a3d64d2e906e4 0 (*) {'user': 'test'} (glob)
   d2ae7f538514cd87c17547b0de4cea71fe1af9fb 0 
{cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b} (*) {'user': 'test'} (glob)
   177f92b773850b59254aa5e923436f921b55483b 
b346ab9a313db8537ecf96fca3ca3ca984ef3bd7 0 (*) {'user': 'test'} (glob)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 1 of 4 V2] obsolete: track node versions

2017-03-27 Thread Jun Wu
Thanks for the detailed reply. I have replies on multiple subtopics. But I
deleted topics that I think are less important, to make the most important
topic clear, and make the discussion more efficient. If you think I need to
respond to more topics, feel free to point me at them.

Excerpts from Pierre-Yves David's message of 2017-03-27 09:14:53 +0200:

[snip]

> Simple practical example with date:
> ---
> 
> [time-1] repo-B: changeset C-A is pulled from repo-A
> [time-2] repo-A: changeset C-A is rewritten as C-B (time-2)
> [time-3] repo-B: changeset C-C is marked as superceeded by C-A (time-3)
> [time-4] repo-B: Changeset C-B (with marker) is pulled from repo-A
> 
> The markers pointing from C-B to C-A have a version "time-2" but "C-A" 
> now have a version of "time-3". "C-A" become wrongly un-obsoleted (and 

I think we basically disagree here. You said that "C-A" being visible does
not match "the meaning of the user action", could you define "the meaning of
the user action" formally ?

In the "node version" approach, the "meaning of a user action", where action
is "creating a marker", is defined as below:

  When the user replace changeset X with Y, the marker X -> Y gets created,
  they want Y to be visible, regardless of what previous (local or remote)
  attempts hiding it.

So the un-obsolete behavior is expected.

> C-B is in strange state). The fact C-A is un-obsoleted here is a bug and 

Strictly speaking, C-B still has an obsoleted precursor of C-A. If we draw a
2-D graph, where Y-axis is revision, X-axis is time, and markers are edges,
the above case could be represented like:

   ^ rev
   |  C-C
   |\
   |  C-A   C-A
   | \
   | C-B
   |
   +---> time

Note that there are 2 "C-A"s, the right one is visible, the left one is not.
We show users the right one. Internally, the code could distinguish the
different of 2 "C-A"s, if it wants.

> do not match the meaning of the user action. This is a regression from 
> what evolution is currently able to achieve.

[snip] 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


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

2017-03-27 Thread Pierre-Yves David



On 03/27/2017 12:32 AM, Kostia Balytskyi wrote:

# HG changeset patch
# User Kostia Balytskyi 
# 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. 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.


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.



'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 < 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 < .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@mercuri

Re: [PATCH 03 of 10 shelve-ext v4] shelve: rename stripnodes to nodestoprune

2017-03-27 Thread Kostia Balytskyi
On 21/03/2017 22:03, Jun Wu wrote:

> Excerpts from Augie Fackler's message of 2017-03-21 17:58:54 -0400:
>> On Sat, Mar 11, 2017 at 01:00:22PM -0800, Kostia Balytskyi wrote:
>>> # HG changeset patch
>>> # User Kostia Balytskyi 
>>> # Date 1489191523 28800
>>> #  Fri Mar 10 16:18:43 2017 -0800
>>> # Node ID 8063b183d2396f14093ebe84da9cedf17b13881d
>>> # Parent  13c8fb8e722fd0563a83e601bb784694535268f1
>>> shelve: rename stripnodes to nodestoprune
>> I (sadly) found some things I at least need answered in later patches,
>> but the first three patches are queued. Thanks!
>>
>> One thing I didn't ask elsewhere but that might be worth exploring is
>> how this series can/should interact with the safecleanupnode() work
>> junw did over in histedit. It feels at least vaguely related.
> I think we want to move the "strip nodes, optionally write obs markers"
> logic to core. It will simplify code and make it easier to migrate to
> root-based hidden w/o obsstore.

Makes sense. Please see patch "[PATCH RFC] repo: add an ability to hide nodes 
in an appropriate way".

>
>>> Since we are introducing obs-based shelve, we are no longer
>>> stripping temporary nodes, we are obsoleting them. Therefore
>>> it looks like stipnodes would be a misleading name, while
>>> prune has a connotaion of "strip but with obsolescense", so
>>> nodestoprune seems like a good rename.
>>>
>>> diff --git a/hgext/shelve.py b/hgext/shelve.py
>>> --- a/hgext/shelve.py
>>> +++ b/hgext/shelve.py
>>> @@ -184,7 +184,7 @@ class shelvedstate(object):
>>>   wctx = nodemod.bin(fp.readline().strip())
>>>   pendingctx = nodemod.bin(fp.readline().strip())
>>>   parents = [nodemod.bin(h) for h in fp.readline().split()]
>>> -stripnodes = [nodemod.bin(h) for h in fp.readline().split()]
>>> +nodestoprune = [nodemod.bin(h) for h in fp.readline().split()]
>>>   branchtorestore = fp.readline().strip()
>>>   keep = fp.readline().strip() == cls._keep
>>>   except (ValueError, TypeError) as err:
>>> @@ -198,7 +198,7 @@ class shelvedstate(object):
>>>   obj.wctx = repo[wctx]
>>>   obj.pendingctx = repo[pendingctx]
>>>   obj.parents = parents
>>> -obj.stripnodes = stripnodes
>>> +obj.nodestoprune = nodestoprune
>>>   obj.branchtorestore = branchtorestore
>>>   obj.keep = keep
>>>   except error.RepoLookupError as err:
>>> @@ -207,7 +207,7 @@ class shelvedstate(object):
>>>   return obj
>>>
>>>   @classmethod
>>> -def save(cls, repo, name, originalwctx, pendingctx, stripnodes,
>>> +def save(cls, repo, name, originalwctx, pendingctx, nodestoprune,
>>>branchtorestore, keep=False):
>>>   fp = repo.vfs(cls._filename, 'wb')
>>>   fp.write('%i\n' % cls._version)
>>> @@ -217,7 +217,7 @@ class shelvedstate(object):
>>>   fp.write('%s\n' %
>>>' '.join([nodemod.hex(p) for p in 
>>> repo.dirstate.parents()]))
>>>   fp.write('%s\n' %
>>> - ' '.join([nodemod.hex(n) for n in stripnodes]))
>>> + ' '.join([nodemod.hex(n) for n in nodestoprune]))
>>>   fp.write('%s\n' % branchtorestore)
>>>   fp.write('%s\n' % (cls._keep if keep else cls._nokeep))
>>>   fp.close()
>>> @@ -569,7 +569,7 @@ def unshelveabort(ui, repo, state, opts)
>>>   raise
>>>
>>>   mergefiles(ui, repo, state.wctx, state.pendingctx)
>>> -repair.strip(ui, repo, state.stripnodes, backup=False,
>>> +repair.strip(ui, repo, state.nodestoprune, backup=False,
>>>topic='shelve')
>>>   finally:
>>>   shelvedstate.clear(repo)
>>> @@ -642,12 +642,12 @@ def unshelvecontinue(ui, repo, state, op
>>>   shelvectx = state.pendingctx
>>>   else:
>>>   # only strip the shelvectx if the rebase produced it
>>> -state.stripnodes.append(shelvectx.node())
>>> +state.nodestoprune.append(shelvectx.node())
>>>
>>>   mergefiles(ui, repo, state.wctx, shelvectx)
>>>   restorebranch(ui, repo, state.branchtorestore)
>>>
>>> -repair.strip(ui, repo, state.stripnodes, backup=False, 
>>> topic='shelve')
>>> +repair.strip(ui, repo, state.nodestoprune, backup=False, 
>>> topic='shelve')
>>>   shelvedstate.clear(repo)
>>>   unshelvecleanup(ui, repo, state.name, opts)
>>>   ui.status(_("unshelve of '%s' complete\n") % state.name)
>>> @@ -699,9 +699,9 @@ def _rebaserestoredcommit(ui, repo, opts
>>>   except error.InterventionRequired:
>>>   tr.close()
>>>
>>> -stripnodes = [repo.changelog.node(rev)
>>> -  for rev in xrange(oldtiprev, len(repo))]
>>> -shelvedstate.save(repo, basename, pctx, tmpwctx, stripnodes,
>>> +nodestoprune = [repo.changelog.node(rev)
>>> + 

Re: [PATCH 4 of 4] summary: display obsolete state of parents

2017-03-27 Thread Denis Laxalde

Yuya Nishihara a écrit :

On Sat, 25 Mar 2017 14:09:19 +0100, Denis Laxalde wrote:

# HG changeset patch
# User Denis Laxalde 
# Date 1490437808 -3600
#  Sat Mar 25 11:30:08 2017 +0100
# Node ID af7190c42c1cd6ba16a9bd83ad54208d89f343fe
# Parent  94a70894c394268b845f427f7643738a70e336bb
# Available At https://bitbucket.org/dlax/hg-work
#  hg pull https://bitbucket.org/dlax/hg-work -r af7190c42c1c
# EXP-Topic obsolete-ui
summary: display obsolete state of parents

Extend the "parent: " lines in summary to display "(obsolete)" when the parent
is obsolete.


The code looks good, but I have no idea how the bulk bikeshedding about evolve
naming ended.



Are you referring to https://www.mercurial-scm.org/wiki/CEDVocabulary ?
It seems to me that the "obsolete" term is accepted (nor really
discussed in this page at least).
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 2 of 4] templatekw: add an "obsolete" keyword

2017-03-27 Thread Denis Laxalde

Yuya Nishihara a écrit :

On Sat, 25 Mar 2017 14:09:17 +0100, Denis Laxalde wrote:

# HG changeset patch
# User Denis Laxalde 
# Date 1490434451 -3600
#  Sat Mar 25 10:34:11 2017 +0100
# Node ID c4c825f1402861e4b988395ac3deebcc6b5292cf
# Parent  130358da2eb894aa04e3bb731d0ccae84205c2ba
# Available At https://bitbucket.org/dlax/hg-work
#  hg pull https://bitbucket.org/dlax/hg-work -r c4c825f14028
# EXP-Topic obsolete-ui
templatekw: add an "obsolete" keyword

This keyword is a Boolean that indicates if a changeset is obsolete.

diff --git a/mercurial/templatekw.py b/mercurial/templatekw.py
--- a/mercurial/templatekw.py
+++ b/mercurial/templatekw.py
@@ -514,6 +514,12 @@ def shownode(repo, ctx, templ, **args):
 """
 return ctx.hex()

+@templatekeyword('obsolete')
+def showobsolete(repo, ctx, templ, **args):
+"""Boolean. True if this changeset is obsolete.
+"""
+return ctx.obsolete()


Seems fine, but this conflicts with the string version provided by evolve.
Have we decided to switch to boolean?



Looking at history of evolve's definition I see that there used to be
several possible string values before f4047fba5e90 but now there's only
one: 
https://bitbucket.org/marmoute/mutable-history/commits/f4047fba5e9060009aea62598416991961374243

Will there be several values again? If no, a Boolean seems natural to
me. Otherwise, I can certainly align with evolve's version.

(CC Pierre-Yves and Martin for advice)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[Bug 5515] New: zsh completion: after abbreviation 'bo' for 'bookmarks' the wrong set of completions is offered.

2017-03-27 Thread mercurial-bugs
https://bz.mercurial-scm.org/show_bug.cgi?id=5515

Bug ID: 5515
   Summary: zsh completion: after abbreviation 'bo' for
'bookmarks' the wrong set of completions is offered.
   Product: Mercurial
   Version: 4.1.1
  Hardware: All
OS: All
Status: UNCONFIRMED
  Severity: bug
  Priority: wish
 Component: third-party
  Assignee: bugzi...@mercurial-scm.org
  Reporter: mtie...@gmx.com
CC: mercurial-devel@mercurial-scm.org

Hi, 

Summary:
I use zsh completion provided in contrib/.
A minor issue I found is when using the abbreviation 'bo' for 'bookmarks'
the set of completions offered are the 'global' ones instead of the 
'bookmarks'-specific ones.

Example:
when typing
$ hg bo --re
it completes to
$ hg bo --repository=

Instead it should offer the same completions as when using 'bookmark'
$ hg bookmark --re
 -- option --
--rename  -- rename a given bookmark
--repository  -- repository root directory
--rev -- revision

Analysis:
In
https://www.mercurial-scm.org/repo/hg-stable/file/ed5b25874d99/contrib/zsh_completion
it goes wrong in lines 96/97:
In l.96 it searches for 'bo*' from the front of the list of all command
finding 'bookmarks'.
In the condition in l.97 it compares the previously found match with the first
match starting from the back of the list of all commands ('bookmark'). The 
condition is therefore not fulfilled and the original command ('bo') is not
replaced with 'bookmarks'. This leads to the completion offering the 'normal'
completions in l.122/123. 

To me it is not obvious why the implementation uses once the first match from 
the start the list of all commands and once from the end. Using the always
the first one works for me in my daily work but I do not know enough about
Mercurial to be sure that this is the way to go.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 3] histedit: test that an aborted histedit can be rerun (with obsolete)

2017-03-27 Thread Pierre-Yves David



On 03/26/2017 08:42 PM, Jun Wu wrote:

Excerpts from Jun Wu's message of 2017-03-26 09:59:43 -0700:

I'm -1 on this series.

If the goal is to workaround obsolete cycles, the code could do additional
checks "if the destination is obsoleted" and do "hg touch"-like thing
automatically.


FYI I have sent the series doing the above.


I've commented on that series too, your new behavior still introduce 
issue in distributed case.


I think we should backout for now (since this is a regression, that 
exist on default right now).
In the mean time we take a step back and think about the situation cold 
headed.


See comment on the other series for details,

Cheers,

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 1 of 5] repair: use ProgrammingError

2017-03-27 Thread Pierre-Yves David

Sure, that seems like a good idea. Can you see to it ?
(I just replied on repair because this is the first changeset in the series)

On 03/27/2017 08:57 AM, Jun Wu wrote:

I think ProgrammingError should all have that hint. So this is not repair.py
specific.

Excerpts from Pierre-Yves David's message of 2017-03-27 08:27:51 +0200:


On 03/27/2017 02:14 AM, Jun Wu wrote:

# HG changeset patch
# User Jun Wu 
# Date 1490572408 25200
#  Sun Mar 26 16:53:28 2017 -0700
# Node ID 92d7eff3f4c7e44e8aab62b486bda78a37b73b83
# Parent  e86eb75e74ce1b0803c26d86a229b9b711f6d76a
# Available At https://bitbucket.org/quark-zju/hg-draft
#  hg pull https://bitbucket.org/quark-zju/hg-draft  -r 92d7eff3f4c7
repair: use ProgrammingError

diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -164,6 +164,5 @@ def strip(ui, repo, nodelist, backup=Tru
 if curtr is not None:
 del curtr  # avoid carrying reference to transaction for nothing
-msg = _('programming error: cannot strip from inside a transaction')
-raise error.Abort(msg, hint=_('contact your extension maintainer'))
+raise error.ProgrammingError('cannot strip from inside a transaction')


AS much as I like the idea of moving to Programming error, this patches
series drops an important hint. Can we have a version that preserve the
hint ?



--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 3] histedit: revive commits on demand

2017-03-27 Thread Pierre-Yves David



On 03/26/2017 08:41 PM, Jun Wu wrote:

# HG changeset patch
# User Jun Wu 
# Date 1490552007 25200
#  Sun Mar 26 11:13:27 2017 -0700
# Node ID b6766d75404fb8c5d26af016caa76f44b47ce156
# Parent  336512ee2f947f07149e399a84927f9d820d2b62
# Available At https://bitbucket.org/quark-zju/hg-draft
#  hg pull https://bitbucket.org/quark-zju/hg-draft -r b6766d75404f
histedit: revive commits on demand

This is to address the "histedit --abort" issue mentioned in [1].


There are still issues with that approach. For example in this 
distributed case:


1) Alice send changesets to Bob,
2) Bob try to histedit something in the stack, this fails
3) Bob --abort the histedit
4) Bob tell Alice to perform the operation herself since she know the 
code better and can resolve the conflict

5) Alice histedit with success

If step (3) created markers for the histedit partial result, these 
markers will apply to the result of (5) and create confusion/change 
"loss" when the two states are gathered.


This is a non-trivial question and there is good reason why rebase and 
histedit do not use obsolescence marker to restore the repository to its 
former state.


note: this is a case were a local-light-weight "archiving" mechanism 
could be handy.



This solution is safer than what [1] proposed because it does not use unsafe
history writing ("strip"). The "strip" has caused several repo corruptions
in production.


I'm sure upstream would be happy to have some bug report about this and 
maybe some investigation about this corruption. Strip corrupting 
repository would happily rank a "critical".



It's similar to what "hg touch" in mutable-history does [3], without using
random number. So it could be more reliably tested.


The reason why 'touch' use random number is to ensure operation in one 
repository will not -unexpectedly- affect the one in another repo (kind 
of like here). We could use sometimes else than random but it cannot be 
a simple sequential number.


Note that using random will not solve the issue here since (5) does not 
even know yet that the markers in (3) exists.



Note that if we have obsolete marker versions, and obsoleted revisions are
revived automatically, as I proposed in [2], the "revive" hack will be
longer be necessary.


Please see my (fresh) reply to that other series.

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 1 of 4 V2] obsolete: track node versions

2017-03-27 Thread Pierre-Yves David
This is a group reply because there is interesting bit in many different 
emails, see the final link section below for details about the email I 
refer to using [#].


Two different issues
=

Just to clarify: there is two different issues at play here:

*unintentional-cycle*: cycle of obsolescence markers is an unavoidable 
possibility inherent to the fact we deal with a distributed system.


*intentional-cycle*: an idea from Jun to use "node-version" and 
obsolescence cycle to revive obsolete changesets while keeping the same 
hash. So far, locally creating cycle have been disallowed since dealing 
with cycle is advanced and complex. Jun proposed to lift this in this 
series [1].



Some background
===

Jun and I had a short discussion Sunday late afternoon at the sprint 
(referred in [2]). In that discussion, jun mentioned the idea of using 
obsstore index to arbitrate cycle. At that point, I was thinking about 
unintentional-cycle and I suggested digging in the direction of using 
the "date" because it is a global property.


"unintentional-cycles" are expected to be "rare", dealing with them as 
an error case that requires user intervention seems fine (as the rest of 
instability handling). I dunno if using date will help presenting the 
issue well to the end user, but that was at least a global properly so 
any repo would see the same thing. Simplest handling I can think for 
unintended cycle is (1) detect them (2) ask the user (3) create a 
newnode (or maker but probably node) to record the solution (4) be done.



Main issue with current proposal: distribution
==

There is multiple issues with the current proposal, but here the main 
one that looks like a definitive blocker.


The node version proposal introduces a new concept of version for "node" 
and "markers" and some logic to disables all markers with a version 
lower than the node it affect. ("dates" of marker are used as node 
version, but the same issue stand with generation maker).


The core issue here have been pointed by Gregory Szorc in [4]. Since our 
system is distributed, it is impossible to make sure the date/generation 
have the meaning they attempt to have.


Simple practical example with date:
---

[time-1] repo-B: changeset C-A is pulled from repo-A
[time-2] repo-A: changeset C-A is rewritten as C-B (time-2)
[time-3] repo-B: changeset C-C is marked as superceeded by C-A (time-3)
[time-4] repo-B: Changeset C-B (with marker) is pulled from repo-A

The markers pointing from C-B to C-A have a version "time-2" but "C-A" 
now have a version of "time-3". "C-A" become wrongly un-obsoleted (and 
C-B is in strange state). The fact C-A is un-obsoleted here is a bug and 
do not match the meaning of the user action. This is a regression from 
what evolution is currently able to achieve.


The fact patch 3 of this series had to update the evolve tests is a good 
example of how the current approaches break some simple assumption in 
current evolution behavior.


Simple practical example with generation number:


Using generation number would make the simple example above works. 
However they have the same class of issues in a sightly different way. 
It is impossible to make sure the local generation number is in phase 
with the other repositories. Here is another simple example using Jun 
proposal to "revive" changesets:


repo-B: pull C-A from repo-A
repo-B: rewrite C-A as C-C (marker: A → B generation 0)
repo-B: undo the rewrite   (marker: B → A generation 1)
repo-A: rewrite C-A as C-B (marker A → C generation 0)
repo-B: push C-A to repo-A (+ markers)

The markers pointing from C-B to C-A have a generation "0" but "C-A" now 
have a version a generation of "1". "C-A" become wrongly un-obsoleted 
(and C-B is in strange state). The fact C-A become un-obsoleted here is 
a bug and do not match the meaning of the user action. This also a 
regression from what evolution is currently able to achieves too.


Conclusion
--

Approaches based on node and marker versions are unable to cope with the 
distributed nature of Mercurial and evolution. One looking for the 
hash-preservation-stone should go down more viable alley


Further Though
--

Something based on ordering markers themselves -might- still be 
something leading to a solution (as still pointed by Greg in [4]).


However, by definition, markers creations might happens in parallel at 
the same time in multiple repositories, and their is no defined order 
between the two "branches". The complexity of tracking -and- using this 
information seems high at first glances. Jun is talking about possible 
dag building in [5]. The various property of obsmarkers and their 
combinations with the changesets dag makes me doubt of that having a 
viable complexity for such DAG, but I'm not seen anything concrete about 
Jun idea yet so he