[PATCH 4 of 7] tags: make argument 'tagtype' optional in '_updatetags'
# 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
# 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'
# 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
# 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'
# 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'
# 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
# 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)
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
# 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
# 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'
# 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
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"
> 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
# 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
# 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
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
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
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'
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)
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
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
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'
# 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'
# 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
# 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
# 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
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
# 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)
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
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
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
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
> 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
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
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
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
(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
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
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
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()
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
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
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
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
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
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
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
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__()
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
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`
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
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
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'
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)
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)
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
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)
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
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
# 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
# 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
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
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
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
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
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.
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)
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
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
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
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