[PATCH 8 of 8 py3] py3: stop exporting urlparse from pycompat and util (API)

[Gregory Szorc]

# HG changeset patch
# User Gregory Szorc 
# Date 1490161669 25200
#  Tue Mar 21 22:47:49 2017 -0700
# Node ID 30c6ed61a0921cc1fddd2c99a1db519f70934a24
# Parent  ec6ce29cad3eb47053b3f3296fb81bc0532ab9d9
py3: stop exporting urlparse from pycompat and util (API)

There are no consumers of this in tree.

Functions formerly available on this object/module can now be accessed
via {pycompat,util}.urlreq.

diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
--- a/mercurial/pycompat.py
+++ b/mercurial/pycompat.py
@@ -22,14 +22,12 @@ if not ispy3:
 import httplib
 import Queue as _queue
 import SocketServer as socketserver
-import urlparse
 import xmlrpclib
 else:
 import http.client as httplib
 import pickle
 import queue as _queue
 import socketserver
-import urllib.parse as urlparse
 import xmlrpc.client as xmlrpclib
 
 if ispy3:
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -60,7 +60,6 @@ stdin = pycompat.stdin
 stdout = pycompat.stdout
 stringio = pycompat.stringio
 urlerr = pycompat.urlerr
-urlparse = pycompat.urlparse
 urlreq = pycompat.urlreq
 xmlrpclib = pycompat.xmlrpclib
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 7 of 8 py3] check-code: recommend util.urlreq when importing urlparse

[Gregory Szorc]

# HG changeset patch
# User Gregory Szorc 
# Date 1490161577 25200
#  Tue Mar 21 22:46:17 2017 -0700
# Node ID ec6ce29cad3eb47053b3f3296fb81bc0532ab9d9
# Parent  13b41c9e0b911c3dc1dfe8ceb319fb28391d31ca
check-code: recommend util.urlreq when importing urlparse

diff --git a/contrib/check-code.py b/contrib/check-code.py
--- a/contrib/check-code.py
+++ b/contrib/check-code.py
@@ -330,7 +330,7 @@ pypats = [
 (r'^import cStringIO', "don't use cStringIO.StringIO, use util.stringio"),
 (r'^import urllib', "don't use urllib, use util.urlreq/util.urlerr"),
 (r'^import SocketServer', "don't use SockerServer, use util.socketserver"),
-(r'^import urlparse', "don't use urlparse, use util.urlparse"),
+(r'^import urlparse', "don't use urlparse, use util.urlreq"),
 (r'^import xmlrpclib', "don't use xmlrpclib, use util.xmlrpclib"),
 (r'^import cPickle', "don't use cPickle, use util.pickle"),
 (r'^import pickle', "don't use pickle, use util.pickle"),
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 1 of 8 py3] pycompat: alias urlreq.unquote to unquote_to_bytes

[Gregory Szorc]

# HG changeset patch
# User Gregory Szorc 
# Date 1490160011 25200
#  Tue Mar 21 22:20:11 2017 -0700
# Node ID 285f48d5644ea070f717473af077e6728df6ea82
# Parent  102f291807c92864a2231e5e925d6cd64783bb59
pycompat: alias urlreq.unquote to unquote_to_bytes

Previously, urlreq.unquote aliased to urllib.parse.unquote,
which returned a str/unicode. We like bytes, so switch urlreq.unquote
to dispatch to urllib.parse.unquote_to_bytes.

This required a minor helper function to register an alias under a
different name from which it points. If this turns into a common
pattern, we could likely teach _registeralias to accept tuple
values defining the mapping. Until then, I didn't feel like
adding complexity to _registeralias.

diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
--- a/mercurial/pycompat.py
+++ b/mercurial/pycompat.py
@@ -268,6 +268,10 @@ class _pycompatstub(object):
 (item.replace(sysstr('_'), sysstr('')).lower(), (origin, item))
 for item in items)
 
+def _registeralias(self, origin, attr, name):
+"""Alias ``origin``.``attr`` as ``name``"""
+self._aliases[sysstr(name)] = (origin, sysstr(attr))
+
 def __getattr__(self, name):
 try:
 origin, item = self._aliases[name]
@@ -337,8 +341,8 @@ else:
 "splitpasswd",
 "splitport",
 "splituser",
-"unquote",
 ))
+urlreq._registeralias(urllib.parse, "unquote_to_bytes", "unquote")
 import urllib.request
 urlreq._registeraliases(urllib.request, (
 "AbstractHTTPHandler",
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 4 of 8 py3] pycompat: define urlreq.urlparse and urlreq.unparse aliases

[Gregory Szorc]

# HG changeset patch
# User Gregory Szorc 
# Date 1490160857 25200
#  Tue Mar 21 22:34:17 2017 -0700
# Node ID 0082c745f5a97449d8a0f9e3bf00db1ebdacedde
# Parent  242fec51a193b396f1783383d830d97babad9208
pycompat: define urlreq.urlparse and urlreq.unparse aliases

Currently, we export urlparse via util.urlparse then
call util.urlparse.urlparse() and util.urlparse.urlunparse()
in a few places. This is the only url* module exported from
pycompat, making it a one-off. So let's transition to urlreq
to match everything else.

Yes, we double import "urlparse" now on Python 2. This will
be cleaned up in a subsequent patch.

Also, the Python 3 functions trade in str/unicode not bytes.
So we'll likely need to write a custom implementation that
speaks bytes. But moving everyone to an abstracted API
is a good first step.

diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
--- a/mercurial/pycompat.py
+++ b/mercurial/pycompat.py
@@ -287,6 +287,7 @@ if not ispy3:
 import SimpleHTTPServer
 import urllib2
 import urllib
+import urlparse
 urlreq._registeraliases(urllib, (
 "addclosehook",
 "addinfourl",
@@ -317,6 +318,10 @@ if not ispy3:
 "Request",
 "urlopen",
 ))
+urlreq._registeraliases(urlparse, (
+"urlparse",
+"urlunparse",
+))
 urlerr._registeraliases(urllib2, (
 "HTTPError",
 "URLError",
@@ -339,6 +344,8 @@ else:
 "splitpasswd",
 "splitport",
 "splituser",
+"urlparse",
+"urlunparse",
 ))
 urlreq._registeralias(urllib.parse, "unquote_to_bytes", "unquote")
 import urllib.request
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 3 of 8 py3] pycompat: remove urlunquote alias

[Gregory Szorc]

# HG changeset patch
# User Gregory Szorc 
# Date 1490160496 25200
#  Tue Mar 21 22:28:16 2017 -0700
# Node ID 242fec51a193b396f1783383d830d97babad9208
# Parent  2a95cd875bfaaec90197a28d33e112ee675066e3
pycompat: remove urlunquote alias

It is duplicated by urlreq.unquote and is unused. Kill it.

We retain the imports because it is re-exported via util.urlparse,
which is used elsewhere.

Since we no longer access attributes of urlparse at module load time,
this change /should/ result in that module reverting to a lazy module.

diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
--- a/mercurial/pycompat.py
+++ b/mercurial/pycompat.py
@@ -23,7 +23,6 @@ if not ispy3:
 import Queue as _queue
 import SocketServer as socketserver
 import urlparse
-urlunquote = urlparse.unquote
 import xmlrpclib
 else:
 import http.client as httplib
@@ -31,7 +30,6 @@ else:
 import queue as _queue
 import socketserver
 import urllib.parse as urlparse
-urlunquote = urlparse.unquote_to_bytes
 import xmlrpc.client as xmlrpclib
 
 if ispy3:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 2 of 2] rebase: ignore commands.rebase.requiredest if HGPLAIN=1

[Martin von Zweigbergk via Mercurial-devel]

# HG changeset patch
# User Martin von Zweigbergk 
# Date 1490156812 25200
#  Tue Mar 21 21:26:52 2017 -0700
# Node ID 23f767fb3142615597c4d3c2d5b4404e6c10f57a
# Parent  2558f3d814f50681641fff9815d30129de2ab5ad
rebase: ignore commands.rebase.requiredest if HGPLAIN=1

diff -r 2558f3d814f5 -r 23f767fb3142 hgext/rebase.py
--- a/hgext/rebase.py   Tue Mar 21 21:22:00 2017 -0700
+++ b/hgext/rebase.py   Tue Mar 21 21:26:52 2017 -0700
@@ -686,7 +686,8 @@
 # Validate input and define rebasing points
 destf = opts.get('dest', None)
 
-if ui.config('commands', 'rebase.requiredest', False):
+if (not ui.plain() and
+ui.config('commands', 'rebase.requiredest', False)):
 if not destf:
 raise error.Abort(_('you must specify a destination'),
   hint=_('use: hg rebase -d REV'))
diff -r 2558f3d814f5 -r 23f767fb3142 tests/test-rebase-base.t
--- a/tests/test-rebase-base.t  Tue Mar 21 21:22:00 2017 -0700
+++ b/tests/test-rebase-base.t  Tue Mar 21 21:26:52 2017 -0700
@@ -413,3 +413,7 @@
   $ hg rebase -d 1
   rebasing 2:5db65b93a12b "cc" (tip)
   saved backup bundle to 
$TESTTMP/repo/.hg/strip-backup/5db65b93a12b-4fb789ec-backup.hg (glob)
+  $ hg rebase -d 0 -r . -q
+  $ HGPLAIN=1 hg rebase
+  rebasing 2:889b0bc6a730 "cc" (tip)
+  saved backup bundle to 
$TESTTMP/repo/.hg/strip-backup/889b0bc6a730-41ec4f81-backup.hg (glob)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 4 of 4 V2] obsolete: allow cycles

[Jun Wu]

Excerpts from Gregory Szorc's message of 2017-03-21 19:32:06 -0700:
> On Tue, Mar 21, 2017 at 3:15 PM, Jun Wu  wrote:
> 
> > Excerpts from Augie Fackler's message of 2017-03-21 18:03:33 -0400:
> > > > As long as exchange sends dates, markers are globally ordered, and they
> > > > behave no different then the local case.
> > > >
> > > > Say Alice has A -> B created on date1, Bob has B -> A with date2. If
> > date2 >
> > > > date1, Bob wins. The time of pulling or the pull direction does not
> > matter.
> > > > Only the global version (date) matters. If date2 == date1, two markers
> > > > "cancel out" and both A and B become visible.  Previously no matter
> > what the
> > > > dates are, A and B are both permanently invisible.
> > >
> > > I do worry a little about someone with a bogus clock years in the
> > > future pumping out malicious nodes that can't be overridden, but I
> > > suppose in that case your "way out" is that you perturb the hash of
> > > the revision(s) you want to keep. Seem good enough?
> >
> > With a bogus clock, power users could also write arbitrary markers with a
> > newer date to solve the issue.
> >
> 
> Hold up.
> 
> Wall clock times cannot be trusted. This is like a cardinal rule of writing
> software in the real world. Yet this patch appears to not only rely on
> local wall clock times being ordered but also relies on wall clocks from
> different machines being ordered. That's a double no-no. Sorting by wall
> clocks will lead to unexpected behavior which in the eyes of an end-user is
> a bug.
> 
> Sorting by wall clocks is hardly ever a good idea. I have serious
> reservations about using wall times to determine obsolescence marker
> precedence.

The date idea was suggested by marmoute at the sprint. I was thinking about
the physical offsets of markers in "obsstore" before.

Using "date" for sorting is better than using physical offsets, because date
is stored in markers and is a global thing, while offsets are local, which
means you need to have some special non-trivial logic to deal with exchange.
This reason alone is enough to convince me to switch from offsets.

I'd note that for some systems like Zookeeper, clocks must be treated *very*
seriously. But for the obsstore, clocks are not that important. Because the
worst case is some visibility issue of commits. No data loss. No repo
corruption.  No consistency issue. And could be fixed by power commands.

Not to mention most systems have sane clocks, and most markers are date
unrelated - only those cycles matter - cycles are uncommon.

I think dsop summaried this up well:

  Mar 13 10:57:26  junw: so basically it boils down to: using date is
  not perefct, it makes the solution easy and elegant and if clocks on
  computers are wrong, the user might have a non-optimal user experience,
  but we never loose data

I've been thinking about the cycle problem for a long time and don't think
there is a better solution practically. The current approach (tens of lines)
is probably the most elegant thing I've ever contributed to the list. You're
encouraged to suggest new ideas. But if the new idea is like some fancy
format change plus some fancy conflict resolution during exchange, which
sounds like thousands of lines, I think it's reasonable to say no-no to it.

> >
> > The whole point of the series is to make it possible to reuse hashes.
> > That's
> > required for a nature user-experience of "hg unamend", "hg unstrip",
> > "hg unabsorb" and maybe a general purposed "hg undo", etc.
> >
> > Yes, the undo reusing hash behavior may be solved in some other way, like
> > striping the obsstore, or "inhibit". But I believe the obscycle is the most
> > elegant, bug-free way. And it deals with exchange just well.
> > ___
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel 
> >
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 2 of 2 v2] rebase: add flag to require destination

[Gregory Szorc]

On Mon, Mar 20, 2017 at 8:17 AM, Ryan McElroy  wrote:

> Any objections here?
>

No. I agree with your approach here. Implement the granular knobs first.
Then build the "multiknobs" later.



>
> On 3/16/17 1:55 AM, Ryan McElroy wrote:
>
>>
>>
>> On 3/14/17 8:26 PM, David Soria Parra wrote:
>>
>>> On Tue, Mar 14, 2017 at 05:56:16PM -0700, Ryan McElroy wrote:
>>>
 # HG changeset patch
 # User Ryan McElroy 
 # Date 1489538624 25200
 #  Tue Mar 14 17:43:44 2017 -0700
 # Node ID 8d8b783803f43d5e2d86916c39e9554139752fe6
 # Parent  2dc26c57e60e7e7bf46a276e8a498a9746bd9271
 rebase: add flag to require destination

 This looks good to me. I was wondering if we want to provide separate
>>> knobs for
>>> these commands which might lead to config overhead or provide more
>>> comprehensive
>>> "ui" improvement knobs such as "commands.requiredest" to move people to
>>> a better
>>> model in logical steps.
>>>
>>> e.g. I am a user who likes a slightly enhanced user experience.
>>> ui.compat= is a
>>> bit too much for me, but update destinations is a good idea. Do i have
>>> to find
>>> all places where we use destinations to update or do I want to select a
>>> logical
>>> step?
>>>
>>> I personally think while fine granualar steps are nice, I'd probably lean
>>> towards logical steps as it provides a more consistent behavior for
>>> users (e.g.
>>> assume an extension Y that we don't know of can opt into using
>>> "commands.requiredest", which at the moment it cannot unless it depends
>>> on
>>> "commands.update.requiredest" which is missleading.
>>>
>>
>> I'm not against this direction, but I think what I have proposed here is
>> stillt he first right step. Once we have a bunch of granular knobs like
>> these ones, we can then work towards "multiknobs" when we have the config
>> registry concept to tie options together more, and then the compatibility
>> levels are just the biggest "multiknobs".
>>
>>
>> ___
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>>
>
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] [RFC] dispatch: setup color before pager

[Gregory Szorc]

On Tue, Mar 21, 2017 at 6:11 PM, Matt Harbison 
wrote:

> (+CC: indygreg, since he added this functionality in 4e02418b4236)
>
> On Tue, 21 Mar 2017 17:36:29 -0400, Augie Fackler  wrote:
>
> On Mon, Mar 20, 2017 at 12:53:19AM -0400, Matt Harbison wrote:
>>
>>> # HG changeset patch
>>> # User Matt Harbison 
>>> # Date 1489985511 14400
>>> #  Mon Mar 20 00:51:51 2017 -0400
>>> # Node ID 2a323109f8e7ec5548dd506c2cff05f27fa20c1e
>>> # Parent  6e72bb689e57c985b138c580acb4cad8053dc180
>>> [RFC] dispatch: setup color before pager
>>>
>>
>> Oy, these two I just dunno. I'll defer to other Windows experts.
>>
>
> Heh. You have no idea how sad my Sunday was :-/
>
> Just to be clear, these last two can't be taken as-is.  The output on
> Windows without a pager is black text on a green background.
>
> I did a little more playing, and if we don't do something, this will be a
> regression.  In 4.1, both of these commands colorize in MSYS:
>
> [extensions]
> color =
>
> [pager]
> pager = less
>
> $ hg log -l 5 --config extensions.pager= --config color.pagermode=ansi
>
> $ hg log -l 5 --config color.pagermode=ansi
>
>
> Now, neither of them do (both are now paged).  You have to set
> '--pager=no' to get the color back, or add '--config color.mode=ansi'.  But
> the latter breaks the output when disabling pager.  And even if that
> magically worked, that doesn't help with switching between MSYS and
> cmd.exe, which is why I'd love to make 'auto' smarter.  (I'd rather not
> resort to %include hacks).
>

So the whole reason behind 4e02418b4236 (as documented in the commit
message) is that different pagers support different escape sequences. At
the time, I couldn't figure out an elegant way to detect the pager's
capabilities at run time. So, I added a config option that could be defined
in certain environments (like a controlled msys environment where `less`
was installed as the pager such as the one that Mozilla uses to build
Firefox) so that pager+color would "just work" in both cmd.exe and an msys
environment. More info at
https://bugzilla.mozilla.org/show_bug.cgi?id=545432#c59 and below (search
for my name).

Core Mercurial should focus on behavior in a stock cmd.exe environment and
be best-effort - but not broken - for cygwin, msys, etc. This is what
a7d98606 aimed to achieve. If someone is running a non-cmd.exe
terminal, I think it is up to the packager to enable a reasonable default
(such as Mozilla setting color.pagermode in our MozillaBuild msys
environment).

Also, if you want a new challenge, the Windows console apparently supports
24-bit color (
https://blogs.msdn.microsoft.com/commandline/2016/09/22/24-bit-color-in-the-windows-console/)
and ANSI+VT-100 escape sequences now.
https://github.com/symfony/symfony/pull/18385 and
https://github.com/Microsoft/BashOnWindows/issues/1173 are somewhat
informative. We could likely detect the Windows version and active terminal
to call SetConsoleMode with ENABLE_VIRTUAL_TERMINAL_PROCESSING. Then
pager.color=ansi should "just work" on Windows.


>
> IIRC, the inter-operation between these two extensions was why the ability
> to register an afterload callback was added.  I'll keep looking at this,
> but I suspect we need a color+pager expert more than a Windows expert.


I think that afterload callback was always buggy and should be fixed or
removed :/


>
>
>
>>> With the previous order, color was None for commands invoked with
>>> --pager=yes at
>>> the time the pager was spawned, which meant the switch over to ANSI mode
>>> on
>>> Windows didn't occur.  That left the color.win32print() in 'auto' mode,
>>> which
>>> meant no color in the pager output.  Non Windows platforms are capable of
>>> printing color with this option.
>>>
>>> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
>>> --- a/mercurial/dispatch.py
>>> +++ b/mercurial/dispatch.py
>>> @@ -760,6 +760,13 @@
>>>  for ui_ in uis:
>>>  ui_.setconfig('ui', 'interactive', 'off', '-y')
>>>
>>> +# setup color handling
>>> +coloropt = options['color']
>>> +for ui_ in uis:
>>> +if coloropt:
>>> +ui_.setconfig('ui', 'color', coloropt, '--color')
>>> +color.setup(ui_)
>>> +
>>>  if util.parsebool(options['pager']):
>>>  ui.pager('internal-always-' + cmd)
>>>  elif options['pager'] != 'auto':
>>> @@ -769,13 +776,6 @@
>>>  for ui_ in uis:
>>>  ui_.insecureconnections = True
>>>
>>> -# setup color handling
>>> -coloropt = options['color']
>>> -for ui_ in uis:
>>> -if coloropt:
>>> -ui_.setconfig('ui', 'color', coloropt, '--color')
>>> -color.setup(ui_)
>>> -
>>>  if options['version']:
>>>  return commands.version_(ui)
>>>  if options['help']:
>>> ___
>>> 

Re: [PATCH 2 of 2 v2] rebase: add flag to require destination

[Augie Fackler]

On Mon, Mar 20, 2017 at 03:17:47PM +, Ryan McElroy wrote:
> Any objections here?

Hearing none, queued.

Do we still think that the 'compat' knob belongs here (as we thought
with [behavior]), or should it live under [ui]?

>
>
> On 3/16/17 1:55 AM, Ryan McElroy wrote:
> >
> >
> >On 3/14/17 8:26 PM, David Soria Parra wrote:
> >>On Tue, Mar 14, 2017 at 05:56:16PM -0700, Ryan McElroy wrote:
> >>># HG changeset patch
> >>># User Ryan McElroy 
> >>># Date 1489538624 25200
> >>>#  Tue Mar 14 17:43:44 2017 -0700
> >>># Node ID 8d8b783803f43d5e2d86916c39e9554139752fe6
> >>># Parent  2dc26c57e60e7e7bf46a276e8a498a9746bd9271
> >>>rebase: add flag to require destination
> >>>
> >>This looks good to me. I was wondering if we want to provide separate
> >>knobs for
> >>these commands which might lead to config overhead or provide more
> >>comprehensive
> >>"ui" improvement knobs such as "commands.requiredest" to move people to
> >>a better
> >>model in logical steps.
> >>
> >>e.g. I am a user who likes a slightly enhanced user experience.
> >>ui.compat= is a
> >>bit too much for me, but update destinations is a good idea. Do i have
> >>to find
> >>all places where we use destinations to update or do I want to select a
> >>logical
> >>step?
> >>
> >>I personally think while fine granualar steps are nice, I'd probably
> >>lean
> >>towards logical steps as it provides a more consistent behavior for
> >>users (e.g.
> >>assume an extension Y that we don't know of can opt into using
> >>"commands.requiredest", which at the moment it cannot unless it depends
> >>on
> >>"commands.update.requiredest" which is missleading.
> >
> >I'm not against this direction, but I think what I have proposed here is
> >stillt he first right step. Once we have a bunch of granular knobs like
> >these ones, we can then work towards "multiknobs" when we have the config
> >registry concept to tie options together more, and then the compatibility
> >levels are just the biggest "multiknobs".
> >
> >
> >___
> >Mercurial-devel mailing list
> >Mercurial-devel@mercurial-scm.org
>
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 04 of 10] scmutil: add a rcpath-like method to return multiple config sources

[Augie Fackler]

On Mon, Mar 13, 2017 at 09:44:18PM -0700, Jun Wu wrote:
> Excerpts from David Soria Parra's message of 2017-03-13 21:37:57 -0700:
> > On Mon, Mar 13, 2017 at 08:44:49PM -0700, Jun Wu wrote:
> > > scmutil: add a rcpath-like method to return multiple config sources
> >
> > nice series :)

I also like this series. As far as I can tell, a v2 with dsop and
yuya's comments addressed would be ready to land. Can I have that?

> >
> > > +def rccomponents():
> > > +'''return an ordered [(type, obj)] about where to load configs.
> > > +
> > > +respect $HGRCPATH. if $HGRCPATH is empty, only .hg/hgrc of current 
> > > repo is
> > > +used. if $HGRCPATH is not set, the platform default will be used.
> > > +
> > > +if a directory is provided, *.rc files under it will be used.
> > > +
> > > +type could be either 'path' or 'items', if type is 'path', obj is a 
> > > string,
> > > +and is the config file path. if type is 'items', obj is a list of 
> > > (section,
> > > +name, value, source) that should fill the config directly.
> > > +'''
> > > +global _rccomponents
> > > +if _rccomponents is None:
> > > +if 'HGRCPATH' in encoding.environ:
> > > +# assume HGRCPATH is all about user configs so environments, 
> > > so
> > > +# environments can be overrided.
> > I think this should be 'environments can be 'overridden'. You duplicated 'so
> > environments'.
>
> Good catch. My Vim was behaving weird recently.
>
> >
> > > +_rccomponents = [('items', envconfig(_sysenvlist))]
> > > +for p in 
> > > encoding.environ['HGRCPATH'].split(pycompat.ospathsep):
> > > +if not p:
> > > +continue
> > > +p = util.expandpath(p)
> > Just a nit, maybe move this into a helper function?
>
> Good advice. The code was copy-pasted from rcpath()
>
> > def readpath(p):
> > p = util.expandpath(p)
> > if os.path.isdir(p):
> > entries = [(f,k) for (f,k) in osutil.listdir(p) if 
> > f.endswith('.rc')]
> > for f, kind in entries:
> > yield ('path', os.path.join(p,f))
> > else
> > return ('path', p)
> >
> > _rccomponents.extend(readpath(p))
> > > +if os.path.isdir(p):
> > > +for f, kind in osutil.listdir(p):
> > > +if f.endswith('.rc'):
> > > +_rccomponents.append(('path', 
> > > os.path.join(p, f)))
> > > +else:
> > > +_rccomponents.append(('path', p))
> > > +else:
> > > +syspath, userpath = osrcpath()
> > > +
> > > +def pathize(path):
> > > +return ('path', path)
> > > +
> > > +_rccomponents = map(pathize, syspath)
> > > +_rccomponents.append(('items', envconfig(_sysenvlist)))
> > > +_rccomponents.extend(map(pathize, userpath))
> >
> > I see we barely use map and prefer list-comprehensions. I don't mind 
> > either, i
> > am just not sure if we have a coding style around it.
>
> I don't feel strong. The "map" version seems to be easier to read in this
> case.
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 09 of 10 shelve-ext v4] shelve: add obs-based unshelve functionality

[Augie Fackler]

On Sat, Mar 11, 2017 at 01:00:28PM -0800, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi 
> # Date 1489265667 28800
> #  Sat Mar 11 12:54:27 2017 -0800
> # Node ID fb764ce7dd0b6b93cdb4449f5bb52164521b0aad
> # Parent  e06d7fd580a798de7818adae8b65eb166ea1defd
> shelve: add obs-based unshelve functionality

Can you clarify the relationship of patches 6 and 9 for me? Right now
they both at a headline level do the same thing, which seems unlikely.

>
> Obsolescense-based unshelve works as follows:
> 1. Instead of stripping temporary nodes, markers are created to
> obsolete them.
> 2. Restoring commit is just finding it in an unfiltered repo.
> 3. '--keep' is only passed to rebase on traditional unshelves
> (and thus traditional rebases), becuase we want markers to be
> created fro obsolete-based rebases.
> 4. 'hg unshelve' uses unfiltered repo to perform rebases
> because we want rebase to be able to create markers between original
> and new commits. 'rebaseskipobsolete' is disabled to make rebase not
> skip the commit altogether.
> 5. As per sprint discussions, hiding commits with obsmarkers is
> not ideal. This is okay for now, as long as obsshelve is an
> experimental feature. In future, once a better approach to
> hiding things is implemented, we can change the way obsshelve
> works (and also change its name to not refer to obsolescense).
>
> I have a test for the case when shelve node has been stripped before
> unshelve call, that test is together with ~30 commits I was talking
> about in patch 1.
>
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -25,6 +25,7 @@ from __future__ import absolute_import
>  import collections
>  import errno
>  import itertools
> +import time
>
>  from mercurial.i18n import _
>  from mercurial import (
> @@ -250,8 +251,13 @@ class shelvedstate(object):
>
>  def prunenodes(self):
>  """Cleanup temporary nodes from the repo"""
> -repair.strip(self.ui, self.repo, self.nodestoprune, backup=False,
> - topic='shelve')
> +if self.obsshelve:
> +unfi = self.repo.unfiltered()
> +relations = [(unfi[n], ()) for n in self.nodestoprune]
> +obsolete.createmarkers(self.repo, relations)
> +else:
> +repair.strip(self.ui, self.repo, self.nodestoprune, backup=False,
> + topic='shelve')
>
>  def cleanupoldbackups(repo):
>  vfs = vfsmod.vfs(repo.join(backupdir))
> @@ -665,9 +671,14 @@ def unshelvecontinue(ui, repo, state, op
>  util.rename(repo.join('unshelverebasestate'),
>  repo.join('rebasestate'))
>  try:
> -rebase.rebase(ui, repo, **{
> -'continue' : True
> -})
> +# if shelve is obs-based, we want rebase to be able
> +# to create markers to already-obsoleted commits
> +_repo = repo.unfiltered() if state.obsshelve else repo
> +with ui.configoverride({('experimental', 'rebaseskipobsolete'):
> +'off'}, 'unshelve'):
> +rebase.rebase(ui, _repo, **{
> +'continue' : True,
> +})
>  except Exception:
>  util.rename(repo.join('rebasestate'),
>  repo.join('unshelverebasestate'))
> @@ -707,30 +718,58 @@ def _commitworkingcopychanges(ui, repo,
>  with ui.configoverride({('ui', 'quiet'): True}):
>  node = cmdutil.commit(ui, repo, commitfunc, [], tempopts)
>  tmpwctx = repo[node]
> +ui.debug("temporary working copy commit: %s:%s\n" %
> + (tmpwctx.rev(), nodemod.short(node)))
>  return tmpwctx, addedbefore
>
> -def _unshelverestorecommit(ui, repo, basename):
> +def _unshelverestorecommit(ui, repo, basename, obsshelve):
>  """Recreate commit in the repository during the unshelve"""
>  with ui.configoverride({('ui', 'quiet'): True}):
> -shelvedfile(repo, basename, 'hg').applybundle()
> -shelvectx = repo['tip']
> +if obsshelve:
> +md = shelvedfile(repo, basename, 'oshelve').readobsshelveinfo()
> +shelvenode = nodemod.bin(md['node'])
> +repo = repo.unfiltered()
> +try:
> +shelvectx = repo[shelvenode]
> +except error.RepoLookupError:
> +m = _("shelved node %s not found in repo")
> +raise error.Abort(m % md['node'])
> +else:
> +shelvedfile(repo, basename, 'hg').applybundle()
> +shelvectx = repo['tip']
>  return repo, shelvectx
>
>  def _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev, basename, pctx,
> -  tmpwctx, shelvectx, branchtorestore):
> +  tmpwctx, shelvectx, branchtorestore, obsshelve):
>  """Rebase restored commit from its original location to a destination"""

Re: [PATCH 1 of 3] pager: fix the invocation of `more` on Windows

[Jun Wu]

Excerpts from Augie Fackler's message of 2017-03-21 17:34:39 -0400:
> On Mon, Mar 20, 2017 at 12:53:17AM -0400, Matt Harbison wrote:
> > pager: fix the invocation of `more` on Windows
> 
> I've taken this one. And I'm sad.
> 
> (Thanks! I blame windows for the sadness, not you.)

This does not look like a complete fix. Users setting "pager.pager=more", or
"pager.pager=somethingelse" will still have issues.

A better fix seems to be disable shell=False for Windows.

> >
> > After 9335dc6b2a9c, invoking `more` no longer works.  Instead, a warning is
> > printed and the pager is disabled.  Invoking `more.com` works.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] [RFC] dispatch: setup color before pager

[Augie Fackler]

On Mon, Mar 20, 2017 at 12:53:19AM -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison 
> # Date 1489985511 14400
> #  Mon Mar 20 00:51:51 2017 -0400
> # Node ID 2a323109f8e7ec5548dd506c2cff05f27fa20c1e
> # Parent  6e72bb689e57c985b138c580acb4cad8053dc180
> [RFC] dispatch: setup color before pager

Oy, these two I just dunno. I'll defer to other Windows experts.

>
> With the previous order, color was None for commands invoked with --pager=yes 
> at
> the time the pager was spawned, which meant the switch over to ANSI mode on
> Windows didn't occur.  That left the color.win32print() in 'auto' mode, which
> meant no color in the pager output.  Non Windows platforms are capable of
> printing color with this option.
>
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -760,6 +760,13 @@
>  for ui_ in uis:
>  ui_.setconfig('ui', 'interactive', 'off', '-y')
>
> +# setup color handling
> +coloropt = options['color']
> +for ui_ in uis:
> +if coloropt:
> +ui_.setconfig('ui', 'color', coloropt, '--color')
> +color.setup(ui_)
> +
>  if util.parsebool(options['pager']):
>  ui.pager('internal-always-' + cmd)
>  elif options['pager'] != 'auto':
> @@ -769,13 +776,6 @@
>  for ui_ in uis:
>  ui_.insecureconnections = True
>
> -# setup color handling
> -coloropt = options['color']
> -for ui_ in uis:
> -if coloropt:
> -ui_.setconfig('ui', 'color', coloropt, '--color')
> -color.setup(ui_)
> -
>  if options['version']:
>  return commands.version_(ui)
>  if options['help']:
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 1 of 3] pager: fix the invocation of `more` on Windows

[Augie Fackler]

On Mon, Mar 20, 2017 at 12:53:17AM -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison 
> # Date 1489983573 14400
> #  Mon Mar 20 00:19:33 2017 -0400
> # Node ID a32b379ffe151b8b094dcd67ef5b6bd551203dbe
> # Parent  b1bebed54e9371708e1cd1eba63f0cedadfdf035
> pager: fix the invocation of `more` on Windows

I've taken this one. And I'm sad.

(Thanks! I blame windows for the sadness, not you.)

>
> After 9335dc6b2a9c, invoking `more` no longer works.  Instead, a warning is
> printed and the pager is disabled.  Invoking `more.com` works.
>
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -839,7 +839,10 @@
>  # less(1), not more(1), and on debian it's
>  # sensible-pager(1). We should probably also give the system
>  # default editor command similar treatment.
> -envpager = encoding.environ.get('PAGER', 'more')
> +envpager = 'more'
> +if pycompat.osname == 'nt':
> +envpager = 'more.com'
> +envpager = encoding.environ.get('PAGER', envpager)
>  pagercmd = self.config('pager', 'pager', envpager)
>  if not pagercmd:
>  return
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] graphlog: draw multiple edges towards null node (issue5440)

[Augie Fackler]

On Mon, Mar 20, 2017 at 11:24:45AM +, Ryan McElroy wrote:
> On 3/20/17 8:22 AM, Yuya Nishihara wrote:
> ># HG changeset patch
> ># User Yuya Nishihara 
> ># Date 1489978255 -32400
> >#  Mon Mar 20 11:50:55 2017 +0900
> ># Node ID db70a30dde5182e704b02b0e47a79d3ecfafdd49
> ># Parent  cede8eaf55e2fe68434c273cfd066512b5ca112e
> >graphlog: draw multiple edges towards null node (issue5440)

queued, thanks

> >
> >Before, edge (r, null) was processed only once by newparents. However what
> >we really need is just stripping the edge (null, null).
> >
>

[...]

> I'd like to re-state for the record my wish that for subtle bugfixes like
> this that we could split the fix into two patches: a patch that adds a test
> exhibiting the broken behavior, and then the fix with the corrections to the
> test. That would make it *so much easier* to review a patch like this.

+1

>
> For the curious (like me), the previous (broken) behavior was this (note the
> missing /):
>
>@  1 bar
>|
>| o  0 foo
> |
>o  -1
>
> The difference in the test output with this patch is thus:
>
>@  1 bar
>|
>| o  0 foo
> -  |
> +  |/
>o  -1
>
> Overall, this change looks good to me.
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Phillip Cohen]

A tuple or subobject works for me.

On Tue, Mar 21, 2017 at 1:45 PM, Jun Wu  wrote:
> Excerpts from Ryan McElroy's message of 2017-03-21 20:32:56 +:
>>
>> On 3/21/17 7:34 PM, Jun Wu wrote:
>> > Excerpts from Phillip Cohen's message of 2017-03-21 12:21:33 -0700:
>> >>> Have you actually tried if "commandname" is the command name after 
>> >>> resolving
>> >>> alias?
>> >> It is, for command aliases. For example `hg sf` will correctly return 
>> >> `absorb`.
>> > "sf" is unambiguous because it's hard-coded by absorb.py.
>> >
>> > I think Ryan is more interested in ambiguous cases caused by user-defined
>> > [alias].
>>
>> I'm actually interested in our internal logging. Today, our
>> command-guessing code in the wrapper is really quite terrible and
>> doesn't normalize as well as I would like. We should be using
>> commandname instead, it sounds like, and passing that back to our
>> logging wrapper. I would sure hate to lose that functionality right
>> after I just learned it exists already (I was thinking about adding it
>> otherwise).
>>
>> I'm only slightly concerned about users not knowing what their aliases
>> are. I'm much more interested in debug-ability and consistency in
>> reporting for stats aggregation tools like what we have and other
>> enterprise deployments will want. So I think that commandname is
>> important to have.
>
> Maybe they can be merged into one value: (cmdname, [fullargs]) so we
> populate ui less. The pattern is seen in other places like "execve":
>
>   int execve(const char *path, char *const argv[], char *const envp[]);
>
> The first argument "path" is like "cmdname", the unambiguous entry point.
> Followed by command line arguments.
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Ryan McElroy]

On 3/21/17 7:34 PM, Jun Wu wrote:

Excerpts from Phillip Cohen's message of 2017-03-21 12:21:33 -0700:

Have you actually tried if "commandname" is the command name after resolving
alias?

It is, for command aliases. For example `hg sf` will correctly return `absorb`.

"sf" is unambiguous because it's hard-coded by absorb.py.

I think Ryan is more interested in ambiguous cases caused by user-defined
[alias].


I'm actually interested in our internal logging. Today, our 
command-guessing code in the wrapper is really quite terrible and 
doesn't normalize as well as I would like. We should be using 
commandname instead, it sounds like, and passing that back to our 
logging wrapper. I would sure hate to lose that functionality right 
after I just learned it exists already (I was thinking about adding it 
otherwise).


I'm only slightly concerned about users not knowing what their aliases 
are. I'm much more interested in debug-ability and consistency in 
reporting for stats aggregation tools like what we have and other 
enterprise deployments will want. So I think that commandname is 
important to have.

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

Re: [PATCH 7 of 7] tests: add a test case verifying that mq respects --no-git option

[Ryan McElroy]

Overall this series looks good to me, except for this last patch. See 
inline comments. For now, I'd take the rest of this series if we're okay 
with the BC break, and just drop this patch while we figure out the mq 
stuff.


On 3/21/17 5:08 PM, Alexander Fomin wrote:

# HG changeset patch
# User Alexander Fomin
# Date 1490113938 25200
#  Tue Mar 21 09:32:18 2017 -0700
# Node ID 9a11a79f6bcdd1134484ddd8eace997b55e7073a
# Parent  e9044ade1523e847877f4eee1d4e06734e2aa4cd
tests: add a test case verifying that mq respects --no-git option

This patch adds a test case to verify that --no-git option still works in mq
after making it explicitly request binary diff even in Git mode (issue5510).

diff --git a/tests/test-mq.t b/tests/test-mq.t
--- a/tests/test-mq.t
+++ b/tests/test-mq.t
@@ -1162,6 +1162,21 @@ check binary patches can be popped and p
8ba2a2f3e77b55d03051ff9c24ad65e7  bucephalus
  
  
+check binary patches respect --no-git

+
+  $ cat > writebin.py < import sys
+  > path = sys.argv[1]
+  > open(path, 'wb').write('BIN\x42RY')


Hex 42 is the character 'B', isn't it? So this isn't binary at all.

Also, binary detection just searches for \x00 I think. So that would be 
more appropriate to use here.



+  > EOF
+  $ python writebin.py answer


Rather than creating a little python program, I think you could just use 
printf here:


  $ printf 'BIN\x00RY' > answer


+
+  $ python "$TESTDIR/md5sum.py" answer
+  ce0b4fda508e3d9f9ece98f8e823b6f7  answer


What is the reason for the md5sum here? Did you want to check 
round-tripping? (but I don't see that here)



+  $ hg add answer
+  $ hg qnew -f --no-git addanswer


What does --no-git do before this patch series? I don't see any 
differences in patch files with or without --no-git today, so I'm not 
sure it's actually respected today.



+  $ grep diff .hg/patches/addanswer
+  diff -r [a-f0-9]* -r [a-f0-9]* answer (re)
  
  strip again
  

   



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

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Augie Fackler]

On Tue, Mar 21, 2017 at 3:29 PM, Jun Wu  wrote:
>> I feel obligated to remind you that we don't offer any stability
>> promises on internals, so if this is a righteous cleanup (using req or
>> similar instead of ui), it's probably worth doing in any case.
>
> I know. In theory we can also drop "extsetup" and only keep "uisetup", or
> drop the support for "extsetup()" without the ui argument. But I guess we
> didn't do that for a reason.

I'm probably going to nibble around dropping support for extsetup
without ui in Python 3. Seems worthwhile as an incremental nudge.

>
>> Note that we're coming up on an interesting moment when we'll know
>> extensions required significant cleanup anyway (python 3), so we might
>> want to evaluate other internal structure cleanups while we're
>> breaking everyone anyway.
>
> Even if we have done replacing "ui" with "req" everywhere, I don't think
> that's better - the request object practically becomes the new "ui" object
> with all kinds of features.
>
> If I guessed correctly, what you want is a new layer inserted, like:
>
> We have a "root" object. "root" could be either "ui" or "request".
>
> To access configs, use: root.config.get, root.config.set, and there is
> no root.setconfig.
>
> To access I/O related features, use: root.io.fin, root.io.fout, ...,
> there is no root.fin.
>
> Just split ui functions to subobjects.
>
> My point is the "root" object does not have to be "request". It can be done
> using the "ui" object incrementally, using a similar approach like what we
> did for "repo.join -> repo.vfs.join". It's more realistic.

Sure, that sounds fine too.

>
> Anyway, the long-term refactoring shouldn't block this simple patch. And I
> think it's reasonable to add "ui.args" for now.

In abstract, this is how we get god objects. I think in this
particular case the progress might be worth the change, but in general
I'm totally willing to block simple patches if they add bad conceptual
layering violations that make future maintenance harder.

(I'll defer to Ryan's part of the discussion for now, and will look at
this series again when there's a resend or some other indication it
needs my attention.)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Jun Wu]

Excerpts from Ryan McElroy's message of 2017-03-21 18:56:19 +:
> 
> On 3/21/17 6:50 PM, Jun Wu wrote:
> > Excerpts from Augie Fackler's message of 2017-03-21 14:35:43 -0400:
> >> On Mon, Mar 20, 2017 at 12:39:11PM +, Ryan McElroy wrote:
> >>> Overall, I like the functionality this series adds, but I'm less convinced
> >>> on the specific implementation. Adding more stuff to the junkyard that is
> >>> the "ui" class doesn't feel awesome, and I'm concerned with how it will
> >>> interact with chg and a future direction of ui immutability.
> >> +1 overall on this paragraph - please avoid adding things to ui if you can.
> >>
> >> (The rest of this thread appears to be going in a healthy direction,
> >> so by all means carry on yourselves and do a resend when you've worked
> >> out something you're happy with.)
> > I think ui.args is useful. We can probably avoid "ui.commandname" (use
> > "ui.args[0]" as an approximate).
> >
> > It's currently painful for anyone who wants the "sys.argv" information, for
> > example the journal extension. For now, it has to be done via wrapping
> > dispatch.runcommand.
> 
> I agree that this seems useful, and cleaning up journal to not need to 
> wrap this would be nice. I also can't think of a better place to put 
> this info than 'ui' right now. I'm leaning towards saying "okay".
> 
> Unfortunately, it can't replace `ui.commandname` I'm afraid: `hg st` 
> might become "status" or it might become "log" because I have an alias. 
> We can't rely on the full command line args to reliably determine what 
> command we dispatch to because of the indirection layer.

The journal extension does not care about aliases. It only needs "fullargs".

Have you actually tried if "commandname" is the command name after resolving
alias?

I'd argue that "fullargs" is simple and correct, as it's what you typed
as-is. As a user, you should know what aliases you have.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 4 of 4 py3] py3: prove `hg {add,addremove,commit} all work

[Augie Fackler]

# HG changeset patch
# User Augie Fackler 
# Date 1489902476 14400
#  Sun Mar 19 01:47:56 2017 -0400
# Node ID c45ada02713a079e62f67340f84727ba593e5a3b
# Parent  3af735529b5f475e5c53ffaf7b136eecb97fcd61
py3: prove `hg {add,addremove,commit} all work

We can't do a second commit in a repo yet, because pure-Python bdiff
is broken on Python 3. That is probably a good next step.

diff --git a/tests/test-check-py3-commands.t b/tests/test-check-py3-commands.t
--- a/tests/test-check-py3-commands.t
+++ b/tests/test-check-py3-commands.t
@@ -122,4 +122,44 @@ Test bytes-ness of policy.policy with HG
 
 `hg init` can create empty repos
 
-  $ $PYTHON3 `which hg` init emptyrepo
+  $ $PYTHON3 `which hg` init py3repo
+  $ cd py3repo
+  $ echo "This is the file 'iota'." > iota
+  $ $PYTHON3 $HGBIN status
+  ? iota
+  $ $PYTHON3 $HGBIN add iota
+  $ $PYTHON3 $HGBIN status
+  A iota
+  $ $PYTHON3 $HGBIN commit --message 'commit performed in Python 3'
+  $ $PYTHON3 $HGBIN status
+
+TODO: bdiff is broken on Python 3 so we can't do a second commit yet,
+when that works remove this rollback command.
+  $ hg rollback
+  repository tip rolled back to revision -1 (undo commit)
+  working directory now based on revision -1
+
+  $ mkdir A
+  $ echo "This is the file 'mu'." > A/mu
+  $ $PYTHON3 $HGBIN addremove
+  adding A/mu
+  $ $PYTHON3 $HGBIN status
+  A A/mu
+  A iota
+  $ HGEDITOR='echo message > ' $PYTHON3 $HGBIN commit
+  $ $PYTHON3 $HGBIN status
+
+Prove the repo is valid using the Python 2 `hg`:
+  $ hg verify
+  checking changesets
+  checking manifests
+  crosschecking files in changesets and manifests
+  checking files
+  2 files, 1 changesets, 2 total revisions
+  $ hg log
+  changeset:   0:e825505ba339
+  tag: tip
+  user:test
+  date:Thu Jan 01 00:00:00 1970 +
+  summary: message
+  
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 1 of 4 py3] similar: avoid sorting and making copies of addedfiles and removedfiles sets

[Augie Fackler]

# HG changeset patch
# User Augie Fackler 
# Date 1489903420 14400
#  Sun Mar 19 02:03:40 2017 -0400
# Node ID a4745fd9219ed5b408bfc0403a4a8e6acd41df6c
# Parent  66c3ae6d886cae0e3a3cff6a0058e2d2a866fd9d
similar: avoid sorting and making copies of addedfiles and removedfiles sets

The process of porting to Python 3 exposed some weirdness in this
code: workingfilectx doesn't define rich comparison operators, so in
Python 2 the default comparison devolved to id(value), which is the
pointer address of the object. Inspection of _findexactmatches and
_findsimilarmatches revealed that they didn't care about the sort
order of the data, so we remove the sort (and potentially make
addremove faster since it's not sorting things). We now have to do one
little extra set dance in order to not mutate the addedfiles set
during its iteration, but that's a small price to pay for the
resulting cleaner nature of the code.

diff --git a/mercurial/similar.py b/mercurial/similar.py
--- a/mercurial/similar.py
+++ b/mercurial/similar.py
@@ -107,13 +107,14 @@ def findrenames(repo, added, removed, th
 if fp in parentctx and parentctx[fp].size() > 0])
 
 # Find exact matches.
-for (a, b) in _findexactmatches(repo,
-sorted(addedfiles), sorted(removedfiles)):
-addedfiles.remove(b)
+addedremove = set()
+for (a, b) in _findexactmatches(repo, addedfiles, removedfiles):
+addedremove.add(b)
 yield (a.path(), b.path(), 1.0)
+addedfiles -= addedremove
 
 # If the user requested similar files to be matched, search for them also.
 if threshold < 1.0:
 for (a, b, score) in _findsimilarmatches(repo,
-sorted(addedfiles), sorted(removedfiles), threshold):
+addedfiles, removedfiles, threshold):
 yield (a.path(), b.path(), score)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Augie Fackler]

(bah, sorry for dropping the list)

On Tue, Mar 21, 2017 at 3:05 PM, Jun Wu  wrote:
>>
>> Would it make more sense to have a "request" or "invocation" object that
>> has a ui (and then, eventually, transitively has a config)?
>
> I'm not sure if I follow. The problem is that extensions only have "ui",
> they do not have the "request" object. If they have, then the problem is
> solved.
>
> If we add "ui.req", it's not better than "ui.args".
>
> If we "incrementally" replace "ui" with "req" everywhere, it's a huge
> amount of work with all kinds of BC issues.

I feel obligated to remind you that we don't offer any stability
promises on internals, so if this is a righteous cleanup (using req or
similar instead of ui), it's probably worth doing in any case.

Note that we're coming up on an interesting moment when we'll know
extensions required significant cleanup anyway (python 3), so we might
want to evaluate other internal structure cleanups while we're
breaking everyone anyway.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

RE: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Jun Wu]

+list

> -Original Message-
> From: Augie Fackler [mailto:r...@durin42.com]
> Sent: Tuesday, March 21, 2017 11:55 AM
> To: Jun Wu 
> Subject: Re: [PATCH 3 of 3] localrepo: pass args and command running as
> store/write lock metadata
> 
> > I think ui.args is useful. We can probably avoid "ui.commandname" (use
> > "ui.args[0]" as an approximate).
> 
> Would it make more sense to have a "request" or "invocation" object that
> has a ui (and then, eventually, transitively has a config)?

I'm not sure if I follow. The problem is that extensions only have "ui",
they do not have the "request" object. If they have, then the problem is
solved.

If we add "ui.req", it's not better than "ui.args".

If we "incrementally" replace "ui" with "req" everywhere, it's a huge
amount of work with all kinds of BC issues.

If we make "req" globally accessible, it's not better than "sys.argv".

Anyway, I think "ui.args" is clean and easy and I don't see any better
solutions.

> I agree that it's painful, but I'd also rather not keep tossing things in the 
> ui
> dumpster.

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

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Ryan McElroy]

On 3/21/17 6:50 PM, Jun Wu wrote:

Excerpts from Augie Fackler's message of 2017-03-21 14:35:43 -0400:

On Mon, Mar 20, 2017 at 12:39:11PM +, Ryan McElroy wrote:

Overall, I like the functionality this series adds, but I'm less convinced
on the specific implementation. Adding more stuff to the junkyard that is
the "ui" class doesn't feel awesome, and I'm concerned with how it will
interact with chg and a future direction of ui immutability.

+1 overall on this paragraph - please avoid adding things to ui if you can.

(The rest of this thread appears to be going in a healthy direction,
so by all means carry on yourselves and do a resend when you've worked
out something you're happy with.)

I think ui.args is useful. We can probably avoid "ui.commandname" (use
"ui.args[0]" as an approximate).

It's currently painful for anyone who wants the "sys.argv" information, for
example the journal extension. For now, it has to be done via wrapping
dispatch.runcommand.


I agree that this seems useful, and cleaning up journal to not need to 
wrap this would be nice. I also can't think of a better place to put 
this info than 'ui' right now. I'm leaning towards saying "okay".


Unfortunately, it can't replace `ui.commandname` I'm afraid: `hg st` 
might become "status" or it might become "log" because I have an alias. 
We can't rely on the full command line args to reliably determine what 
command we dispatch to because of the indirection layer.



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

[PATCH 3 of 7] cmdutil: treat all files as text when doing a record

[Alexander Fomin]

# HG changeset patch
# User Alexander Fomin 
# Date 1490104984 25200
#  Tue Mar 21 07:03:04 2017 -0700
# Node ID a4b6c5df3e64195cb8ba124d9dd2428483cfc6a6
# Parent  6e4849bc2c126815e8edfe176257883907596361
cmdutil: treat all files as text when doing a record

This patch makes cmdutil treat all files as text when when doing a record.
It has no immediate effect, but rather allows to change hg behaviour with
regarding to binary diffs generation in Git mode (issue5510).

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -162,6 +162,8 @@ def dorecord(ui, repo, commitfunc, cmdsu
 diffopts = patch.difffeatureopts(ui, opts=opts, whitespace=True)
 diffopts.nodates = True
 diffopts.git = True
+# Explicitly request binary diffs in Git mode
+diffopts.text = True
 diffopts.showfunc = True
 originaldiff = patch.diff(repo, changes=status, opts=diffopts)
 originalchunks = patch.parsepatch(originaldiff)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 6 of 7] tests: add test cases for binary diffs in Git mode

[Alexander Fomin]

# HG changeset patch
# User Alexander Fomin 
# Date 1490113366 25200
#  Tue Mar 21 09:22:46 2017 -0700
# Node ID e9044ade1523e847877f4eee1d4e06734e2aa4cd
# Parent  98301edf86daecef6ef43357b0934c351f58574d
tests: add test cases for binary diffs in Git mode

This patch adds some test cases to verify binary diffs are not generated in
Git mode unless explicitly requested by user (issue5510).

diff --git a/tests/test-diff-binary-file.t b/tests/test-diff-binary-file.t
--- a/tests/test-diff-binary-file.t
+++ b/tests/test-diff-binary-file.t
@@ -19,6 +19,10 @@
 
   $ hg diff --nodates -r 0 -r 2
 
+  $ hg diff --git -r 0 -r 1
+  diff --git a/binfile.bin b/binfile.bin
+  Binary file binfile.bin has changed
+
   $ hg diff --git -a -r 0 -r 1
   diff --git a/binfile.bin b/binfile.bin
   index 
37ba3d1c6f17137d9c5f5776fa040caf5fe73ff9..58dc31a9e2f40f74ff3b45903f7d620b8e5b7356
@@ -44,6 +48,10 @@
   diff --git a/binfile.bin b/binfile.bin
   Binary file binfile.bin has changed
 
+  $ HGPLAIN=1 hg diff --git -r 0 -r 1
+  diff --git a/binfile.bin b/binfile.bin
+  Binary file binfile.bin has changed
+
   $ HGPLAIN=1 hg diff --config diff.nobinary=True --git -a -r 0 -r 1
   diff --git a/binfile.bin b/binfile.bin
   index 
37ba3d1c6f17137d9c5f5776fa040caf5fe73ff9..58dc31a9e2f40f74ff3b45903f7d620b8e5b7356
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 2 of 7] transplant: treat all files as text when applying a changeset

[Alexander Fomin]

# HG changeset patch
# User Alexander Fomin 
# Date 1490051515 25200
#  Mon Mar 20 16:11:55 2017 -0700
# Node ID 6e4849bc2c126815e8edfe176257883907596361
# Parent  3c43572fec91b9caa83dc3402532046013b84e07
transplant: treat all files as text when applying a changeset

This patch makes transplant treat all files as text when when applying a
changeset. It has no immediate effect, but rather allows to change hg
behaviour with regarding to binary diffs generation in Git mode (issue5510).

diff --git a/hgext/transplant.py b/hgext/transplant.py
--- a/hgext/transplant.py
+++ b/hgext/transplant.py
@@ -143,6 +143,8 @@ class transplanter(object):
 pulls = []
 diffopts = patch.difffeatureopts(self.ui, opts)
 diffopts.git = True
+# Enable binary diffs in Git mode
+diffopts.text = True
 
 lock = tr = None
 try:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 4 of 7] tests: explicitly require binary diffs generation in Git mode

[Alexander Fomin]

# HG changeset patch
# User Alexander Fomin 
# Date 1490105477 25200
#  Tue Mar 21 07:11:17 2017 -0700
# Node ID b1eb6801f90a45ef43f17a22d9835ccba4ac190b
# Parent  a4b6c5df3e64195cb8ba124d9dd2428483cfc6a6
tests: explicitly require binary diffs generation in Git mode

This patch makes existing tests explicitly request binary diffs generation by
using -a option even in Git mode.
It has no immediate effect, but rather allows to change hg behaviour with
regarding to binary diffs generation in Git mode (issue5510).

diff --git a/tests/test-diff-binary-file.t b/tests/test-diff-binary-file.t
--- a/tests/test-diff-binary-file.t
+++ b/tests/test-diff-binary-file.t
@@ -19,7 +19,7 @@
 
   $ hg diff --nodates -r 0 -r 2
 
-  $ hg diff --git -r 0 -r 1
+  $ hg diff --git -a -r 0 -r 1
   diff --git a/binfile.bin b/binfile.bin
   index 
37ba3d1c6f17137d9c5f5776fa040caf5fe73ff9..58dc31a9e2f40f74ff3b45903f7d620b8e5b7356
   GIT binary patch
@@ -44,7 +44,7 @@
   diff --git a/binfile.bin b/binfile.bin
   Binary file binfile.bin has changed
 
-  $ HGPLAIN=1 hg diff --config diff.nobinary=True --git -r 0 -r 1
+  $ HGPLAIN=1 hg diff --config diff.nobinary=True --git -a -r 0 -r 1
   diff --git a/binfile.bin b/binfile.bin
   index 
37ba3d1c6f17137d9c5f5776fa040caf5fe73ff9..58dc31a9e2f40f74ff3b45903f7d620b8e5b7356
   GIT binary patch
@@ -64,7 +64,7 @@
   
 
 
-  $ hg diff --git -r 2 -r 3
+  $ hg diff --git -a -r 2 -r 3
   diff --git a/binfile.bin b/nonbinfile
   copy from binfile.bin
   copy to nonbinfile
diff --git a/tests/test-git-export.t b/tests/test-git-export.t
--- a/tests/test-git-export.t
+++ b/tests/test-git-export.t
@@ -342,7 +342,7 @@ Binary diff:
 
   $ cp "$TESTDIR/binfile.bin" .
   $ hg add binfile.bin
-  $ hg diff --git > b.diff
+  $ hg diff --git -a > b.diff
   $ cat b.diff
   diff --git a/binfile.bin b/binfile.bin
   new file mode 100644
diff --git a/tests/test-import-eol.t b/tests/test-import-eol.t
--- a/tests/test-import-eol.t
+++ b/tests/test-import-eol.t
@@ -131,7 +131,7 @@ Test --eol and binary patches
   $ hg ci -Am addb
   adding b
   $ $PYTHON -c 'file("b", "wb").write("a\x00\nc\r\nd")'
-  $ hg diff --git > bin.diff
+  $ hg diff --git -a > bin.diff
   $ hg revert --no-backup b
 
 binary patch with --eol
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 7 of 7] tests: add a test case verifying that mq respects --no-git option

[Alexander Fomin]

# HG changeset patch
# User Alexander Fomin 
# Date 1490113938 25200
#  Tue Mar 21 09:32:18 2017 -0700
# Node ID 9a11a79f6bcdd1134484ddd8eace997b55e7073a
# Parent  e9044ade1523e847877f4eee1d4e06734e2aa4cd
tests: add a test case verifying that mq respects --no-git option

This patch adds a test case to verify that --no-git option still works in mq
after making it explicitly request binary diff even in Git mode (issue5510).

diff --git a/tests/test-mq.t b/tests/test-mq.t
--- a/tests/test-mq.t
+++ b/tests/test-mq.t
@@ -1162,6 +1162,21 @@ check binary patches can be popped and p
   8ba2a2f3e77b55d03051ff9c24ad65e7  bucephalus
 
 
+check binary patches respect --no-git
+
+  $ cat > writebin.py < import sys
+  > path = sys.argv[1]
+  > open(path, 'wb').write('BIN\x42RY')
+  > EOF
+  $ python writebin.py answer
+
+  $ python "$TESTDIR/md5sum.py" answer
+  ce0b4fda508e3d9f9ece98f8e823b6f7  answer
+  $ hg add answer
+  $ hg qnew -f --no-git addanswer
+  $ grep diff .hg/patches/addanswer
+  diff -r [a-f0-9]* -r [a-f0-9]* answer (re)
 
 strip again
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 5 of 7] patch: require text mode (-a) for binary patches in git mode (issue5510) (BC)

[Alexander Fomin]

# HG changeset patch
# User Alexander Fomin 
# Date 1490105843 25200
#  Tue Mar 21 07:17:23 2017 -0700
# Node ID 98301edf86daecef6ef43357b0934c351f58574d
# Parent  b1eb6801f90a45ef43f17a22d9835ccba4ac190b
patch: require text mode (-a) for binary patches in git mode (issue5510) (BC)

This changeset makes patch explicitly request binary diffs generation by using
-a/--text option even when in Git mode.

diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -2358,7 +2358,8 @@ def diffhunks(repo, node1=None, node2=No
 # Buffer the whole output until we are sure it can be generated
 return list(difffn(opts.copy(git=False), losedata))
 except GitDiffRequired:
-return difffn(opts.copy(git=True), None)
+# Explicitly request binary diffs if Git mode is required
+return difffn(opts.copy(git=True, text=True), None)
 else:
 return difffn(opts, None)
 
@@ -2552,7 +2553,7 @@ def trydiff(repo, revs, ctx1, ctx2, modi
 elif revs and not repo.ui.quiet:
 header.append(diffline(path1, revs))
 
-if binary and opts.git and not opts.nobinary:
+if binary and opts.git and opts.text and not opts.nobinary:
 text = mdiff.b85diff(content1, content2)
 if text:
 header.append('index %s..%s' %
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 1 of 7] mq: treat all files as text in Git mode

[Alexander Fomin]

# HG changeset patch
# User Alexander Fomin 
# Date 1490051288 25200
#  Mon Mar 20 16:08:08 2017 -0700
# Node ID 3c43572fec91b9caa83dc3402532046013b84e07
# Parent  65969cd351b73228642d1e491e8c78b20d85405a
mq: treat all files as text in Git mode

This patch makes mq treat all files as text when in Git mode.
It has no immediate effect, but rather allows to change hg behaviour with
regarding to binary diffs generation in Git mode (issue5510).

diff --git a/hgext/mq.py b/hgext/mq.py
--- a/hgext/mq.py
+++ b/hgext/mq.py
@@ -1180,6 +1180,8 @@ class queue(object):
 if date:
 date = util.parsedate(date)
 diffopts = self.diffopts({'git': opts.get('git')})
+if diffopts.git:
+diffopts.text = True
 if opts.get('checkname', True):
 self.checkpatchname(patchfn)
 inclsubs = checksubstate(repo)
@@ -2609,6 +2611,8 @@ def new(ui, repo, patch, *args, **opts):
 msg = cmdutil.logmessage(ui, opts)
 q = repo.mq
 opts['msg'] = msg
+if opts['git']:
+opts['text'] = True
 setupheaderopts(ui, opts)
 q.new(repo, patch, *args, **opts)
 q.savedirty()
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Augie Fackler]

On Mon, Mar 20, 2017 at 12:39:11PM +, Ryan McElroy wrote:
> Overall, I like the functionality this series adds, but I'm less convinced
> on the specific implementation. Adding more stuff to the junkyard that is
> the "ui" class doesn't feel awesome, and I'm concerned with how it will
> interact with chg and a future direction of ui immutability.

+1 overall on this paragraph - please avoid adding things to ui if you can.

(The rest of this thread appears to be going in a healthy direction,
so by all means carry on yourselves and do a resend when you've worked
out something you're happy with.)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH evolve-ext] tests: add glob for Windows

[Ryan McElroy]

On 3/17/17 3:32 AM, Matt Harbison wrote:

# HG changeset patch
# User Matt Harbison 
# Date 1489720627 14400
#  Thu Mar 16 23:17:07 2017 -0400
# Node ID ab5c2bef148bfb60e1956a55de4b7ba00ebe1817
# Parent  6a3248558b6929378450b572bb27406afe703ffd
tests: add glob for Windows


This looks good to me.



diff --git a/tests/test-inhibit.t b/tests/test-inhibit.t
--- a/tests/test-inhibit.t
+++ b/tests/test-inhibit.t
@@ -911,7 +911,7 @@
$ cd not-inhibit
$ hg book -d foo
$ hg pull
-  pulling from $TESTTMP/inhibit
+  pulling from $TESTTMP/inhibit (glob)
searching for changes
no changes found
adding remote bookmark foo
diff --git a/tests/test-stablesort.t b/tests/test-stablesort.t
--- a/tests/test-stablesort.t
+++ b/tests/test-stablesort.t
@@ -138,7 +138,7 @@
updating to branch default
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg -R repo_B pull --rev 13
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -146,7 +146,7 @@
added 4 changesets with 0 changes to 0 files (+1 heads)
(run 'hg heads' to see heads, 'hg merge' to merge)
$ hg -R repo_B pull --rev 14
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -154,7 +154,7 @@
added 1 changesets with 0 changes to 0 files (+1 heads)
(run 'hg heads .' to see heads, 'hg merge' to merge)
$ hg -R repo_B pull
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -204,7 +204,7 @@
updating to branch default
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg -R repo_C pull --rev 12
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -212,7 +212,7 @@
added 2 changesets with 0 changes to 0 files (+1 heads)
(run 'hg heads' to see heads, 'hg merge' to merge)
$ hg -R repo_C pull --rev 15
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -220,7 +220,7 @@
added 4 changesets with 0 changes to 0 files (+1 heads)
(run 'hg heads .' to see heads, 'hg merge' to merge)
$ hg -R repo_C pull
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -270,7 +270,7 @@
updating to branch default
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg -R repo_D pull --rev 10
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -278,7 +278,7 @@
added 5 changesets with 0 changes to 0 files
(run 'hg update' to get a working copy)
$ hg -R repo_D pull --rev 15
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -286,7 +286,7 @@
added 4 changesets with 0 changes to 0 files (+1 heads)
(run 'hg heads' to see heads, 'hg merge' to merge)
$ hg -R repo_D pull
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -404,7 +404,7 @@
updating to branch default
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg -R repo_E pull --rev e7d9710d9fc6
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -420,7 +420,7 @@
updating to branch default
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg -R repo_F pull --rev d62d843c9a01
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -436,7 +436,7 @@
updating to branch default
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg -R repo_G pull --rev 43227190fef8
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
@@ -444,7 +444,7 @@
added 1 changesets with 0 changes to 0 files (+1 heads)
(run 'hg heads' to see heads, 'hg merge' to merge)
$ hg -R repo_G pull --rev 2702dd0c91e7
-  pulling from $TESTTMP/repo_A
+  pulling from $TESTTMP/repo_A (glob)
searching for changes
adding changesets
adding manifests
diff --git a/tests/test-topic-dest.t b/tests/test-topic-dest.t
--- a/tests/test-topic-dest.t
+++ b/tests/test-topic-dest.t
@@ -276,7 +276,7 @@
$ hg add other
$ hg ci -m 'c_other'
$ hg pull -r default --rebase
-  pulling from $TESTTMP/jungle
+  pulling from 

Re: [PATCH evolve-ext] checks: correct the shebang line filtering for python files

[Ryan McElroy]

On 3/17/17 1:38 AM, Matt Harbison wrote:

# HG changeset patch
# User Matt Harbison 
# Date 1489713417 14400
#  Thu Mar 16 21:16:57 2017 -0400
# Node ID 6a3248558b6929378450b572bb27406afe703ffd
# Parent  e9d5f54765a27e09d35f48dda23db7e6f5b8320a
checks: correct the shebang line filtering for python files

As it is, the only related file is docs/test2rst.py, which was covered by **.py.
Not sure if it matters, but most patterns in core tests are for "#!.*?python".
(Though there are a couple "#!.*python" tests.)

diff --git a/tests/test-check-flake8.t b/tests/test-check-flake8.t
--- a/tests/test-check-flake8.t
+++ b/tests/test-check-flake8.t
@@ -14,5 +14,5 @@
  
  run flake8 if it exists; if it doesn't, then just skip
  
-  $ hg files -0 'set:(**.py or grep("^!#.*python")) - removed()' 2>/dev/null \

+  $ hg files -0 'set:(**.py or grep("^#!.*python")) - removed()' 2>/dev/null \


This looks obviously correct to me. I wouldn't mind a test that prevents 
regressions here but I don't think it's a blocker.



> | xargs -0 flake8
diff --git a/tests/test-check-pyflakes.t b/tests/test-check-pyflakes.t
--- a/tests/test-check-pyflakes.t
+++ b/tests/test-check-pyflakes.t
@@ -7,5 +7,5 @@
  run pyflakes on all tracked files ending in .py or without a file ending
  (skipping binary file random-seed)

This comment looks out of date? Can you update it?
  
-  $ hg locate 'set:(**.py or grep("^!#.*python")) - removed()' 2>/dev/null \

+  $ hg locate 'set:(**.py or grep("^#!.*python")) - removed()' 2>/dev/null \
> | xargs pyflakes 2>/dev/null


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

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Jun Wu]

Excerpts from Ryan McElroy's message of 2017-03-21 17:18:21 +:
> On 3/21/17 4:40 PM, Jun Wu wrote:
> > Excerpts from Ryan McElroy's message of 2017-03-20 12:39:11 +:
> >> Overall, I like the functionality this series adds, but I'm less
> >> convinced on the specific implementation. Adding more stuff to the
> >> junkyard that is the "ui" class doesn't feel awesome, and I'm concerned
> >> with how it will interact with chg and a future direction of ui
> >> immutability.
> >>
> >> I have an alternate suggestion for how to achieve this visibility into
> >> what is going on in a repo:
> >>
> >> Instead of having this information inside of a lock, what if every
> >> process that started up in a repo tried to create a file in
> >> .hg/processes/ (or something) of the form "info.". Then you
> >> wouldn't need to pass this data through to the lock command, wouldn't
> >> need to plumb it through the ui class, and combining the lock info with
> >> the processes info would still be straightforward.
> >>
> >> This would allow you to touch only the dispatch logic and achieve
> >> something even cooler: you'd be able to see all of the processes running
> >> in a repo (even read processes, as long as the reader has write access
> >> to the repo's .hg/).
> >>
> >> Thoughts on this alternate approach?
> > Two questions:
> >
> > 1. Who is responsible for garbage collecting those pid files, in case of
> > crashes?
> > 2. Linux has pid namespace, pid could conflict.
> >
> > I think it's better to just rely on whatever the operating system provides
> > to convert pid to args.
> >
> 
> 1. Could be the same as abandoned lockfile cleanup (or maybe we just 
> leave them as graves honoring the processes that came and died in this 
> repo -- joking)
> 2. Already breaks mercurial locks badly due to abandoned lockfile 
> cleanup, so I'm a little less concerned about that -- but there is a 
> case that I might care about which would be a read-only process 
> overwriting the lock holder's "pidfile"

With Docker becomes more popular, it's more common to have these situations
nowadays. So I think we have to deal with pid namespaces.

The lock file is not that fragile - normal Python code crash won't cause it
to be orphaned because the interpretor still executes "finally" block, it
has to be the crash of the interpretor or a forced kill.

> These points do raise the complexity and risk of my proposed approach. I 
> would rather have this information *somehow* than worrying about exactly 
> how, and the current approach in this series seems lower risk. So I'm 
> okay going with the approach suggested in these patches (but with fixes 
> for the issues raised).
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Ryan McElroy]

On 3/21/17 4:40 PM, Jun Wu wrote:

Excerpts from Ryan McElroy's message of 2017-03-20 12:39:11 +:

Overall, I like the functionality this series adds, but I'm less
convinced on the specific implementation. Adding more stuff to the
junkyard that is the "ui" class doesn't feel awesome, and I'm concerned
with how it will interact with chg and a future direction of ui
immutability.

I have an alternate suggestion for how to achieve this visibility into
what is going on in a repo:

Instead of having this information inside of a lock, what if every
process that started up in a repo tried to create a file in
.hg/processes/ (or something) of the form "info.". Then you
wouldn't need to pass this data through to the lock command, wouldn't
need to plumb it through the ui class, and combining the lock info with
the processes info would still be straightforward.

This would allow you to touch only the dispatch logic and achieve
something even cooler: you'd be able to see all of the processes running
in a repo (even read processes, as long as the reader has write access
to the repo's .hg/).

Thoughts on this alternate approach?

Two questions:

1. Who is responsible for garbage collecting those pid files, in case of
crashes?
2. Linux has pid namespace, pid could conflict.

I think it's better to just rely on whatever the operating system provides
to convert pid to args.



1. Could be the same as abandoned lockfile cleanup (or maybe we just 
leave them as graves honoring the processes that came and died in this 
repo -- joking)
2. Already breaks mercurial locks badly due to abandoned lockfile 
cleanup, so I'm a little less concerned about that -- but there is a 
case that I might care about which would be a read-only process 
overwriting the lock holder's "pidfile"


These points do raise the complexity and risk of my proposed approach. I 
would rather have this information *somehow* than worrying about exactly 
how, and the current approach in this series seems lower risk. So I'm 
okay going with the approach suggested in these patches (but with fixes 
for the issues raised).

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

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Jun Wu]

Excerpts from Ryan McElroy's message of 2017-03-20 12:39:11 +:
> Overall, I like the functionality this series adds, but I'm less 
> convinced on the specific implementation. Adding more stuff to the 
> junkyard that is the "ui" class doesn't feel awesome, and I'm concerned 
> with how it will interact with chg and a future direction of ui 
> immutability.
> 
> I have an alternate suggestion for how to achieve this visibility into 
> what is going on in a repo:
> 
> Instead of having this information inside of a lock, what if every 
> process that started up in a repo tried to create a file in 
> .hg/processes/ (or something) of the form "info.". Then you 
> wouldn't need to pass this data through to the lock command, wouldn't 
> need to plumb it through the ui class, and combining the lock info with 
> the processes info would still be straightforward.
>
> This would allow you to touch only the dispatch logic and achieve 
> something even cooler: you'd be able to see all of the processes running 
> in a repo (even read processes, as long as the reader has write access 
> to the repo's .hg/).
> 
> Thoughts on this alternate approach?

Two questions:

1. Who is responsible for garbage collecting those pid files, in case of
   crashes?
2. Linux has pid namespace, pid could conflict.

I think it's better to just rely on whatever the operating system provides
to convert pid to args.

> 
> Some nitpicks inline as well.
> 
> On 3/20/17 7:49 AM, Phil Cohen wrote:
> > # HG changeset patch
> > # User Phil Cohen 
> > # Date 1489996027 25200
> > #  Mon Mar 20 00:47:07 2017 -0700
> > # Node ID f37121209a2bbcde572e986f2b038bf2da7f954a
> > # Parent  1ca023fb02cbe4747e2b5b625866cfa538cbebd3
> > localrepo: pass args and command running as store/write lock metadata
> >
> > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> > --- a/mercurial/localrepo.py
> > +++ b/mercurial/localrepo.py
> > @@ -10,6 +10,7 @@
> >   import errno
> >   import hashlib
> >   import inspect
> > +import json
> >   import os
> >   import random
> >   import time
> > @@ -1355,10 +1356,15 @@
> >   if parentenvvar is not None:
> >   parentlock = encoding.environ.get(parentenvvar)
> >   try:
> > +metadata = json.dumps({
> > +'cmd': self.ui.commandname,
> > +'args': self.ui.args
> > +})
> >   l = lockmod.lock(vfs, lockname, 0, releasefn=releasefn,
> >acquirefn=acquirefn, desc=desc,
> >inheritchecker=inheritchecker,
> > - parentlock=parentlock)
> > + parentlock=parentlock,
> > + metadata=metadata)
> 
> It wasn't clear to me that metadata should be a string and not arbitrary 
> data that would be "somehow" serialized. It might be good to name (in 
> the previous patch) the variable "infostring" rather than "metadata" so 
> that this is more self-documenting.
> 
> Alternatively, have the lock code do the JSON serialization, and allow 
> metadata to be a list of dict of data that the lock will do the 
> serialization for, if present. That help's guarantee that lock.info will 
> always contain JSON serialized data.
> 
> >   except error.LockHeld as inst:
> >   if not wait:
> >   raise
> > diff --git a/tests/test-repo-lock-writes-metadata.t 
> > b/tests/test-repo-lock-writes-metadata.t
> > new file mode 100644
> > --- /dev/null
> > +++ b/tests/test-repo-lock-writes-metadata.t
> > @@ -0,0 +1,38 @@
> > +Make a repo
> > +
> > +  $ hg init test
> > +  $ cd test
> > +  $ echo file > file
> > +  $ hg commit -Aqm initial
> 
> Nit-pick: I'd love some whitespace after the previous test instead of 
> before the next test.
> 
> > +Make an extension
> > +
> > +  $ cat << EOT > lockinfo.py
> > +  > from contextlib import nested
> > +  > from mercurial import (cmdutil, extensions)
> > +  > cmdtable = {}
> > +  > command = cmdutil.command(cmdtable)
> > +  > @command('locktest')
> > +  > def locktest(ui, repo, *args, **opts):
> > +  >   def readf(path):
> > +  >  try:
> > +  >with open(path, "r") as f:
> > +  >   return f.read()
> > +  >  except IOError as e:
> > +  > return ""
> > +  >   def status(str):
> > +  > print(str, readf(".hg/wlock.info"), readf(".hg/store/lock.info"))
> > +  >   status("No lock, shouldn't have either file:")
> > +  >   with repo.wlock():
> > +  >   status("Have the write lock:")
> > +  >   with repo.lock():
> > +  > status("Also have the store lock:")
> > +  >   status("Just the write lock:")
> > +  >   status("Neither:")
> > +  > EOT
> > +Test store and write lock
> > +  $ hg locktest -v --config extensions.x=lockinfo.py
> > +  ("No lock, shouldn't have either file:", '', '')
> > +  ('Have the write lock:', '{"cmd": "locktest", "args": ["locktest", 
> > "-v"]}', '')
> > +  ('Also 

Re: [PATCH 1 of 3] dispatch: store args and command run on ui

[Yuya Nishihara]

On Mon, 20 Mar 2017 00:49:05 -0700, Phil Cohen wrote:
> # HG changeset patch
> # User Phil Cohen 
> # Date 1489995201 25200
> #  Mon Mar 20 00:33:21 2017 -0700
> # Node ID 2c1d5a02ec533055f182b0cc1280107fbfb76206
> # Parent  291951ad070b3fa39dd1d83503aa1011a20d9a21
> dispatch: store args and command run on ui

> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -160,6 +160,8 @@
>  self._colormode = None
>  self._terminfoparams = {}
>  self._styles = {}
> +self.commandname = None
> +self.args = None
>  
>  if src:
>  self.fout = src.fout

Perhaps ui should copy src.args to self if given.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] localrepo: pass args and command running as store/write lock metadata

[Yuya Nishihara]

On Mon, 20 Mar 2017 00:49:07 -0700, Phil Cohen wrote:
> # HG changeset patch
> # User Phil Cohen 
> # Date 1489996027 25200
> #  Mon Mar 20 00:47:07 2017 -0700
> # Node ID f37121209a2bbcde572e986f2b038bf2da7f954a
> # Parent  1ca023fb02cbe4747e2b5b625866cfa538cbebd3
> localrepo: pass args and command running as store/write lock metadata
> 
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -10,6 +10,7 @@
>  import errno
>  import hashlib
>  import inspect
> +import json
>  import os
>  import random
>  import time
> @@ -1355,10 +1356,15 @@
>  if parentenvvar is not None:
>  parentlock = encoding.environ.get(parentenvvar)
>  try:
> +metadata = json.dumps({
> +'cmd': self.ui.commandname,
> +'args': self.ui.args
> +})

json requires commandname and args are valid unicode, but they aren't. It's 90%
wrong to use the json module directly in Mercurial codebase.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] rebase: use matcher to optimize manifestmerge

[Yuya Nishihara]

On Mon, 20 Mar 2017 19:10:08 -0700, Durham Goode wrote:
> On 3/20/17 6:29 PM, Yuya Nishihara wrote:
> > On Mon, 20 Mar 2017 17:14:38 -0700, Durham Goode wrote:
> >> On 3/20/17 1:14 AM, Yuya Nishihara wrote:
> >>> On Sun, 19 Mar 2017 12:00:58 -0700, Durham Goode wrote:
>  # HG changeset patch
>  # User Durham Goode 
>  # Date 1489949694 25200
>  #  Sun Mar 19 11:54:54 2017 -0700
>  # Node ID 800c452bf1a44f9f817174c69443121f4ed4c3b8
>  # Parent  d598e42fa629195ecf43f438b71603df9fb66d6d
>  rebase: use matcher to optimize manifestmerge
> 
>  The old merge code would call manifestmerge and calculate the complete 
>  diff
>  between the source to the destination. In many cases, like rebase, the 
>  vast
>  majority of differences between the source and destination are irrelevant
>  because they are differences between the destination and the common 
>  ancestor
>  only, and therefore don't affect the merge. Since most actions are 
>  'keep', all
>  the effort to compute them is wasted.
> 
>  Instead, let's compute the difference between the source and the common 
>  ancestor
>  and only perform the diff of those files against the merge destination. 
>  When
>  using treemanifest, this lets us avoid loading almost the entire tree 
>  when
>  rebasing from a very old ancestor. This speeds up rebase of an old stack 
>  of 27
>  commits by 20x.
> >>>
> >>> Looks generally good to me, but this needs more eyes.
> >>>
>  @@ -819,6 +819,27 @@ def manifestmerge(repo, wctx, p2, pa, br
>   if any(wctx.sub(s).dirty() for s in wctx.substate):
>   m1['.hgsubstate'] = modifiednodeid
> 
>  +# Don't use m2-vs-ma optimization if:
>  +# - ma is the same as m1 or m2, which we're just going to diff 
>  again later
>  +# - The matcher is set already, so we can't override it
>  +# - The caller specifically asks for a full diff, which is useful 
>  during bid
>  +#   merge.
>  +if (pa not in ([wctx, p2] + wctx.parents()) and
>  +matcher is None and not forcefulldiff):
> >>>
> >>> Is this optimization better for normal merge where m2 might be far from 
> >>> m1?
> >>
> >> I'm not sure what you mean by 'normal merge'.  You mean like an 'hg
> >> merge'?  Or like an hg update?  Any merge where you are merging in a
> >> small branch will benefit from this (like hg up @ && hg merge
> >> myfeaturebranch).
> >
> > 'hg merge'. What I had in mind was 'hg up stable && hg merge default' where
> > pa::p2 would be likely to be as large as p1:p2.
> 
> In that case, it may not save any time, but the time would still be 
> O(number of incoming changes).

Right, that wouldn't be a reason to avoid m2-vs-ma optimization.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 1 of 3 v2] posix: use local reference to unlink

[Yuya Nishihara]

On Tue, 21 Mar 2017 06:53:18 -0700, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy 
> # Date 1490104228 25200
> #  Tue Mar 21 06:50:28 2017 -0700
> # Node ID 281fd76b392c38bd5299198810f85f9e3c2034be
> # Parent  527a247f114f8af37326805fd6cce923f9ac6453
> posix: use local reference to unlink

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

Re: [PATCH 10 of 11] vfs: use tryunlink

[Yuya Nishihara]

On Tue, 21 Mar 2017 06:54:32 -0700, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy 
> # Date 1490104228 25200
> #  Tue Mar 21 06:50:28 2017 -0700
> # Node ID ba24c5cdac8654da7d0d510e3643226e69cef187
> # Parent  684d201e116b65e6dff5f9494de064591da57b07
> vfs: use tryunlink
> 
> diff --git a/mercurial/vfs.py b/mercurial/vfs.py
> --- a/mercurial/vfs.py
> +++ b/mercurial/vfs.py
> @@ -401,10 +401,7 @@ class vfs(abstractvfs):
>  def symlink(self, src, dst):
>  self.audit(dst)
>  linkname = self.join(dst)
> -try:
> -os.unlink(linkname)
> -except OSError:
> -pass
> +util.tryunlink(linkname)

I tracked history of this code down to 41ad4105dde9, and there seems no
practical reason to suppress all kinds of OSError. Looks good.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 01 of 11] util: add tryunlink function

[Yuya Nishihara]

On Tue, 21 Mar 2017 06:54:23 -0700, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy 
> # Date 1490104228 25200
> #  Tue Mar 21 06:50:28 2017 -0700
> # Node ID 5c9cdd8046845f76e169885ed490a603b911d0d4
> # Parent  49de4dfb282e2ad4dc91328ffd7fc396ee92b4a0
> util: add tryunlink function

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

Re: [PATCH v2] localrepo: improve vfs documentation

[Yuya Nishihara]

On Tue, 21 Mar 2017 06:51:52 -0700, Ryan McElroy wrote:
> # HG changeset patch
> # User Ryan McElroy 
> # Date 1490104242 25200
> #  Tue Mar 21 06:50:42 2017 -0700
> # Node ID 99b12351af995446ad4f9d52b3fda0b591611e69
> # Parent  527a247f114f8af37326805fd6cce923f9ac6453
> localrepo: improve vfs documentation

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

Re: [PATCH 01 of 11] util: add tryunlink function

[Ryan McElroy]

Argh, I missed --flag v2 on this series. Apologies.

I also split this from the previous three patches because they are 
actually unrelated cleanups.


On 3/21/17 1:54 PM, Ryan McElroy wrote:

# HG changeset patch
# User Ryan McElroy 
# Date 1490104228 25200
#  Tue Mar 21 06:50:28 2017 -0700
# Node ID 5c9cdd8046845f76e169885ed490a603b911d0d4
# Parent  49de4dfb282e2ad4dc91328ffd7fc396ee92b4a0
util: add tryunlink function

Throughout mercurial cdoe, there is a common pattern of attempting to remove
a file and ignoring ENOENT errors. Let's move this into a common function to
allow for cleaner code.

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1617,6 +1617,14 @@ def unlinkpath(f, ignoremissing=False):
  except OSError:
  pass
  
+def tryunlink(f):

+"""Attempt to remove a file, ignoring ENOENT errors."""
+try:
+unlink(f)
+except OSError as e:
+if e.errno != errno.ENOENT:
+raise
+
  def makedirs(name, mode=None, notindexed=False):
  """recursive directory creation with parent mode inheritance
  


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

[PATCH 03 of 11] vfs: add tryunlink method

[Ryan McElroy]

# HG changeset patch
# User Ryan McElroy 
# Date 1490104228 25200
#  Tue Mar 21 06:50:28 2017 -0700
# Node ID 05060c142764cd34c9ad16127bd1b562f9548bd7
# Parent  6652e6d191c677f1188252f1bb4e3e7e88bfcf1c
vfs: add tryunlink method

Thoughout hg code, we see a pattern of attempting to remove a file and then
catching and ignoring any errors due to a missing file in the calling code.
Let's unify this pattern in a single implementation in the vfs layer.

diff --git a/mercurial/vfs.py b/mercurial/vfs.py
--- a/mercurial/vfs.py
+++ b/mercurial/vfs.py
@@ -223,6 +223,10 @@ class abstractvfs(object):
 def unlink(self, path=None):
 return util.unlink(self.join(path))
 
+def tryunlink(self, path=None):
+"""Attempt to remove a file, ignoring missing file errors."""
+util.tryunlink(self.join(path))
+
 def unlinkpath(self, path=None, ignoremissing=False):
 return util.unlinkpath(self.join(path), ignoremissing=ignoremissing)
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 04 of 11] shelve: use tryunlink

[Ryan McElroy]

# HG changeset patch
# User Ryan McElroy 
# Date 1490104228 25200
#  Tue Mar 21 06:50:28 2017 -0700
# Node ID aa0458baf1599d8dc4c35d48609bd64141c38a09
# Parent  05060c142764cd34c9ad16127bd1b562f9548bd7
shelve: use tryunlink

diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -236,11 +236,7 @@ def cleanupoldbackups(repo):
 continue
 base = f[:-(1 + len(patchextension))]
 for ext in shelvefileextensions:
-try:
-vfs.unlink(base + '.' + ext)
-except OSError as err:
-if err.errno != errno.ENOENT:
-raise
+vfs.tryunlink(base + '.' + ext)
 
 def _aborttransaction(repo):
 '''Abort current transaction for shelve/unshelve, but keep dirstate
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 3 of 3 v2] util: unify unlinkpath

[Ryan McElroy]

# HG changeset patch
# User Ryan McElroy 
# Date 1490104228 25200
#  Tue Mar 21 06:50:28 2017 -0700
# Node ID 49de4dfb282e2ad4dc91328ffd7fc396ee92b4a0
# Parent  9fbbf8d286cce1b48b3defc6c50a09596367
util: unify unlinkpath

Previously, there were two slightly different versions of unlinkpath between
windows and posix, but these differences were eliminated in previous patches.
Now we can unify these two code paths inside of the util module.

diff --git a/mercurial/posix.py b/mercurial/posix.py
--- a/mercurial/posix.py
+++ b/mercurial/posix.py
@@ -536,19 +536,6 @@ def gethgcmd():
 def makedir(path, notindexed):
 os.mkdir(path)
 
-def unlinkpath(f, ignoremissing=False):
-"""unlink and remove the directory if it is empty"""
-try:
-unlink(f)
-except OSError as e:
-if not (ignoremissing and e.errno == errno.ENOENT):
-raise
-# try removing directories that might now be empty
-try:
-removedirs(os.path.dirname(f))
-except OSError:
-pass
-
 def lookupreg(key, name=None, scope=None):
 return None
 
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -136,7 +136,6 @@ statislink = platform.statislink
 testpid = platform.testpid
 umask = platform.umask
 unlink = platform.unlink
-unlinkpath = platform.unlinkpath
 username = platform.username
 
 # Python compatibility
@@ -1605,6 +1604,19 @@ class atomictempfile(object):
 else:
 self.close()
 
+def unlinkpath(f, ignoremissing=False):
+"""unlink and remove the directory if it is empty"""
+try:
+unlink(f)
+except OSError as e:
+if not (ignoremissing and e.errno == errno.ENOENT):
+raise
+# try removing directories that might now be empty
+try:
+removedirs(os.path.dirname(f))
+except OSError:
+pass
+
 def makedirs(name, mode=None, notindexed=False):
 """recursive directory creation with parent mode inheritance
 
diff --git a/mercurial/windows.py b/mercurial/windows.py
--- a/mercurial/windows.py
+++ b/mercurial/windows.py
@@ -385,19 +385,6 @@ def removedirs(name):
 break
 head, tail = os.path.split(head)
 
-def unlinkpath(f, ignoremissing=False):
-"""unlink and remove the directory if it is empty"""
-try:
-unlink(f)
-except OSError as e:
-if not (ignoremissing and e.errno == errno.ENOENT):
-raise
-# try removing directories that might now be empty
-try:
-removedirs(os.path.dirname(f))
-except OSError:
-pass
-
 def rename(src, dst):
 '''atomically rename file src to dst, replacing dst if it exists'''
 try:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 1 of 3 v2] posix: use local reference to unlink

[Ryan McElroy]

# HG changeset patch
# User Ryan McElroy 
# Date 1490104228 25200
#  Tue Mar 21 06:50:28 2017 -0700
# Node ID 281fd76b392c38bd5299198810f85f9e3c2034be
# Parent  527a247f114f8af37326805fd6cce923f9ac6453
posix: use local reference to unlink

We have a local reference to os.unlink in module scope, but we still used
os.unlink inside functions. This changes util to use the local reference,
which will pave the way for combining duplicated code in future patches.

diff --git a/mercurial/posix.py b/mercurial/posix.py
--- a/mercurial/posix.py
+++ b/mercurial/posix.py
@@ -105,7 +105,7 @@ def setflags(f, l, x):
 fp = open(f)
 data = fp.read()
 fp.close()
-os.unlink(f)
+unlink(f)
 try:
 os.symlink(data, f)
 except OSError:
@@ -118,7 +118,7 @@ def setflags(f, l, x):
 if stat.S_ISLNK(s):
 # switch link to file
 data = os.readlink(f)
-os.unlink(f)
+unlink(f)
 fp = open(f, "w")
 fp.write(data)
 fp.close()
@@ -187,9 +187,9 @@ def checkexec(path):
 # check-exec is exec and check-no-exec is not exec
 return True
 # checknoexec exists but is exec - delete it
-os.unlink(checknoexec)
+unlink(checknoexec)
 # checkisexec exists but is not exec - delete it
-os.unlink(checkisexec)
+unlink(checkisexec)
 
 # check using one file, leave it as checkisexec
 checkdir = cachedir
@@ -210,7 +210,7 @@ def checkexec(path):
 return True
 finally:
 if fn is not None:
-os.unlink(fn)
+unlink(fn)
 except (IOError, OSError):
 # we don't care, the user probably won't be able to commit anyway
 return False
@@ -248,12 +248,12 @@ def checklink(path):
 try:
 os.symlink(target, name)
 if cachedir is None:
-os.unlink(name)
+unlink(name)
 else:
 try:
 os.rename(name, checklink)
 except OSError:
-os.unlink(name)
+unlink(name)
 return True
 except OSError as inst:
 # link creation might race, try again
@@ -268,7 +268,7 @@ def checklink(path):
 except OSError as inst:
 # sshfs might report failure while successfully creating the link
 if inst[0] == errno.EIO and os.path.exists(name):
-os.unlink(name)
+unlink(name)
 return False
 
 def checkosfilename(path):
@@ -539,7 +539,7 @@ def makedir(path, notindexed):
 def unlinkpath(f, ignoremissing=False):
 """unlink and remove the directory if it is empty"""
 try:
-os.unlink(f)
+unlink(f)
 except OSError as e:
 if not (ignoremissing and e.errno == errno.ENOENT):
 raise
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 2 of 3 v2] posix: use local reference to removedirs

[Ryan McElroy]

# HG changeset patch
# User Ryan McElroy 
# Date 1490104228 25200
#  Tue Mar 21 06:50:28 2017 -0700
# Node ID 9fbbf8d286cce1b48b3defc6c50a09596367
# Parent  281fd76b392c38bd5299198810f85f9e3c2034be
posix: use local reference to removedirs

We have a local reference to os.removedirs in module scope, but we still used
os.removedirs inside functions. This changes util to use the local reference,
which will pave the way for combining duplicated code in future patches.

diff --git a/mercurial/posix.py b/mercurial/posix.py
--- a/mercurial/posix.py
+++ b/mercurial/posix.py
@@ -545,7 +545,7 @@ def unlinkpath(f, ignoremissing=False):
 raise
 # try removing directories that might now be empty
 try:
-os.removedirs(os.path.dirname(f))
+removedirs(os.path.dirname(f))
 except OSError:
 pass
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH v2] localrepo: improve vfs documentation

[Ryan McElroy]

# HG changeset patch
# User Ryan McElroy 
# Date 1490104242 25200
#  Tue Mar 21 06:50:42 2017 -0700
# Node ID 99b12351af995446ad4f9d52b3fda0b591611e69
# Parent  527a247f114f8af37326805fd6cce923f9ac6453
localrepo: improve vfs documentation

At the beginning of March, I promised Yuya that I would follow up a comment I
made on a patch with improved documention for these vfs objects. Also hat tip
to Pierre-Yves for adding the documentation here in the first place.

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -260,11 +260,13 @@ class localrepository(object):
 
 def __init__(self, baseui, path, create=False):
 self.requirements = set()
-# vfs to access the working copy
+# wvfs: rooted at the repository root, used to access the working copy
 self.wvfs = vfsmod.vfs(path, expandpath=True, realpath=True)
-# vfs to access the content of the repository
+# vfs: rooted at .hg, used to access repo files outside of .hg/store
 self.vfs = None
-# vfs to access the store part of the repository
+# svfs: usually rooted at .hg/store, used to access repository history
+# If this is a shared repository, this vfs may point to another
+# repository's .hg/store directory.
 self.svfs = None
 self.root = self.wvfs.base
 self.path = self.wvfs.join(".hg")
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 13 of 14] vfs: use unlink

[Ryan McElroy]

On 3/21/17 11:30 AM, Ryan McElroy wrote:



On 3/21/17 3:28 AM, Jun Wu wrote:

Excerpts from Ryan McElroy's message of 2017-03-20 19:10:56 -0700:

# HG changeset patch
# User Ryan McElroy 
# Date 1490059858 25200
#  Mon Mar 20 18:30:58 2017 -0700
# Node ID 912717e0a0921df8cb8440ce69185850e53788ba
# Parent  f405f3bb1b9686322545a61d2f8df45ff53b4527
vfs: use unlink

diff --git a/mercurial/vfs.py b/mercurial/vfs.py
--- a/mercurial/vfs.py
+++ b/mercurial/vfs.py
@@ -400,10 +400,7 @@ class vfs(abstractvfs):
  def symlink(self, src, dst):
  self.audit(dst)
  linkname = self.join(dst)
-try:
-os.unlink(linkname)
-except OSError:
-pass
+util.tryunlink(linkname)

This could probably be changed to "self.tryunlink(dst)".

Sure, that would work for me too.


Actually, inspecting the code more, this would be a bad change. 
Currently, the join only happens once, and the joined path is reused 
multiple times. It makes more sense to keep it this way.



util.makedirs(os.path.dirname(linkname), self.createmode)


___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=Jw8rundaE7TbmqBYd1txIQ=Nn7tE60VQ1b5sIq5uOLoPMtxQAnZVZmpx7aCKF3wXtg=C_CXEsd2lAxKXz_2kyfusEobHGt_1YioCaenrZ0f_l8= 



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

Re: [PATCH 3 of 5 py3] ui: convert to/from Unicode on Python 3 in ui.editor()

[Yuya Nishihara]

On Mon, 20 Mar 2017 21:56:49 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler 
> # Date 1489901890 14400
> #  Sun Mar 19 01:38:10 2017 -0400
> # Node ID 70ec92cd06b503cdba456f069c0f65bd71987a6e
> # Parent  4136d2f283afab8b143a5392144652c079dedbaf
> ui: convert to/from Unicode on Python 3 in ui.editor()
> 
> I considered making this I/O be done in terms of bytes, but that would
> cause an observable regression for Windows users, as non-binary-mode
> open does EOL conversion there. There are probably new encoding
> dragons lurking here, so we may want to switch to using binary mode
> and doing EOL conversion ourselves.

This is lossy conversion we should avoid, but I've taken this as it will
never break things on Python 2.

Another workaround is to abuse latin-1 unicode as a fat bytes on Python 3:

  f = fdopen(fd, 'w', encoding='latin-1')
  f.write(text.decode('latin-1'))

  f = open(name, 'r', encoding='latin-1')
  f.read().encode('latin-1')
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 1 of 4 V3] histedit: add a method to cleanup nodes safely

[Yuya Nishihara]

On Mon, 20 Mar 2017 17:15:28 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu 
> # Date 1489464645 25200
> #  Mon Mar 13 21:10:45 2017 -0700
> # Node ID 016fcb6e44e0e903d590b63135fa785955068737
> # Parent  44c591f634584c721778c5a77edeb04cd919ac43
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #  hg pull https://bitbucket.org/quark-zju/hg-draft -r 
> 016fcb6e44e0
> histedit: add a method to cleanup nodes safely

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

Re: [PATCH 5 of 5 py3] commit: keep opts as byteskwargs as much as possible

[Yuya Nishihara]

On Tue, 21 Mar 2017 15:51:54 +0530, Pulkit Goyal wrote:
> On Tue, Mar 21, 2017 at 7:26 AM, Augie Fackler  wrote:
> 
> > # HG changeset patch
> > # User Augie Fackler 
> > # Date 1489900808 14400
> > #  Sun Mar 19 01:20:08 2017 -0400
> > # Node ID cdcbc8ac3d4b38099eab928691002e3d004487a5
> > # Parent  6928c4d2776711c0dbf2f37bb9ee7dfbb959b0db
> > commit: keep opts as byteskwargs as much as possible
> >
> > This fixes many flag-handling bugs on 'hg commit' in Python 3.
> >
> 
> ​The series looks good to me. :)​

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

Re: [PATCH] pure: use int instead of long

[Yuya Nishihara]

On Mon, 20 Mar 2017 21:58:20 -0700, Martin von Zweigbergk via Mercurial-devel 
wrote:
> # HG changeset patch
> # User Martin von Zweigbergk 
> # Date 1490071228 25200
> #  Mon Mar 20 21:40:28 2017 -0700
> # Node ID 8360d7a65480d45606ee61ca737beefd2087b731
> # Parent  f42ec07db6a995d972fee8ce2a7fb122c5372109
> pure: use int instead of long
> 
> Similar to the recent 73aa13bc8dac (revlog: use int instead of long,
> 2017-03-19).

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

Re: [PATCH] py3: prove hg status works

[Yuya Nishihara]

On Tue, 21 Mar 2017 07:24:03 +0530, Rishabh Madan wrote:
> # HG changeset patch
> # User Rishabh Madan 
> # Date 1490061133 -19800
> #  Tue Mar 21 07:22:13 2017 +0530
> # Node ID 89d88651eb6f449b8822102a8a300bec9b50a586
> # Parent  b357ba1ce1636181f699706e0648d70ace62e592
> py3: prove hg status works

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

Re: [PATCH 8 of 8] shelve: get rid of ui.backupconfig

[Kostia Balytskyi]

FYI, a very similar patch is a part of the latest shevle series I sent.

On 16/03/2017 21:57, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu 
> # Date 1489699661 25200
> #  Thu Mar 16 14:27:41 2017 -0700
> # Node ID 4f98fa7baebed161fe94c98661a8c404dd82a87d
> # Parent  8f87b407da24ee802e271f517dbc96ad32f21779
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #  hg pull https://bitbucket.org/quark-zju/hg-draft -r 
> 4f98fa7baebe
> shelve: get rid of ui.backupconfig
>
> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -315,15 +315,14 @@ def getcommitfunc(extra, interactive, ed
>   if hasmq:
>   saved, repo.mq.checkapplied = repo.mq.checkapplied, False
> -backup = repo.ui.backupconfig('phases', 'new-commit')
> +override = {('phases', 'new-commit'): phases.secret}
>   try:
> -repo.ui.setconfig('phases', 'new-commit', phases.secret)
>   editor_ = False
>   if editor:
>   editor_ = cmdutil.getcommiteditor(editform='shelve.shelve',
> **opts)
> -return repo.commit(message, shelveuser, opts.get('date'), match,
> -   editor=editor_, extra=extra)
> +with repo.ui.configoverride(override):
> +return repo.commit(message, shelveuser, opts.get('date'),
> +   match, editor=editor_, extra=extra)
>   finally:
> -repo.ui.restoreconfig(backup)
>   if hasmq:
>   repo.mq.checkapplied = saved
> @@ -851,7 +850,5 @@ def _dounshelve(ui, repo, *shelved, **op
>   oldquiet = ui.quiet
>   lock = tr = None
> -forcemerge = ui.backupconfig('ui', 'forcemerge')
>   try:
> -ui.setconfig('ui', 'forcemerge', opts.get('tool', ''), 'unshelve')
>   lock = repo.lock()
>   
> @@ -867,23 +864,26 @@ def _dounshelve(ui, repo, *shelved, **op
>   # to the original pctx.
>   
> -tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts,
> - tmpwctx)
> -
> -repo, shelvectx = _unshelverestorecommit(ui, repo, basename, 
> oldquiet)
> -_checkunshelveuntrackedproblems(ui, repo, shelvectx)
> -branchtorestore = ''
> -if shelvectx.branch() != shelvectx.p1().branch():
> -branchtorestore = shelvectx.branch()
> +override = {('ui', 'forcemerge'): opts.get('tool', '')}
> +with ui.configoverride(override, 'unshelve'):
> +tmpwctx, addedbefore = _commitworkingcopychanges(ui, repo, opts,
> + tmpwctx)
>   
> -shelvectx = _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev,
> -  basename, pctx, tmpwctx, shelvectx,
> -  branchtorestore)
> -mergefiles(ui, repo, pctx, shelvectx)
> -restorebranch(ui, repo, branchtorestore)
> -_forgetunknownfiles(repo, shelvectx, addedbefore)
> +repo, shelvectx = _unshelverestorecommit(ui, repo, basename,
> + oldquiet)
> +_checkunshelveuntrackedproblems(ui, repo, shelvectx)
> +branchtorestore = ''
> +if shelvectx.branch() != shelvectx.p1().branch():
> +branchtorestore = shelvectx.branch()
>   
> -shelvedstate.clear(repo)
> -_finishunshelve(repo, oldtiprev, tr)
> -unshelvecleanup(ui, repo, basename, opts)
> +shelvectx = _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev,
> +  basename, pctx, tmpwctx,
> +  shelvectx, branchtorestore)
> +mergefiles(ui, repo, pctx, shelvectx)
> +restorebranch(ui, repo, branchtorestore)
> +_forgetunknownfiles(repo, shelvectx, addedbefore)
> +
> +shelvedstate.clear(repo)
> +_finishunshelve(repo, oldtiprev, tr)
> +unshelvecleanup(ui, repo, basename, opts)
>   finally:
>   ui.quiet = oldquiet
> @@ -891,5 +891,4 @@ def _dounshelve(ui, repo, *shelved, **op
>   tr.release()
>   lockmod.release(lock)
> -ui.restoreconfig(forcemerge)
>   
>   @command('shelve',
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

Re: [PATCH 04 of 14] util: add tryunlink function

[Ryan McElroy]

On 3/21/17 3:25 AM, Jun Wu wrote:

Excerpts from Ryan McElroy's message of 2017-03-20 19:10:47 -0700:

# HG changeset patch
# User Ryan McElroy 
# Date 1490059858 25200
#  Mon Mar 20 18:30:58 2017 -0700
# Node ID 6c9e676047b5dc2469cd9c00ca82e9389a287839
# Parent  d35d6cde9c2e428a7e77f35d2cd745696bec072a
util: add tryunlink function

Throughout mercurial cdoe, there is a common pattern of attempting to remove
a file and ignoring ENOENT errors. Let's move this into a common function to
allow for cleaner code.

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1617,6 +1617,19 @@ def unlinkpath(f, ignoremissing=False):
  except OSError:
  pass
  
+def tryunlink(f):

+"""
+Attempt to remove a file, ignoring ENOENT errors. Return True if deleted,
+False otherwise.
+"""
+try:
+unlink(f)
+return True
+except OSError as e:
+if e.errno != errno.ENOENT:
+raise
+return False
+

Is the return value useful? Since it can raise to indicate failure, I think
we can just remove "return"s here. The return value is not used in the
following patches.


There were a few patches I dropped before sending due to test failures 
in largefiles that I'm looking into -- and those were the ones that used 
the return value.


I can drop the return value and clean up a few typos in commit messages 
I noticed in a v2. If I need the return value in the future, it's easy 
enough to add it back.


There are a bunch of additional places that currently use unlinkpath 
(but will never remove the containing directory) that I'd like to use 
with this new method as well, but I didn't want to send out a series of 30.


I'll send a v2 later today. Thanks for the review!


  def makedirs(name, mode=None, notindexed=False):
  """recursive directory creation with parent mode inheritance
  


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

Re: [PATCH 13 of 14] vfs: use unlink

[Ryan McElroy]

On 3/21/17 3:28 AM, Jun Wu wrote:

Excerpts from Ryan McElroy's message of 2017-03-20 19:10:56 -0700:

# HG changeset patch
# User Ryan McElroy 
# Date 1490059858 25200
#  Mon Mar 20 18:30:58 2017 -0700
# Node ID 912717e0a0921df8cb8440ce69185850e53788ba
# Parent  f405f3bb1b9686322545a61d2f8df45ff53b4527
vfs: use unlink

diff --git a/mercurial/vfs.py b/mercurial/vfs.py
--- a/mercurial/vfs.py
+++ b/mercurial/vfs.py
@@ -400,10 +400,7 @@ class vfs(abstractvfs):
  def symlink(self, src, dst):
  self.audit(dst)
  linkname = self.join(dst)
-try:
-os.unlink(linkname)
-except OSError:
-pass
+util.tryunlink(linkname)

This could probably be changed to "self.tryunlink(dst)".

Sure, that would work for me too.


  
  util.makedirs(os.path.dirname(linkname), self.createmode)
  


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

Re: [PATCH 5 of 5 py3] commit: keep opts as byteskwargs as much as possible

[Pulkit Goyal]

On Tue, Mar 21, 2017 at 7:26 AM, Augie Fackler  wrote:

> # HG changeset patch
> # User Augie Fackler 
> # Date 1489900808 14400
> #  Sun Mar 19 01:20:08 2017 -0400
> # Node ID cdcbc8ac3d4b38099eab928691002e3d004487a5
> # Parent  6928c4d2776711c0dbf2f37bb9ee7dfbb959b0db
> commit: keep opts as byteskwargs as much as possible
>
> This fixes many flag-handling bugs on 'hg commit' in Python 3.
>

​The series looks good to me. :)​
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel