[Bug 5530] New: Allow bisect to accept a filtering revset

2017-04-07 Thread mercurial-bugs
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)

2017-04-07 Thread Jun Wu
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)

2017-04-07 Thread Matt Harbison

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)"

2017-04-07 Thread Gregory Szorc
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 

[Bug 5529] New: Tests crash python on some versions of OS X

2017-04-07 Thread mercurial-bugs
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=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

2017-04-07 Thread Pierre-Yves David



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.
+  > 

Re: [PATCH 2 of 3 V2] bundle2: use bundleerrorparts for error parts with unbounded parameters

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Jun Wu
# 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

2017-04-07 Thread Jun Wu
# 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

2017-04-07 Thread Siddharth Agarwal

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 

Re: [PATCH 1 of 3 V2] bundle2: add separate handling for error part creation

2017-04-07 Thread Siddharth Agarwal

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

2017-04-07 Thread Siddharth Agarwal

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

2017-04-07 Thread Gregory Szorc
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

2017-04-07 Thread Gregory Szorc
# 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

2017-04-07 Thread Gregory Szorc
# 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

2017-04-07 Thread Gregory Szorc
# 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

2017-04-07 Thread Gregory Szorc
# 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

2017-04-07 Thread Pulkit Goyal
# 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

2017-04-07 Thread Pulkit Goyal
# 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()

2017-04-07 Thread Pulkit Goyal
# 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()"

2017-04-07 Thread Jun Wu
# 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

2017-04-07 Thread Jun Wu
# 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

2017-04-07 Thread Pulkit Goyal
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

2017-04-07 Thread Pierre-Yves David

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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Olivier Trempe
# 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

2017-04-07 Thread Yuya Nishihara
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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Pierre-Yves David

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

2017-04-07 Thread Peter Kronenberg
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

2017-04-07 Thread Peter Kronenberg
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

2017-04-07 Thread Jun Wu
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

2017-04-07 Thread Kostia Balytskyi
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=DwICaQ=5VD0RTtNlTh3ycd41
> b3MUw=Pp-gQYFgs4tKlSFPF5kfCw=sQIiGyuq_0IQmaF9PP8F0DCU2b0lP5av_lDck
> g3idtk=BSi9_RvTDzP8V3KckQYSrbpXpL2g6A85sCIcc2dID4w=

--
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

2017-04-07 Thread Kostia Balytskyi
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

2017-04-07 Thread Jun Wu
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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Jun Wu
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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Jun Wu
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

2017-04-07 Thread Ryan McElroy

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

2017-04-07 Thread Ryan McElroy

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

2017-04-07 Thread Yuya Nishihara
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

2017-04-07 Thread Yuya Nishihara
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

2017-04-07 Thread Yuya Nishihara
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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Jun Wu
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

2017-04-07 Thread Kostia Balytskyi
# 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

2017-04-07 Thread Kostia Balytskyi
# 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

2017-04-07 Thread Ryan McElroy



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

2017-04-07 Thread Ryan McElroy

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

2017-04-07 Thread Augie Fackler
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

2017-04-07 Thread Ryan McElroy
# 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)

2017-04-07 Thread Ryan McElroy
# 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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Yuya Nishihara
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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Yuya Nishihara
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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread mercurial-bugs
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

2017-04-07 Thread Yuya Nishihara
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

2017-04-07 Thread Yuya Nishihara
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

2017-04-07 Thread Yuya Nishihara
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

2017-04-07 Thread Yuya Nishihara
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

2017-04-07 Thread Yuya Nishihara
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 2] test-check-code: do not use xargs

2017-04-07 Thread Yuya Nishihara
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 2 of 4] py3: replace str() with pycompat.bytestr()

2017-04-07 Thread Yuya Nishihara
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 3 of 4 V2] run-tests: support per-line conditional output in tests

2017-04-07 Thread Matt Harbison

> 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 

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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Pierre-Yves David

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

2017-04-07 Thread Pulkit Goyal
# 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()

2017-04-07 Thread Pulkit Goyal
# 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

2017-04-07 Thread Pulkit Goyal
# 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

2017-04-07 Thread Pulkit Goyal
# 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

2017-04-07 Thread Pierre-Yves David



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

2017-04-07 Thread Pierre-Yves David

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, 

Re: [PATCH 3 of 4 V2] run-tests: support per-line conditional output in tests

2017-04-07 Thread Ryan McElroy

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 

[PATCH 2 of 2] hgweb: position the "followlines" box close to latest cursor position

2017-04-07 Thread Denis Laxalde
# 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 2 of 2] test-check-code: do not use xargs

2017-04-07 Thread Ryan McElroy

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