[Bug 5530] New: Allow bisect to accept a filtering revset
https://bz.mercurial-scm.org/show_bug.cgi?id=5530 Bug ID: 5530 Summary: Allow bisect to accept a filtering revset Product: Mercurial Version: default branch Hardware: PC OS: Windows Status: UNCONFIRMED Severity: feature Priority: wish Component: Mercurial Assignee: bugzi...@mercurial-scm.org Reporter: matt_harbi...@yahoo.com CC: mercurial-devel@mercurial-scm.org In a large monolithic repository of many components, not every changeset applies to the component being bisected. It would be nice to be able to specify a subset of the full range, for example with 'file(..)'. Consider an exaggerated case where the range is a linear 100..200, but only {100, 200} are relevant. That's ~6 uselessly duplicated tests. Even if you recognize that the test for a given changeset is redundant, skipping only moves to the next commit instead of cutting the range in half. A hack is to use a shell script as the command that automates the skips, and prompts for good/bad: filter=... # revset match=$(hg log -r $HG_NODE -T "{ifcontains(rev, revset(\"${filter}\"), 'y')}") if [[ $match != 'y' ]]; then # Filtered out: skip exit 125 fi # prompt for good/bad/skip The problems are the small movement for each skip, it won't work on Windows without MSYS, and also a full checkout is done for each irrelevant version. I tried using --noupdate, and then updating in the script once past the skip check. But the script must be getting called while a lock is held, as it deadlocks. Is this something that narrow clone also needs, or is this a case of just saving the revset in the bisect state and applying it? -- 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 07 of 10 V5] rcutil: let environ override system configs (BC)
Excerpts from Matt Harbison's message of 2017-04-07 23:14:20 -0400: > That did the trick, thanks. Out of curiosity, how did you figure you the > problem from that error message? xargs would have been the last thing I > would have suspected. I didn't figure it out from the error message - that's why I asked for your environment to be able to reproduce. The test is one big command piped together. I rewrote the pipe to use temporary files. "hg locate" output differs - a couple of files were added. Running check-code.py on added files showed no problem. Then I started to suspect argv length limit. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 07 of 10 V5] rcutil: let environ override system configs (BC)
On Fri, 07 Apr 2017 01:13:20 -0400, Jun Wu wrote: Excerpts from Matt Harbison's message of 2017-04-07 00:35:42 -0400: > Just to confirm: if you run "make local", it will say "cl.exe ..." > instead > of "gcc ..." (where gcc could come from MinGW). Yep. $ make local | grep cl.exe c:\Program Files (x86)\Microsoft Visual Studio 9.0\VC\BIN\amd64\cl.exe /c /nologo /Ox /MD /W3 /GS- /DNDEBUG -Ic:\Python27\include -Ic:\Python27\PC /Tcmercurial/bdiff.c /Fobuild\temp.win-amd64-2.7\Release\mercurial/bdiff.obj ... > And you are using Python for > Windows, not some cygwin or mingw Python. Correct. 2.7.13, from the python.org Windows installer. > And the shell is provided by MinGW. I've never had cygwin or anything else installed. The titlebar says "MINGW32", and $SHELL is /bin/sh. Not sure if there's something specific I should check here. Thanks. I have sent a fix. That did the trick, thanks. Out of curiosity, how did you figure you the problem from that error message? xargs would have been the last thing I would have suspected. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: A possible explanation for random "stream ended unexpectedly (got m bytes, expected n)"
On Sat, Mar 25, 2017 at 10:34 AM, Gregory Szorc wrote: > On Sat, Mar 25, 2017 at 4:19 AM, Mike Hommey wrote: > >> Hi, >> >> I don't know about you, but occasionally, I've hit "stream ended >> unexpectedly (got m bytes, expected n)" errors that didn't make sense. >> Retrying would always work. >> >> Recently, I was trying to use signal.setitimer and a signal handler for >> some memory profiling on git-cinnabar, which uses mercurial as a >> library, and got "stream ended 4 unexpectedly (got m bytes, expected n)" >> *very* reproducibly. Like, with an interval timer firing every second, >> it took only a few seconds to hit the error during a clone. >> >> I'm pretty sure this can be reproduced with a similar setup in mercurial >> itself. >> >> Now, the reason this happens in this case is that, the code that fails >> does: >> >> def readexactly(stream, n): >> '''read n bytes from stream.read and abort if less was available''' >> s = stream.read(n) >> if len(s) < n: >> raise error.Abort(_("stream ended unexpectedly" >>" (got %d bytes, expected %d)") >> % (len(s), n)) >> return s >> >> ... and thanks to POSIX, interrupted reads can lead to short reads. So, >> you request n bytes, and get less, just because something interrupted >> the process. The problem then is that python doesn't let you know why >> you just got a short read, and you have to figure that out on your own. >> >> The same kind of problem is also true to some extent on writes. >> >> Now, the problem is that this piece of code is likely the most visible >> place where the issue exists, but there are many other places in the >> mercurial code base that are likely affected. >> >> And while the signal.setitimer case is a corner case (and I ended up >> using a separate thread to work around the problem ; my code wasn't >> interruption safe either anyways), I wonder if those random "stream >> ended unexpectedly (got m bytes, expected n)" errors I was getting under >> normal circumstances are not just a manifestation of the same underlying >> issue, which is that the code doesn't like interrupted reads. >> >> Disclaimer: I'm not going to work on fixing this issue, but I figured >> I'd let you know, in case someone wants to look into it more deeply. >> > > Thank you for writing this up. This "stream ended unexpectedly" has been a > thorn in my side for a while, as it comes up frequently in Mozilla's CI > with a frequency somewhere between 1 in 100-1000. Even retrying failed > operations multiple times isn't enough to overcome it > > I have long suspected interrupted system calls as a likely culprit. > However, when I initially investigated this a few months ago, I found that > Python's I/O APIs retry automatically for EINTR. See > https://hg.python.org/cpython/file/54c93e0fe79b/Lib/socket.py#l365 for > example. This /should/ make e.g. socket._fileobject.read() resilient > against signal interruption. (If Python's I/O APIs didn't do this, tons of > programs would break. Also, the semantics of .read() are such that it is > always supposed to retrieve all available bytes until EOF - at least for > some io ABCs. read1() exists to perform at most 1 system call.) > > But you provide compelling evidence that signal interruptions do in fact > result in truncated reads. So something is obviously going wrong. > > The stream.read() in changegroup.readexactly() actually follows a pretty > complicated path. Here is what happens when reading from an HTTP response: > > 1. stream is a mercurial.httppeer.readerproxy > 2. The readerproxy is constructed from util._zlibengine.decompressorreader > and a mercurial.keepalive.HTTPResponse instance > 3. decompressorreader takes the file object interface from the > HTTPResponse, constructs an util.filechunkiter, feeds that into a > zlib.decompressobj and turns that into a util.chunkbuffer > 4. When you stream.read(), it has to traverse through a util.chunkbuffer, > a generator of zlib decompressed chunks, a util.filechunkiter, and HTTP > chunked transfer decoding before it finally gets to a recv() in > socket._fileobject.read(). > > Any of those things could have a bug where a signal interrupts things or a > truncated read isn't retried. I suspect our bug is somewhere in this chain. > > I have some other wrinkles to add. > > Mozilla sees these stream failures much more frequently on Windows. They > do happen on Linux and OS X in the wild. But they are >10x more frequent on > Windows. I'm pretty sure we're running 2.7.12 on Windows (on my insistence > to help isolate an old Python as the source of the bug). > > Mozilla also sees "unexpected negative part header" errors. e.g. > https://bugzilla.mozilla.org/show_bug.cgi?id=148. The code in bundle2 > that is emitting this error also uses changegroup.readexactly. I've long > suspected the cause of this is a bit flip or reading a truncated or > uninitialized 4 byte field. I would not at all be surprised
[Bug 5529] New: Tests crash python on some versions of OS X
https://bz.mercurial-scm.org/show_bug.cgi?id=5529 Bug ID: 5529 Summary: Tests crash python on some versions of OS X Product: Mercurial Version: default branch Hardware: Macintosh OS: Mac OS Status: UNCONFIRMED Severity: bug Priority: normal Component: Mercurial Assignee: bugzi...@mercurial-scm.org Reporter: matt_harbi...@yahoo.com CC: duri...@gmail.com, mercurial-devel@mercurial-scm.org Created attachment 1956 --> https://bz.mercurial-scm.org/attachment.cgi?id=1956&action=edit crashreporter output I'm on 10.10, with python 2.7.13 (I don't recall if it was from python.org or via brew). Many tests are triggering CrashReporter on this system, which I bisected back to this: changeset: 30576:270b077d434b parent: 30574:1156ec81f709 user:Augie Fackler date:Thu Nov 10 16:07:24 2016 -0500 summary: run-tests: forward Python USER_BASE from site (issue5425) On a clean install of 10.9.5 on another machine, there were no problems. I didn't get a chance to try on 10.11+. Sample output when things went south below (note the PyThreadState_Get() reference). Also attached the CrashReporter output, in case that's helpful. MacPro64:tests atto$ ./run-tests.py --local -j16 .ss..s.s.s --- /usr/local/mercurial/tests/test-convert-cvs.t +++ /usr/local/mercurial/tests/test-convert-cvs.t.err @@ -81,55 +81,29 @@ a fixed difference from UTC. $ TZ=US/Hawaii hg convert --config convert.localtimezone=True src src-hg - initializing destination src-hg repository - connecting to $TESTTMP/cvsrepo - scanning source... - collecting CVS rlog - 5 log entries - cvslog hook: 5 entries - creating changesets - 3 changeset entries - cvschangesets hook: 3 changesets - sorting... - converting... - 2 Initial revision - 1 ci0 - 0 import - updating tags + Fatal Python error: PyThreadState_Get: no current thread + $TESTTMP.sh: line 80: 42151 Abort trap: 6 TZ=US/Hawaii hg convert --config convert.localtimezone=True src src-hg + [134] $ hgcat a - a - $ hgcat b/c - c - c + abort: No such file or directory: 'src-hg' + [255] + $ hgcat b/c + abort: No such file or directory: 'src-hg' + [255] convert fresh repo with --filemap -- 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 V2] bundle2: handle long error params on the unbundling side
On 04/07/2017 09:55 PM, Siddharth Agarwal wrote: On 4/6/17 05:50, Pierre-Yves David wrote: On 04/05/2017 05:49 AM, Siddharth Agarwal wrote: # HG changeset patch # User Siddharth Agarwal # Date 1491349317 25200 # Tue Apr 04 16:41:57 2017 -0700 # Node ID 52572916e2ae57e92d22d718e469a7c0928e8f5e # Parent 21c811d141254489398a83affa46e066bf2f6b94 bundle2: handle long error params on the unbundling side As the tests establish, the unbundling side can now present full parameters. We retain tests to simulate an old client, and add tests to simulate an old server talking to a new client. diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py --- a/mercurial/bundle2.py +++ b/mercurial/bundle2.py @@ -1556,11 +1556,37 @@ def handlereplycaps(op, inpart): class AbortFromPart(error.Abort): """Sub-class of Abort that denotes an error from a bundle2 part.""" +def _errorparams(inpart): +"""Read error parameters from the payload if so specified. + +This should only be used for errors generated via bundleerrorpart.""" +params = inpart.params +if params.get('longparams') != 'payload': +return params +# avoid modifying the params dict in inpart +params = params.copy() +# don't use inpart._unpack/_readexact because they read the underlying +# stream (with payload chunk sizes etc) -- instead read from the +# higher-level stream I double back this comment. They are internal method for the bundle2 machinery. Maybe we should update their docstring to make this clearer? maybe even doublescore (__ prefix) them. Could you clarify what you mean by "double back"? I just checked the definition and I've way off target here :-) What I meant was: "I agree with this comment" I would be in favor of __ prefixing them. Only consideration would be that they're on unpackermixin so everything that uses it would want to switch to the __ methods. Ha, I missed the fact it is a mixin. This mean we cannot use the '__' mangling[1], since it would makes it "inaccessible" to the class that needs it. I see three option left: 1) clarify their docstring 2) move to the verbose __xx__ (eg: __unpack__) 3) change nothing (meh). I think I would prefer to go for the doc update (1) first and see if this is enough or if the confusion arise again in the future. [1]https://docs.python.org/2/tutorial/classes.html#private-variables-and-class-local-references +lensize = struct.calcsize(_ferrorlongparamsize) +def readsize(): +return struct.unpack(_ferrorlongparamsize, + changegroup.readexactly(inpart, lensize))[0] + +numparams = readsize() +for _param in xrange(numparams): +namesize = readsize() +name = changegroup.readexactly(inpart, namesize) +valuesize = readsize() +value = changegroup.readexactly(inpart, valuesize) +params[name] = value +return params + @parthandler('error:abort', ('message', 'hint')) def handleerrorabort(op, inpart): """Used to transmit abort error over the wire""" -raise AbortFromPart(inpart.params['message'], -hint=inpart.params.get('hint')) +params = _errorparams(inpart) +raise AbortFromPart(params['message'], hint=params.get('hint')) @parthandler('error:pushkey', ('namespace', 'key', 'new', 'old', 'ret', 'in-reply-to')) @@ -1576,11 +1602,12 @@ def handleerrorpushkey(op, inpart): @parthandler('error:unsupportedcontent', ('parttype', 'params')) def handleerrorunsupportedcontent(op, inpart): """Used to transmit unknown content error over the wire""" +params = _errorparams(inpart) kwargs = {} -parttype = inpart.params.get('parttype') +parttype = params.get('parttype') if parttype is not None: kwargs['parttype'] = parttype -params = inpart.params.get('params') +params = params.get('params') if params is not None: kwargs['params'] = params.split('\0') @@ -1589,7 +1616,8 @@ def handleerrorunsupportedcontent(op, in @parthandler('error:pushraced', ('message',)) def handleerrorpushraced(op, inpart): """Used to transmit push race error over the wire""" -raise error.ResponseError(_('push failed:'), inpart.params['message']) +params = _errorparams(inpart) +raise error.ResponseError(_('push failed:'), params['message']) @parthandler('listkeys', ('namespace',)) def handlelistkeys(op, inpart): diff --git a/tests/test-bundle2-exchange.t b/tests/test-bundle2-exchange.t --- a/tests/test-bundle2-exchange.t +++ b/tests/test-bundle2-exchange.t @@ -464,6 +464,10 @@ Setting up > bundler.newpart('test:abort') > if reason == 'abort-long': > bundler.newpart('test:abort-long') + > if reason == 'abort-legacy': + > # Use bundler.newpart rather than bundler.newerrorpart to create this part. + > # This simulates a Mercurial server < 4.2. + > bundler.newpart('error:abort
Re: [PATCH 2 of 3 V2] bundle2: use bundleerrorparts for error parts with unbounded parameters
On 04/07/2017 09:28 PM, Siddharth Agarwal wrote: On 4/6/17 05:50, Pierre-Yves David wrote: small note: This test seems to be checking the bundle2 format and API more than the target feature. This seems superfluous or at least wrongly located. Could you elaborate about what you mean? There's a wire transmission going on. This is an example of an abort that would have crashed the server earlier. Now it no longer does. How is it superfluous? I'm referring to this comment: + > # Make sure error messages are more than 4k long to ensure they work + > # across payload chunks The payload chunks is an internal details of the bundle2 protocol itself, it is already abstracted to the code handling a part. So you should not have to test it when testing your part handler. If you need/want to add tests for payload above 4K, that would be a lower level tests that belong to `test-bundle2-format.t` I hope this clarifies my previous comment. The rest of this patch (testing cases that previous aborted) is fine and I agree we want that tested. Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2] test-flagprocessor: add a case about hg status
# HG changeset patch # User Jun Wu # Date 1491587813 25200 # Fri Apr 07 10:56:53 2017 -0700 # Node ID 0613dc70b604ad9a2abb548e223981defe753c7f # Parent c39e7c4b535c654d5b2f7790efebff2909986a04 # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 0613dc70b604 test-flagprocessor: add a case about hg status This shows how "hg status" is wrong - nothing changed but the file is labeled as "M". diff --git a/tests/test-flagprocessor.t b/tests/test-flagprocessor.t --- a/tests/test-flagprocessor.t +++ b/tests/test-flagprocessor.t @@ -240,2 +240,9 @@ 1 files changed, 1 insertions(+), 0 deletions(-) + $ rm bundle.hg bundle-again.hg + +# TEST: hg status + + $ hg status + M base64 + $ hg diff ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2] filelog: use slow path to calculate size
# HG changeset patch # User Jun Wu # Date 1491597357 25200 # Fri Apr 07 13:35:57 2017 -0700 # Node ID 48542079e2580e3889eb396c60acee9e1ce36d59 # Parent 0613dc70b604ad9a2abb548e223981defe753c7f # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 48542079e258 filelog: use slow path to calculate size Previously the code tries to be smart, and use "rawsize" (= "size" in filelog's parent class revlog) as a fast path, if there is no rename. But that does not actually work - testing if it's a rename calls the slow path already. Besides, testing rename is also insufficient - it's really the metadata header that should be tested, not rename. This patch uses slow path in all cases. Since the actual slow read is not avoidable, it should not hurt performance. This corrects "hg status" output when flag processor is involved. Related methods are: basectx.status -> workingctx._buildstatus -> workingctx._dirstatestatus -> workingctx._checklookup -> filectx.cmp -> filelog.cmp As a side effect, the "XXX" comment, "-4" hack are removed, the table in "verify.py" is simplified, and test-filelog.py is fixed. diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -821,8 +821,5 @@ class basefilectx(object): if (fctx._filenode is None -and (self._repo._encodefilterpats - # if file data starts with '\1\n', empty metadata block is - # prepended, which adds 4 bytes to filelog.size(). - or self.size() - 4 == fctx.size()) +and self._repo._encodefilterpats or self.size() == fctx.size()): return self._filelog.cmp(self._filenode, fctx.data()) diff --git a/mercurial/filelog.py b/mercurial/filelog.py --- a/mercurial/filelog.py +++ b/mercurial/filelog.py @@ -69,13 +69,15 @@ class filelog(revlog.revlog): """return the size of a given revision""" -# for revisions with renames, we have to go the slow way -node = self.node(rev) -if self.renamed(node): -return len(self.read(node)) if self.iscensored(rev): return 0 -# XXX if self.read(node).startswith("\1\n"), this returns (size+4) -return super(filelog, self).size(rev) +# "rawsize" is faster. but rawsize could be wrong when: +# 1. content starts with "\1\n" (metadata header). usually it's a +#rename. rare cases it's a binary file with that header +# 2. flag processor is involved, so the content can be changed +# checking the metadata header requires reading the content already. +# so we cannot avoid reading the content anyway, "rawsize" fastpath +# is not really useful. +return len(self.read(self.node(rev))) def cmp(self, node, text): diff --git a/mercurial/verify.py b/mercurial/verify.py --- a/mercurial/verify.py +++ b/mercurial/verify.py @@ -413,13 +413,10 @@ class verifier(object): # - #rawsize() | L1 | L1 | L1| L1 -# size() | L1 | L2-LM | L1(*) | L1 (?) +# size() | L1 | L2-LM | L2-LM | L3-LM # len(rawtext) | L2 | L2 | L2| L2 #len(text) | L2 | L2 | L2| L3 -# len(read()) | L2 | L2-LM | L2-LM | L3 (?) +# len(read()) | L2 | L2-LM | L2-LM | L3-LM # # LM: length of metadata, depending on rawtext -# (*): not ideal, see comment in filelog.size -# (?): could be "- len(meta)" if the resolved content has -# rename metadata # # Checks needed to be done: diff --git a/tests/test-filelog.py.out b/tests/test-filelog.py.out --- a/tests/test-filelog.py.out +++ b/tests/test-filelog.py.out @@ -1,2 +1,1 @@ -ERROR: FIXME: This is a known failure of filelog.size for data starting with \1\n OK. diff --git a/tests/test-flagprocessor.t b/tests/test-flagprocessor.t --- a/tests/test-flagprocessor.t +++ b/tests/test-flagprocessor.t @@ -245,4 +245,3 @@ $ hg status - M base64 $ hg diff ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 3 V2] bundle2: handle long error params on the unbundling side
On 4/6/17 05:50, Pierre-Yves David wrote: On 04/05/2017 05:49 AM, Siddharth Agarwal wrote: # HG changeset patch # User Siddharth Agarwal # Date 1491349317 25200 # Tue Apr 04 16:41:57 2017 -0700 # Node ID 52572916e2ae57e92d22d718e469a7c0928e8f5e # Parent 21c811d141254489398a83affa46e066bf2f6b94 bundle2: handle long error params on the unbundling side As the tests establish, the unbundling side can now present full parameters. We retain tests to simulate an old client, and add tests to simulate an old server talking to a new client. diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py --- a/mercurial/bundle2.py +++ b/mercurial/bundle2.py @@ -1556,11 +1556,37 @@ def handlereplycaps(op, inpart): class AbortFromPart(error.Abort): """Sub-class of Abort that denotes an error from a bundle2 part.""" +def _errorparams(inpart): +"""Read error parameters from the payload if so specified. + +This should only be used for errors generated via bundleerrorpart.""" +params = inpart.params +if params.get('longparams') != 'payload': +return params +# avoid modifying the params dict in inpart +params = params.copy() +# don't use inpart._unpack/_readexact because they read the underlying +# stream (with payload chunk sizes etc) -- instead read from the +# higher-level stream I double back this comment. They are internal method for the bundle2 machinery. Maybe we should update their docstring to make this clearer? maybe even doublescore (__ prefix) them. Could you clarify what you mean by "double back"? I would be in favor of __ prefixing them. Only consideration would be that they're on unpackermixin so everything that uses it would want to switch to the __ methods. +lensize = struct.calcsize(_ferrorlongparamsize) +def readsize(): +return struct.unpack(_ferrorlongparamsize, + changegroup.readexactly(inpart, lensize))[0] + +numparams = readsize() +for _param in xrange(numparams): +namesize = readsize() +name = changegroup.readexactly(inpart, namesize) +valuesize = readsize() +value = changegroup.readexactly(inpart, valuesize) +params[name] = value +return params + @parthandler('error:abort', ('message', 'hint')) def handleerrorabort(op, inpart): """Used to transmit abort error over the wire""" -raise AbortFromPart(inpart.params['message'], -hint=inpart.params.get('hint')) +params = _errorparams(inpart) +raise AbortFromPart(params['message'], hint=params.get('hint')) @parthandler('error:pushkey', ('namespace', 'key', 'new', 'old', 'ret', 'in-reply-to')) @@ -1576,11 +1602,12 @@ def handleerrorpushkey(op, inpart): @parthandler('error:unsupportedcontent', ('parttype', 'params')) def handleerrorunsupportedcontent(op, inpart): """Used to transmit unknown content error over the wire""" +params = _errorparams(inpart) kwargs = {} -parttype = inpart.params.get('parttype') +parttype = params.get('parttype') if parttype is not None: kwargs['parttype'] = parttype -params = inpart.params.get('params') +params = params.get('params') if params is not None: kwargs['params'] = params.split('\0') @@ -1589,7 +1616,8 @@ def handleerrorunsupportedcontent(op, in @parthandler('error:pushraced', ('message',)) def handleerrorpushraced(op, inpart): """Used to transmit push race error over the wire""" -raise error.ResponseError(_('push failed:'), inpart.params['message']) +params = _errorparams(inpart) +raise error.ResponseError(_('push failed:'), params['message']) @parthandler('listkeys', ('namespace',)) def handlelistkeys(op, inpart): diff --git a/tests/test-bundle2-exchange.t b/tests/test-bundle2-exchange.t --- a/tests/test-bundle2-exchange.t +++ b/tests/test-bundle2-exchange.t @@ -464,6 +464,10 @@ Setting up > bundler.newpart('test:abort') > if reason == 'abort-long': > bundler.newpart('test:abort-long') + > if reason == 'abort-legacy': + > # Use bundler.newpart rather than bundler.newerrorpart to create this part. + > # This simulates a Mercurial server < 4.2. + > bundler.newpart('error:abort', [('message', 'Abandon ship too!')]) > if reason == 'unknown': > bundler.newpart('test:unknown') > if reason == 'race': @@ -480,9 +484,16 @@ Setting up > # across payload chunks > raise error.Abort('a' * 8192, hint="don't panic") > + > def overrideerrorparams(orig, inpart): + > # simulate Mercurial < 4.2 if this config is set + > if inpart.ui.configbool('failpush', 'legacyerrorparams'): + > return inpart.params + > return orig(inpart) note: they are a devel.legacy group of configuration intended at providing "downgrade" swith for testing. Maybe you should add something there for
Re: [PATCH 1 of 3 V2] bundle2: add separate handling for error part creation
On 4/6/17 05:50, Pierre-Yves David wrote: While I understand the need for unbounded generic error:Abort, I not sure we should extend this to pushraced and unsupported content one. They have defined enough use case where limiting the message and hint to 255 char seems reasonable (in the exception themselves). Limiting the scope of this more complex creation and handling to error:Abort. What do you think ? My view is that if we're doing this for abort anyway, we might as well do this for the other part types. * unsupportedcontent is arbitrary so it can easily go beyond 255 when you concatenate the params together. A misbehaving client can easily crash the server that way. * pushraced is less likely but there's still a non-zero chance that a confluence of events can cause this. I think the incremental amount of work required to support it is minimal, so we might as well do it in my opinion. I think we could get away with a simpler way to add the long message to the payload. You current approach implies packing the parameters name and length using our own binary encoding into the payload. This imply tedious binary manipulation on both end. Instead we could leverage more of the bundle2 format. In case of too long message or hint, we add a corresponding 'message_long' or 'hint_long' advisory parameter to the part. This parameter contains a 'start:end' range of bytes in the paylog containing the message. That way most of the binary encoding is handled by the native bundle2 machinery and less work is needed on the receiving side. What do you think ? It'll probably lead to simpler code overall. I like it. I would want to store the full message in the payload though, as we're truncating the message in params for older clients in a friendly-ish way. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 3 V2] bundle2: use bundleerrorparts for error parts with unbounded parameters
On 4/6/17 05:50, Pierre-Yves David wrote: small note: This test seems to be checking the bundle2 format and API more than the target feature. This seems superfluous or at least wrongly located. Could you elaborate about what you mean? There's a wire transmission going on. This is an example of an abort that would have crashed the server earlier. Now it no longer does. How is it superfluous? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2] tests: add test demonstrating buggy path handling
On Fri, Apr 7, 2017 at 11:52 AM, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc > # Date 1491590783 25200 > # Fri Apr 07 11:46:23 2017 -0700 > # Branch stable > # Node ID f8ba1fb4458b60b1d129f97d1f47a542aa0daf98 > # Parent 68f263f52d2e3e2798b4f1e55cb665c6b043f93b > tests: add test demonstrating buggy path handling > Forgot STABLE in the email flag. Sorry for the spam. > > `hg debugupgraderepo` is currently buggy with regards to path > handling when copying files in .hg/store/. Specifically, it applies > the store filename encoding to paths instead of operating on raw > files. > > This commit adds a test demonstrating the buggy behavior. > > diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t > --- a/tests/test-upgrade-repo.t > +++ b/tests/test-upgrade-repo.t > @@ -310,3 +310,38 @@ old store should be backed up >undo.phaseroots > >$ cd .. > + > +store files with special filenames aren't encoded during copy > + > + $ hg init store-filenames > + $ cd store-filenames > + $ touch foo > + $ hg -q commit -A -m initial > + $ touch .hg/store/.XX_special_filename > + > + $ hg debugupgraderepo --run > + upgrade will perform the following actions: > + > + requirements > + preserved: dotencode, fncache, generaldelta, revlogv1, store > + > + beginning upgrade... > + repository locked and read-only > + creating temporary repository to stage migrated data: > $TESTTMP/store-filenames/.hg/upgrade.* (glob) > + (it is safe to interrupt this process any time before data migration > completes) > + migrating 3 total revisions (1 in filelogs, 1 in manifests, 1 in > changelog) > + migrating 109 bytes in store; 107 bytes tracked data > + migrating 1 filelogs containing 1 revisions (0 bytes in store; 0 bytes > tracked data) > + finished migrating 1 filelog revisions across 1 filelogs; change in > size: 0 bytes > + migrating 1 manifests containing 1 revisions (46 bytes in store; 45 > bytes tracked data) > + finished migrating 1 manifest revisions across 1 manifests; change in > size: 0 bytes > + migrating changelog containing 1 revisions (63 bytes in store; 62 bytes > tracked data) > + finished migrating 1 changelog revisions; change in size: 0 bytes > + finished migrating 3 total revisions; total change in store size: 0 > bytes > + copying phaseroots > + copying .XX_special_filename > + removing temporary repository $TESTTMP/store-filenames/.hg/upgrade.* > (glob) > + abort: No such file or directory: $TESTTMP/store-filenames/.hg/ > store/~2e_x_x__special__filename > + [255] > + > + $ cd .. > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2 STABLE V2] tests: add test demonstrating buggy path handling
# HG changeset patch # User Gregory Szorc # Date 1491590783 25200 # Fri Apr 07 11:46:23 2017 -0700 # Branch stable # Node ID f8ba1fb4458b60b1d129f97d1f47a542aa0daf98 # Parent 68f263f52d2e3e2798b4f1e55cb665c6b043f93b tests: add test demonstrating buggy path handling `hg debugupgraderepo` is currently buggy with regards to path handling when copying files in .hg/store/. Specifically, it applies the store filename encoding to paths instead of operating on raw files. This commit adds a test demonstrating the buggy behavior. diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t --- a/tests/test-upgrade-repo.t +++ b/tests/test-upgrade-repo.t @@ -310,3 +310,38 @@ old store should be backed up undo.phaseroots $ cd .. + +store files with special filenames aren't encoded during copy + + $ hg init store-filenames + $ cd store-filenames + $ touch foo + $ hg -q commit -A -m initial + $ touch .hg/store/.XX_special_filename + + $ hg debugupgraderepo --run + upgrade will perform the following actions: + + requirements + preserved: dotencode, fncache, generaldelta, revlogv1, store + + beginning upgrade... + repository locked and read-only + creating temporary repository to stage migrated data: $TESTTMP/store-filenames/.hg/upgrade.* (glob) + (it is safe to interrupt this process any time before data migration completes) + migrating 3 total revisions (1 in filelogs, 1 in manifests, 1 in changelog) + migrating 109 bytes in store; 107 bytes tracked data + migrating 1 filelogs containing 1 revisions (0 bytes in store; 0 bytes tracked data) + finished migrating 1 filelog revisions across 1 filelogs; change in size: 0 bytes + migrating 1 manifests containing 1 revisions (46 bytes in store; 45 bytes tracked data) + finished migrating 1 manifest revisions across 1 manifests; change in size: 0 bytes + migrating changelog containing 1 revisions (63 bytes in store; 62 bytes tracked data) + finished migrating 1 changelog revisions; change in size: 0 bytes + finished migrating 3 total revisions; total change in store size: 0 bytes + copying phaseroots + copying .XX_special_filename + removing temporary repository $TESTTMP/store-filenames/.hg/upgrade.* (glob) + abort: No such file or directory: $TESTTMP/store-filenames/.hg/store/~2e_x_x__special__filename + [255] + + $ cd .. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2 STABLE V2] repair: use rawvfs when copying extra store files
# HG changeset patch # User Gregory Szorc # Date 1491590980 25200 # Fri Apr 07 11:49:40 2017 -0700 # Branch stable # Node ID 8b073104522c69a3892288a5ed35e4e60cdc2033 # Parent f8ba1fb4458b60b1d129f97d1f47a542aa0daf98 repair: use rawvfs when copying extra store files If we use the normal vfs, store encoding will be applied when we .join() the path to be copied. This results in attempting to copy a file that (likely) doesn't exist. Using the rawvfs operates on the raw file path, which is returned by vfs.readdir(). Users at Mozilla are encountering this, as I've instructed them to run `hg debugupgraderepo` to upgrade to generaldelta. While Mercurial shouldn't deposit any files under .hg/store that require encoding, it is possible for e.g. .DS_Store files to be created by the operating system. diff --git a/mercurial/repair.py b/mercurial/repair.py --- a/mercurial/repair.py +++ b/mercurial/repair.py @@ -874,8 +874,8 @@ def _upgraderepo(ui, srcrepo, dstrepo, r continue srcrepo.ui.write(_('copying %s\n') % p) -src = srcrepo.store.vfs.join(p) -dst = dstrepo.store.vfs.join(p) +src = srcrepo.store.rawvfs.join(p) +dst = dstrepo.store.rawvfs.join(p) util.copyfile(src, dst, copystat=True) _upgradefinishdatamigration(ui, srcrepo, dstrepo, requirements) diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t --- a/tests/test-upgrade-repo.t +++ b/tests/test-upgrade-repo.t @@ -340,8 +340,15 @@ store files with special filenames aren' finished migrating 3 total revisions; total change in store size: 0 bytes copying phaseroots copying .XX_special_filename + data fully migrated to temporary repository + marking source repository as being upgraded; clients will be unable to read from repository + starting in-place swap of repository data + replaced files will be backed up at $TESTTMP/store-filenames/.hg/upgradebackup.* (glob) + replacing store... + store replacement complete; repository was inconsistent for 0.0s + finalizing requirements file and making repository readable again removing temporary repository $TESTTMP/store-filenames/.hg/upgrade.* (glob) - abort: No such file or directory: $TESTTMP/store-filenames/.hg/store/~2e_x_x__special__filename - [255] + copy of old repository backed up at $TESTTMP/store-filenames/.hg/upgradebackup.* (glob) + the old repository will not be deleted; remove it to free up disk space once the upgraded repository is verified $ cd .. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2] repair: use rawvfs when copying extra store files
# HG changeset patch # User Gregory Szorc # Date 1491590980 25200 # Fri Apr 07 11:49:40 2017 -0700 # Branch stable # Node ID 8b073104522c69a3892288a5ed35e4e60cdc2033 # Parent f8ba1fb4458b60b1d129f97d1f47a542aa0daf98 repair: use rawvfs when copying extra store files If we use the normal vfs, store encoding will be applied when we .join() the path to be copied. This results in attempting to copy a file that (likely) doesn't exist. Using the rawvfs operates on the raw file path, which is returned by vfs.readdir(). Users at Mozilla are encountering this, as I've instructed them to run `hg debugupgraderepo` to upgrade to generaldelta. While Mercurial shouldn't deposit any files under .hg/store that require encoding, it is possible for e.g. .DS_Store files to be created by the operating system. diff --git a/mercurial/repair.py b/mercurial/repair.py --- a/mercurial/repair.py +++ b/mercurial/repair.py @@ -874,8 +874,8 @@ def _upgraderepo(ui, srcrepo, dstrepo, r continue srcrepo.ui.write(_('copying %s\n') % p) -src = srcrepo.store.vfs.join(p) -dst = dstrepo.store.vfs.join(p) +src = srcrepo.store.rawvfs.join(p) +dst = dstrepo.store.rawvfs.join(p) util.copyfile(src, dst, copystat=True) _upgradefinishdatamigration(ui, srcrepo, dstrepo, requirements) diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t --- a/tests/test-upgrade-repo.t +++ b/tests/test-upgrade-repo.t @@ -340,8 +340,15 @@ store files with special filenames aren' finished migrating 3 total revisions; total change in store size: 0 bytes copying phaseroots copying .XX_special_filename + data fully migrated to temporary repository + marking source repository as being upgraded; clients will be unable to read from repository + starting in-place swap of repository data + replaced files will be backed up at $TESTTMP/store-filenames/.hg/upgradebackup.* (glob) + replacing store... + store replacement complete; repository was inconsistent for 0.0s + finalizing requirements file and making repository readable again removing temporary repository $TESTTMP/store-filenames/.hg/upgrade.* (glob) - abort: No such file or directory: $TESTTMP/store-filenames/.hg/store/~2e_x_x__special__filename - [255] + copy of old repository backed up at $TESTTMP/store-filenames/.hg/upgradebackup.* (glob) + the old repository will not be deleted; remove it to free up disk space once the upgraded repository is verified $ cd .. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2] tests: add test demonstrating buggy path handling
# HG changeset patch # User Gregory Szorc # Date 1491590783 25200 # Fri Apr 07 11:46:23 2017 -0700 # Branch stable # Node ID f8ba1fb4458b60b1d129f97d1f47a542aa0daf98 # Parent 68f263f52d2e3e2798b4f1e55cb665c6b043f93b tests: add test demonstrating buggy path handling `hg debugupgraderepo` is currently buggy with regards to path handling when copying files in .hg/store/. Specifically, it applies the store filename encoding to paths instead of operating on raw files. This commit adds a test demonstrating the buggy behavior. diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t --- a/tests/test-upgrade-repo.t +++ b/tests/test-upgrade-repo.t @@ -310,3 +310,38 @@ old store should be backed up undo.phaseroots $ cd .. + +store files with special filenames aren't encoded during copy + + $ hg init store-filenames + $ cd store-filenames + $ touch foo + $ hg -q commit -A -m initial + $ touch .hg/store/.XX_special_filename + + $ hg debugupgraderepo --run + upgrade will perform the following actions: + + requirements + preserved: dotencode, fncache, generaldelta, revlogv1, store + + beginning upgrade... + repository locked and read-only + creating temporary repository to stage migrated data: $TESTTMP/store-filenames/.hg/upgrade.* (glob) + (it is safe to interrupt this process any time before data migration completes) + migrating 3 total revisions (1 in filelogs, 1 in manifests, 1 in changelog) + migrating 109 bytes in store; 107 bytes tracked data + migrating 1 filelogs containing 1 revisions (0 bytes in store; 0 bytes tracked data) + finished migrating 1 filelog revisions across 1 filelogs; change in size: 0 bytes + migrating 1 manifests containing 1 revisions (46 bytes in store; 45 bytes tracked data) + finished migrating 1 manifest revisions across 1 manifests; change in size: 0 bytes + migrating changelog containing 1 revisions (63 bytes in store; 62 bytes tracked data) + finished migrating 1 changelog revisions; change in size: 0 bytes + finished migrating 3 total revisions; total change in store size: 0 bytes + copying phaseroots + copying .XX_special_filename + removing temporary repository $TESTTMP/store-filenames/.hg/upgrade.* (glob) + abort: No such file or directory: $TESTTMP/store-filenames/.hg/store/~2e_x_x__special__filename + [255] + + $ cd .. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 3 of 3 V2] py3: add pycompat.unicode and add it to importer
# HG changeset patch # User Pulkit Goyal <7895pul...@gmail.com> # Date 1491588351 -19800 # Fri Apr 07 23:35:51 2017 +0530 # Node ID 7ce54c695ff7a5637b6798dec0fc6cf4f7b22098 # Parent 1724a314de7c22936d33b09066e9f06197437391 py3: add pycompat.unicode and add it to importer On python 3, builtins.unicode does not exist. diff -r 1724a314de7c -r 7ce54c695ff7 mercurial/__init__.py --- a/mercurial/__init__.py Fri Apr 07 16:00:44 2017 +0530 +++ b/mercurial/__init__.py Fri Apr 07 23:35:51 2017 +0530 @@ -283,7 +283,8 @@ continue r, c = t.start l = (b'; from mercurial.pycompat import ' - b'delattr, getattr, hasattr, setattr, xrange, open\n') + b'delattr, getattr, hasattr, setattr, xrange, ' + b'open, unicode\n') for u in tokenize.tokenize(io.BytesIO(l).readline): if u.type in (tokenize.ENCODING, token.ENDMARKER): continue @@ -323,7 +324,7 @@ # ``replacetoken`` or any mechanism that changes semantics of module # loading is changed. Otherwise cached bytecode may get loaded without # the new transformation mechanisms applied. -BYTECODEHEADER = b'HG\x00\x09' +BYTECODEHEADER = b'HG\x00\x10' class hgloader(importlib.machinery.SourceFileLoader): """Custom module loader that transforms source code. diff -r 1724a314de7c -r 7ce54c695ff7 mercurial/pycompat.py --- a/mercurial/pycompat.py Fri Apr 07 16:00:44 2017 +0530 +++ b/mercurial/pycompat.py Fri Apr 07 23:35:51 2017 +0530 @@ -174,6 +174,7 @@ hasattr = _wrapattrfunc(builtins.hasattr) setattr = _wrapattrfunc(builtins.setattr) xrange = builtins.range +unicode = str def open(name, mode='r', buffering=-1): return builtins.open(name, sysstr(mode), buffering) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 3 V2] py3: add a bytes version of urllib.parse.urlencode() to pycompat.py
# HG changeset patch # User Pulkit Goyal <7895pul...@gmail.com> # Date 1491561044 -19800 # Fri Apr 07 16:00:44 2017 +0530 # Node ID 1724a314de7c22936d33b09066e9f06197437391 # Parent 50b531cb22c78a068c5effd84eb3c931187b5b71 py3: add a bytes version of urllib.parse.urlencode() to pycompat.py urllib.parse.urlencode() returns unicodes on Python 3. This commit adds a method which will take its output and encode it to bytes so that we can use bytes consistently. diff -r 50b531cb22c7 -r 1724a314de7c mercurial/pycompat.py --- a/mercurial/pycompat.py Fri Apr 07 13:46:35 2017 +0530 +++ b/mercurial/pycompat.py Fri Apr 07 16:00:44 2017 +0530 @@ -399,4 +399,11 @@ s = urllib.parse.quote_from_bytes(s, safe=safe) return s.encode('ascii', 'strict') +# urllib.parse.urlencode() returns str. We use this function to make +# sure we return bytes. +def urlencode(query, doseq=False): +s = urllib.parse.urlencode(query, doseq=doseq) +return s.encode('ascii') + urlreq.quote = quote +urlreq.urlencode = urlencode ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 3 V2] py3: replace str() with bytes()
# HG changeset patch # User Pulkit Goyal <7895pul...@gmail.com> # Date 1491552995 -19800 # Fri Apr 07 13:46:35 2017 +0530 # Node ID 50b531cb22c78a068c5effd84eb3c931187b5b71 # Parent 6b32872e4896993efe0abcc13b6d9c4c9b8687b7 py3: replace str() with bytes() diff -r 6b32872e4896 -r 50b531cb22c7 mercurial/hg.py --- a/mercurial/hg.py Fri Apr 07 13:45:33 2017 +0530 +++ b/mercurial/hg.py Fri Apr 07 13:46:35 2017 +0530 @@ -103,7 +103,7 @@ if u.fragment: branch = u.fragment u.fragment = None -return str(u), (branch, branches or []) +return bytes(u), (branch, branches or []) schemes = { 'bundle': bundlerepo, diff -r 6b32872e4896 -r 50b531cb22c7 mercurial/util.py --- a/mercurial/util.py Fri Apr 07 13:45:33 2017 +0530 +++ b/mercurial/util.py Fri Apr 07 13:46:35 2017 +0530 @@ -2799,7 +2799,7 @@ user, passwd = self.user, self.passwd try: self.user, self.passwd = None, None -s = str(self) +s = bytes(self) finally: self.user, self.passwd = user, passwd if not self.user: @@ -2854,7 +2854,7 @@ u = url(u) if u.passwd: u.passwd = '***' -return str(u) +return bytes(u) def removeauth(u): '''remove all authentication information from a url string''' ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] metadataonlyctx: replace "changeset()[0]" to "manifestnode()"
# HG changeset patch # User Jun Wu # Date 1491588163 25200 # Fri Apr 07 11:02:43 2017 -0700 # Node ID 0d57661098e3f93e7d1ab4e206d87d39c6ce4f84 # Parent c39e7c4b535c654d5b2f7790efebff2909986a04 # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 0d57661098e3 metadataonlyctx: replace "changeset()[0]" to "manifestnode()" As Yuya pointed out [1], "changeset()[0]" could be simplified to "manifestnode()". I didn't notice that method earlier. It should definitely be used - it's easier to read, and faster. [1]: https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-March/095716.html diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -2055,8 +2055,8 @@ class metadataonlyctx(committablectx): # manifests of our commit parents mp1, mp2 = self.manifestctx().parents -if p1 != nullid and p1.changeset()[0] != mp1: +if p1 != nullid and p1.manifestnode() != mp1: raise RuntimeError('can\'t reuse the manifest: ' 'its p1 doesn\'t match the new ctx p1') -if p2 != nullid and p2.changeset()[0] != mp2: +if p2 != nullid and p2.manifestnode() != 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
[PATCH] test-check-pylint: match its output
# HG changeset patch # User Jun Wu # Date 1491588594 25200 # Fri Apr 07 11:09:54 2017 -0700 # Node ID 58c9ffb81e676df4613c86c9ebd75836f1567ced # Parent 0d57661098e3f93e7d1ab4e206d87d39c6ce4f84 # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 58c9ffb81e67 test-check-pylint: match its output "pylint --version" shows: pylint 2.0.0, astroid 1.5.0 Python 2.7.13 (default, Dec 21 2016, 07:16:46) [GCC 6.2.1 20160830] I got "Your code has been rated at 10.00/10" every time and didn't know how to turn it off. Therefore the fix. diff --git a/tests/test-check-pylint.t b/tests/test-check-pylint.t --- a/tests/test-check-pylint.t +++ b/tests/test-check-pylint.t @@ -14,2 +14,6 @@ Current checks: > --enable=W0102 --reports=no \ > mercurial hgext hgext3rd + (?) + (?) + Your code has been rated at 10.00/10 (?) + (?) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] py3: add a bytes version of urllib.parse.urlencode() to pycompat.py
On Fri, Apr 7, 2017 at 5:48 PM, Yuya Nishihara wrote: > On Fri, 07 Apr 2017 16:29:40 +0530, Pulkit Goyal wrote: > > # HG changeset patch > > # User Pulkit Goyal <7895pul...@gmail.com> > > # Date 1491561044 -19800 > > # Fri Apr 07 16:00:44 2017 +0530 > > # Node ID c3e934121b61d54184e914bda587281b4184eabf > > # Parent beedb62e62a78a2f45ef8bd0b72c51f14619ee7f > > py3: add a bytes version of urllib.parse.urlencode() to pycompat.py > > > > urllib.parse.urlencode() returns unicodes on Python 3. This commit adds a > > method which will take its output and encode it to bytes so that we can > use > > bytes consistently. > > > > diff -r beedb62e62a7 -r c3e934121b61 mercurial/pycompat.py > > --- a/mercurial/pycompat.py Fri Apr 07 15:58:04 2017 +0530 > > +++ b/mercurial/pycompat.py Fri Apr 07 16:00:44 2017 +0530 > > @@ -399,4 +399,12 @@ > > s = urllib.parse.quote_from_bytes(s, safe=safe) > > return s.encode('ascii', 'strict') > > > > +# urllib.parse.urlencode() returns str. We use this function to make > > +# sure we return bytes. > > +def urlencode(query, doseq=False, safe='', encoding=None, > errors=None): > > Nit: better to not provide safe, encoding and errors arguments which don't > exist in Python 3. Other than that, the patch looks good to me. > > These arguments are present in Python 3.5 but not in Python 3.0. I will drop them though as they are not on Python 2 also. One general thing, when you are referring to Python 3 here, are you also considering the version older than Python 3.5 as my current approach is to look for things(documentation) in 3.5 only. I don't care about previous versions. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2 V2] util: add a way to issue deprecation warning without a UI object
On 04/06/2017 05:44 PM, Yuya Nishihara wrote: On Thu, 6 Apr 2017 16:09:07 +0200, Pierre-Yves David wrote: On 04/06/2017 03:58 PM, Yuya Nishihara wrote: On Tue, 04 Apr 2017 11:58:49 +0200, Pierre-Yves David wrote: # HG changeset patch # User Pierre-Yves David # Date 1491296609 -7200 # Tue Apr 04 11:03:29 2017 +0200 # Node ID 3992b843a7832bf23f2430dc2308907d1f5ba7f9 # Parent 2632df096fc0ac7582382b1f94ea4b9ad0bce8f2 # EXP-Topic vfs.cleanup # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ # hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 3992b843a783 util: add a way to issue deprecation warning without a UI object Our current deprecation warning mechanism rely on ui object. They are case where we cannot have access to the UI object. On a general basis we avoid using the python mechanism for deprecation warning because up to Python 2.6 it is exposing warning to unsuspecting user who cannot do anything to deal with them. So we build a "safe" strategy to hide this warnings behind a flag in an environment variable. The test runner set this flag so that tests show these warning. This will help us marker API as deprecated for extensions to update their code. diff --git a/mercurial/util.py b/mercurial/util.py --- a/mercurial/util.py +++ b/mercurial/util.py @@ -38,6 +38,7 @@ import tempfile import textwrap import time import traceback +import warnings import zlib from . import ( @@ -155,6 +156,31 @@ def bitsfrom(container): bits |= bit return bits +# python 2.6 still have deprecation warning enabled by default. We do not want +# to display anything to standard user so detect if we are running test and +# only use python deprecation warning in this case. +_dowarn = bool(encoding.environ.get('HGEMITWARNINGS')) +if _dowarn: +# explicitly unfilter our warning for python 2.7 +# +# The option of setting PYTHONWARNINGS in the test runner was investigated. +# However, module name set through PYTHONWARNINGS was exactly matched, so +# we cannot set 'mercurial' and have it match eg: 'mercurial.scmutil'. This +# makes the whole PYTHONWARNINGS thing useless for our usecase. +warnings.filterwarnings('default', '', DeprecationWarning, 'mercurial') +warnings.filterwarnings('default', '', DeprecationWarning, 'hgext') +warnings.filterwarnings('default', '', DeprecationWarning, 'hgext3rd') + +def nouideprecwarn(msg, version, stacklevel=1): +"""Issue an python native deprecation warning + +This is a noop outside of tests, use 'ui.deprecwarn' when possible. +""" +if _dowarn: +msg += ("\n(compatibility will be dropped after Mercurial-%s," +" update your code.)") % version +warnings.warn(msg, DeprecationWarning, stacklevel + 1) Sorry for late reply, but can this DeprecationWarning really reach extension authors who need the warning? The test runner have been updated to make sure they appears and official recommendation is to use the test runner from the core repository. So extensions author following the good practice will get the warning (I've caught a couple already in client extensions). The other one will get bitten and have one extra incentive to follow the good practice :-) You are the core developer. I believe many extensions wouldn't have tests, and if they do, the authors wouldn't run tests unless they change their code. Having tests and running them for each new Mercurial release is also something I would put into good pratice o:-) But a develwarn will hopefully be noticed while running. For example, you've fixed several opener uses in TortoiseHg probably because you'd see warnings. I've more hope of developer running tests than in developer knowing about the 'devel.all-warnings=1' option. I've have it set because I'm the one who introduced it. I wonder who else does? But I see your point, using the same function as for the rest increase chance of people seeing them. If dirty hack allowed, I would do something like the following: # util.py def _deprecwarn(msg, version): pass # somewhere ui is available, maybe in dispatch.py util._deprecwarn = ui.deprecwarn That is a diry hack. I would prefer we did not used it. Yeah, that is dirty and I don't like it. But I'm okay with it as long as it is a temporary hack. If you think the dirty hack is worth the potential extra exposure, I'm fine with it. However, I'm confused about your usage of "temporary hack" here. Why are you using temporary? Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4 NEW-CONCEPT] track-tags: write all tag changes to a file
On 04/07/2017 05:25 PM, Yuya Nishihara wrote: On Fri, 7 Apr 2017 15:11:16 +0200, Pierre-Yves David wrote: On 04/06/2017 05:27 PM, Yuya Nishihara wrote: On Thu, 6 Apr 2017 01:09:47 +0200, Pierre-Yves David wrote: On 04/05/2017 10:54 PM, Jun Wu wrote: I don't think "writing things that hook *may* need to filesystem" is a good approach. It introduces unnecessary overhead if the hook does not need that information. I do not find the overhead concerning. The thing to keep in mind is that if they are many things to write down, this also means many thing changed during the transaction. So the overhead is likely minimal. In the tags cases, just updating the various tags related cache is going to be much more expensive that writing this to disk. I agree Pierre-Yves in that I/O overhead wouldn't matter. On the other hand, I think the slowness of calling back an hg command doesn't matter, too. The cost of a round-trip to Mercurial is close to a minimum of 0.1s. And every hooks who needs fine transaction data will have to pay it. Possibly multiple time. One the other hand writing down a file is a one time operation that do not add overhead to each hook. So I'm still leaning toward the file solution. As the journal extension collects similar information for bookmarks, can't we integrate the tag tracking with the journal? Journal is currently an extension, and an experimental one. I would prefer not to be scope bloated into solving the two points aboves before getting theses features done. Journal is also tracking name movement, not transaction. We would have to teach journal about transaction first (that is probably something useful to do, but more scope bloat :-) ) Okay, so this tags.changes file describes the current transaction, which is a different concept than the journal. I agree. I prefer not introducing new file format which we'll have to document and maintain forever, but I have no better idea. Can we move forward with a file for now? The feature is experimental so we have time to think of an alternative before we enforce backward compatibility. Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH V3] fsmonitor: match watchman and filesystem encoding
# HG changeset patch # User Olivier Trempe # Date 1488981822 18000 # Wed Mar 08 09:03:42 2017 -0500 # Branch stable # Node ID 867e2a3b106825dbd637aa404115301f9df70bfd # Parent 68f263f52d2e3e2798b4f1e55cb665c6b043f93b fsmonitor: match watchman and filesystem encoding watchman's paths encoding can differ from filesystem encoding. For example, on Windows, it's always utf-8. Before this patch, on Windows, mismatch in path comparison between fsmonitor state and osutil.statfiles would yield a clean status for added/modified files. In addition to status reporting wrong results, this leads to files being discarded from changesets while doing history editing operations such as rebase. Benchmark: There is a little overhead at module import: python -m timeit "import hgext.fsmonitor" Windows before patch: 100 loops, best of 3: 0.563 usec per loop Windows after patch: 100 loops, best of 3: 0.583 usec per loop Linx before patch: 100 loops, best of 3: 0.579 usec per loop Linux after patch: 100 loops, best of 3: 0.588 usec per loop 1 calls to _watchmantofsencoding: python -m timeit -s "from hgext.fsmonitor import _watchmantofsencoding, _fixencoding" "fname = '/path/to/file'" "for i in range(1):" "if _fixencoding: fname = _watchmantofsencoding(fname)" Windows (_fixencoding is True): 100 loops, best of 3: 19.5 msec per loop Linux (_fixencoding is False): 100 loops, best of 3: 3.08 msec per loop diff -r 68f263f52d2e -r 867e2a3b1068 hgext/fsmonitor/__init__.py --- a/hgext/fsmonitor/__init__.py Mon Apr 03 17:34:24 2017 -0400 +++ b/hgext/fsmonitor/__init__.py Wed Mar 08 09:03:42 2017 -0500 @@ -91,14 +91,17 @@ from __future__ import absolute_import +import codecs import hashlib import os import stat +import sys from mercurial.i18n import _ from mercurial import ( context, encoding, +error, extensions, localrepo, merge, @@ -110,6 +113,7 @@ from mercurial import match as matchmod from . import ( +pywatchman, state, watchmanclient, ) @@ -159,6 +163,28 @@ sha1.update('\0') return sha1.hexdigest() +_watchmanencoding = pywatchman.encoding.get_local_encoding() +_fsencoding = sys.getfilesystemencoding() or sys.getdefaultencoding() +_fixencoding = codecs.lookup(_watchmanencoding) != codecs.lookup(_fsencoding) + +def _watchmantofsencoding(path): +"""Fix path to match watchman and local filesystem encoding + +watchman's paths encoding can differ from filesystem encoding. For example, +on Windows, it's always utf-8. +""" +try: +decoded = path.decode(_watchmanencoding) +except UnicodeDecodeError as e: +raise error.Abort(str(e), hint='watchman encoding error') + +try: +encoded = decoded.encode(_fsencoding, 'strict') +except UnicodeEncodeError as e: +raise error.Abort(str(e)) + +return encoded + def overridewalk(orig, self, match, subrepos, unknown, ignored, full=True): '''Replacement for dirstate.walk, hooking into Watchman. @@ -303,6 +329,8 @@ # for name case changes. for entry in result['files']: fname = entry['name'] +if _fixencoding: +fname = _watchmantofsencoding(fname) if switch_slashes: fname = fname.replace('\\', '/') if normalize: ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4 NEW-CONCEPT] track-tags: write all tag changes to a file
On Fri, 7 Apr 2017 15:11:16 +0200, Pierre-Yves David wrote: > On 04/06/2017 05:27 PM, Yuya Nishihara wrote: > > On Thu, 6 Apr 2017 01:09:47 +0200, Pierre-Yves David wrote: > >> On 04/05/2017 10:54 PM, Jun Wu wrote: > >>> I don't think "writing things that hook *may* need to filesystem" is a > >>> good > >>> approach. It introduces unnecessary overhead if the hook does not need > >>> that > >>> information. > >> > >> I do not find the overhead concerning. > >> > >> The thing to keep in mind is that if they are many things to write down, > >> this also means many thing changed during the transaction. So the > >> overhead is likely minimal. In the tags cases, just updating the various > >> tags related cache is going to be much more expensive that writing this > >> to disk. > > > > I agree Pierre-Yves in that I/O overhead wouldn't matter. On the other hand, > > I think the slowness of calling back an hg command doesn't matter, too. > > The cost of a round-trip to Mercurial is close to a minimum of 0.1s. And > every hooks who needs fine transaction data will have to pay it. > Possibly multiple time. One the other hand writing down a file is a one > time operation that do not add overhead to each hook. > So I'm still leaning toward the file solution. > > > As the journal extension collects similar information for bookmarks, can't > > we integrate the tag tracking with the journal? > > Journal is currently an extension, and an experimental one. I would > prefer not to be scope bloated into solving the two points aboves before > getting theses features done. > > Journal is also tracking name movement, not transaction. We would have > to teach journal about transaction first (that is probably something > useful to do, but more scope bloat :-) ) Okay, so this tags.changes file describes the current transaction, which is a different concept than the journal. I agree. I prefer not introducing new file format which we'll have to document and maintain forever, but I have no better idea. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 8] bundle: allow bundle command to use changegroup3 in tests
On 04/07/2017 04:44 PM, Jun Wu wrote: Excerpts from Pierre-Yves David's message of 2017-04-07 16:39:03 +0200: Is there any reasons why we can't just have bundle spec to support cg3? martinvonz reminds me that the treemanifest part is tricky. You mean that currently, your cg3 bundle does not support treemanifest? If so, I'm not sure this is a blocker since treemanifest are still experimental and already unsupported by 'hg bundle'. afaik, cg3 is also experimental. Okay, so it seems fine to not expose it in bundlespec Out of curiosity, any idea of why it is still experimental? 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 8] bundle: allow bundle command to use changegroup3 in tests
Excerpts from Pierre-Yves David's message of 2017-04-07 16:39:03 +0200: > >> Is there any reasons why we can't just have bundle spec to support cg3? > > > > martinvonz reminds me that the treemanifest part is tricky. > > You mean that currently, your cg3 bundle does not support treemanifest? > > If so, I'm not sure this is a blocker since treemanifest are still > experimental and already unsupported by 'hg bundle'. afaik, cg3 is also experimental. > > Cheers, > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 8] bundlerepo: make baserevision return raw text
On Fri, 7 Apr 2017 07:04:40 -0700, Jun Wu wrote: > Excerpts from Yuya Nishihara's message of 2017-04-07 22:56:54 +0900: > > On Thu, 6 Apr 2017 19:08:12 -0700, Jun Wu wrote: > > > # HG changeset patch > > > # User Jun Wu > > > # Date 1491525809 25200 > > > # Thu Apr 06 17:43:29 2017 -0700 > > > # Node ID f05275cd80eb1afde5c36470fadf224a44733c45 > > > # Parent 2a254e0cac392c1e0af8bbf0645ecb02b2352f8c > > > # Available At https://bitbucket.org/quark-zju/hg-draft > > > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > > > f05275cd80eb > > > bundlerepo: make baserevision return raw text > > > > It might be better to pass 'raw' argument to baserevision(), but I'm not > > sure. > > Since no deltas are built against external content in all cases, and base > revision is related to delta application. I think it's safer to not allow > raw=False. > > > > > > @@ -211,5 +211,10 @@ class bundlemanifest(bundlerevlog, manif > > > result = '%s' % self.fulltextcache[node] > > > else: > > > -result = manifest.manifestrevlog.revision(self, nodeorrev) > > > +try: > > > +result = manifest.manifestrevlog.revision(self, > > > nodeorrev, > > > + raw=True) > > > +except TypeError: > > > +# some manifestrevlog implementation does not accept > > > "raw" > > > +result = manifest.manifestrevlog.revision(self, > > > nodeorrev) > > > return result > > > > I think the general rule is to change the API and fix non-core extensions. > > I agree. I was a bit worried that it may make the series too long. But it > turns out the try block is actually not necessary here. > > It should be just: > > result = manifest.manifestrevlog.revision(self, nodeorrev, raw=True) Dropped try-except and queued, thanks! ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 8] bundle: allow bundle command to use changegroup3 in tests
On 04/07/2017 04:14 PM, Jun Wu wrote: Excerpts from Pierre-Yves David's message of 2017-04-07 16:07:08 +0200: On 04/07/2017 04:08 AM, Jun Wu wrote: # HG changeset patch # User Jun Wu # Date 1491523318 25200 # Thu Apr 06 17:01:58 2017 -0700 # Node ID 3d62d68ed4245359b5ae5b6b6c1959a15ffa84e9 # Parent 45761ef1bc935b1fab74adccf2541ef854b1c2eb # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 3d62d68ed424 bundle: allow bundle command to use changegroup3 in tests Since bundle2 writes changegroup version, we can just reuse the bundle2 format for changegroup3. This won't enable the bundle command to write changegroup3 in the wild, since exchange.parsebundlespec only returns changegroup2. Is there any reasons why we can't just have bundle spec to support cg3? martinvonz reminds me that the treemanifest part is tricky. You mean that currently, your cg3 bundle does not support treemanifest? If so, I'm not sure this is a blocker since treemanifest are still experimental and already unsupported by 'hg bundle'. Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Announcing Config Express Extension, version 0.1.0
Hello, Mathias de Maré and I are happy to announce the very first release of the config express extension. The goal of the extensions is to help server owner and company to monitor their client configuration. This first version have rough edges and does not contains much. A server can suggest and standard configuration to its client (on pull). A client can send its Mercurial version and some of its configuration value to the server (on push). These data will be available to hook. They are much more coming: %includes, and repository format supports. Automated check server side, writing to user-wide configuration, etc… A 0.2.0 version is probably not too far behind. Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Installation-specific ini file
I've installed a server on my Windows machine under Apache. I want to create an installation-specific mercurial.ini, so that it doesn't use the ini file in my User directory. But I can't find the proper location. I have the hg.exe file in c:\Python27\Scripts. I thought that mercurial.ini should go into the same directory, but that doesn't seem to work. What is the proper location? thanks Peter ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Server logs
I've successfully set up a Mercurial server on Windows running under Apache. But I can't find where the Mercurial logs are. I see access.log and error.log, but that just contains non-Hg specific info about the transactions. Where does the Hg Server log go? thanks Peter ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 8] bundle: allow bundle command to use changegroup3 in tests
Excerpts from Pierre-Yves David's message of 2017-04-07 16:07:08 +0200: > > On 04/07/2017 04:08 AM, Jun Wu wrote: > > # HG changeset patch > > # User Jun Wu > > # Date 1491523318 25200 > > # Thu Apr 06 17:01:58 2017 -0700 > > # Node ID 3d62d68ed4245359b5ae5b6b6c1959a15ffa84e9 > > # Parent 45761ef1bc935b1fab74adccf2541ef854b1c2eb > > # Available At https://bitbucket.org/quark-zju/hg-draft > > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > > 3d62d68ed424 > > bundle: allow bundle command to use changegroup3 in tests > > > > Since bundle2 writes changegroup version, we can just reuse the bundle2 > > format for changegroup3. > > > > This won't enable the bundle command to write changegroup3 in the wild, > > since exchange.parsebundlespec only returns changegroup2. > > Is there any reasons why we can't just have bundle spec to support cg3? martinvonz reminds me that the treemanifest part is tricky. > > The usual way to go here is: > 1) add a way to specify the bundle content you want as a bundlespec > 2) automatically upgrade to the minimal subset we need to not loose > information when special feature is used. (in your case cg3) > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
RE: [PATCH 1 of 2 shelve-ext] shelve: move node-pruning functionality to be member of shelvedstate
I am fine with renaming nodestoprune/prunenodes, but not to cleanupnodes: there are already 14 lines with word "cleanup" in shelve.py. If you can come up with a better name, you can change it always (especially given that class member is already nodestoprune). -Original Message- From: Pierre-Yves David [mailto:pierre-yves.da...@ens-lyon.org] Sent: Friday, 7 April, 2017 15:02 To: Ryan McElroy ; Kostia Balytskyi ; mercurial-devel@mercurial-scm.org Subject: Re: [PATCH 1 of 2 shelve-ext] shelve: move node-pruning functionality to be member of shelvedstate On 04/07/2017 03:58 PM, Ryan McElroy wrote: > On 4/7/17 2:46 PM, Kostia Balytskyi wrote: >> # HG changeset patch >> # User Kostia Balytskyi # Date 1491570726 25200 >> # Fri Apr 07 06:12:06 2017 -0700 >> # Node ID c70f98920d904c94abe17f0b4fd141928dfa3e84 >> # Parent f6d77af84ef3e936b15634759df2718d5363b78a >> shelve: move node-pruning functionality to be member of shelvedstate >> >> This is just a piece of refactoring that I'd like to get in. It seems >> harmless to me and will still be valualbe in future, when better >> hiding mechanism is introduced. > > I agree with the direction of the cleanup. I agree with the direction too but using "prune" might get confusing here. I would recommend using another word without an obsolete markers connotation. For example cleanupnodes seems fine What do you think? >> diff --git a/hgext/shelve.py b/hgext/shelve.py >> --- a/hgext/shelve.py >> +++ b/hgext/shelve.py >> @@ -234,6 +234,11 @@ class shelvedstate(object): >> def clear(cls, repo): >> repo.vfs.unlinkpath(cls._filename, ignoremissing=True) >> +def prunenodes(self, ui, repo): >> +"""Cleanup temporary nodes from the repo""" >> +repair.strip(ui, repo, self.nodestoprune, backup=False, >> + topic='shelve') >> + > > "prune" is definitely a confusing name to use here. If the goal is to > be "agnostic" to the type of node removal going on, call it "removenodes". > If you want to be maximally clear, just call it "stripnodes" and > rename it later when there's an alternate. > >> def cleanupoldbackups(repo): >> vfs = vfsmod.vfs(repo.vfs.join(backupdir)) >> maxbackups = repo.ui.configint('shelve', 'maxbackups', 10) @@ >> -583,8 +588,7 @@ def unshelveabort(ui, repo, state, opts) >> raise >> mergefiles(ui, repo, state.wctx, state.pendingctx) >> -repair.strip(ui, repo, state.nodestoprune, backup=False, >> - topic='shelve') >> +state.prunenodes(ui, repo) >> finally: >> shelvedstate.clear(repo) >> ui.warn(_("unshelve of '%s' aborted\n") % state.name) >> @@ -655,7 +659,7 @@ def unshelvecontinue(ui, repo, state, op >> mergefiles(ui, repo, state.wctx, shelvectx) >> restorebranch(ui, repo, state.branchtorestore) >> -repair.strip(ui, repo, state.nodestoprune, backup=False, >> topic='shelve') >> +state.prunenodes(ui, repo) >> _restoreactivebookmark(repo, state.activebookmark) >> shelvedstate.clear(repo) >> unshelvecleanup(ui, repo, state.name, opts) >> > > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Ds > cm.org_mailman_listinfo_mercurial-2Ddevel&d=DwICaQ&c=5VD0RTtNlTh3ycd41 > b3MUw&r=Pp-gQYFgs4tKlSFPF5kfCw&m=sQIiGyuq_0IQmaF9PP8F0DCU2b0lP5av_lDck > g3idtk&s=BSi9_RvTDzP8V3KckQYSrbpXpL2g6A85sCIcc2dID4w&e= -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
RE: [PATCH 2 of 2 shelve-ext] shelve: add shelve type saving and loading
The line can be added later, but it's less extension-friendly this way. Someone may add some new lines to shelvestate and extension-writers would need to adopt for position changes. Reserving this position will mean that core shelve changes affect extensions to a smaller degree. This was one of the reasons I wanted to get simple key-value file in core, but I do not want to port shelvestate to this format now. -Original Message- From: Ryan McElroy Sent: Friday, 7 April, 2017 14:58 To: Kostia Balytskyi ; mercurial-devel@mercurial-scm.org Subject: Re: [PATCH 2 of 2 shelve-ext] shelve: add shelve type saving and loading On 4/7/17 2:46 PM, Kostia Balytskyi wrote: > # HG changeset patch > # User Kostia Balytskyi # Date 1491572377 25200 > # Fri Apr 07 06:39:37 2017 -0700 > # Node ID 3ef1bf4e61ef5c5957a8c5978dfc2785e31dcc42 > # Parent c70f98920d904c94abe17f0b4fd141928dfa3e84 > shelve: add shelve type saving and loading > > I would like to reserve a line in shelvestate file for shelve type used. > It looks like whenever we go with the alternative approach, we will > have to support traditional shelve for some time, so this makes sense. > > diff --git a/hgext/shelve.py b/hgext/shelve.py > --- a/hgext/shelve.py > +++ b/hgext/shelve.py > @@ -173,6 +173,7 @@ class shelvedstate(object): > _nokeep = 'nokeep' > # colon is essential to differentiate from a real bookmark name > _noactivebook = ':no-active-bookmark' > +_traditional = 'traditional' > > @classmethod > def load(cls, repo): > @@ -191,6 +192,9 @@ class shelvedstate(object): > branchtorestore = fp.readline().strip() > keep = fp.readline().strip() == cls._keep > activebook = fp.readline().strip() > +# we do not use shelvekind anywhere now, > +# this is just to reserve a line in a file > +shelvekind = fp.readline().strip() > except (ValueError, TypeError) as err: > raise error.CorruptedState(str(err)) > finally: > @@ -208,6 +212,7 @@ class shelvedstate(object): > obj.activebookmark = '' > if activebook != cls._noactivebook: > obj.activebookmark = activebook > +obj.shelvekind = shelvekind > except error.RepoLookupError as err: > raise error.CorruptedState(str(err)) > > @@ -228,6 +233,8 @@ class shelvedstate(object): > fp.write('%s\n' % branchtorestore) > fp.write('%s\n' % (cls._keep if keep else cls._nokeep)) > fp.write('%s\n' % (activebook or cls._noactivebook)) > +# for now we only have traditional shelves > +fp.write('%s\n' % cls._traditional) > fp.close() > > @classmethod > I don't see much harm here, but I also don't see any benefit. Is there a disadvantage to just adding this line later? Like, is it important to get a line reserved in the file before the freeze, for example? My guess would be "no" because if the line is missing, you can assume "traditional" shelve, which you will have to do anyway for back-compat. So I'm -1 on this patch at this time. However, if there's a stronger reason to reserve the line now, please state that in the commit message clearly and send a v2. For now, I've marked this series as "changes requested" in patchwork. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 8] bundlerepo: make baserevision return raw text
Excerpts from Yuya Nishihara's message of 2017-04-07 22:56:54 +0900: > On Thu, 6 Apr 2017 19:08:12 -0700, Jun Wu wrote: > > # HG changeset patch > > # User Jun Wu > > # Date 1491525809 25200 > > # Thu Apr 06 17:43:29 2017 -0700 > > # Node ID f05275cd80eb1afde5c36470fadf224a44733c45 > > # Parent 2a254e0cac392c1e0af8bbf0645ecb02b2352f8c > > # Available At https://bitbucket.org/quark-zju/hg-draft > > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > > f05275cd80eb > > bundlerepo: make baserevision return raw text > > It might be better to pass 'raw' argument to baserevision(), but I'm not > sure. Since no deltas are built against external content in all cases, and base revision is related to delta application. I think it's safer to not allow raw=False. > > > @@ -211,5 +211,10 @@ class bundlemanifest(bundlerevlog, manif > > result = '%s' % self.fulltextcache[node] > > else: > > -result = manifest.manifestrevlog.revision(self, nodeorrev) > > +try: > > +result = manifest.manifestrevlog.revision(self, nodeorrev, > > + raw=True) > > +except TypeError: > > +# some manifestrevlog implementation does not accept "raw" > > +result = manifest.manifestrevlog.revision(self, nodeorrev) > > return result > > I think the general rule is to change the API and fix non-core extensions. I agree. I was a bit worried that it may make the series too long. But it turns out the try block is actually not necessary here. It should be just: result = manifest.manifestrevlog.revision(self, nodeorrev, raw=True) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 8] bundle: allow bundle command to use changegroup3 in tests
On 04/07/2017 04:08 AM, Jun Wu wrote: # HG changeset patch # User Jun Wu # Date 1491523318 25200 # Thu Apr 06 17:01:58 2017 -0700 # Node ID 3d62d68ed4245359b5ae5b6b6c1959a15ffa84e9 # Parent 45761ef1bc935b1fab74adccf2541ef854b1c2eb # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 3d62d68ed424 bundle: allow bundle command to use changegroup3 in tests Since bundle2 writes changegroup version, we can just reuse the bundle2 format for changegroup3. This won't enable the bundle command to write changegroup3 in the wild, since exchange.parsebundlespec only returns changegroup2. Is there any reasons why we can't just have bundle spec to support cg3? The usual way to go here is: 1) add a way to specify the bundle content you want as a bundlespec 2) automatically upgrade to the minimal subset we need to not loose information when special feature is used. (in your case cg3) -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 8] test-flagprocessor: use changegroup3 in bundle2
Excerpts from Yuya Nishihara's message of 2017-04-07 22:51:38 +0900: > On Thu, 6 Apr 2017 19:08:10 -0700, Jun Wu wrote: > > # HG changeset patch > > # User Jun Wu > > # Date 1491524600 25200 > > # Thu Apr 06 17:23:20 2017 -0700 > > # Node ID f9c75f4ee30e0102d8fb5a65eaee988e7ca30139 > > # Parent 3d62d68ed4245359b5ae5b6b6c1959a15ffa84e9 > > # Available At https://bitbucket.org/quark-zju/hg-draft > > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > > f9c75f4ee30e > > test-flagprocessor: use changegroup3 in bundle2 > > > > This will force "hg bundle" to use changegroup3 in the test. It is > > important since only changegroup3 preserves revlog flags. > > > > diff --git a/tests/flagprocessorext.py b/tests/flagprocessorext.py > > --- a/tests/flagprocessorext.py > > +++ b/tests/flagprocessorext.py > > @@ -8,4 +8,5 @@ import zlib > > from mercurial import ( > > changegroup, > > +exchange, > > extensions, > > filelog, > > @@ -104,4 +105,8 @@ def extsetup(ui): > > revlog.REVIDX_FLAGS_ORDER.extend(flags) > > > > +# Teach exchange to use changegroup 3 > > +for k in exchange._bundlespeccgversions.keys(): > > +exchange._bundlespeccgversions[k] = '03' > > Is this a temporary hack until cg3 bundle is officially supported? Yes. The treemanifest part may be non-trivial. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4 NEW-CONCEPT] track-tags: write all tag changes to a file
On 04/07/2017 04:00 PM, Jun Wu wrote: Excerpts from Pierre-Yves David's message of 2017-04-07 15:55:39 +0200: Yes, since we do not touch the manifest, we can reuse the entry from the originalctx. In the general case, you either touch the '.hgtags' (and you know its value) or you don't and you can reuse the parent value. Okay. I think this series is good as long as it does not introduce full manifest read before being enabled by default. Do you mind I add comments about the manifest performance (including the metadataonlyctx case) as a follow-up after it gets pushed? Nope, go for it. Thanks for flagging these corner cases! 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 2 shelve-ext] shelve: move node-pruning functionality to be member of shelvedstate
On 04/07/2017 03:58 PM, Ryan McElroy wrote: On 4/7/17 2:46 PM, Kostia Balytskyi wrote: # HG changeset patch # User Kostia Balytskyi # Date 1491570726 25200 # Fri Apr 07 06:12:06 2017 -0700 # Node ID c70f98920d904c94abe17f0b4fd141928dfa3e84 # Parent f6d77af84ef3e936b15634759df2718d5363b78a shelve: move node-pruning functionality to be member of shelvedstate This is just a piece of refactoring that I'd like to get in. It seems harmless to me and will still be valualbe in future, when better hiding mechanism is introduced. I agree with the direction of the cleanup. I agree with the direction too but using "prune" might get confusing here. I would recommend using another word without an obsolete markers connotation. For example cleanupnodes seems fine What do you think? diff --git a/hgext/shelve.py b/hgext/shelve.py --- a/hgext/shelve.py +++ b/hgext/shelve.py @@ -234,6 +234,11 @@ class shelvedstate(object): def clear(cls, repo): repo.vfs.unlinkpath(cls._filename, ignoremissing=True) +def prunenodes(self, ui, repo): +"""Cleanup temporary nodes from the repo""" +repair.strip(ui, repo, self.nodestoprune, backup=False, + topic='shelve') + "prune" is definitely a confusing name to use here. If the goal is to be "agnostic" to the type of node removal going on, call it "removenodes". If you want to be maximally clear, just call it "stripnodes" and rename it later when there's an alternate. def cleanupoldbackups(repo): vfs = vfsmod.vfs(repo.vfs.join(backupdir)) maxbackups = repo.ui.configint('shelve', 'maxbackups', 10) @@ -583,8 +588,7 @@ def unshelveabort(ui, repo, state, opts) raise mergefiles(ui, repo, state.wctx, state.pendingctx) -repair.strip(ui, repo, state.nodestoprune, backup=False, - topic='shelve') +state.prunenodes(ui, repo) finally: shelvedstate.clear(repo) ui.warn(_("unshelve of '%s' aborted\n") % state.name) @@ -655,7 +659,7 @@ def unshelvecontinue(ui, repo, state, op mergefiles(ui, repo, state.wctx, shelvectx) restorebranch(ui, repo, state.branchtorestore) -repair.strip(ui, repo, state.nodestoprune, backup=False, topic='shelve') +state.prunenodes(ui, repo) _restoreactivebookmark(repo, state.activebookmark) shelvedstate.clear(repo) unshelvecleanup(ui, repo, state.name, opts) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4 NEW-CONCEPT] track-tags: write all tag changes to a file
Excerpts from Pierre-Yves David's message of 2017-04-07 15:55:39 +0200: > Yes, since we do not touch the manifest, we can reuse the entry from the > originalctx. > > In the general case, you either touch the '.hgtags' (and you know its > value) or you don't and you can reuse the parent value. Okay. I think this series is good as long as it does not introduce full manifest read before being enabled by default. Do you mind I add comments about the manifest performance (including the metadataonlyctx case) as a follow-up after it gets pushed? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2 shelve-ext] shelve: move node-pruning functionality to be member of shelvedstate
On 4/7/17 2:46 PM, Kostia Balytskyi wrote: # HG changeset patch # User Kostia Balytskyi # Date 1491570726 25200 # Fri Apr 07 06:12:06 2017 -0700 # Node ID c70f98920d904c94abe17f0b4fd141928dfa3e84 # Parent f6d77af84ef3e936b15634759df2718d5363b78a shelve: move node-pruning functionality to be member of shelvedstate This is just a piece of refactoring that I'd like to get in. It seems harmless to me and will still be valualbe in future, when better hiding mechanism is introduced. I agree with the direction of the cleanup. diff --git a/hgext/shelve.py b/hgext/shelve.py --- a/hgext/shelve.py +++ b/hgext/shelve.py @@ -234,6 +234,11 @@ class shelvedstate(object): def clear(cls, repo): repo.vfs.unlinkpath(cls._filename, ignoremissing=True) +def prunenodes(self, ui, repo): +"""Cleanup temporary nodes from the repo""" +repair.strip(ui, repo, self.nodestoprune, backup=False, + topic='shelve') + "prune" is definitely a confusing name to use here. If the goal is to be "agnostic" to the type of node removal going on, call it "removenodes". If you want to be maximally clear, just call it "stripnodes" and rename it later when there's an alternate. def cleanupoldbackups(repo): vfs = vfsmod.vfs(repo.vfs.join(backupdir)) maxbackups = repo.ui.configint('shelve', 'maxbackups', 10) @@ -583,8 +588,7 @@ def unshelveabort(ui, repo, state, opts) raise mergefiles(ui, repo, state.wctx, state.pendingctx) -repair.strip(ui, repo, state.nodestoprune, backup=False, - topic='shelve') +state.prunenodes(ui, repo) finally: shelvedstate.clear(repo) ui.warn(_("unshelve of '%s' aborted\n") % state.name) @@ -655,7 +659,7 @@ def unshelvecontinue(ui, repo, state, op mergefiles(ui, repo, state.wctx, shelvectx) restorebranch(ui, repo, state.branchtorestore) -repair.strip(ui, repo, state.nodestoprune, backup=False, topic='shelve') +state.prunenodes(ui, repo) _restoreactivebookmark(repo, state.activebookmark) shelvedstate.clear(repo) unshelvecleanup(ui, repo, state.name, opts) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2 shelve-ext] shelve: add shelve type saving and loading
On 4/7/17 2:46 PM, Kostia Balytskyi wrote: # HG changeset patch # User Kostia Balytskyi # Date 1491572377 25200 # Fri Apr 07 06:39:37 2017 -0700 # Node ID 3ef1bf4e61ef5c5957a8c5978dfc2785e31dcc42 # Parent c70f98920d904c94abe17f0b4fd141928dfa3e84 shelve: add shelve type saving and loading I would like to reserve a line in shelvestate file for shelve type used. It looks like whenever we go with the alternative approach, we will have to support traditional shelve for some time, so this makes sense. diff --git a/hgext/shelve.py b/hgext/shelve.py --- a/hgext/shelve.py +++ b/hgext/shelve.py @@ -173,6 +173,7 @@ class shelvedstate(object): _nokeep = 'nokeep' # colon is essential to differentiate from a real bookmark name _noactivebook = ':no-active-bookmark' +_traditional = 'traditional' @classmethod def load(cls, repo): @@ -191,6 +192,9 @@ class shelvedstate(object): branchtorestore = fp.readline().strip() keep = fp.readline().strip() == cls._keep activebook = fp.readline().strip() +# we do not use shelvekind anywhere now, +# this is just to reserve a line in a file +shelvekind = fp.readline().strip() except (ValueError, TypeError) as err: raise error.CorruptedState(str(err)) finally: @@ -208,6 +212,7 @@ class shelvedstate(object): obj.activebookmark = '' if activebook != cls._noactivebook: obj.activebookmark = activebook +obj.shelvekind = shelvekind except error.RepoLookupError as err: raise error.CorruptedState(str(err)) @@ -228,6 +233,8 @@ class shelvedstate(object): fp.write('%s\n' % branchtorestore) fp.write('%s\n' % (cls._keep if keep else cls._nokeep)) fp.write('%s\n' % (activebook or cls._noactivebook)) +# for now we only have traditional shelves +fp.write('%s\n' % cls._traditional) fp.close() @classmethod I don't see much harm here, but I also don't see any benefit. Is there a disadvantage to just adding this line later? Like, is it important to get a line reserved in the file before the freeze, for example? My guess would be "no" because if the line is missing, you can assume "traditional" shelve, which you will have to do anyway for back-compat. So I'm -1 on this patch at this time. However, if there's a stronger reason to reserve the line now, please state that in the commit message clearly and send a v2. For now, I've marked this series as "changes requested" in patchwork. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 8] bundlerepo: make baserevision return raw text
On Thu, 6 Apr 2017 19:08:12 -0700, Jun Wu wrote: > # HG changeset patch > # User Jun Wu > # Date 1491525809 25200 > # Thu Apr 06 17:43:29 2017 -0700 > # Node ID f05275cd80eb1afde5c36470fadf224a44733c45 > # Parent 2a254e0cac392c1e0af8bbf0645ecb02b2352f8c > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > f05275cd80eb > bundlerepo: make baserevision return raw text It might be better to pass 'raw' argument to baserevision(), but I'm not sure. > @@ -211,5 +211,10 @@ class bundlemanifest(bundlerevlog, manif > result = '%s' % self.fulltextcache[node] > else: > -result = manifest.manifestrevlog.revision(self, nodeorrev) > +try: > +result = manifest.manifestrevlog.revision(self, nodeorrev, > + raw=True) > +except TypeError: > +# some manifestrevlog implementation does not accept "raw" > +result = manifest.manifestrevlog.revision(self, nodeorrev) > return result I think the general rule is to change the API and fix non-core extensions. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 8] test-flagprocessor: use changegroup3 in bundle2
On Thu, 6 Apr 2017 19:08:10 -0700, Jun Wu wrote: > # HG changeset patch > # User Jun Wu > # Date 1491524600 25200 > # Thu Apr 06 17:23:20 2017 -0700 > # Node ID f9c75f4ee30e0102d8fb5a65eaee988e7ca30139 > # Parent 3d62d68ed4245359b5ae5b6b6c1959a15ffa84e9 > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > f9c75f4ee30e > test-flagprocessor: use changegroup3 in bundle2 > > This will force "hg bundle" to use changegroup3 in the test. It is > important since only changegroup3 preserves revlog flags. > > diff --git a/tests/flagprocessorext.py b/tests/flagprocessorext.py > --- a/tests/flagprocessorext.py > +++ b/tests/flagprocessorext.py > @@ -8,4 +8,5 @@ import zlib > from mercurial import ( > changegroup, > +exchange, > extensions, > filelog, > @@ -104,4 +105,8 @@ def extsetup(ui): > revlog.REVIDX_FLAGS_ORDER.extend(flags) > > +# Teach exchange to use changegroup 3 > +for k in exchange._bundlespeccgversions.keys(): > +exchange._bundlespeccgversions[k] = '03' Is this a temporary hack until cg3 bundle is officially supported? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 8] bundle: allow bundle command to use changegroup3 in tests
On Thu, 6 Apr 2017 19:08:09 -0700, Jun Wu wrote: > # HG changeset patch > # User Jun Wu > # Date 1491523318 25200 > # Thu Apr 06 17:01:58 2017 -0700 > # Node ID 3d62d68ed4245359b5ae5b6b6c1959a15ffa84e9 > # Parent 45761ef1bc935b1fab74adccf2541ef854b1c2eb > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > 3d62d68ed424 > bundle: allow bundle command to use changegroup3 in tests The overall change looks good to me. I have a few questions in line. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4 NEW-CONCEPT] track-tags: write all tag changes to a file
On 04/07/2017 03:50 PM, Jun Wu wrote: Excerpts from Pierre-Yves David's message of 2017-04-07 14:52:44 +0200: On 04/06/2017 08:18 PM, Jun Wu wrote: Thanks for the clarification. My concern about performance is not writing the file, but calculating the content to be written. I'd like to see more clarification about whether and when this new code path triggers a full manifest read, since reading a flat manifest is painfully slow on a large repo. They is a cache for the changeset → fnode mapping. And this cache is exchanged over the wire so I've not worried. Worst case we'll need a pass to strengthen the cache warming at commit time (as part of the "better use the cache before getting out of experimental"). So overall we should not triggers full manifest read. Does this address your concerns? IIUC, you are assuming that in the transaction, when a new commit is written, the manifest is known. However, that's not always the case. For example, if I do "hg metaedit" (there could be more cases like this in the future), it creates a new changelog node, but could reuse the manifest node directly without reading that manfiest. That's what metadataonlyctx is used for. Will you be able to avoid reading manifest to warm the cache in this case? Yes, since we do not touch the manifest, we can reuse the entry from the originalctx. In the general case, you either touch the '.hgtags' (and you know its value) or you don't and you can reuse the parent value. Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4 NEW-CONCEPT] track-tags: write all tag changes to a file
Excerpts from Pierre-Yves David's message of 2017-04-07 14:52:44 +0200: > > On 04/06/2017 08:18 PM, Jun Wu wrote: > > Thanks for the clarification. > > > > My concern about performance is not writing the file, but calculating the > > content to be written. I'd like to see more clarification about whether and > > when this new code path triggers a full manifest read, since reading a flat > > manifest is painfully slow on a large repo. > > They is a cache for the changeset → fnode mapping. And this cache is > exchanged over the wire so I've not worried. Worst case we'll need a > pass to strengthen the cache warming at commit time (as part of the > "better use the cache before getting out of experimental"). So overall > we should not triggers full manifest read. > > Does this address your concerns? IIUC, you are assuming that in the transaction, when a new commit is written, the manifest is known. However, that's not always the case. For example, if I do "hg metaedit" (there could be more cases like this in the future), it creates a new changelog node, but could reuse the manifest node directly without reading that manfiest. That's what metadataonlyctx is used for. Will you be able to avoid reading manifest to warm the cache in this case? > > Cheers, > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2 shelve-ext] shelve: move node-pruning functionality to be member of shelvedstate
# HG changeset patch # User Kostia Balytskyi # Date 1491570726 25200 # Fri Apr 07 06:12:06 2017 -0700 # Node ID c70f98920d904c94abe17f0b4fd141928dfa3e84 # Parent f6d77af84ef3e936b15634759df2718d5363b78a shelve: move node-pruning functionality to be member of shelvedstate This is just a piece of refactoring that I'd like to get in. It seems harmless to me and will still be valualbe in future, when better hiding mechanism is introduced. diff --git a/hgext/shelve.py b/hgext/shelve.py --- a/hgext/shelve.py +++ b/hgext/shelve.py @@ -234,6 +234,11 @@ class shelvedstate(object): def clear(cls, repo): repo.vfs.unlinkpath(cls._filename, ignoremissing=True) +def prunenodes(self, ui, repo): +"""Cleanup temporary nodes from the repo""" +repair.strip(ui, repo, self.nodestoprune, backup=False, + topic='shelve') + def cleanupoldbackups(repo): vfs = vfsmod.vfs(repo.vfs.join(backupdir)) maxbackups = repo.ui.configint('shelve', 'maxbackups', 10) @@ -583,8 +588,7 @@ def unshelveabort(ui, repo, state, opts) raise mergefiles(ui, repo, state.wctx, state.pendingctx) -repair.strip(ui, repo, state.nodestoprune, backup=False, - topic='shelve') +state.prunenodes(ui, repo) finally: shelvedstate.clear(repo) ui.warn(_("unshelve of '%s' aborted\n") % state.name) @@ -655,7 +659,7 @@ def unshelvecontinue(ui, repo, state, op mergefiles(ui, repo, state.wctx, shelvectx) restorebranch(ui, repo, state.branchtorestore) -repair.strip(ui, repo, state.nodestoprune, backup=False, topic='shelve') +state.prunenodes(ui, repo) _restoreactivebookmark(repo, state.activebookmark) shelvedstate.clear(repo) unshelvecleanup(ui, repo, state.name, opts) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2 shelve-ext] shelve: add shelve type saving and loading
# HG changeset patch # User Kostia Balytskyi # Date 1491572377 25200 # Fri Apr 07 06:39:37 2017 -0700 # Node ID 3ef1bf4e61ef5c5957a8c5978dfc2785e31dcc42 # Parent c70f98920d904c94abe17f0b4fd141928dfa3e84 shelve: add shelve type saving and loading I would like to reserve a line in shelvestate file for shelve type used. It looks like whenever we go with the alternative approach, we will have to support traditional shelve for some time, so this makes sense. diff --git a/hgext/shelve.py b/hgext/shelve.py --- a/hgext/shelve.py +++ b/hgext/shelve.py @@ -173,6 +173,7 @@ class shelvedstate(object): _nokeep = 'nokeep' # colon is essential to differentiate from a real bookmark name _noactivebook = ':no-active-bookmark' +_traditional = 'traditional' @classmethod def load(cls, repo): @@ -191,6 +192,9 @@ class shelvedstate(object): branchtorestore = fp.readline().strip() keep = fp.readline().strip() == cls._keep activebook = fp.readline().strip() +# we do not use shelvekind anywhere now, +# this is just to reserve a line in a file +shelvekind = fp.readline().strip() except (ValueError, TypeError) as err: raise error.CorruptedState(str(err)) finally: @@ -208,6 +212,7 @@ class shelvedstate(object): obj.activebookmark = '' if activebook != cls._noactivebook: obj.activebookmark = activebook +obj.shelvekind = shelvekind except error.RepoLookupError as err: raise error.CorruptedState(str(err)) @@ -228,6 +233,8 @@ class shelvedstate(object): fp.write('%s\n' % branchtorestore) fp.write('%s\n' % (cls._keep if keep else cls._nokeep)) fp.write('%s\n' % (activebook or cls._noactivebook)) +# for now we only have traditional shelves +fp.write('%s\n' % cls._traditional) fp.close() @classmethod ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 8] bundle: allow bundle command to use changegroup3 in tests
On 4/7/17 3:08 AM, Jun Wu wrote: # HG changeset patch # User Jun Wu # Date 1491523318 25200 # Thu Apr 06 17:01:58 2017 -0700 # Node ID 3d62d68ed4245359b5ae5b6b6c1959a15ffa84e9 # Parent 45761ef1bc935b1fab74adccf2541ef854b1c2eb bundle: allow bundle command to use changegroup3 in tests Since bundle2 writes changegroup version, we can just reuse the bundle2 format for changegroup3. This won't enable the bundle command to write changegroup3 in the wild, since exchange.parsebundlespec only returns changegroup2. It unlocks tests to override exchange.parsebundlespec and get "hg bundle" write changegroup3. diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -1377,7 +1377,9 @@ def bundle(ui, repo, fname, dest=None, * bversion = 'HG10' + bcompression bcompression = None +elif cgversion in ('02', '03'): +bversion = 'HG20' else: -assert cgversion == '02' -bversion = 'HG20' +raise error.ProgrammingError( +'bundle: unexpected changegroup version %s' % cgversion) # TODO compression options should be derived from bundlespec parsing. A test of this abort would be a nice-to-have add-on. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 8 of 8] test-flagprocessor: remove unnecessary greps
On 4/7/17 3:08 AM, Jun Wu wrote: # HG changeset patch # User Jun Wu # Date 1491530511 25200 # Thu Apr 06 19:01:51 2017 -0700 # Node ID fd2bcb9a6182d5abaa06cf6b3fd8a19ab0223750 # Parent be7f687c98afa3e03ffee53b95024b74c2585485 test-flagprocessor: remove unnecessary greps I have reviewed this series and it looks good to me. I'll mark it as pre-reviewed. I will have one or two non-blocking comments coming in on the series. This series fixes another set of nasty raw vs processed issues in core. Thanks for taking the time to create an easy to follow "story" of the patches here once again... creating the test in a compact form and fixing issues one by one in easy-to-review manner. I really appreciate the effort you put in to make reviewable patches -- I know it's not easy in some of these cases. The "2>&1 | egrep ..." code is used for removing uninteresting parts from tracebacks. Now the test does not dump tracebacks, they can be removed. diff --git a/tests/test-flagprocessor.t b/tests/test-flagprocessor.t --- a/tests/test-flagprocessor.t +++ b/tests/test-flagprocessor.t @@ -188,5 +188,5 @@ 4 changesets found $ hg --config extensions.strip= strip -r 2 --no-backup --force -q - $ hg -R bundle.hg log --stat -T '{rev} {desc}\n' base64 2>&1 | egrep -v '^(\*\*| )' + $ hg -R bundle.hg log --stat -T '{rev} {desc}\n' base64 5 branching base64 | 2 +- @@ -214,7 +214,6 @@ - $ hg bundle -R bundle.hg --base 1 bundle-again.hg -q 2>&1 | egrep -v '^(\*\*| )' - [1] - $ hg -R bundle-again.hg log --stat -T '{rev} {desc}\n' base64 2>&1 | egrep -v '^(\*\*| )' + $ hg bundle -R bundle.hg --base 1 bundle-again.hg -q + $ hg -R bundle-again.hg log --stat -T '{rev} {desc}\n' base64 5 branching base64 | 2 +- ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Friendly warning: 4.2 freeze expected to start in one week
Nobody has had time to come up with alternatives to "no work outside of the stable branch" yet, so be advised that the freeze for 4.2 will likely start a week from today. Monday the 18th at the latest. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2] tests: move update requiredest test to own test file
# HG changeset patch # User Ryan McElroy # Date 1491568863 25200 # Fri Apr 07 05:41:03 2017 -0700 # Node ID 3d5f4ddecd504abf742f89c7c5d27064e301bda8 # Parent d5e0f7fbcbbb21bf358362fbdcd693a5e0988b86 tests: move update requiredest test to own test file More tests for this flag are coming in upcoming patches. diff --git a/tests/test-update-dest.t b/tests/test-update-dest.t new file mode 100644 --- /dev/null +++ b/tests/test-update-dest.t @@ -0,0 +1,23 @@ +Test update.requiredest + $ cd $TESTTMP + $ cat >> $HGRCPATH < [commands] + > update.requiredest = True + > EOF + $ hg init repo + $ cd repo + $ echo a >> a + $ hg commit -qAm aa + $ hg up + abort: you must specify a destination + (for example: hg update ".::") + [255] + $ hg up . + 0 files updated, 0 files merged, 0 files removed, 0 files unresolved + $ HGPLAIN=1 hg up + 0 files updated, 0 files merged, 0 files removed, 0 files unresolved + $ hg --config commands.update.requiredest=False up + 0 files updated, 0 files merged, 0 files removed, 0 files unresolved + + $ cd .. + diff --git a/tests/test-update-names.t b/tests/test-update-names.t --- a/tests/test-update-names.t +++ b/tests/test-update-names.t @@ -88,23 +88,3 @@ Test that warning is printed if cwd is d (consider changing to repo root: $TESTTMP/r1/r4) #endif - - $ cd $TESTTMP - $ cat >> $HGRCPATH < [commands] - > update.requiredest = True - > EOF - $ hg init dest - $ cd dest - $ echo a >> a - $ hg commit -qAm aa - $ hg up - abort: you must specify a destination - (for example: hg update ".::") - [255] - $ hg up . - 0 files updated, 0 files merged, 0 files removed, 0 files unresolved - $ HGPLAIN=1 hg up - 0 files updated, 0 files merged, 0 files removed, 0 files unresolved - $ hg --config commands.update.requiredest=False up - 0 files updated, 0 files merged, 0 files removed, 0 files unresolved ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2] pull: abort pull --update if config requires destination (issue5528)
# HG changeset patch # User Ryan McElroy # Date 1491571910 25200 # Fri Apr 07 06:31:50 2017 -0700 # Node ID 1a7c8af860b2bec0349661a82d2f449c9d2944bb # Parent 3d5f4ddecd504abf742f89c7c5d27064e301bda8 pull: abort pull --update if config requires destination (issue5528) diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -3935,6 +3935,12 @@ def pull(ui, repo, source="default", **o Returns 0 on success, 1 if an update had unresolved files. """ + +if ui.configbool('commands', 'update.requiredest') and opts.get('update'): +msg = _('update destination required by configuration') +hint = _('use hg pull followed by hg update DEST') +raise error.Abort(msg, hint=hint) + source, branches = hg.parseurl(ui.expandpath(source), opts.get('branch')) ui.status(_('pulling from %s\n') % util.hidepassword(source)) other = hg.peer(repo, opts, source) diff --git a/tests/test-update-dest.t b/tests/test-update-dest.t --- a/tests/test-update-dest.t +++ b/tests/test-update-dest.t @@ -21,3 +21,15 @@ Test update.requiredest $ cd .. +Check update.requiredest interaction with pull --update + $ hg clone repo clone + updating to branch default + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + $ cd repo + $ echo a >> a + $ hg commit -qAm aa + $ cd ../clone + $ hg pull --update + abort: update destination required by configuration + (use hg pull followed by hg update DEST) + [255] ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 8 of 9] util: support multi-threaded zstandard compression for bundles
On 04/07/2017 03:11 PM, Yuya Nishihara wrote: On Thu, 6 Apr 2017 10:51:36 -0700, Gregory Szorc wrote: Does anyone see any value in adding it as an experimental config option in Mercurial? Or should we drop the feature for now? I'd kinda like it there to make it easier to experiment with. But if others aren't comfortable with it, I totally understand. An experimental config makes sense to me. My only concern was a hard bug in C layer, but that would be okay-ish for experimental features. +1, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 8 of 9] util: support multi-threaded zstandard compression for bundles
On Thu, 6 Apr 2017 10:51:36 -0700, Gregory Szorc wrote: > Does anyone see any value in adding it as an experimental config option in > Mercurial? Or should we drop the feature for now? I'd kinda like it there > to make it easier to experiment with. But if others aren't comfortable with > it, I totally understand. An experimental config makes sense to me. My only concern was a hard bug in C layer, but that would be okay-ish for experimental features. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4 NEW-CONCEPT] track-tags: write all tag changes to a file
On 04/06/2017 05:27 PM, Yuya Nishihara wrote: On Thu, 6 Apr 2017 01:09:47 +0200, Pierre-Yves David wrote: On 04/05/2017 10:54 PM, Jun Wu wrote: I don't think "writing things that hook *may* need to filesystem" is a good approach. It introduces unnecessary overhead if the hook does not need that information. I do not find the overhead concerning. The thing to keep in mind is that if they are many things to write down, this also means many thing changed during the transaction. So the overhead is likely minimal. In the tags cases, just updating the various tags related cache is going to be much more expensive that writing this to disk. I agree Pierre-Yves in that I/O overhead wouldn't matter. On the other hand, I think the slowness of calling back an hg command doesn't matter, too. The cost of a round-trip to Mercurial is close to a minimum of 0.1s. And every hooks who needs fine transaction data will have to pay it. Possibly multiple time. One the other hand writing down a file is a one time operation that do not add overhead to each hook. So I'm still leaning toward the file solution. As the journal extension collects similar information for bookmarks, can't we integrate the tag tracking with the journal? Journal is currently an extension, and an experimental one. I would prefer not to be scope bloated into solving the two points aboves before getting theses features done. Journal is also tracking name movement, not transaction. We would have to teach journal about transaction first (that is probably something useful to do, but more scope bloat :-) ) Maybe a hook will receive some pointer to select the corresponding journal entry, and run "hg journal". (we could use hg journal --pending, to refer to the current transaction, that is something to think about while adding transaction awareness to journal.) -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] fsmonitor: match watchman and filesystem encoding
On Thu, 6 Apr 2017 12:46:38 -0400, Olivier Trempe wrote: > On Thu, Apr 6, 2017 at 9:45 AM, Yuya Nishihara wrote: > > > > +def _watchmantofsencoding(path): > > > > +"""Fix path to match watchman and local filesystem encoding > > > > + > > > > +watchman's paths encoding can differ from filesystem encoding. > > For example, > > > > +on Windows, it's always utf-8. > > > > +""" > > > > +try: > > > > +decoded = path.decode(_watchmanencoding) > > > > +except UnicodeDecodeError as e: > > > > +raise error.Abort(e, hint='watchman encoding error') > > > > > > Does this need to be str(e)? > > > > Perhaps. > > > > > > > + > > > > +return decoded.encode(_fsencoding, 'replace') > > > > Maybe it's better to catch exception here. Encoding error would be more > > likely > > to happen because Windows ANSI charset is generally narrower than UTF-*. > > > > You mean setting the error handler to 'strict' rather than 'replace' and > wrap the call in a try except block? Yes. > Or just wrap the call in a try except block, but keep the 'replace' error > handler? > Using the 'replace' error handler is necessary here to match the behavior > of osutil.listdir It appears 'mbcs' codec replaces unknown characters no matter if 'strict' is specified or not. Perhaps that would be done by WideCharToMultiByte(). I think using 'strict' here is more consistent because osutil.listdir() handles nothing about encoding in Python layer. https://msdn.microsoft.com/en-us/library/windows/desktop/dd374130(v=vs.85).aspx ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4 NEW-CONCEPT] track-tags: write all tag changes to a file
On 04/06/2017 08:18 PM, Jun Wu wrote: Thanks for the clarification. My concern about performance is not writing the file, but calculating the content to be written. I'd like to see more clarification about whether and when this new code path triggers a full manifest read, since reading a flat manifest is painfully slow on a large repo. They is a cache for the changeset → fnode mapping. And this cache is exchanged over the wire so I've not worried. Worst case we'll need a pass to strengthen the cache warming at commit time (as part of the "better use the cache before getting out of experimental"). So overall we should not triggers full manifest read. Does this address your concerns? Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[Bug 5528] New: commands.update.requiredest is ignored with pull --update
https://bz.mercurial-scm.org/show_bug.cgi?id=5528 Bug ID: 5528 Summary: commands.update.requiredest is ignored with pull --update Product: Mercurial Version: 4.1-rc Hardware: All OS: All Status: UNCONFIRMED Severity: feature Priority: wish Component: Mercurial Assignee: bugzi...@mercurial-scm.org Reporter: r...@fb.com CC: mercurial-devel@mercurial-scm.org Similar to issue5514, this issue is that `hg pull --update` doesn't abort as it should when commands.update.requiredest is set to true. See more discussion about this in #5514. -- 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 1 of 4 V2] run-tests: prevent a (glob) declaration from reordering (?) lines
On Thu, 06 Apr 2017 21:11:40 -0400, Matt Harbison wrote: > # HG changeset patch > # User Matt Harbison > # Date 1491444033 14400 > # Wed Apr 05 22:00:33 2017 -0400 > # Node ID c0789426514615cd841a31f60688516f2cdeaae0 > # Parent 81abd0d12c8641df666d356f6033d84cd55977a8 > run-tests: prevent a (glob) declaration from reordering (?) lines Queued the series, many thanks. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 4 V2] run-tests: support per-line conditional output in tests
On Thu, 06 Apr 2017 21:11:42 -0400, Matt Harbison wrote: > # HG changeset patch > # User Matt Harbison > # Date 1491448647 14400 > # Wed Apr 05 23:17:27 2017 -0400 > # Node ID 8e3cf915084723551d0b68094cede7e65f49cf39 > # Parent 7c5e861f39fbffdfae0e31d1edba419a62391afe > run-tests: support per-line conditional output in tests > @@ -341,6 +361,12 @@ >xyz >+ echo *SALT* 15 0 (glob) >*SALT* 15 0 (glob) > + + printf 'zyx\nwvu\ntsr\n' Globbed out quotes. IIRC, it is shell dependent. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 4] py3: alias unicode to str on Python 3
On Fri, 07 Apr 2017 16:29:39 +0530, Pulkit Goyal wrote: > # HG changeset patch > # User Pulkit Goyal <7895pul...@gmail.com> > # Date 1491560884 -19800 > # Fri Apr 07 15:58:04 2017 +0530 > # Node ID beedb62e62a78a2f45ef8bd0b72c51f14619ee7f > # Parent c08552f3a4ab883aac255bc7d8efb7637ba245d8 > py3: alias unicode to str on Python 3 > > diff -r c08552f3a4ab -r beedb62e62a7 mercurial/scmutil.py > --- a/mercurial/scmutil.pyFri Apr 07 13:46:35 2017 +0530 > +++ b/mercurial/scmutil.pyFri Apr 07 15:58:04 2017 +0530 > @@ -36,6 +36,9 @@ > > termsize = scmplatform.termsize > > +if pycompat.ispy3: > +unicode = str Let's add pycompat.unicode and/or importer magic. We'll need the unicode type at several places. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 4] py3: use pycompat.byteskwargs() to convert opts to bytes
On Fri, 07 Apr 2017 16:29:37 +0530, Pulkit Goyal wrote: > # HG changeset patch > # User Pulkit Goyal <7895pul...@gmail.com> > # Date 1491552933 -19800 > # Fri Apr 07 13:45:33 2017 +0530 > # Node ID 6b32872e4896993efe0abcc13b6d9c4c9b8687b7 > # Parent f6d77af84ef3e936b15634759df2718d5363b78a > py3: use pycompat.byteskwargs() to convert opts to bytes Queued this, thanks. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] py3: add a bytes version of urllib.parse.urlencode() to pycompat.py
On Fri, 07 Apr 2017 16:29:40 +0530, Pulkit Goyal wrote: > # HG changeset patch > # User Pulkit Goyal <7895pul...@gmail.com> > # Date 1491561044 -19800 > # Fri Apr 07 16:00:44 2017 +0530 > # Node ID c3e934121b61d54184e914bda587281b4184eabf > # Parent beedb62e62a78a2f45ef8bd0b72c51f14619ee7f > py3: add a bytes version of urllib.parse.urlencode() to pycompat.py > > urllib.parse.urlencode() returns unicodes on Python 3. This commit adds a > method which will take its output and encode it to bytes so that we can use > bytes consistently. > > diff -r beedb62e62a7 -r c3e934121b61 mercurial/pycompat.py > --- a/mercurial/pycompat.py Fri Apr 07 15:58:04 2017 +0530 > +++ b/mercurial/pycompat.py Fri Apr 07 16:00:44 2017 +0530 > @@ -399,4 +399,12 @@ > s = urllib.parse.quote_from_bytes(s, safe=safe) > return s.encode('ascii', 'strict') > > +# urllib.parse.urlencode() returns str. We use this function to make > +# sure we return bytes. > +def urlencode(query, doseq=False, safe='', encoding=None, errors=None): Nit: better to not provide safe, encoding and errors arguments which don't exist in Python 3. Other than that, the patch looks good to me. > +s = urllib.parse.urlencode(query, doseq=doseq, safe=safe, > +encoding=encoding, errors=errors) > +return s.encode('ascii') ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 4] py3: replace str() with pycompat.bytestr()
On Fri, 07 Apr 2017 16:29:38 +0530, Pulkit Goyal wrote: > # HG changeset patch > # User Pulkit Goyal <7895pul...@gmail.com> > # Date 1491552995 -19800 > # Fri Apr 07 13:46:35 2017 +0530 > # Node ID c08552f3a4ab883aac255bc7d8efb7637ba245d8 > # Parent 6b32872e4896993efe0abcc13b6d9c4c9b8687b7 > py3: replace str() with pycompat.bytestr() > > diff -r 6b32872e4896 -r c08552f3a4ab mercurial/hg.py > --- a/mercurial/hg.py Fri Apr 07 13:45:33 2017 +0530 > +++ b/mercurial/hg.py Fri Apr 07 13:46:35 2017 +0530 > @@ -31,6 +31,7 @@ > merge as mergemod, > node, > phases, > +pycompat, > repoview, > scmutil, > sshpeer, > @@ -103,7 +104,7 @@ > if u.fragment: > branch = u.fragment > u.fragment = None > -return str(u), (branch, branches or []) > +return pycompat.bytestr(u), (branch, branches or []) Could be bytes(u) since util.url implements __bytes__(). ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2] test-check-code: do not use xargs
On Fri, 7 Apr 2017 09:27:49 +0100, Ryan McElroy wrote: > On 4/7/17 6:13 AM, Jun Wu wrote: > > # HG changeset patch > > # User Jun Wu > > # Date 1491541846 25200 > > # Thu Apr 06 22:10:46 2017 -0700 > > # Node ID 86847271e19bf2bb87d36db7d0418f1b5e52a4b3 > > # Parent 9a84025f33b33ba68a7c0bac763f9c756e1f81d0 > > test-check-code: do not use xargs > > This series looks good to me. Marked as pre-reviewed in patchwork. Queued, thanks. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 4 V2] run-tests: support per-line conditional output in tests
> On Apr 7, 2017, at 5:23 AM, Ryan McElroy wrote: > >> On 4/7/17 2:11 AM, Matt Harbison wrote: >> # HG changeset patch >> # User Matt Harbison >> # Date 1491448647 14400 >> # Wed Apr 05 23:17:27 2017 -0400 >> # Node ID 8e3cf915084723551d0b68094cede7e65f49cf39 >> # Parent 7c5e861f39fbffdfae0e31d1edba419a62391afe >> run-tests: support per-line conditional output in tests > > This series looks like a huge win for test de-duplication! Thanks for working > on it. > >> >> Duplicating entire tests just because the output is different is both error >> prone and can make the tests harder to read. This harnesses the existing >> '(?)' >> infrastructure, both to improve readability, and because it seemed like the >> path >> of least resistance. >> >> The form is: >> >> $ test_cmd >> output (hghave-feature !) # required if hghave.has_feature(), else optional >> out2 (no-hghave-feature2 !) # req if not hghave.has_feature2(), else >> optional > > My only question is: why is it optional if the feature is not present? It > seems like maybe we should have two things: > > ! = must be present iff feature > !? = must be present if feature > > (NOTE: that the first is two-f "iff" -- as in "if and only if" -- so if the > feature is not present, and the line is still there, that would be a failure. > The second one is the one-f "if", as in, "optional if not true".) It's an inherent property of the (?) infrastructure that was used, rather than a design goal. I have no problem with someone refining it like you propose. But I think that splitting will only prevent cruft from accumulating (which isn't a bad thing). I'm struggling to come up with a scenario where we want to fail the iff case other than because the condition is unnecessary. > Regardless, I think this could be done as a follow-up and I only if it's a > good idea. I've marked this series as pre-reviewed in patchwork. > >> >> I originally extended the '(?)' syntax. For example, this: >> >> 2 r4/.hg/cache/checkisexec (execbit ?) >> >> pretty naturally reads as "checkisexec, if execbit". In some ways though, >> this >> inverts the meaning of '?'. For '(?)', the line is purely optional. In the >> example, it is mandatory iff execbit. Otherwise, it is carried forward as >> optional, to preserve the test output. I tried it the other way, (listing >> 'no-exec' in the example), but that is too confusing to read. Kostia >> suggested >> using '!', and that seems fine. >> >> diff --git a/tests/run-tests.py b/tests/run-tests.py >> --- a/tests/run-tests.py >> +++ b/tests/run-tests.py >> @@ -497,6 +497,12 @@ >> # sans \t, \n and \r >> CDATA_EVIL = re.compile(br"[\000-\010\013\014\016-\037]") >> +# Match feature conditionalized output lines in the form, capturing the >> feature >> +# list in group 2, and the preceeding line output in group 1: >> +# >> +# output..output (feature !)\n >> +optline = re.compile(b'(.+) \((.+?) !\)\n$') >> + >> def cdatasafe(data): >> """Make a string safe to include in a CDATA block. >> @@ -1271,8 +1277,19 @@ >> if r: >> els.pop(i) >> break >> -if el and el.endswith(b" (?)\n"): >> -optional.append(i) >> +if el: >> +if el.endswith(b" (?)\n"): >> +optional.append(i) >> +else: >> +m = optline.match(el) >> +if m: >> +conditions = [c for c in m.group(2).split(' >> ')] >> + >> +if self._hghave(conditions)[0]: >> +lout = el >> +else: >> +optional.append(i) >> + >> i += 1 >>if r: >> @@ -1298,8 +1315,10 @@ >> # clean up any optional leftovers >> while expected.get(pos, None): >> el = expected[pos].pop(0) >> -if el and not el.endswith(b" (?)\n"): >> -break >> +if el: >> +if (not optline.match(el) >> +and not el.endswith(b" (?)\n")): >> +break >> postout.append(b' ' + el) >>if lcmd: >> @@ -1371,6 +1390,12 @@ >> if el.endswith(b" (?)\n"): >> retry = "retry" >> el = el[:-5] + b"\n" >> +else: >> +m = optline.match(el) >> +if m: >> +el = m.group(1) + b"\n" >> +retry = "retry" >> + >> if el.endswith(b" (esc)\n"): >> if PYTHON3: >> el = el[:-7].decode('unicode_escape') + '\n' >> diff --git a/tests/test-run-tests.t b/tests/test
Re: [PATCH 1 of 4 V2] obsolete: track node versions
On 04/05/2017 11:37 PM, Durham Goode wrote: I respond inline, but I'm also happy to drop discussion about evolve-like user experience changes and instead focus on the proposal about just making hidden commits a separate system. So we can discuss the various topics incrementally. I think most of the important point raised in this email are already covered (or open) in the more fresh thread. So I'm going to drop this with the hope to save everyones time. Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Hidden Commits in 4.3
On 04/06/2017 12:02 AM, Durham Goode wrote: On 4/5/17 4:06 AM, Pierre-Yves David wrote: […] The change proposed to evolution is problematic. As explain in my reply to the other thread[1], at minimum, removing the link between obsolescence and visibility is destroying the "global-state" property we currently have. This put in disarray the whole ability to use evolution to exchange and collaborate on mutable history, the very thing evolution has been built for. I've not seen strong-enough rationals for derailing the current evolution plan and design. The simpler implementation model, the simpler mental model, the simpler ability for commands to use hiding, and the lack of tie-in with other more divisive features is the reason for this proposal. With this proposal we can have the majority of the benefits quickly, without notable changes to the existing core UI, and in a intuitive form for users. I agree that having a local-only hiding mechanism would be a win. However obsolescence and the hiding computed from it must stay fully independent from it. Changeset-evolution relies on building a global state[1] mixing it with local-only elements breaks this property. I believe these benefits outweigh the downside to evolve's current goal, […] I think other members of the community would have to weigh in on whether this trade off is worth it, since it is a subjective decision. This is not a "small downside" to evolution goal. This is a full stop to our ability to ship changeset-evolution. The founding goal of changeset-evolution is to allow exchange and collaboration around mutable history[2]. The local benefit are "just" a nice side effect, but the concept is fully design around distribution. This is a problem no other DVCS solves and we have a plan, on track, to complete the concept and solve it. That will give Mercurial clear edge over others. The current proposal is destroying at least one pillard currently holding the changeset-evolution plan (global state), without replacement. As of today, this kills our ability to complete and ship changeset-evolution to all Mercurial user in the next year or two. This is not a small subjective adjustment, this is a full discontinuation of concept critical to the future of Mercurial. Cheers, -- Pierre-Yves David [1] https://www.mercurial-scm.org/wiki/CEDConcept#Global_State [2] https://www.mercurial-scm.org/wiki/CEDConcept#Changeset_Evolution_Goal ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 4] py3: use pycompat.byteskwargs() to convert opts to bytes
# HG changeset patch # User Pulkit Goyal <7895pul...@gmail.com> # Date 1491552933 -19800 # Fri Apr 07 13:45:33 2017 +0530 # Node ID 6b32872e4896993efe0abcc13b6d9c4c9b8687b7 # Parent f6d77af84ef3e936b15634759df2718d5363b78a py3: use pycompat.byteskwargs() to convert opts to bytes We have converted opts to unicodes before passing them. diff -r f6d77af84ef3 -r 6b32872e4896 mercurial/commands.py --- a/mercurial/commands.py Thu Apr 06 14:41:42 2017 +0200 +++ b/mercurial/commands.py Fri Apr 07 13:45:33 2017 +0530 @@ -2027,6 +2027,7 @@ Returns 0 on success. """ +opts = pycompat.byteskwargs(opts) changesets += tuple(opts.get('rev', [])) if not changesets: changesets = ['.'] @@ -3185,6 +3186,7 @@ Returns 0 if there are incoming changes, 1 otherwise. """ +opts = pycompat.byteskwargs(opts) if opts.get('graph'): cmdutil.checkunsupportedgraphflags([], opts) def display(other, chlist, displayer): ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 4] py3: replace str() with pycompat.bytestr()
# HG changeset patch # User Pulkit Goyal <7895pul...@gmail.com> # Date 1491552995 -19800 # Fri Apr 07 13:46:35 2017 +0530 # Node ID c08552f3a4ab883aac255bc7d8efb7637ba245d8 # Parent 6b32872e4896993efe0abcc13b6d9c4c9b8687b7 py3: replace str() with pycompat.bytestr() diff -r 6b32872e4896 -r c08552f3a4ab mercurial/hg.py --- a/mercurial/hg.py Fri Apr 07 13:45:33 2017 +0530 +++ b/mercurial/hg.py Fri Apr 07 13:46:35 2017 +0530 @@ -31,6 +31,7 @@ merge as mergemod, node, phases, +pycompat, repoview, scmutil, sshpeer, @@ -103,7 +104,7 @@ if u.fragment: branch = u.fragment u.fragment = None -return str(u), (branch, branches or []) +return pycompat.bytestr(u), (branch, branches or []) schemes = { 'bundle': bundlerepo, diff -r 6b32872e4896 -r c08552f3a4ab mercurial/util.py --- a/mercurial/util.py Fri Apr 07 13:45:33 2017 +0530 +++ b/mercurial/util.py Fri Apr 07 13:46:35 2017 +0530 @@ -2799,7 +2799,7 @@ user, passwd = self.user, self.passwd try: self.user, self.passwd = None, None -s = str(self) +s = pycompat.bytestr(self) finally: self.user, self.passwd = user, passwd if not self.user: @@ -2854,7 +2854,7 @@ u = url(u) if u.passwd: u.passwd = '***' -return str(u) +return pycompat.bytestr(u) def removeauth(u): '''remove all authentication information from a url string''' ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 4 of 4] py3: add a bytes version of urllib.parse.urlencode() to pycompat.py
# HG changeset patch # User Pulkit Goyal <7895pul...@gmail.com> # Date 1491561044 -19800 # Fri Apr 07 16:00:44 2017 +0530 # Node ID c3e934121b61d54184e914bda587281b4184eabf # Parent beedb62e62a78a2f45ef8bd0b72c51f14619ee7f py3: add a bytes version of urllib.parse.urlencode() to pycompat.py urllib.parse.urlencode() returns unicodes on Python 3. This commit adds a method which will take its output and encode it to bytes so that we can use bytes consistently. diff -r beedb62e62a7 -r c3e934121b61 mercurial/pycompat.py --- a/mercurial/pycompat.py Fri Apr 07 15:58:04 2017 +0530 +++ b/mercurial/pycompat.py Fri Apr 07 16:00:44 2017 +0530 @@ -399,4 +399,12 @@ s = urllib.parse.quote_from_bytes(s, safe=safe) return s.encode('ascii', 'strict') +# urllib.parse.urlencode() returns str. We use this function to make +# sure we return bytes. +def urlencode(query, doseq=False, safe='', encoding=None, errors=None): +s = urllib.parse.urlencode(query, doseq=doseq, safe=safe, +encoding=encoding, errors=errors) +return s.encode('ascii') + urlreq.quote = quote +urlreq.urlencode = urlencode ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 3 of 4] py3: alias unicode to str on Python 3
# HG changeset patch # User Pulkit Goyal <7895pul...@gmail.com> # Date 1491560884 -19800 # Fri Apr 07 15:58:04 2017 +0530 # Node ID beedb62e62a78a2f45ef8bd0b72c51f14619ee7f # Parent c08552f3a4ab883aac255bc7d8efb7637ba245d8 py3: alias unicode to str on Python 3 diff -r c08552f3a4ab -r beedb62e62a7 mercurial/scmutil.py --- a/mercurial/scmutil.py Fri Apr 07 13:46:35 2017 +0530 +++ b/mercurial/scmutil.py Fri Apr 07 15:58:04 2017 +0530 @@ -36,6 +36,9 @@ termsize = scmplatform.termsize +if pycompat.ispy3: +unicode = str + class status(tuple): '''Named tuple with a list of files per status. The 'deleted', 'unknown' and 'ignored' properties are only relevant to the working copy. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Hidden Commits in 4.3
On 04/06/2017 02:47 PM, Ryan McElroy wrote: > […] That's why I want to focus on a scalable hidden storage solution that everyone can use (including evolve), To clarify this point. They are no need to plug evolution to a new storage system, the current approach is scalable[1]. In addition, as explain in my reply to Jun[2] hiding from obs-markers is a static computation from a global-state[3] and we cannot let other mechanism affects it. So we should not mix its implementation with something user manipulable. Both hiding source can peacefully coexists, and I agree that local hiding is useful. So let us implement local hiding independently from obsolescence. [1] current implementation is slow, but adding a trivial "hasmarkers" cache will solve this. This will happens in the next month as side effect of my work on obsmarkers discovery. [2] https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-April/096360.html [3] https://www.mercurial-scm.org/wiki/CEDConcept#Global_State -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Hidden Commits in 4.3
On 04/06/2017 02:46 AM, Jun Wu wrote: obsstore-based hidden is broken by design, since it cannot unhide commits: $ hg export . > patch.txt $ hg prune . # could also be done by pulling obsmarkers from a remote # peer, or it could be amend / metaedit etc. $ hg import --exact --bypass patch.txt # cannot really import the patch The case you describe above work as intended. By using "hg prune ." you globally marks the changeset as an undesirable dead-end (obsolete-pruned)[1]. re-obtaining the same changesets in this repository or in another will not affect this global state. We are just adding global informations together: The changeset exists + a prune marker exists == this changeset is obsolete. This global state is a critical part of changeset evolution, and the visibility rules computed from obsolescence are fully parts of this state[2]. The uses can change their mind and bring the changes back to life. We currently have a basic command `hg touch` doing so (more useful friendly alternative are in project), To "bring" this changeset back to life, new information needs to be added. In addition, currently the changesets is "brought back" with a different hash. A new changeset node "NEW" and an obsolescence markers marking "NEW as a replacement for OLD" are created. This hash changes is necessary as the current implementation of evolution relies on "monotonic rewrite"[3] that ensure different actions will always creates a new nodes. These monotonic rewrites comes with many useful property to simplify the problems evolution as to solves[4]. The hash change is not the best thing from user experience perspective, but it practice the impact is very minor[5]. They -might- be ways to introduce new mechanism that would lift the need for monotonic rewrites while still solving the same problems. So far, such mechanism are unknown and concrete plan does not exists. The current roadmap focus on being able to ship evolve to all with monotonic rewrites. [1] https://www.mercurial-scm.org/wiki/CEDConcept#Obsolescence_markers_semantic [2] https://www.mercurial-scm.org/wiki/CEDConcept#Global_State [3] https://www.mercurial-scm.org/wiki/CEDConcept#Monotonic_rewrite [4] https://www.mercurial-scm.org/wiki/CEDConcept#Problem_this_Easily_Solve [5] https://www.mercurial-scm.org/wiki/CEDConcept#User_Impact The new ui requires the ability to unhide commits. So hidden-ness should not be affected by obsstore. That means extra work is needed to make sure commands like amend also write to the new hidden store if they write obsolete markers to hide commits today. I think most people agree on this. I just want to make it clear as Pierre-Yves does not mention this aspect in his long text. So far, I'm not seen a rational for hash-preservation been a -requirement- nor a group of people agreeing it is a -requirement-. Sure it would be nice, but "nice" is different from -required-. This is probably a core point we need to iron out in this discussion. On 04/05/2017 11:22 PM, Jun Wu wrote: […] There are some questions that I'm waiting for your answers: - Define "user intention" about adding a obsmarker formally [1] I've started formally writing some of the evolution concept. Does this section fits your need: * https://www.mercurial-scm.org/wiki/CEDConcept#obs-marker:_General_Principle - Why "an unified hidden store" does not work [2] Changeset evolution is based on a global state created by the obsolescence markers. The hiding from evolution is a computed property of this global state. It -cannot- be delegated to an independent, local- only, user manipulable hiding mechanism. Hiding from obsolescence markers is a "static" property that -must- remains independent from the local hiding mechanism (See previous [2] for details). Local-hiding has its own merit and would help various usecase. The two mechanism can perfectly exists side by side. On 04/05/2017 11:50 PM, Jun Wu wrote: I think it's unnecessary to have "internal changeset" concept separated from the general-purposed hidden. They are some case where having explicit internal changeset are useful: Case 1: looking at hidden changeset, When the user want to look or query the set of hidden changeset (eg: user remember he hided something he now needs). Commands will be run to see `hg log --hidden` or even select `hg log --hidden --rev 'hidden() and file('foo/bar') and date(-7)'`. Without an internal changesets, this command will display and select internal changesets that are irrelevant/scary to the user. Having explicit "internal" changesets allow to filter them out of such search from the user. Case 2: History of hiding I can expect user to request a command displaying the history of what they hide/archived[6]. They could use it to undo things. Without distinction between archived and real changesets, this history will list hiding and changesets from internal operation, sur
Re: [PATCH 3 of 4 V2] run-tests: support per-line conditional output in tests
On 4/7/17 2:11 AM, Matt Harbison wrote: # HG changeset patch # User Matt Harbison # Date 1491448647 14400 # Wed Apr 05 23:17:27 2017 -0400 # Node ID 8e3cf915084723551d0b68094cede7e65f49cf39 # Parent 7c5e861f39fbffdfae0e31d1edba419a62391afe run-tests: support per-line conditional output in tests This series looks like a huge win for test de-duplication! Thanks for working on it. Duplicating entire tests just because the output is different is both error prone and can make the tests harder to read. This harnesses the existing '(?)' infrastructure, both to improve readability, and because it seemed like the path of least resistance. The form is: $ test_cmd output (hghave-feature !) # required if hghave.has_feature(), else optional out2 (no-hghave-feature2 !) # req if not hghave.has_feature2(), else optional My only question is: why is it optional if the feature is not present? It seems like maybe we should have two things: ! = must be present iff feature !? = must be present if feature (NOTE: that the first is two-f "iff" -- as in "if and only if" -- so if the feature is not present, and the line is still there, that would be a failure. The second one is the one-f "if", as in, "optional if not true".) Regardless, I think this could be done as a follow-up and I only if it's a good idea. I've marked this series as pre-reviewed in patchwork. I originally extended the '(?)' syntax. For example, this: 2 r4/.hg/cache/checkisexec (execbit ?) pretty naturally reads as "checkisexec, if execbit". In some ways though, this inverts the meaning of '?'. For '(?)', the line is purely optional. In the example, it is mandatory iff execbit. Otherwise, it is carried forward as optional, to preserve the test output. I tried it the other way, (listing 'no-exec' in the example), but that is too confusing to read. Kostia suggested using '!', and that seems fine. diff --git a/tests/run-tests.py b/tests/run-tests.py --- a/tests/run-tests.py +++ b/tests/run-tests.py @@ -497,6 +497,12 @@ # sans \t, \n and \r CDATA_EVIL = re.compile(br"[\000-\010\013\014\016-\037]") +# Match feature conditionalized output lines in the form, capturing the feature +# list in group 2, and the preceeding line output in group 1: +# +# output..output (feature !)\n +optline = re.compile(b'(.+) \((.+?) !\)\n$') + def cdatasafe(data): """Make a string safe to include in a CDATA block. @@ -1271,8 +1277,19 @@ if r: els.pop(i) break -if el and el.endswith(b" (?)\n"): -optional.append(i) +if el: +if el.endswith(b" (?)\n"): +optional.append(i) +else: +m = optline.match(el) +if m: +conditions = [c for c in m.group(2).split(' ')] + +if self._hghave(conditions)[0]: +lout = el +else: +optional.append(i) + i += 1 if r: @@ -1298,8 +1315,10 @@ # clean up any optional leftovers while expected.get(pos, None): el = expected[pos].pop(0) -if el and not el.endswith(b" (?)\n"): -break +if el: +if (not optline.match(el) +and not el.endswith(b" (?)\n")): +break postout.append(b' ' + el) if lcmd: @@ -1371,6 +1390,12 @@ if el.endswith(b" (?)\n"): retry = "retry" el = el[:-5] + b"\n" +else: +m = optline.match(el) +if m: +el = m.group(1) + b"\n" +retry = "retry" + if el.endswith(b" (esc)\n"): if PYTHON3: el = el[:-7].decode('unicode_escape') + '\n' 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 @@ -39,6 +39,19 @@ $ rm hg #endif +Features for testing optional lines +=== + + $ cat > hghaveaddon.py < import hghave + > @hghave.check("custom", "custom hghave feature") + > def has_custom(): + > return True + > @hghave.check("missing", "missing hghave feature") + > def has_missing(): + > return False + > EOF + an empty test === @@ -67,6 +80,13 @@ > def (?) > 456 (?) > xyz + > $ printf 'zyx\nwvu\ntsr\n' + > abc (?) + > zyx (custom !) + > wvu + > no_print (no-custom !) + > tsr (no-missing !) + > missing (missing !) > EOF
[PATCH 1 of 2] hgweb: add a floating tooltip to invite on followlines action
# HG changeset patch # User Denis Laxalde # Date 1491498909 -7200 # Thu Apr 06 19:15:09 2017 +0200 # Node ID e8cc0233064b9aaf4e99efa960d857c9884266c4 # Parent e0dc40530c5aa514feb6a09cf79ab6a3aa2ec331 # Available At http://hg.logilab.org/users/dlaxalde/hg # hg pull http://hg.logilab.org/users/dlaxalde/hg -r e8cc0233064b hgweb: add a floating tooltip to invite on followlines action In followlines.js, we create a element to draw attention of users on "followlines" feature. The element shows up on hover of source lines after one second and follows the cursor. After first click (start line selection), the text changes and indicates that next click will terminate selection. diff --git a/mercurial/templates/static/followlines.js b/mercurial/templates/static/followlines.js --- a/mercurial/templates/static/followlines.js +++ b/mercurial/templates/static/followlines.js @@ -17,6 +17,34 @@ document.addEventListener('DOMContentLoa return; } +// tooltip to invite on lines selection +var tooltip = document.createElement('div'); +tooltip.id = 'followlines-tooltip'; +tooltip.classList.add('hidden'); +var initTooltipText = 'click to start following lines history from here'; +tooltip.textContent = initTooltipText; +sourcelines.appendChild(tooltip); + +var tooltipTimeoutID; +//* move the "tooltip" with cursor (top-right) and show it after 1s */ +function moveAndShowTooltip(e) { +if (typeof tooltipTimeoutID !== 'undefined') { +// avoid accumulation of timeout callbacks (blinking) +window.clearTimeout(tooltipTimeoutID); +} +tooltip.classList.add('hidden'); +var x = (e.clientX + 10) + 'px', +y = (e.clientY - 20) + 'px'; +tooltip.style.top = y; +tooltip.style.left = x; +tooltipTimeoutID = window.setTimeout(function() { +tooltip.classList.remove('hidden'); +}, 1000); +} + +// on mousemove, show tooltip close to cursor position +sourcelines.addEventListener('mousemove', moveAndShowTooltip); + // retrieve all direct children of var spans = Array.prototype.filter.call( sourcelines.children, @@ -65,6 +93,10 @@ document.addEventListener('DOMContentLoa // registered for other click with target return; } + +// update tooltip text +tooltip.textContent = 'click again to terminate line block selection here'; + var startId = parseInt(startElement.id.slice(1)); startElement.classList.add(lineSelectedCSSClass); // CSS @@ -83,13 +115,25 @@ document.addEventListener('DOMContentLoa // remove this event listener sourcelines.removeEventListener('click', lineSelectEnd); +// hide tooltip and disable motion tracking +tooltip.classList.add('hidden'); +sourcelines.removeEventListener('mousemove', moveAndShowTooltip); +window.clearTimeout(tooltipTimeoutID); + +//* restore initial "tooltip" state */ +function restoreTooltip() { +tooltip.textContent = initTooltipText; +sourcelines.addEventListener('mousemove', moveAndShowTooltip); +} + // compute line range (startId, endId) var endId = parseInt(endElement.id.slice(1)); if (endId == startId) { // clicked twice the same line, cancel and reset initial state -// (CSS and event listener for selection start) +// (CSS, event listener for selection start, tooltip) removeSelectedCSSClass(); sourcelines.addEventListener('click', lineSelectStart); +restoreTooltip(); return; } var inviteElement = endElement; @@ -118,6 +162,8 @@ document.addEventListener('DOMContentLoa sourcelines.removeEventListener('click', cancel); // remove styles on selected lines removeSelectedCSSClass(); +// restore tooltip element +restoreTooltip(); } // bind cancel event to click on diff --git a/mercurial/templates/static/style-paper.css b/mercurial/templates/static/style-paper.css --- a/mercurial/templates/static/style-paper.css +++ b/mercurial/templates/static/style-paper.css @@ -320,6 +320,22 @@ div.followlines-link { font-family: sans-serif; } +div#followlines-tooltip { + display: none; + position: fixed; + background-color: #ffc; + border: 1px solid #999; + padding: 2px; +} + +.sourcelines:hover > div#followlines-tooltip { + display: inline; +} + +.sourcelines:hover > div#followlines-tooltip.hidden { + display: none; +} + .sourcelines > a { display: inline-block; position: absolute; ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org h
[PATCH 2 of 2] hgweb: position the "followlines" box close to latest cursor position
# HG changeset patch # User Denis Laxalde # Date 1491499444 -7200 # Thu Apr 06 19:24:04 2017 +0200 # Node ID 59f99f6267b84e06589334b1057a98e8051fb562 # Parent e8cc0233064b9aaf4e99efa960d857c9884266c4 # Available At http://hg.logilab.org/users/dlaxalde/hg # hg pull http://hg.logilab.org/users/dlaxalde/hg -r 59f99f6267b8 hgweb: position the "followlines" box close to latest cursor position diff --git a/mercurial/templates/static/followlines.js b/mercurial/templates/static/followlines.js --- a/mercurial/templates/static/followlines.js +++ b/mercurial/templates/static/followlines.js @@ -25,6 +25,14 @@ document.addEventListener('DOMContentLoa tooltip.textContent = initTooltipText; sourcelines.appendChild(tooltip); +//* position "element" on top-right of cursor */ +function positionTopRight(element, event) { +var x = (event.clientX + 10) + 'px', +y = (event.clientY - 20) + 'px'; +element.style.top = y; +element.style.left = x; +} + var tooltipTimeoutID; //* move the "tooltip" with cursor (top-right) and show it after 1s */ function moveAndShowTooltip(e) { @@ -33,10 +41,7 @@ document.addEventListener('DOMContentLoa window.clearTimeout(tooltipTimeoutID); } tooltip.classList.add('hidden'); -var x = (e.clientX + 10) + 'px', -y = (e.clientY - 20) + 'px'; -tooltip.style.top = y; -tooltip.style.left = x; +positionTopRight(tooltip, e); tooltipTimeoutID = window.setTimeout(function() { tooltip.classList.remove('hidden'); }, 1000); @@ -152,6 +157,8 @@ document.addEventListener('DOMContentLoa var div = divAndButton[0], button = divAndButton[1]; inviteElement.appendChild(div); +// set position close to cursor (top-right) +positionTopRight(div, e); //** event handler for cancelling selection */ function cancel() { diff --git a/mercurial/templates/static/style-paper.css b/mercurial/templates/static/style-paper.css --- a/mercurial/templates/static/style-paper.css +++ b/mercurial/templates/static/style-paper.css @@ -293,7 +293,7 @@ div#followlines { border: 1px solid #CCC; border-radius: 5px; padding: 4px; - position: absolute; + position: fixed; } div.followlines-cancel { ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4] templater: make _hybrid provide more list/dict-like methods
On 4/4/17 5:00 PM, Yuya Nishihara wrote: # HG changeset patch # User Yuya Nishihara # Date 1491312719 -32400 # Tue Apr 04 22:31:59 2017 +0900 # Node ID 236441d523b03a227a0562497cbd060f795d3e13 # Parent a78c9de39862cd089aaddd9536902c7c52dfe1e6 templater: make _hybrid provide more list/dict-like methods Sorry for the slow review. I've looked over this series and it looks like a good improvement to me. Being able to filter things like extra or files through json is a big win in my opinion! I've marked as pre-reviewed in patchwork. So the JSON filter works. diff --git a/mercurial/templatekw.py b/mercurial/templatekw.py --- a/mercurial/templatekw.py +++ b/mercurial/templatekw.py @@ -28,6 +28,7 @@ class _hybrid(object): and to access raw values: - "{ifcontains(file, files, ...)}", "{ifcontains(key, extras, ...)}" - "{get(extras, key)}" +- "{files|json}" """ def __init__(self, gen, values, makemap, joinfmt): @@ -43,8 +44,11 @@ class _hybrid(object): return x in self._values def __len__(self): return len(self._values) +def __iter__(self): +return iter(self._values) def __getattr__(self, name): -if name != 'get': +if name not in ('get', 'items', 'iteritems', 'iterkeys', 'itervalues', +'keys', 'values'): raise AttributeError(name) return getattr(self._values, name) diff --git a/tests/test-command-template.t b/tests/test-command-template.t --- a/tests/test-command-template.t +++ b/tests/test-command-template.t @@ -3392,6 +3392,13 @@ Test get function: hg: parse error: get() expects a dict as first argument [255] +Test json filter applied to hybrid object: + + $ hg log -r0 -T '{files|json}\n' + ["a"] + $ hg log -r0 -T '{extras|json}\n' + {"branch": "default"} + Test localdate(date, tz) function: $ TZ=JST-09 hg log -r0 -T '{date|localdate|isodate}\n' ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2] test-check-code: do not use xargs
On 4/7/17 6:13 AM, Jun Wu wrote: # HG changeset patch # User Jun Wu # Date 1491541846 25200 # Thu Apr 06 22:10:46 2017 -0700 # Node ID 86847271e19bf2bb87d36db7d0418f1b5e52a4b3 # Parent 9a84025f33b33ba68a7c0bac763f9c756e1f81d0 test-check-code: do not use xargs This series looks good to me. Marked as pre-reviewed in patchwork. We have too many files, and passing them via arguments could cause strange errors on some platforms [1]. Since check-code.py can now take "-" and read file names from stdin, use it instead of xargs to avoid the argv size limit. Isn't xargs supposed to be aware of the argv limit and split up calls appropriately? Seems like a bug on MinGW's xargs if it's not working there. Regardless, the fix seems good. [1]: https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-April/096346.html diff --git a/tests/test-check-code.t b/tests/test-check-code.t --- a/tests/test-check-code.t +++ b/tests/test-check-code.t @@ -9,5 +9,5 @@ New errors are not allowed. Warnings are $ hg locate -X contrib/python-zstandard -X hgext/fsmonitor/pywatchman | - > sed 's-\\-/-g' | xargs "$check_code" --warnings --per-file=0 || false + > sed 's-\\-/-g' | "$check_code" --warnings --per-file=0 - || false contrib/perf.py:859: > r.revision(r.node(x)) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel