[PATCH 3 of 3] revset: leverage internal _rev() function to implement rev()

[Yuya Nishihara]

# HG changeset patch
# User Yuya Nishihara 
# Date 1584765728 -32400
#  Sat Mar 21 13:42:08 2020 +0900
# Node ID 98b60b759e2623db637a2ff68eadd5b7e6a8dc12
# Parent  1c189a6e5aa76bf71d5078597fd01c1f959244a2
revset: leverage internal _rev() function to implement rev()

Now 'rev(n)' is identical to 'present(_rev(n))'.

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2082,19 +2082,11 @@ def removes(repo, subset, x):
 
 @predicate(b'rev(number)', safe=True)
 def rev(repo, subset, x):
-"""Revision with the given numeric identifier.
-"""
-# i18n: "rev" is a keyword
-l = getargs(x, 1, 1, _(b"rev requires one argument"))
+"""Revision with the given numeric identifier."""
 try:
-# i18n: "rev" is a keyword
-l = int(getstring(l[0], _(b"rev requires a number")))
-except (TypeError, ValueError):
-# i18n: "rev" is a keyword
-raise error.ParseError(_(b"rev expects a number"))
-if l not in repo.changelog and l not in _virtualrevs:
+return _rev(repo, subset, x)
+except error.RepoLookupError:
 return baseset()
-return subset & baseset([l])
 
 
 @predicate(b'_rev(number)', safe=True)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 2 of 3] revset: fix crash by repo.revs('%d', tip + 1)

[Yuya Nishihara]

# HG changeset patch
# User Yuya Nishihara 
# Date 1584765579 -32400
#  Sat Mar 21 13:39:39 2020 +0900
# Node ID 1c189a6e5aa76bf71d5078597fd01c1f959244a2
# Parent  d0aa302e184789f1af3048814d23d0a4cedcca15
revset: fix crash by repo.revs('%d', tip + 1)

IndexError shouldn't be raised from a revset predicate. The error message
is copied from scmutil.revsymbol().

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2109,7 +2109,10 @@ def _rev(repo, subset, x):
 # i18n: "rev" is a keyword
 raise error.ParseError(_(b"rev expects a number"))
 if l not in _virtualrevs:
-repo.changelog.node(l)  # check that the rev exists
+try:
+repo.changelog.node(l)  # check that the rev exists
+except IndexError:
+raise error.RepoLookupError(_(b"unknown revision '%d'") % l)
 return subset & baseset([l])
 
 
diff --git a/tests/test-template-functions.t b/tests/test-template-functions.t
--- a/tests/test-template-functions.t
+++ b/tests/test-template-functions.t
@@ -1269,6 +1269,12 @@ default. join() should agree with the de
   2147483647
   $ hg log -T '{revset("%d", rev)}\n' -r'null'
   -1
+  $ hg log -T '{revset("%d", rev + 1)}\n' -r'tip'
+  abort: unknown revision '3'!
+  [255]
+  $ hg log -T '{revset("%d", rev - 1)}\n' -r'null'
+  abort: unknown revision '-2'!
+  [255]
 
 Invalid arguments passed to revset()
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 1 of 3] revset: allow repo.revs('%d', wdirrev)

[Yuya Nishihara]

# HG changeset patch
# User Yuya Nishihara 
# Date 1584764867 -32400
#  Sat Mar 21 13:27:47 2020 +0900
# Node ID d0aa302e184789f1af3048814d23d0a4cedcca15
# Parent  c1c2f6e0cd9f819389c18245d791c906023d9545
revset: allow repo.revs('%d', wdirrev)

Otherwise we can't write repo.revs('null:%d', subset.max()) to build
a smartset covering the range {null .. tip} + {wdir} if subset includes
wdir.

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2108,7 +2108,8 @@ def _rev(repo, subset, x):
 except (TypeError, ValueError):
 # i18n: "rev" is a keyword
 raise error.ParseError(_(b"rev expects a number"))
-repo.changelog.node(l)  # check that the rev exists
+if l not in _virtualrevs:
+repo.changelog.node(l)  # check that the rev exists
 return subset & baseset([l])
 
 
diff --git a/tests/test-template-functions.t b/tests/test-template-functions.t
--- a/tests/test-template-functions.t
+++ b/tests/test-template-functions.t
@@ -1263,6 +1263,13 @@ default. join() should agree with the de
   5:13207e5a10d9fd28ec424934298e176197f2c67f,
   4:bbe44766e73d5f11ed2177f1838de10c53ef3e74
 
+%d parameter handling:
+
+  $ hg log -T '{revset("%d", rev)}\n' -r'wdir()'
+  2147483647
+  $ hg log -T '{revset("%d", rev)}\n' -r'null'
+  -1
+
 Invalid arguments passed to revset()
 
   $ hg log -T '{revset("%whatever", 0)}\n'
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH] phabricator: remove *-argument from _getdrevs()

[Yuya Nishihara]

# HG changeset patch
# User Yuya Nishihara 
# Date 1584766870 -32400
#  Sat Mar 21 14:01:10 2020 +0900
# Node ID c1c2f6e0cd9f819389c18245d791c906023d9545
# Parent  a7f8c657a3f08d1e71ee112f797df011eea7a078
phabricator: remove *-argument from _getdrevs()

It can't take more than one specs arguments per len(*specs).

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1632,19 +1632,19 @@ def getdiffmeta(diff):
 return meta
 
 
-def _getdrevs(ui, stack, *specs):
+def _getdrevs(ui, stack, specs):
 """convert user supplied DREVSPECs into "Differential Revision" dicts
 
 See ``hg help phabread`` for how to specify each DREVSPEC.
 """
-if len(*specs) > 0:
+if len(specs) > 0:
 
 def _formatspec(s):
 if stack:
 s = b':(%s)' % s
 return b'(%s)' % s
 
-spec = b'+'.join(pycompat.maplist(_formatspec, *specs))
+spec = b'+'.join(pycompat.maplist(_formatspec, specs))
 
 drevs = querydrev(ui, spec)
 if drevs:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8318: setup: build C extensions with -Werror=declaration-after-statement

[yuja (Yuya Nishihara)]

yuja added a comment.


  >   MSVC 2008 still needs declarations at the top of the scope.
  
  Can you copy this to the code so we can recall why we enable this weird
  warning option?
  
  > +if os.name != 'nt':
  > +common_cflags = ['-Werror=declaration-after-statement']

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8318/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8318

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: D8318: setup: build C extensions with -Werror=declaration-after-statement

[Yuya Nishihara]

>   MSVC 2008 still needs declarations at the top of the scope.

Can you copy this to the code so we can recall why we enable this weird
warning option?

> +if os.name != 'nt':
> +common_cflags = ['-Werror=declaration-after-statement']
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8281: narrow: escape includepats/excludepats when sending over the wire

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  In D8281#124246 , @spectral 
wrote:
  
  > In D8281#124129 , @durin42 
wrote:
  >
  >> In D8281#124058 , @marmoute 
wrote:
  >>
  >>> Since narrow is still experimental, I don't think we should try too hard 
for backward compatibility. We could introduce a new end-point for a new 
encoding and drop the old one in a couple of version.
  >>
  >> +0, honestly. I won't require it, but I'd really rather we shaved this yak 
_now_ rather than when narrow has even more users.
  >
  > I'm getting a bit frustrated with how much time I've spent on this, made 
worse by the fact that I agree with everything everyone's saying and so it's 
not like I'm frustrated at the review process, just how slow I've been at 
accomplishing this.
  
  I know the feeling, thanks a lot for revisiting you original plan.
  
  > So, before I go down another rabbit hole, here's what I'm thinking:
  >
  > - Server emits a new capability `narrow-exp-1-escaped` (in addition to the 
current `narrow-exp-1`, this is not replacing the existing capability)
  > - wireprototypes's map will change these items from csv to csv.escaped
  > - Compatible clients will detect this capability from the server and send 
items of type csv.escaped during getbundle with keys like 
`.escaped` (ex: `include.escaped`). If the server doesn't support 
csv.escaped, the client sends with the old names (unescaped).
  > - The escaping will be urllibcompat.quote
  > - The server will strip the `.escaped` suffix on the keys, split on comma, 
and urllibcompat.unquote the individual items
  
  This looks overall good to me.
  
  > - I'm *not* expecting to do anything about `\` -> `/` conversion.
  
  Does this means the client side is expected to enforce using `/` as the 
directory separator ?
  
  > Since these are part of `getbundle`, I haven't found a way of doing this 
that's not one of:
  >
  > - a custom escaping mechanism that's backwards compatible
  > - adding a capability and renaming the keys that are sent (so the server 
can tell when it needs to unescape)
  > - having the client always send duplicate items (i.e. send `include` and 
`include.escaped`). I'm not even sure that older servers would tolerate 
receiving keys they aren't expecting.
  
  It would not work. Server would reject unknown arguments to `getbundle`.
  
  > - having the client only escape when necessary (i.e. it includes a comma), 
and then always send the path as include.escaped (which runs into the problem 
of old servers rejecting).
  
  In my opinion, I don't think we get much benefit of conditional escaping. So 
keeping things simple seems better.
  
  > - having the server always unescape and the client always escape. This 
breaks the server's ability to interact with older clients that aren't escaping 
(which we'll need to support for at least a week or two).
  
  As much as I think we don't need strong BC on narrow because it is 
experimental, have a couple of version that can still speak to each other is 
preferable.
  
  > For the non-getbundle parts (I think the wireproto command is 'widen'), we 
can easily make a widen2 or something, but it's probably easier to just keep 
the same command name and do the same thing as in getbundle: detect the 
capability, send as foo.escaped if supported.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8281/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8281

To: spectral, durin42, martinvonz, #hg-reviewers, marmoute
Cc: marmoute, mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8304: cext: move variable declaration to the top of the block for C89 support

[mharbison72 (Matt Harbison)]

mharbison72 added a comment.


  In D8304#124121 , @marmoute 
wrote:
  
  > Great, @mharbison72 can you submit a patch ?
  
  D8318 

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8318: setup: build C extensions with -Werror=declaration-after-statement

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  MSVC 2008 still needs declarations at the top of the scope.  I added it to the
  3rd party code too in case somebody vendors a new version with a problem-
  they'll get an early warning.  Clang seems to ignore this (at least on 10.14
  with Xcode 10), and gcc 7.4 will error out as desired on Ubuntu 18.04.  Thanks
  to Yuya for remembering the name of the option.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8318

AFFECTED FILES
  setup.py

CHANGE DETAILS

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -1268,6 +1268,11 @@
 ]
 common_include_dirs = ['mercurial']
 
+common_cflags = []
+
+if os.name != 'nt':
+common_cflags = ['-Werror=declaration-after-statement']
+
 osutil_cflags = []
 osutil_ldflags = []
 
@@ -1441,18 +1446,21 @@
 'mercurial.cext.base85',
 ['mercurial/cext/base85.c'],
 include_dirs=common_include_dirs,
+extra_compile_args=common_cflags,
 depends=common_depends,
 ),
 Extension(
 'mercurial.cext.bdiff',
 ['mercurial/bdiff.c', 'mercurial/cext/bdiff.c'] + xdiff_srcs,
 include_dirs=common_include_dirs,
+extra_compile_args=common_cflags,
 depends=common_depends + ['mercurial/bdiff.h'] + xdiff_headers,
 ),
 Extension(
 'mercurial.cext.mpatch',
 ['mercurial/mpatch.c', 'mercurial/cext/mpatch.c'],
 include_dirs=common_include_dirs,
+extra_compile_args=common_cflags,
 depends=common_depends,
 ),
 Extension(
@@ -1466,6 +1474,7 @@
 'mercurial/cext/revlog.c',
 ],
 include_dirs=common_include_dirs,
+extra_compile_args=common_cflags,
 depends=common_depends
 + ['mercurial/cext/charencode.h', 'mercurial/cext/revlog.h',],
 ),
@@ -1473,7 +1482,7 @@
 'mercurial.cext.osutil',
 ['mercurial/cext/osutil.c'],
 include_dirs=common_include_dirs,
-extra_compile_args=osutil_cflags,
+extra_compile_args=common_cflags + osutil_cflags,
 extra_link_args=osutil_ldflags,
 depends=common_depends,
 ),
@@ -1482,6 +1491,7 @@
 [
 
'mercurial/thirdparty/zope/interface/_zope_interface_coptimizations.c',
 ],
+extra_compile_args=common_cflags,
 ),
 Extension(
 'mercurial.thirdparty.sha1dc',
@@ -1490,9 +1500,12 @@
 'mercurial/thirdparty/sha1dc/lib/sha1.c',
 'mercurial/thirdparty/sha1dc/lib/ubc_check.c',
 ],
+extra_compile_args=common_cflags,
 ),
 Extension(
-'hgext.fsmonitor.pywatchman.bser', 
['hgext/fsmonitor/pywatchman/bser.c']
+'hgext.fsmonitor.pywatchman.bser',
+['hgext/fsmonitor/pywatchman/bser.c'],
+extra_compile_args=common_cflags,
 ),
 RustStandaloneExtension(
 'mercurial.rustext', 'hg-cpython', 'librusthg', py3_features='python3'
@@ -1503,11 +1516,11 @@
 sys.path.insert(0, 'contrib/python-zstandard')
 import setup_zstd
 
-extmodules.append(
-setup_zstd.get_c_extension(
-name='mercurial.zstd', root=os.path.abspath(os.path.dirname(__file__))
-)
+zstd = setup_zstd.get_c_extension(
+name='mercurial.zstd', root=os.path.abspath(os.path.dirname(__file__))
 )
+zstd.extra_compile_args += common_cflags
+extmodules.append(zstd)
 
 try:
 from distutils import cygwinccompiler



To: mharbison72, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8317: cext: move more variable declarations to the top of the block for C89 support

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  These instances aren't compiled on Windows, so they don't matter much.  But 
they
  do get flagged by `-Werror=declaration-after-statement` in the next patch.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8317

AFFECTED FILES
  mercurial/cext/osutil.c

CHANGE DETAILS

diff --git a/mercurial/cext/osutil.c b/mercurial/cext/osutil.c
--- a/mercurial/cext/osutil.c
+++ b/mercurial/cext/osutil.c
@@ -810,9 +810,10 @@
/* Check the memory we can use. Typically, argv[i] and
 * argv[i + 1] are continuous. */
for (i = 0; i < argc; ++i) {
+   size_t len;
if (argv[i] > argvend || argv[i] < argvstart)
break; /* not continuous */
-   size_t len = strlen(argv[i]);
+   len = strlen(argv[i]);
argvend = argv[i] + len + 1 /* '\0' */;
}
if (argvend > argvstart) /* sanity check */
@@ -1170,9 +1171,9 @@
 {
int sig = 0;
int r;
+   sigset_t set;
if (!PyArg_ParseTuple(args, "i", &sig))
return NULL;
-   sigset_t set;
r = sigemptyset(&set);
if (r != 0)
return PyErr_SetFromErrno(PyExc_OSError);



To: mharbison72, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6846: packaging: script the building of a MacOS installer using a custom python

[mharbison72 (Matt Harbison)]

mharbison72 added a comment.


  In D6846#124054 , @marmoute 
wrote:
  
  > What's the status of this ? @mharbison72  id the linking problem got solved 
?
  
  I've been too busy to get back to this, and probably won't for the short 
term, anyway.
  
  It would probably also be a good to have a general idea when we drop py2 
completely.  I'd be content leaving py2 around for awhile for various reasons, 
but I don't want to sink a bunch of time into something that lasts for one 
release.  (I think a PyOxidizer build would end up being a lot different 
looking than this.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6846/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6846

To: mharbison72, #hg-reviewers
Cc: indygreg, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8281: narrow: escape includepats/excludepats when sending over the wire

[mharbison72 (Matt Harbison)]

mharbison72 added a comment.


  In D8281#124246 , @spectral 
wrote:
  
  > - I'm *not* expecting to do anything about `\` -> `/` conversion.
  
  So would there be some interoperability issue between Windows and not-Windows 
if paths aren't pconverted, if paths can also come from the server as Martin 
mentioned?  Is there anything here that makes it more difficult to pconvert in 
the future?  (I assume it only came up in the first place to allow the custom 
escaping.  I understand your frustration, so I'm not looking to sign you up for 
more work.  But I only know about narrow from a very high conceptual level, so 
I figure I might as well ask now and save this info for later.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8281/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8281

To: spectral, durin42, martinvonz, #hg-reviewers, marmoute
Cc: marmoute, mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8281: narrow: escape includepats/excludepats when sending over the wire

[spectral (Kyle Lippincott)]

spectral added a comment.


  In D8281#124247 , @martinvonz 
wrote:
  
  > In D8281#124246 , @spectral 
wrote:
  >
  >> - Server emits a new capability `narrow-exp-1-escaped` (in addition to the 
current `narrow-exp-1`, this is not replacing the existing capability)
  >
  > nit: I *think* the "1" in the name was supposed to be a version number, so 
the new capability's name would be `narrow-exp-2`.
  
  Yes, I had assumed that as well. This isn't really a new version of the 
protocol, though, just a minor tweak, and it's primarily to the 'csv' type used 
in getbundle (see the current version of this patch that adds 'qcsv' for the 
actual locations it's used). Honestly I went back and forth between announcing 
it as `getbundle-csv-escaped` and something related to narrow (and ended up on 
narrow, as you see, since while it's generically useful besides narrow, nothing 
else needs it today, and future things wouldn't need this to be announced 
forever - they'll always have used foo.escaped and been transmitted as escaped).
  
  > Maybe no one uses the "widen" command so we don't even need to worry about 
compatibility there?
  
  I don't know how often it's used :) I just know that there's something *not* 
getbundle-related in narrowwirepeer.py (looks like it's called `narrow_widen` 
that needed to be modified or else the tests wouldn't pass. I honestly don't 
even know if we're using it internally at Google right now. If not, that's 
fewer things to change, which I'm OK with :)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8281/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8281

To: spectral, durin42, martinvonz, #hg-reviewers, marmoute
Cc: marmoute, mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8281: narrow: escape includepats/excludepats when sending over the wire

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  In D8281#124246 , @spectral 
wrote:
  
  > In D8281#124129 , @durin42 
wrote:
  >
  >> In D8281#124058 , @marmoute 
wrote:
  >>
  >>> Since narrow is still experimental, I don't think we should try too hard 
for backward compatibility. We could introduce a new end-point for a new 
encoding and drop the old one in a couple of version.
  >>
  >> +0, honestly. I won't require it, but I'd really rather we shaved this yak 
_now_ rather than when narrow has even more users.
  >
  > I'm getting a bit frustrated with how much time I've spent on this, made 
worse by the fact that I agree with everything everyone's saying and so it's 
not like I'm frustrated at the review process, just how slow I've been at 
accomplishing this. 
  > So, before I go down another rabbit hole, here's what I'm thinking:
  >
  > - Server emits a new capability `narrow-exp-1-escaped` (in addition to the 
current `narrow-exp-1`, this is not replacing the existing capability)
  
  nit: I *think* the "1" in the name was supposed to be a version number, so 
the new capability's name would be `narrow-exp-2`.
  
  > - wireprototypes's map will change these items from csv to csv.escaped
  > - Compatible clients will detect this capability from the server and send 
items of type csv.escaped during getbundle with keys like 
`.escaped` (ex: `include.escaped`). If the server doesn't support 
csv.escaped, the client sends with the old names (unescaped).
  > - The escaping will be urllibcompat.quote
  > - The server will strip the `.escaped` suffix on the keys, split on comma, 
and urllibcompat.unquote the individual items
  > - I'm *not* expecting to do anything about `\` -> `/` conversion.
  
  This all sounds good to me.
  
  > Since these are part of `getbundle`, I haven't found a way of doing this 
that's not one of:
  >
  > - a custom escaping mechanism that's backwards compatible
  > - adding a capability and renaming the keys that are sent (so the server 
can tell when it needs to unescape)
  > - having the client always send duplicate items (i.e. send `include` and 
`include.escaped`). I'm not even sure that older servers would tolerate 
receiving keys they aren't expecting.
  > - having the client only escape when necessary (i.e. it includes a comma), 
and then always send the path as include.escaped (which runs into the problem 
of old servers rejecting).
  > - having the server always unescape and the client always escape. This 
breaks the server's ability to interact with older clients that aren't escaping 
(which we'll need to support for at least a week or two).
  >
  > For the non-getbundle parts (I think the wireproto command is 'widen'), we 
can easily make a widen2 or something, but it's probably easier to just keep 
the same command name and do the same thing as in getbundle: detect the 
capability, send as foo.escaped if supported.
  
  Maybe no one uses the "widen" command so we don't even need to worry about 
compatibility there?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8281/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8281

To: spectral, durin42, martinvonz, #hg-reviewers, marmoute
Cc: marmoute, mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8281: narrow: escape includepats/excludepats when sending over the wire

[spectral (Kyle Lippincott)]

spectral added a comment.


  In D8281#124129 , @durin42 wrote:
  
  > In D8281#124058 , @marmoute 
wrote:
  >
  >> Since narrow is still experimental, I don't think we should try too hard 
for backward compatibility. We could introduce a new end-point for a new 
encoding and drop the old one in a couple of version.
  >
  > +0, honestly. I won't require it, but I'd really rather we shaved this yak 
_now_ rather than when narrow has even more users.
  
  I'm getting a bit frustrated with how much time I've spent on this, made 
worse by the fact that I agree with everything everyone's saying and so it's 
not like I'm frustrated at the review process, just how slow I've been at 
accomplishing this.
  
  So, before I go down another rabbit hole, here's what I'm thinking:
  
  - Server emits a new capability `narrow-exp-1-escaped` (in addition to the 
current `narrow-exp-1`, this is not replacing the existing capability)
  - wireprototypes's map will change these items from csv to csv.escaped
  - Compatible clients will detect this capability from the server and send 
items of type csv.escaped during getbundle with keys like 
`.escaped` (ex: `include.escaped`). If the server doesn't support 
csv.escaped, the client sends with the old names (unescaped).
  - The escaping will be urllibcompat.quote
  - The server will strip the `.escaped` suffix on the keys, split on comma, 
and urllibcompat.unquote the individual items
  - I'm *not* expecting to do anything about `\` -> `/` conversion.
  
  Since these are part of `getbundle`, I haven't found a way of doing this 
that's not one of:
  
  - a custom escaping mechanism that's backwards compatible
  - adding a capability and renaming the keys that are sent (so the server can 
tell when it needs to unescape)
  - having the client always send duplicate items (i.e. send `include` and 
`include.escaped`). I'm not even sure that older servers would tolerate 
receiving keys they aren't expecting.
  - having the client only escape when necessary (i.e. it includes a comma), 
and then always send the path as include.escaped (which runs into the problem 
of old servers rejecting).
  - having the server always unescape and the client always escape. This breaks 
the server's ability to interact with older clients that aren't escaping (which 
we'll need to support for at least a week or two).
  
  For the non-getbundle parts (I think the wireproto command is 'widen'), we 
can easily make a widen2 or something, but it's probably easier to just keep 
the same command name and do the same thing as in getbundle: detect the 
capability, send as foo.escaped if supported.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8281/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8281

To: spectral, durin42, martinvonz, #hg-reviewers, marmoute
Cc: marmoute, mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8316: testlib: adjust wait-on-file timeout according to the global test timeout

[marmoute (Pierre-Yves David)]

marmoute created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Lets assume that if test timeout have been set to be twice as long, it means
  local timeout should be twice as long too.
  
  I am not aware of any case were extending timeout for file based 
synchronisation
  was necessary, but the safety seems simple to implements.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8316

AFFECTED FILES
  tests/run-tests.py
  tests/testlib/wait-on-file

CHANGE DETAILS

diff --git a/tests/testlib/wait-on-file b/tests/testlib/wait-on-file
--- a/tests/testlib/wait-on-file
+++ b/tests/testlib/wait-on-file
@@ -10,6 +10,12 @@
 fi
 
 timer="$1"
+
+# if the test timeout have been extended, explicitly extend the provided timer
+if [ "$HGTEST_TIMEOUT_DEFAULT" -lt "$HGTEST_TIMEOUT" ]; then
+timer=$(( ("$timer" * "$HGTEST_TIMEOUT") / $HGTEST_TIMEOUT_DEFAULT ))
+fi
+
 wait_on="$2"
 create=""
 if [ $# -eq 3 ]; then
diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -321,7 +321,7 @@
 if 'java' in sys.platform:
 IMPL_PATH = b'JYTHONPATH'
 
-defaults = {
+default_defaults = {
 'jobs': ('HGTEST_JOBS', multiprocessing.cpu_count()),
 'timeout': ('HGTEST_TIMEOUT', 180),
 'slowtimeout': ('HGTEST_SLOWTIMEOUT', 1500),
@@ -329,6 +329,8 @@
 'shell': ('HGTEST_SHELL', 'sh'),
 }
 
+defaults = default_defaults.copy()
+
 
 def canonpath(path):
 return os.path.realpath(os.path.expanduser(path))
@@ -1322,6 +1324,9 @@
 env['TESTTMP'] = _bytes2sys(self._testtmp)
 env['TESTNAME'] = self.name
 env['HOME'] = _bytes2sys(self._testtmp)
+formated_timeout = _bytes2sys(b"%d" % default_defaults['timeout'][1])
+env['HGTEST_TIMEOUT_DEFAULT'] = formated_timeout
+env['HGTEST_TIMEOUT'] = _bytes2sys(b"%d" % self._timeout)
 # This number should match portneeded in _getport
 for port in xrange(3):
 # This list should be parallel to _portmap in _getreplacements



To: marmoute, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8189: testlib: add a small scrip to help process to synchronise using file

[marmoute (Pierre-Yves David)]

marmoute updated this revision to Diff 20856.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8189?vs=20392&id=20856

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8189/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8189

AFFECTED FILES
  tests/testlib/wait-on-file

CHANGE DETAILS

diff --git a/tests/testlib/wait-on-file b/tests/testlib/wait-on-file
new file mode 100755
--- /dev/null
+++ b/tests/testlib/wait-on-file
@@ -0,0 +1,32 @@
+#!/bin/bash
+#
+# wait up to TIMEOUT seconds until a WAIT_ON_FILE is created.
+#
+# In addition, this script can create CREATE_FILE once it is ready to wait.
+
+if [ $# -lt 2 ] || [ $# -gt 3 ]; then
+echo $#
+echo "USAGE: $0 TIMEOUT WAIT_ON_FILE [CREATE_FILE]"
+fi
+
+timer="$1"
+wait_on="$2"
+create=""
+if [ $# -eq 3 ]; then
+create="$3"
+fi
+
+if [ -n "$create" ];
+then
+touch "$create"
+create=""
+fi
+while [ "$timer" -gt 0 ] && [ ! -f "$wait_on" ];
+do
+timer=$(( timer - 1))
+sleep 0.01
+done
+if [ "$timer" -le 0 ]; then
+echo "file not created after $1 seconds: $wait_on" >&2
+exit 1
+fi



To: marmoute, #hg-reviewers
Cc: durin42, mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8190: nodemap: test that concurrent process don't see the pending transaction

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  In D8190#124147 , @durin42 wrote:
  
  > In D8190#124146 , @marmoute 
wrote:
  >
  >> In D8190#124141 , @martinvonz 
wrote:
  >>
  >>> 
  >>
  >> This new patch works, but I don't like the idea of not testing two fully 
independant process. Running a process through hooks is quite different and who 
know how that semantic could evolve.
  >
  > Can we wrap the `hg` invocation in the script in enough `env -u VAR` 
business that we can know it won't see the pending transaction? That seems like 
it ought to be eminently doable. In fact, it might even merit an environment 
variable of its own, eg `HG_IGNORE_PENDING_TXN=1 hg ...` for use in other 
scripts.
  
  Invocation of independant process will always be easier to trust dans 
something spawn by the process we are racing with. In addition, hooks are not 
as fine grained as we needs for some of the race condition we have to checks. 
So inter-process signaling will be needed and so far doing this through the 
file system has proven a solid and reliable approach. So having a generic 
solution for this kind of synchronisation and getting an official helper for it 
seems valuable. Since the approach is suitable here, I don't see a reason not 
to use it.
  
  >> The feature provided by the `wait-on-file` script is useful and the 
approach have been sucessfully used for a while in the Mercurial codebase (eg: 
test-push-race.t, test-bookmarks-corner-case.t, …). and extensions. The need 
for such fine grained synchronisation will appears again, and that script will 
be needed again. So I would rather use that script with the current semantic 
(and whaterver reasonable update are requested to the implementation) and keep 
two fully independant processes in that tests.
  >
  > I'm not terribly swayed by this argument: I've invested nonzero time in 
*fixing* flakes that use this (anti-)pattern, so I'd like to avoid 
proliferating it. Encoding it as a helper script pushes it towards being an 
acceptable pattern that folks will use without critical thought. :(
  
  I am not aware of case where this pattern (communication between concurrent 
process using the file system) was problematic. Do you have any example to 
point to?
  
  As far as I understand, you main concern with the current script was the use 
of some (quite long) timeout to catch up stall situation. You suggested to use 
the global test timeout, a solution currently unsuitable because of issue6125 
 (and probably less 
developper friendly overall). Having a standard script to do process 
synchronisation through the FS (An approach perfectly fine as far as I know) 
mean we could adjust all users in one go (eg: timeout adjustement depending on 
the global test timeout, removal of the timeout, etc…).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8190/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8190

To: marmoute, #hg-reviewers
Cc: martinvonz, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8189: testlib: add a small scrip to help process to synchronise using file

[marmoute (Pierre-Yves David)]

marmoute added a comment.
marmoute added a subscriber: durin42.


  In D8189#123522 , @marmoute 
wrote:
  
  > In D8189#123323 , @durin42 
wrote:
  >
  >> […]
  >> I'm also not happy about the 1-second floor this puts on the step. Doesn't 
sleep(1) support sub-second sleeps on all platforms at this point?
  >
  > I am extremly sad too. But last time I checked, it was still not the case. 
We detect plateform and use small increment on better plateform (but I would 
rather follow up for that).
  
  Good news, even is sub-second is not expect to work on all plateforms, I 
found out that we already use it in our test suite. So any plateform that does 
not support it are already broken. I'll send an update soon.
  
  I am adding a small change to have the local timeout adjust itself according 
to the global time out. I am not aware of real live issues with the local 
timeout, but adding that logic is simple enough.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8189/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8189

To: marmoute, #hg-reviewers
Cc: durin42, mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8315: rust-status: only involve ignore mechanism when needed

[Raphaël Gomès]

Alphare created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This prevents unnecessary fallbacks to Python, improving performance for
  `hg update` for instance.
  
  On Mozilla-Central a noop update goes from 1.6s down to 700ms.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8315

AFFECTED FILES
  rust/hg-core/src/dirstate/status.rs
  rust/hg-core/src/matchers.rs
  rust/hg-cpython/src/dirstate/status.rs

CHANGE DETAILS

diff --git a/rust/hg-cpython/src/dirstate/status.rs 
b/rust/hg-cpython/src/dirstate/status.rs
--- a/rust/hg-cpython/src/dirstate/status.rs
+++ b/rust/hg-cpython/src/dirstate/status.rs
@@ -127,7 +127,7 @@
 &dmap,
 &matcher,
 &root_dir,
-&ignore_files,
+ignore_files,
 StatusOptions {
 check_exec,
 last_normal_time,
@@ -163,7 +163,7 @@
 &dmap,
 &matcher,
 &root_dir,
-&ignore_files,
+ignore_files,
 StatusOptions {
 check_exec,
 last_normal_time,
@@ -217,7 +217,7 @@
 &dmap,
 &matcher,
 &root_dir,
-&ignore_files,
+ignore_files,
 StatusOptions {
 check_exec,
 last_normal_time,
diff --git a/rust/hg-core/src/matchers.rs b/rust/hg-core/src/matchers.rs
--- a/rust/hg-core/src/matchers.rs
+++ b/rust/hg-core/src/matchers.rs
@@ -24,12 +24,12 @@
 PatternSyntax,
 };
 
-use micro_timer::timed;
+use std::borrow::ToOwned;
 use std::collections::HashSet;
 use std::fmt::{Display, Error, Formatter};
 use std::iter::FromIterator;
 use std::ops::Deref;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 #[derive(Debug, PartialEq)]
 pub enum VisitChildrenSet<'a> {
@@ -478,7 +478,8 @@
 let mut prefixes = vec![];
 
 for SubInclude { prefix, root, path } in subincludes.into_iter() {
-let (match_fn, warnings) = get_ignore_function(&[path], root)?;
+let (match_fn, warnings) =
+get_ignore_function(vec![path.to_path_buf()], root)?;
 all_warnings.extend(warnings);
 prefixes.push(prefix.to_owned());
 submatchers.insert(prefix.to_owned(), match_fn);
@@ -549,12 +550,11 @@
 /// Parses all "ignore" files with their recursive includes and returns a
 /// function that checks whether a given file (in the general sense) should be
 /// ignored.
-#[timed]
 pub fn get_ignore_function<'a>(
-all_pattern_files: &[impl AsRef],
+all_pattern_files: Vec,
 root_dir: impl AsRef,
 ) -> PatternResult<(
-impl for<'r> Fn(&'r HgPath) -> bool + Sync,
+Box Fn(&'r HgPath) -> bool + Sync + 'a>,
 Vec,
 )> {
 let mut all_patterns = vec![];
@@ -564,12 +564,15 @@
 let (patterns, warnings) =
 get_patterns_from_file(pattern_file, &root_dir)?;
 
-all_patterns.extend(patterns);
+all_patterns.extend(patterns.to_owned());
 all_warnings.extend(warnings);
 }
 let (matcher, warnings) = IncludeMatcher::new(all_patterns, root_dir)?;
 all_warnings.extend(warnings);
-Ok((move |path: &HgPath| matcher.matches(path), all_warnings))
+Ok((
+Box::new(move |path: &HgPath| matcher.matches(path)),
+all_warnings,
+))
 }
 
 impl<'a> IncludeMatcher<'a> {
diff --git a/rust/hg-core/src/dirstate/status.rs 
b/rust/hg-core/src/dirstate/status.rs
--- a/rust/hg-core/src/dirstate/status.rs
+++ b/rust/hg-core/src/dirstate/status.rs
@@ -33,7 +33,7 @@
 fs::{read_dir, DirEntry},
 io::ErrorKind,
 ops::Deref,
-path::Path,
+path::{Path, PathBuf},
 };
 
 /// Wrong type of file from a `BadMatch`
@@ -94,6 +94,9 @@
 }
 
 type IoResult = std::io::Result;
+/// `Box` is syntactic sugar for `Box`, so add
+/// an explicit lifetime here to not fight `'static` bounds "out of nowhere".
+type IgnoreFnType<'a> = Box Fn(&'r HgPath) -> bool + Sync + 'a>;
 
 /// Dates and times that are outside the 31-bit signed range are compared
 /// modulo 2^31. This should prevent hg from behaving badly with very large
@@ -312,8 +315,8 @@
 root_dir: impl AsRef + Sync + Send + Copy + 'a,
 dmap: &'a DirstateMap,
 old_results: &'a FastHashMap, Dispatch>,
-ignore_fn: &'a (impl for<'r> Fn(&'r HgPath) -> bool + Sync),
-dir_ignore_fn: &'a (impl for<'r> Fn(&'r HgPath) -> bool + Sync),
+ignore_fn: &'a IgnoreFnType,
+dir_ignore_fn: &'a IgnoreFnType,
 options: StatusOptions,
 filename: HgPathBuf,
 dir_entry: DirEntry,
@@ -393,8 +396,8 @@
 root_dir: impl AsRef + Sync + Send + Copy + 'a,
 dmap: &'a DirstateMap,
 old_results: &'a FastHashMap, Dispatch>,
-ignore_fn: &'a (impl for<'r> Fn(&'r HgPath) -> bool + Sync),
-dir_ignore_fn:

D8313: phabricator: extract logic to print the status when posting a commit

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This will make it easier to list each commit when folding.  That makes the
  output less confusing because it matches the output of `--confirm` and the
  revisions listed on the command line.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8313

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1163,6 +1163,26 @@
 return [entry[b'phid'] for entry in data]
 
 
+def _print_phabsend_action(ui, ctx, newrevid, action):
+"""print the ``action`` that occurred when posting ``ctx`` for review
+
+This is a utility function for the sending phase of ``phabsend``, which
+makes it easier to show a status for all local commits with `--fold``.
+"""
+actiondesc = ui.label(
+{
+b'created': _(b'created'),
+b'skipped': _(b'skipped'),
+b'updated': _(b'updated'),
+}[action],
+b'phabricator.action.%s' % action,
+)
+drevdesc = ui.label(b'D%d' % newrevid, b'phabricator.drev')
+nodedesc = ui.label(bytes(ctx), b'phabricator.node')
+desc = ui.label(ctx.description().split(b'\n')[0], b'phabricator.desc')
+ui.write(_(b'%s - %s - %s: %s\n') % (drevdesc, actiondesc, nodedesc, desc))
+
+
 def _amend_diff_properties(unfi, drevid, newnodes, diff):
 """update the local commit list for the ``diff`` associated with ``drevid``
 
@@ -1312,23 +1332,11 @@
 newrevid = revid
 action = b'skipped'
 
-actiondesc = ui.label(
-{
-b'created': _(b'created'),
-b'skipped': _(b'skipped'),
-b'updated': _(b'updated'),
-}[action],
-b'phabricator.action.%s' % action,
-)
-drevdesc = ui.label(b'D%d' % newrevid, b'phabricator.drev')
-nodedesc = ui.label(bytes(ctx), b'phabricator.node')
-desc = ui.label(ctx.description().split(b'\n')[0], b'phabricator.desc')
-ui.write(
-_(b'%s - %s - %s: %s\n') % (drevdesc, actiondesc, nodedesc, desc)
-)
 drevids.append(newrevid)
 lastrevphid = newrevphid
 
+_print_phabsend_action(ui, ctx, newrevid, action)
+
 # Update commit messages and remove tags
 if opts.get(b'amend'):
 unfi = repo.unfiltered()



To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8312: phabricator: extract the logic to amend diff properties to a function

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This will be needed on a separate code path when dealing with folding 
revisions.
  And since we know that will involve adding multiple local commmits to the diff
  properties instead of just one, restructure the logic slightly to allow it.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8312

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1163,6 +1163,20 @@
 return [entry[b'phid'] for entry in data]
 
 
+def _amend_diff_properties(unfi, drevid, newnodes, diff):
+"""update the local commit list for the ``diff`` associated with ``drevid``
+
+This is a utility function for the amend phase of ``phabsend``, which
+converts failures to warning messages.
+"""
+try:
+writediffproperties([unfi[newnode] for newnode in newnodes], diff)
+except util.urlerr.urlerror:
+# If it fails just warn and keep going, otherwise the DREV
+# associations will be lost
+unfi.ui.warnnoi18n(b'Failed to update metadata for D%d\n' % drevid)
+
+
 @vcrcommand(
 b'phabsend',
 [
@@ -1352,17 +1366,10 @@
 newnode = new.commit()
 
 mapping[old.node()] = [newnode]
-# Update diff property
-# If it fails just warn and keep going, otherwise the DREV
-# associations will be lost
-try:
-writediffproperties(
-[unfi[newnode]], diffmap[old.node()]
-)
-except util.urlerr.urlerror:
-ui.warnnoi18n(
-b'Failed to update metadata for D%d\n' % drevid
-)
+
+_amend_diff_properties(
+unfi, drevid, [newnode], diffmap[old.node()]
+)
 # Remove local tags since it's no longer necessary
 tagname = b'D%d' % drevid
 if tagname in repo.tags():



To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8311: phabricator: teach `getoldnodedrevmap()` to handle folded reviews

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The tricky part here is reasoning through all of the possible predecessor
  scenarios.  In the typical case of submitting a folded range and then
  resubmitting it (also folded), filtering the list of commits for the diff 
stored
  on Phabricator through the local predecessor list for each single node will
  result in the typical 1:1 mapping to the old node.
  
  There are edge cases like using `hg fold` within the range prior to
  resubmitting, that will result in mapping to multiple old nodes.  In that 
case,
  the first direct predecessor is needed for the base of the diff, and the last
  direct predecessor is needed for the head of the diff in order to make sure 
that
  the entire range is included in the diff content.  And none of this matters 
for
  commits in the middle of the range, as they are never used.
  
  Fortunately the only crucial thing here is the `drev` number for each node.  
For
  these complicated cases where there are multiple old nodes, simply ignore them
  all.  This will cause `createdifferentialrevision()` to generate a new diff
  (within the same Differential), and avoids complicating the code.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8311

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -483,18 +483,23 @@
 alldiffs = callconduit(
 unfi.ui, b'differential.querydiffs', {b'revisionIDs': drevs}
 )
-getnode = lambda d: bin(getdiffmeta(d).get(b'node', b'')) or None
+
+def getnodes(d, precset):
+# Ignore other nodes that were combined into the Differential
+# that aren't predecessors of the current local node.
+return [n for n in getlocalcommits(d) if n in precset]
+
 for newnode, (force, precset, drev) in toconfirm.items():
 diffs = [
 d for d in alldiffs.values() if int(d[b'revisionID']) == drev
 ]
 
-# "precursors" as known by Phabricator
-phprecset = {getnode(d) for d in diffs}
+# local predecessors known by Phabricator
+phprecset = {n for d in diffs for n in getnodes(d, precset)}
 
 # Ignore if precursors (Phabricator and local repo) do not overlap,
 # and force is not set (when commit message says nothing)
-if not force and not bool(phprecset & precset):
+if not force and not phprecset:
 tagname = b'D%d' % drev
 tags.tag(
 repo,
@@ -519,7 +524,15 @@
 oldnode = lastdiff = None
 if diffs:
 lastdiff = max(diffs, key=lambda d: int(d[b'id']))
-oldnode = getnode(lastdiff)
+oldnodes = getnodes(lastdiff, precset)
+
+# If this commit was the result of `hg fold` after submission,
+# and now resubmitted with --fold, the easiest thing to do is
+# to leave the node clear.  This only results in creating a new
+# diff for the _same_ Differential Revision if this commit is
+# the first or last in the selected range.
+if len(oldnodes) == 1:
+oldnode = oldnodes[0]
 if oldnode and not has_node(oldnode):
 oldnode = None
 
@@ -1667,6 +1680,21 @@
 return _differentialrevisiondescre.sub(uri, ctx.description())
 
 
+def getlocalcommits(diff):
+"""get the set of local commits from a diff object
+
+See ``getdiffmeta()`` for an example diff object.
+"""
+props = diff.get(b'properties') or {}
+commits = props.get(b'local:commits') or {}
+if len(commits) > 1:
+return {bin(c) for c in commits.keys()}
+
+# Storing the diff metadata predates storing `local:commits`, so continue
+# to use that in the --no-fold case.
+return {bin(getdiffmeta(diff).get(b'node', b'')) or None}
+
+
 def getdiffmeta(diff):
 """get commit metadata (date, node, user, p1) from a diff object
 



To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8310: phabricator: teach createdifferentialrevision() to allow a folded commit range

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  No visible changes here, until an option to enable it is added to `phabsend`.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8310

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1014,9 +1014,10 @@
 
 
 def createdifferentialrevision(
-ctx,
+ctxs,
 revid=None,
 parentrevphid=None,
+oldbasenode=None,
 oldnode=None,
 olddiff=None,
 actions=None,
@@ -1027,17 +1028,29 @@
 If revid is None, create a new Differential Revision, otherwise update
 revid. If parentrevphid is not None, set it as a dependency.
 
+If there is a single commit for the new Differential Revision, ``ctxs`` 
will
+be a list of that single context.  Otherwise, it is a list that covers the
+range of changes for the differential, where ``ctxs[0]`` is the first 
change
+to include and ``ctxs[-1]`` is the last.
+
 If oldnode is not None, check if the patch content (without commit message
-and metadata) has changed before creating another diff.
+and metadata) has changed before creating another diff.  For a Revision 
with
+a single commit, ``oldbasenode`` and ``oldnode`` have the same value.  For 
a
+Revision covering multiple commits, ``oldbasenode`` corresponds to
+``ctxs[0]`` the previous time this Revision was posted, and ``oldnode``
+corresponds to ``ctxs[-1]``.
 
 If actions is not None, they will be appended to the transaction.
 """
-basectx = ctx
+ctx = ctxs[-1]
+basectx = ctxs[0]
+
 repo = ctx.repo()
 if oldnode:
 diffopts = mdiff.diffopts(git=True, context=32767)
-oldctx = repo.unfiltered()[oldnode]
-oldbasectx = oldctx
+unfi = repo.unfiltered()
+oldctx = unfi[oldnode]
+oldbasectx = unfi[oldbasenode]
 neednewdiff = getdiff(basectx, ctx, diffopts) != getdiff(
 oldbasectx, oldctx, diffopts
 )
@@ -1056,7 +1069,7 @@
 # pushers could know the correct node metadata.
 assert olddiff
 diff = olddiff
-writediffproperties([ctx], diff)
+writediffproperties(ctxs, diff)
 
 # Set the parent Revision every time, so commit re-ordering is picked-up
 if parentrevphid:
@@ -1076,7 +1089,7 @@
 # this gets assigned to the title.
 fields = util.sortdict()  # sorted for stable wire protocol in tests
 
-for i, _ctx in enumerate([ctx]):
+for i, _ctx in enumerate(ctxs):
 # Parse commit message and update related fields.
 desc = _ctx.description()
 info = callconduit(
@@ -,7 +1124,11 @@
 
 revision = callconduit(repo.ui, b'differential.revision.edit', params)
 if not revision:
-raise error.Abort(_(b'cannot create revision for %s') % ctx)
+if len(ctxs) == 1:
+msg = _(b'cannot create revision for %s') % ctx
+else:
+msg = _(b'cannot create revision for %s::%s') % (basectx, ctx)
+raise error.Abort(msg)
 
 return revision, diff
 
@@ -1226,12 +1243,14 @@
 
 # Get Differential Revision ID
 oldnode, olddiff, revid = oldmap.get(ctx.node(), (None, None, None))
+oldbasenode = oldnode
 if oldnode != ctx.node() or opts.get(b'amend'):
 # Create or update Differential Revision
 revision, diff = createdifferentialrevision(
-ctx,
+[ctx],
 revid,
 lastrevphid,
+oldbasenode,
 oldnode,
 olddiff,
 actions,



To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8309: phabricator: combine commit messages into the review when folding commits

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  No visible changes here, until an option to enable it is added to `phabsend`.
  
  This combines the Differential fields like Arcanist does, rather than simply
  concatenating the text blocks.  Aside from populating everything properly in 
the
  web interface, Phabricator fails the review create/update if repeated fields 
are
  seen as would happen with simple concatenation.
  
  On the flip side, now that the Summary and Test Plan fields can contain data
  from multiple commits, we can't just join these fields together to determine 
if
  an amend is needed.  If that were to happen, every single commit in the folded
  range would get amended with the combined commit message, which seems clearly
  wrong.  Aside from making a minor assumption about the content of the
  Differential Revision field (it seems they allow some minor variances with
  spacing), this means that for folded reviews, you can't post it, go to the web
  page add a missing Test Plan, and then get it added to the commit message by
  re-posting it.  I don't think that's a big deal.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8309

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1067,14 +1067,42 @@
 if actions:
 transactions += actions
 
-# Parse commit message and update related fields.
-desc = ctx.description()
-info = callconduit(
-repo.ui, b'differential.parsecommitmessage', {b'corpus': desc}
-)
-for k, v in info[b'fields'].items():
-if k in [b'title', b'summary', b'testPlan']:
-transactions.append({b'type': k, b'value': v})
+# When folding multiple local commits into a single review, arcanist will
+# take the summary line of the first commit as the title, and then
+# concatenate the rest of the remaining messages (including each of their
+# first lines) to the rest of the first commit message (each separated by
+# an empty line), and use that as the summary field.  Do the same here.
+# For commits with only a one line message, there is no summary field, as
+# this gets assigned to the title.
+fields = util.sortdict()  # sorted for stable wire protocol in tests
+
+for i, _ctx in enumerate([ctx]):
+# Parse commit message and update related fields.
+desc = _ctx.description()
+info = callconduit(
+repo.ui, b'differential.parsecommitmessage', {b'corpus': desc}
+)
+
+for k in [b'title', b'summary', b'testPlan']:
+v = info[b'fields'].get(k)
+if not v:
+continue
+
+if i == 0:
+# Title, summary and test plan (if present) are taken verbatim
+# for the first commit.
+fields[k] = v.rstrip()
+continue
+elif k == b'title':
+# Add subsequent titles (i.e. the first line of the commit
+# message) back to the summary.
+k = b'summary'
+
+# Append any current field to the existing composite field
+fields[k] = b'\n\n'.join(filter(None, [fields.get(k), v.rstrip()]))
+
+for k, v in fields.items():
+transactions.append({b'type': k, b'value': v})
 
 params = {b'transactions': transactions}
 if revid is not None:
@@ -1266,7 +1294,7 @@
 old = unfi[rev]
 drevid = drevids[i]
 drev = [d for d in drevs if int(d[b'id']) == drevid][0]
-newdesc = getdescfromdrev(drev)
+newdesc = get_amended_desc(drev, old, False)
 # Make sure commit message contain "Differential Revision"
 if old.description() != newdesc:
 if old.phase() == phases.public:
@@ -1593,6 +1621,33 @@
 return b'\n\n'.join(filter(None, [title, summary, testplan, uri]))
 
 
+def get_amended_desc(drev, ctx, folded):
+"""similar to ``getdescfromdrev``, but supports a folded series of commits
+
+This is used when determining if an individual commit needs to have its
+message amended after posting it for review.  The determination is made for
+each individual commit, even when they were folded into one review.
+"""
+if not folded:
+return getdescfromdrev(drev)
+
+uri = b'Differential Revision: %s' % drev[b'uri']
+
+# Since the commit messages were combined when posting multiple commits
+# with --fold, the fields can't be read from Phabricator here, or *all*
+# affected local revisions will end up with the same commit message after
+# the URI is amended in.  Append in the DREV line, or update it if it
+# exists.  At worst, this means commit message or test

D8308: phabricator: record all local commits used to create a Differential revision

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Arcanist records all of the commits that it squashes into a single review, and
  that info will be helpful when adding similar functionality.  This info is 
used
  when submitting an updated review, so that the extension can recalculate the 
old
  diff and see if a new one is necessary, or if it is just a property update.  
It
  also shows on the `commits` tab in the `Revision Contents` section.
  
  When submitting in the usual 1:1 commit to review mode, the wire protocol is
  unchanged.
  
  The content of `hg:meta` is a bit odd, but such is the problem when folding
  several commits.  The choice for the parent node is obvious, but the `node`
  value uses the tip commit because that seems more natural, and is used 
elsewhere
  to look up the previous diff when updating.  The rest of the attributes follow
  from there.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8308

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -968,42 +968,49 @@
 return diff
 
 
-def writediffproperties(ctx, diff):
-"""write metadata to diff so patches could be applied losslessly"""
+def writediffproperties(ctxs, diff):
+"""write metadata to diff so patches could be applied losslessly
+
+``ctxs`` is the list of commits that created the diff, in ascending order.
+The list is generally a single commit, but may be several when using
+``phabsend --fold``.
+"""
 # creatediff returns with a diffid but query returns with an id
 diffid = diff.get(b'diffid', diff.get(b'id'))
+basectx = ctxs[0]
+tipctx = ctxs[-1]
+
 params = {
 b'diff_id': diffid,
 b'name': b'hg:meta',
 b'data': templatefilters.json(
 {
-b'user': ctx.user(),
-b'date': b'%d %d' % ctx.date(),
-b'branch': ctx.branch(),
-b'node': ctx.hex(),
-b'parent': ctx.p1().hex(),
+b'user': tipctx.user(),
+b'date': b'%d %d' % tipctx.date(),
+b'branch': tipctx.branch(),
+b'node': tipctx.hex(),
+b'parent': basectx.p1().hex(),
 }
 ),
 }
-callconduit(ctx.repo().ui, b'differential.setdiffproperty', params)
+callconduit(basectx.repo().ui, b'differential.setdiffproperty', params)
 
+commits = {}
+for ctx in ctxs:
+commits[ctx.hex()] = {
+b'author': stringutil.person(ctx.user()),
+b'authorEmail': stringutil.email(ctx.user()),
+b'time': int(ctx.date()[0]),
+b'commit': ctx.hex(),
+b'parents': [ctx.p1().hex()],
+b'branch': ctx.branch(),
+}
 params = {
 b'diff_id': diffid,
 b'name': b'local:commits',
-b'data': templatefilters.json(
-{
-ctx.hex(): {
-b'author': stringutil.person(ctx.user()),
-b'authorEmail': stringutil.email(ctx.user()),
-b'time': int(ctx.date()[0]),
-b'commit': ctx.hex(),
-b'parents': [ctx.p1().hex()],
-b'branch': ctx.branch(),
-},
-}
-),
+b'data': templatefilters.json(commits),
 }
-callconduit(ctx.repo().ui, b'differential.setdiffproperty', params)
+callconduit(basectx.repo().ui, b'differential.setdiffproperty', params)
 
 
 def createdifferentialrevision(
@@ -1049,7 +1056,7 @@
 # pushers could know the correct node metadata.
 assert olddiff
 diff = olddiff
-writediffproperties(ctx, diff)
+writediffproperties([ctx], diff)
 
 # Set the parent Revision every time, so commit re-ordering is picked-up
 if parentrevphid:
@@ -1289,7 +1296,9 @@
 # If it fails just warn and keep going, otherwise the DREV
 # associations will be lost
 try:
-writediffproperties(unfi[newnode], diffmap[old.node()])
+writediffproperties(
+[unfi[newnode]], diffmap[old.node()]
+)
 except util.urlerr.urlerror:
 ui.warnnoi18n(
 b'Failed to update metadata for D%d\n' % drevid



To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8307: phabricator: account for `basectx != ctx` when calculating renames

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  No functional changes here because the two are the currently same, but they
  won't be with a `--fold` option.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8307

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -61,6 +61,7 @@
 from mercurial import (
 cmdutil,
 context,
+copies,
 encoding,
 error,
 exthelper,
@@ -859,16 +860,28 @@
 # additional copies we can mark them (moves get removed from removed)
 copiedchanges = {}
 movedchanges = {}
+
+copy = {}
+if basectx != ctx:
+copy = copies.pathcopies(basectx.p1(), ctx)
+
 for fname in added:
 fctx = ctx[fname]
 oldfctx = None
 pchange = phabchange(currentPath=fname)
 
 filemode = gitmode[fctx.flags()]
-renamed = fctx.renamed()
+
+if copy:
+originalfname = copy.get(fname, fname)
+else:
+originalfname = fname
+if fctx.renamed():
+originalfname = fctx.renamed()[0]
+
+renamed = fname != originalfname
 
 if renamed:
-originalfname = renamed[0]
 oldfctx = basectx.p1()[originalfname]
 originalmode = gitmode[oldfctx.flags()]
 pchange.oldPath = originalfname



To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8306: phabricator: add basectx arguments to file related `phabsend` utilities

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This is in support of a future `--fold` option, that allows rolling up several
  commits into a single review with a diff from the start to the end of the 
range.
  
  There are no functional changes yet- the original `ctx` is also passed as the
  new `basectx`, which represents the first commit in the review range (similar 
to
  `qbase` in MQ parlance).  Other functions will need the range of commits, but
  these deal with status or the diffs, so they only need the end points.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8306

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -550,11 +550,11 @@
 return result
 
 
-def getdiff(ctx, diffopts):
+def getdiff(basectx, ctx, diffopts):
 """plain-text diff without header (user, commit message, etc)"""
 output = util.stringio()
 for chunk, _label in patch.diffui(
-ctx.repo(), ctx.p1().node(), ctx.node(), None, opts=diffopts
+ctx.repo(), basectx.p1().node(), ctx.node(), None, opts=diffopts
 ):
 output.write(chunk)
 return output.getvalue()
@@ -661,13 +661,13 @@
 )
 
 
-def maketext(pchange, ctx, fname):
+def maketext(pchange, basectx, ctx, fname):
 """populate the phabchange for a text file"""
 repo = ctx.repo()
 fmatcher = match.exact([fname])
 diffopts = mdiff.diffopts(git=True, context=32767)
 _pfctx, _fctx, header, fhunks = next(
-patch.diffhunks(repo, ctx.p1(), ctx, fmatcher, opts=diffopts)
+patch.diffhunks(repo, basectx.p1(), ctx, fmatcher, opts=diffopts)
 )
 
 for fhunk in fhunks:
@@ -813,25 +813,25 @@
 return True
 
 
-def addremoved(pdiff, ctx, removed):
+def addremoved(pdiff, basectx, ctx, removed):
 """add removed files to the phabdiff. Shouldn't include moves"""
 for fname in removed:
 pchange = phabchange(
 currentPath=fname, oldPath=fname, type=DiffChangeType.DELETE
 )
-oldfctx = ctx.p1()[fname]
+oldfctx = basectx.p1()[fname]
 pchange.addoldmode(gitmode[oldfctx.flags()])
 if not (oldfctx.isbinary() or notutf8(oldfctx)):
-maketext(pchange, ctx, fname)
+maketext(pchange, basectx, ctx, fname)
 
 pdiff.addchange(pchange)
 
 
-def addmodified(pdiff, ctx, modified):
+def addmodified(pdiff, basectx, ctx, modified):
 """add modified files to the phabdiff"""
 for fname in modified:
 fctx = ctx[fname]
-oldfctx = ctx.p1()[fname]
+oldfctx = basectx.p1()[fname]
 pchange = phabchange(currentPath=fname, oldPath=fname)
 filemode = gitmode[fctx.flags()]
 originalmode = gitmode[oldfctx.flags()]
@@ -848,12 +848,12 @@
 makebinary(pchange, fctx)
 addoldbinary(pchange, oldfctx, fctx)
 else:
-maketext(pchange, ctx, fname)
+maketext(pchange, basectx, ctx, fname)
 
 pdiff.addchange(pchange)
 
 
-def addadded(pdiff, ctx, added, removed):
+def addadded(pdiff, basectx, ctx, added, removed):
 """add file adds to the phabdiff, both new files and copies/moves"""
 # Keep track of files that've been recorded as moved/copied, so if there 
are
 # additional copies we can mark them (moves get removed from removed)
@@ -869,7 +869,7 @@
 
 if renamed:
 originalfname = renamed[0]
-oldfctx = ctx.p1()[originalfname]
+oldfctx = basectx.p1()[originalfname]
 originalmode = gitmode[oldfctx.flags()]
 pchange.oldPath = originalfname
 
@@ -914,7 +914,7 @@
 if renamed:
 addoldbinary(pchange, oldfctx, fctx)
 else:
-maketext(pchange, ctx, fname)
+maketext(pchange, basectx, ctx, fname)
 
 pdiff.addchange(pchange)
 
@@ -924,21 +924,21 @@
 pdiff.addchange(movedchange)
 
 
-def creatediff(ctx):
+def creatediff(basectx, ctx):
 """create a Differential Diff"""
 repo = ctx.repo()
 repophid = getrepophid(repo)
 # Create a "Differential Diff" via "differential.creatediff" API
 pdiff = phabdiff(
-sourceControlBaseRevision=b'%s' % ctx.p1().hex(),
+sourceControlBaseRevision=b'%s' % basectx.p1().hex(),
 branch=b'%s' % ctx.branch(),
 )
-modified, added, removed, _d, _u, _i, _c = ctx.p1().status(ctx)
+modified, added, removed, _d, _u, _i, _c = basectx.p1().status(ctx)
 # addadded will remove moved files from removed, so addremoved won't get
 # them
-addadded(pdiff, ctx, added, removed)
-addmodified(pdiff, ctx, modified)
-addremoved(pdiff, ctx, removed)
+addadded(pdiff, basectx, ctx, added, removed)
+addmodified(pdiff, basectx, ctx, modified)
+addre

D8305: phabricator: eliminate a couple of duplicate filectx lookups

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8305

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -819,8 +819,8 @@
 pchange = phabchange(
 currentPath=fname, oldPath=fname, type=DiffChangeType.DELETE
 )
-pchange.addoldmode(gitmode[ctx.p1()[fname].flags()])
 oldfctx = ctx.p1()[fname]
+pchange.addoldmode(gitmode[oldfctx.flags()])
 if not (oldfctx.isbinary() or notutf8(oldfctx)):
 maketext(pchange, ctx, fname)
 
@@ -831,10 +831,10 @@
 """add modified files to the phabdiff"""
 for fname in modified:
 fctx = ctx[fname]
-oldfctx = fctx.p1()
+oldfctx = ctx.p1()[fname]
 pchange = phabchange(currentPath=fname, oldPath=fname)
-filemode = gitmode[ctx[fname].flags()]
-originalmode = gitmode[ctx.p1()[fname].flags()]
+filemode = gitmode[fctx.flags()]
+originalmode = gitmode[oldfctx.flags()]
 if filemode != originalmode:
 pchange.addoldmode(originalmode)
 pchange.addnewmode(filemode)
@@ -846,7 +846,7 @@
 or notutf8(oldfctx)
 ):
 makebinary(pchange, fctx)
-addoldbinary(pchange, fctx.p1(), fctx)
+addoldbinary(pchange, oldfctx, fctx)
 else:
 maketext(pchange, ctx, fname)
 
@@ -864,7 +864,7 @@
 oldfctx = None
 pchange = phabchange(currentPath=fname)
 
-filemode = gitmode[ctx[fname].flags()]
+filemode = gitmode[fctx.flags()]
 renamed = fctx.renamed()
 
 if renamed:



To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8190: nodemap: test that concurrent process don't see the pending transaction

[durin42 (Augie Fackler)]

durin42 added a comment.


  In D8190#124146 , @marmoute 
wrote:
  
  > In D8190#124141 , @martinvonz 
wrote:
  >
  >> 
  >
  > This new patch works, but I don't like the idea of not testing two fully 
independant process. Running a process through hooks is quite different and who 
know how that semantic could evolve.
  
  Can we wrap the `hg` invocation in the script in enough `env -u VAR` business 
that we can know it won't see the pending transaction? That seems like it ought 
to be eminently doable. In fact, it might even merit an environment variable of 
its own, eg `HG_IGNORE_PENDING_TXN=1 hg ...` for use in other scripts.
  
  > The feature provided by the `wait-on-file` script is useful and the 
approach have been sucessfully used for a while in the Mercurial codebase (eg: 
test-push-race.t, test-bookmarks-corner-case.t, …). and extensions. The need 
for such fine grained synchronisation will appears again, and that script will 
be needed again. So I would rather use that script with the current semantic 
(and whaterver reasonable update are requested to the implementation) and keep 
two fully independant processes in that tests.
  
  I'm not terribly swayed by this argument: I've invested nonzero time in 
*fixing* flakes that use this (anti-)pattern, so I'd like to avoid 
proliferating it. Encoding it as a helper script pushes it towards being an 
acceptable pattern that folks will use without critical thought. :(
  
  > As a note, I have been fixing a lot of test flackyness in the past years, 
maybe of them create by sleep only based synchronisation (so not the current 
approach). The existing example of "synchronisation of file with much longer 
timeout" (see previous list) have never been an issue.
  
  I'm pretty sure I've spent time (see above) in fixing flakes of this nature.
  
  > If anyone has a more elegant solution to this *general* problem, I would be 
happy to use it instead, but so far, this it the best reasonable option I have 
up my sleeve.
  
  That's kind of the counter-argument though: rather than a one-size-fits-all 
approach, I'd rather we thought more about getting the right synchronization 
primitives in place on a case-by-case basis until a non-sleepy pattern emerges 
that we can really live with.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8190/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8190

To: marmoute, #hg-reviewers
Cc: martinvonz, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8190: nodemap: test that concurrent process don't see the pending transaction

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  In D8190#124141 , @martinvonz 
wrote:
  
  > The parent patch (D8189 ) was a bit 
controversial. We don't need that patch if we apply the following patch on top 
of this one (making this test sleep-free):
  >
  >   diff --git a/tests/test-persistent-nodemap.t 
b/tests/test-persistent-nodemap.t
  >   --- a/tests/test-persistent-nodemap.t
  >   +++ b/tests/test-persistent-nodemap.t
  >   @@ -320,17 +320,20 @@ An up to date nodemap should be availabl
  >Another process does not see the pending nodemap content during run.
  >   -  $ PATH=$RUNTESTDIR/testlib/:$PATH
  >   +  $ PATH=$PWD:$RUNTESTDIR/testlib/:$PATH
  >   +  $ cat >> log_nodemap <   +  > unset HG_PENDING
  >   +  > hg debugnodemap --metadata > "\$1"
  >   +  > EOS
  >   +  $ chmod +x log_nodemap
  >  $ echo qpoasp > a
  >  $ hg ci -m a2 \
  >   -  > --config "hooks.pretxnclose=wait-on-file 20 sync-repo-read 
sync-txn-pending" \
  >   -  > --config "hooks.txnclose=touch sync-txn-close" > output.txt 2>&1 &
  >   +  > --config "hooks.pretxnclose=log_nodemap pretxn-output" \
  >   +  > --config "hooks.txnclose=log_nodemap posttxn-output"
  >(read the repository while the commit transaction is pending)
  >   -  $ wait-on-file 20 sync-txn-pending && \
  >   -  > hg debugnodemap --metadata && \
  >   -  > wait-on-file 20 sync-txn-close sync-repo-read
  >   +  $ cat pretxn-output
  >  uid:  (glob)
  >  tip-rev: 5004
  >  tip-node: ba87cd9559559e4b91b28cb140d003985315e031
  >   @@ -340,7 +343,7 @@ Another process does not see the pending
  >  data-unused: 192 (pure !)
  >  data-unused: 192 (rust !)
  >  data-unused: 0 (no-pure no-rust !)
  >   -  $ hg debugnodemap --metadata
  >   +  $ cat posttxn-output
  >  uid:  (glob)
  >  tip-rev: 5005
  >  tip-node: bae4d45c759e30f1cb1a40e1382cf0e0414154db
  >   @@ -350,6 +353,3 @@ Another process does not see the pending
  >  data-unused: 448 (pure !)
  >  data-unused: 448 (rust !)
  >  data-unused: 0 (no-pure no-rust !)
  >   -
  >   -  $ cat output.txt
  >   -
  
  This new patch works, but I don't like the idea of not testing two fully 
independant process. Running a process through hooks is quite different and who 
know how that semantic could evolve.
  
  The feature provided by the `wait-on-file` script is useful and the approach 
have been sucessfully used for a while in the Mercurial codebase (eg: 
test-push-race.t, test-bookmarks-corner-case.t, …). and extensions. The need 
for such fine grained synchronisation will appears again, and that script will 
be needed again. So I would rather use that script with the current semantic 
(and whaterver reasonable update are requested to the implementation) and keep 
two fully independant processes in that tests.
  
  As a note, I have been fixing a lot of test flackyness in the past years, 
maybe of them create by sleep only based synchronisation (so not the current 
approach). The existing example of "synchronisation of file with much longer 
timeout" (see previous list) have never been an issue.
  
  If anyone has a more elegant solution to this *general* problem, I would be 
happy to use it instead, but so far, this it the best reasonable option I have 
up my sleeve.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8190/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8190

To: marmoute, #hg-reviewers
Cc: martinvonz, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8190: nodemap: test that concurrent process don't see the pending transaction

[durin42 (Augie Fackler)]

durin42 added a comment.


  In D8190#124141 , @martinvonz 
wrote:
  
  > @durin42, what do you think?
  
  That's *inspired* and looks like an awesome solution to me.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8190/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8190

To: marmoute, #hg-reviewers
Cc: martinvonz, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8190: nodemap: test that concurrent process don't see the pending transaction

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.
martinvonz added subscribers: durin42, martinvonz.


  The parent patch (D8189 ) was a bit 
controversial. We don't need that patch if we apply the following patch on top 
of this one (making this test sleep-free):
  
diff --git a/tests/test-persistent-nodemap.t 
b/tests/test-persistent-nodemap.t
--- a/tests/test-persistent-nodemap.t
+++ b/tests/test-persistent-nodemap.t
@@ -320,17 +320,20 @@ An up to date nodemap should be availabl

 Another process does not see the pending nodemap content during run.

-  $ PATH=$RUNTESTDIR/testlib/:$PATH
+  $ PATH=$PWD:$RUNTESTDIR/testlib/:$PATH
+  $ cat >> log_nodemap < unset HG_PENDING
+  > hg debugnodemap --metadata > "\$1"
+  > EOS
+  $ chmod +x log_nodemap
   $ echo qpoasp > a
   $ hg ci -m a2 \
-  > --config "hooks.pretxnclose=wait-on-file 20 sync-repo-read 
sync-txn-pending" \
-  > --config "hooks.txnclose=touch sync-txn-close" > output.txt 2>&1 &
+  > --config "hooks.pretxnclose=log_nodemap pretxn-output" \
+  > --config "hooks.txnclose=log_nodemap posttxn-output"

 (read the repository while the commit transaction is pending)

-  $ wait-on-file 20 sync-txn-pending && \
-  > hg debugnodemap --metadata && \
-  > wait-on-file 20 sync-txn-close sync-repo-read
+  $ cat pretxn-output
   uid:  (glob)
   tip-rev: 5004
   tip-node: ba87cd9559559e4b91b28cb140d003985315e031
@@ -340,7 +343,7 @@ Another process does not see the pending
   data-unused: 192 (pure !)
   data-unused: 192 (rust !)
   data-unused: 0 (no-pure no-rust !)
-  $ hg debugnodemap --metadata
+  $ cat posttxn-output
   uid:  (glob)
   tip-rev: 5005
   tip-node: bae4d45c759e30f1cb1a40e1382cf0e0414154db
@@ -350,6 +353,3 @@ Another process does not see the pending
   data-unused: 448 (pure !)
   data-unused: 448 (rust !)
   data-unused: 0 (no-pure no-rust !)
-
-  $ cat output.txt
-
  
  @durin42, what do you think?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8190/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8190

To: marmoute, #hg-reviewers
Cc: martinvonz, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: Moving patches traffic to a another list?

[Gregory Szorc]

On Thu, Mar 19, 2020 at 3:06 AM Pierre-Yves David <
pierre-yves.da...@ens-lyon.org> wrote:

> Hello everyone,
>
> After chatting with various people, I would like to suggest that we move
> patches and phabricator away from mercurial-devel@mercurial-scm.org to a
> new mailing list (mercurial-patc...@mercurial-scm.org).
>
> My goal here is to make other email discussion between developer more
> visible and to encourage core-developers to exchange more about their
> ideas. Right now, the patches related noise (especially from phabricator
> who don't thread series) makes other discussion hard to spot and follow.
> I know developers that explicitly don't read the ML but for specific
> keyword.
>
> What do you think?
>

I like the idea of having a lower volume list for non-patch discussions. I
think mercurial-devel is that list. If the list is Phabricator only, we
should disable posting to ensure all discussion is captured on Phabricator
and isn't fragmented across tools.

I support moving all Phabricator traffic to a new list. I'm OK with email
based patches still going to the mercurial-devel list, as volume is low.

I think it would also be interesting to explore a mechanism to add
mercurial-devel CC to a Phabricator review. A use case would be if there is
a particularly controversial change under review and we want to raise
awareness on the developer list to get more eyeballs on it. I anticipate
the number of reviews incurring this would be low. Is this technically
doable?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: Moving patches traffic to a another list?

[Augie Fackler]

> On Mar 20, 2020, at 12:09, Pierre-Yves David  
> wrote:
> 
> 
> 
> On 3/20/20 4:42 PM, Martin von Zweigbergk via Mercurial-devel wrote:
>> On Fri, Mar 20, 2020 at 8:39 AM Pierre-Yves David 
>> mailto:pierre-yves.da...@ens-lyon.org>> 
>> wrote:
>>On 3/20/20 4:19 PM, Yuya Nishihara wrote:
>> > unless we're willing to use the mailing list (or plain emails)
>>extensively.
>>That's what I am trying to achieve here :-), more direct communication
>>between developers.
>> An alternative for that is to set up a new mercurial-devel-discuss@ or 
>> something and encourage design discussions to go to that list. We already 
>> have #mercurial also. I don't think the new mailing list would get much 
>> traffic.
> 
> I am -1 on creating a new list for discussion. We have a lot of subscribers 
> here that I do not want to loose in the process. The goal is to reach out to 
> more people. Not less.

Should we see if we could get only "new patch" mails from phabricator sent 
here, and shove everything else in some other list for archival purposes? It 
sounds like the subject lines are sometimes useful to get folks interested in a 
patch, but the rest is noise...

(To be clear: I have no idea if this can be done.)


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

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

Re: Moving patches traffic to a another list?

[Pierre-Yves David]

On 3/20/20 4:42 PM, Martin von Zweigbergk via Mercurial-devel wrote:


On Fri, Mar 20, 2020 at 8:39 AM Pierre-Yves David 
mailto:pierre-yves.da...@ens-lyon.org>> 
wrote:




On 3/20/20 4:19 PM, Yuya Nishihara wrote:

 > unless we're willing to use the mailing list (or plain emails)
extensively.

That's what I am trying to achieve here :-), more direct communication
between developers.


An alternative for that is to set up a new mercurial-devel-discuss@ or 
something and encourage design discussions to go to that list. We 
already have #mercurial also. I don't think the new mailing list would 
get much traffic.


I am -1 on creating a new list for discussion. We have a lot of 
subscribers here that I do not want to loose in the process. The goal is 
to reach out to more people. Not less.


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

D8172: notify: optional mail threading based on obsmarker

[joerg.sonnenberger (Joerg Sonnenberger)]

joerg.sonnenberger updated this revision to Diff 20843.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8172?vs=20648&id=20843

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8172/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8172

AFFECTED FILES
  hgext/notify.py
  tests/test-notify.t

CHANGE DETAILS

diff --git a/tests/test-notify.t b/tests/test-notify.t
--- a/tests/test-notify.t
+++ b/tests/test-notify.t
@@ -6,8 +6,15 @@
   > EOF
 
   $ cat <> $HGRCPATH
+  > [experimental]
+  > evolution = true
+  > 
   > [extensions]
   > notify=
+  > strip=
+  > 
+  > [phases]
+  > publish=False
   > 
   > [hooks]
   > incoming.notify = python:hgext.notify.hook
@@ -15,6 +22,8 @@
   > [notify]
   > sources = pull
   > diffstat = False
+  > reply-to-predecessor = True
+  > messageidseed = notifyseed
   > 
   > [usersubs]
   > foo@bar = *
@@ -151,6 +160,15 @@
 "From" field of the notification mail. If not set, take the user from the
 pushing repo.  Default: False.
   
+  notify.reply-to-predecessor (EXPERIMENTAL)
+If set and the changeset has a predecessor in the repository, try to thread
+the notification mail with the predecessor. This adds the "In-Reply-To"
+header to the notification mail with a reference to the predecessor with 
the
+smallest revision number. Mail threads can still be torn, especially when
+changesets are folded.
+  
+This option must  be used in combination with "notify.messageidseed".
+  
   If set, the following entries will also be used to customize the
   notifications:
   
@@ -205,7 +223,7 @@
   adding manifests
   adding file changes
   added 1 changesets with 2 changes to 2 files
-  new changesets 00a13f371396
+  new changesets 00a13f371396 (1 drafts)
   MIME-Version: 1.0
   Content-Type: text/plain; charset="us-ascii"
   Content-Transfer-Encoding: 7bit
@@ -266,7 +284,7 @@
   adding manifests
   adding file changes
   added 1 changesets with 2 changes to 2 files
-  new changesets 00a13f371396
+  new changesets 00a13f371396 (1 drafts)
   MIME-Version: 1.0
   Content-Type: text/plain; charset="us-ascii"
   Content-Transfer-Encoding: 7bit
@@ -316,7 +334,7 @@
   adding manifests
   adding file changes
   added 1 changesets with 2 changes to 2 files
-  new changesets 00a13f371396
+  new changesets 00a13f371396 (1 drafts)
   MIME-Version: 1.0
   Content-Type: text/plain; charset="us-ascii"
   Content-Transfer-Encoding: 7bit
@@ -369,7 +387,7 @@
   adding manifests
   adding file changes
   added 2 changesets with 0 changes to 0 files
-  new changesets 3332653e1f3c:fccf66cd0c35
+  new changesets 3332653e1f3c:fccf66cd0c35 (2 drafts)
   MIME-Version: 1.0
   Content-Type: text/plain; charset="us-ascii"
   Content-Transfer-Encoding: 7bit
@@ -436,7 +454,7 @@
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files
-  new changesets 0f25f9c22b4c
+  new changesets 0f25f9c22b4c (1 drafts)
   MIME-Version: 1.0
   Content-Type: text/plain; charset="us-ascii"
   Content-Transfer-Encoding: 8bit
@@ -480,7 +498,7 @@
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files
-  new changesets a846b5f6ebb7
+  new changesets a846b5f6ebb7 (1 drafts)
   notify: sending 2 subscribers 1 changes
   (run 'hg update' to get a working copy)
   $ cat b/mbox | "$PYTHON" $TESTDIR/unwrap-message-id.py | "$PYTHON" 
$TESTTMP/filter.py
@@ -493,7 +511,7 @@
   Subject: long line
   From: t...@test.com
   X-Hg-Notification: changeset a846b5f6ebb7
-  Message-Id:  (glob)
+  Message-Id: 

   To: b...@test.com, foo@bar
   
   changeset a846b5f6ebb7 in b
@@ -543,6 +561,8 @@
   (branches are permanent and global, did you want a bookmark?)
   $ echo a >> a/a
   $ hg --cwd a ci -m test -d '1 0'
+  $ echo a >> a/a
+  $ hg --cwd a ci -m test -d '1 0'
   $ hg --traceback --cwd b pull ../a | \
   >  "$PYTHON" $TESTDIR/unwrap-message-id.py | \
   >  "$PYTHON" $TESTTMP/filter.py
@@ -551,8 +571,8 @@
   adding changesets
   adding manifests
   adding file changes
-  added 1 changesets with 1 changes to 1 files
-  new changesets f7e5aaed4080
+  added 2 changesets with 2 changes to 1 files
+  new changesets f7e5aaed4080:485bf79b9464 (2 drafts)
   MIME-Version: 1.0
   Content-Type: text/plain; charset="us-ascii"
   Content-Transfer-Encoding: 7bit
@@ -561,11 +581,24 @@
   Subject: test
   From: t...@test.com
   X-Hg-Notification: changeset f7e5aaed4080
-  Message-Id:  (glob)
+  Message-Id: 

   To: b...@test.com, foo@bar, not...@example.com
   
   changeset f7e5aaed4080 in b
   description: test
+  MIME-Version: 1.0
+  Content-Type: text/plain; charset="us-ascii"
+  Content-Transfer-Encoding: 7bit
+  X-Test: foo
+  Date: * (glob)
+  Subject: test
+  From: t...@test.com
+  X-Hg-Notification: changeset 485bf79b9464
+  Message-Id: 

+  To: b...@test.com, foo@bar, not...@example.com
+  
+  changeset 485bf79b9464 in b
+  description: test
   (run 'hg update' to get a working copy)

D7296: pycompat: kludge around pytype being confused by __new__

[durin42 (Augie Fackler)]

durin42 added a comment.
durin42 planned changes to this revision.


  I'd like to get back to pytyping all the things, but I can't justify spending 
more time on it. If someone else wants to push things forward I'll gladly 
provide some mentorship.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7296/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

To: durin42, #hg-reviewers, indygreg, dlax
Cc: marmoute, yuja, mjpieters, dlax, indygreg, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8281: narrow: escape includepats/excludepats when sending over the wire

[durin42 (Augie Fackler)]

durin42 added a comment.


  In D8281#124058 , @marmoute 
wrote:
  
  > Since narrow is still experimental, I don't think we should try too hard 
for backward compatibility. We could introduce a new end-point for a new 
encoding and drop the old one in a couple of version.
  
  +0, honestly. I won't require it, but I'd really rather we shaved this yak 
_now_ rather than when narrow has even more users.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8281/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8281

To: spectral, durin42, martinvonz, #hg-reviewers, marmoute
Cc: marmoute, mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8189: testlib: add a small scrip to help process to synchronise using file

[durin42 (Augie Fackler)]

durin42 added a comment.


  In D8189#124124 , @marmoute 
wrote:
  
  > In D8189#124122 , @durin42 
wrote:
  >
  >> I'm still -0 on this: I'd rather we found an approach that didn't require 
sleeping for so long. Perhaps a Python script would be a better fit here?
  >> (I won't block this landing, but I won't push it.)
  >
  > What abotu replacing the sleep 1 by a `python -c "import time; 
time.sleep(0.1)" would that make you happy ?
  
  Happy? No, not really. I think I'd rather it was a Python script, but what 
I'd _really_ rather (and what would actually make me happy) is that we didn't 
have sleep-required steps like this at all. They're inherently racy , and I 
feel like there's got to be a better solution.
  
  At this point I don't have the patience to try and work through this patch, 
so you'll need to find a different reviewer.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8189/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8189

To: marmoute, #hg-reviewers
Cc: durin42, mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8189: testlib: add a small scrip to help process to synchronise using file

[marmoute (Pierre-Yves David)]

marmoute added a comment.
marmoute added a subscriber: durin42.


  In D8189#124122 , @durin42 wrote:
  
  > I'm still -0 on this: I'd rather we found an approach that didn't require 
sleeping for so long. Perhaps a Python script would be a better fit here?
  > (I won't block this landing, but I won't push it.)
  
  What abotu replacing the sleep 1 by a `python -c "import time; 
time.sleep(0.1)" would that make you happy ?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8189/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8189

To: marmoute, #hg-reviewers
Cc: durin42, mharbison72, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: Moving patches traffic to a another list?

[Martin von Zweigbergk via Mercurial-devel]

On Fri, Mar 20, 2020 at 8:39 AM Pierre-Yves David <
pierre-yves.da...@ens-lyon.org> wrote:

>
>
> On 3/20/20 4:19 PM, Yuya Nishihara wrote:
>
> > unless we're willing to use the mailing list (or plain emails)
> extensively.
>
> That's what I am trying to achieve here :-), more direct communication
> between developers.
>

An alternative for that is to set up a new mercurial-devel-discuss@ or
something and encourage design discussions to go to that list. We already
have #mercurial also. I don't think the new mailing list would get much
traffic.


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

D8189: testlib: add a small scrip to help process to synchronise using file

[durin42 (Augie Fackler)]

durin42 added a comment.


  I'm still -0 on this: I'd rather we found an approach that didn't require 
sleeping for so long. Perhaps a Python script would be a better fit here?
  
  (I won't block this landing, but I won't push it.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8189/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8189

To: marmoute, #hg-reviewers
Cc: mharbison72, durin42, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: Moving patches traffic to a another list?

[Martin von Zweigbergk via Mercurial-devel]

I agree with Yuya. I read phabricator threads if the email subject
interests me or if I'm added to the thread for some reason. Otherwise I
mostly ignore the threads there.

On Fri, Mar 20, 2020 at 8:36 AM Yuya Nishihara  wrote:

> On Fri, 20 Mar 2020 15:51:12 +0100, Pierre-Yves David wrote:
> > There are two aspect to my proposal:
> >
> > 1) First, phab is pretty verbose and I tend to miss important patch or
> > discussion starting on phab. Some of my colleague just gave up and won't
> > even try to monitor the flow. So having important discussion happening
> > on a dedicated channel will help people to pick them up. Even if those
> > discussion link to some patch/phab
> >
> > 2) There are multiple large work that would benefit from earlier
> > discussion. Some project requires a serious investment and starting to
> > discuss the project fundamentals once weeks have been spent writing code
> > give sub-optimal results. I would like to encourage more of these
> > discussions, especially since the sprint is pushed back to post-pandemic
> > time.
> >
> > So, just to be sure, If I read your email right, you would be fine with
> > moving the phab notification to a dedicated mailing list?
>
> Yep, that's how I deal with mercurial-devel. Ignore phab and feel safe.
>
> But I have to say that email flow would be almost zero if phab
> notifications
> were filtered out. Interesting topics would be discussed in phabricator in
> general, so I see no point to split the mailing lists unless we're willing
> to use the mailing list (or plain emails) extensively.
> ___
> 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: Moving patches traffic to a another list?

[Pierre-Yves David]

On 3/20/20 4:19 PM, Yuya Nishihara wrote:


unless we're willing to use the mailing list (or plain emails) extensively.


That's what I am trying to achieve here :-), more direct communication 
between developers.


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

Re: Moving patches traffic to a another list?

[Yuya Nishihara]

On Fri, 20 Mar 2020 15:51:12 +0100, Pierre-Yves David wrote:
> There are two aspect to my proposal:
> 
> 1) First, phab is pretty verbose and I tend to miss important patch or 
> discussion starting on phab. Some of my colleague just gave up and won't 
> even try to monitor the flow. So having important discussion happening 
> on a dedicated channel will help people to pick them up. Even if those 
> discussion link to some patch/phab
> 
> 2) There are multiple large work that would benefit from earlier 
> discussion. Some project requires a serious investment and starting to 
> discuss the project fundamentals once weeks have been spent writing code 
> give sub-optimal results. I would like to encourage more of these 
> discussions, especially since the sprint is pushed back to post-pandemic 
> time.
> 
> So, just to be sure, If I read your email right, you would be fine with
> moving the phab notification to a dedicated mailing list?

Yep, that's how I deal with mercurial-devel. Ignore phab and feel safe.

But I have to say that email flow would be almost zero if phab notifications
were filtered out. Interesting topics would be discussed in phabricator in
general, so I see no point to split the mailing lists unless we're willing
to use the mailing list (or plain emails) extensively.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8304: cext: move variable declaration to the top of the block for C89 support

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  Great, @mharbison72 can you submit a patch ?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8304: cext: move variable declaration to the top of the block for C89 support

[yuja (Yuya Nishihara)]

yuja added a comment.


  >   > -Wdeclaration-after-statement for gcc and maybe clang.
  >   Nice, any idea of how to make it verbosely complaining part of the 
test-suite ?
  
  -Werror=declaration-after-statement will prevent the test from running at all.
  I think setup.py can be adjusted to add some extra compiler options.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: D8304: cext: move variable declaration to the top of the block for C89 support

[Yuya Nishihara]

>   > -Wdeclaration-after-statement for gcc and maybe clang.
>   
>   Nice, any idea of how to make it verbosely complaining part of the 
> test-suite ?

-Werror=declaration-after-statement will prevent the test from running at all.
I think setup.py can be adjusted to add some extra compiler options.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8304: cext: move variable declaration to the top of the block for C89 support

[mharbison72 (Matt Harbison)]

mharbison72 added a comment.


  In D8304#124116 , @yuja wrote:
  
  >>   Could we enable some kind of warning to catch this in the tests ?
  >
  > -Wdeclaration-after-statement for gcc and maybe clang.
  
  I can confirm that this works on 18.04, though it doesn't issue a warning (or 
complain about an unknown option) on 10.14 with gcc/clang.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: Moving patches traffic to a another list?

[Pierre-Yves David]

On 3/19/20 8:30 PM, Yuya Nishihara wrote:

On Thu, 19 Mar 2020 11:03:22 +0100, Pierre-Yves David wrote:

After chatting with various people, I would like to suggest that we move
patches and phabricator away from mercurial-devel@mercurial-scm.org to a
new mailing list (mercurial-patc...@mercurial-scm.org).

My goal here is to make other email discussion between developer more
visible and to encourage core-developers to exchange more about their
ideas. Right now, the patches related noise (especially from phabricator
who don't thread series) makes other discussion hard to spot and follow.
I know developers that explicitly don't read the ML but for specific
keyword.


I'm against moving non-phabricator patches away. They aren't spam source
and are low volume these days.


That seems fine. patchbom series are easier to deal with because they 
are threaded and less verbose.



I think development discussion often starts
from patches/phab, so splitting them would make things hard to track.


There are two aspect to my proposal:

1) First, phab is pretty verbose and I tend to miss important patch or 
discussion starting on phab. Some of my colleague just gave up and won't 
even try to monitor the flow. So having important discussion happening 
on a dedicated channel will help people to pick them up. Even if those 
discussion link to some patch/phab


2) There are multiple large work that would benefit from earlier 
discussion. Some project requires a serious investment and starting to 
discuss the project fundamentals once weeks have been spent writing code 
give sub-optimal results. I would like to encourage more of these 
discussions, especially since the sprint is pushed back to post-pandemic 
time.


So, just to be sure, If I read your email right, you would be fine with 
moving the phab notification to a dedicated mailing list?


Cheers,

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

D8304: cext: move variable declaration to the top of the block for C89 support

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  In D8304#124116 , @yuja wrote:
  
  >>   Could we enable some kind of warning to catch this in the tests ?
  >
  > -Wdeclaration-after-statement for gcc and maybe clang.
  
  Nice, any idea of how to make it verbosely complaining part of the test-suite 
?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8232: phabricator: add a helper function to convert DREVSPECs to a DREV dict list

[mharbison72 (Matt Harbison)]

mharbison72 added a comment.


  In D8232#124097 , @yuja wrote:
  
  >>> +def _getdrevs(ui, stack, *specs):
  >>> +"""convert user supplied DREVSPECs into "Differential Revision" dicts
  >>> +
  >>> +See ``hg help phabread`` for how to specify each DREVSPEC.
  >>> +"""
  >>> +if len(*specs) > 0:
  >>
  >>   ^^
  >>
  >> Fixed bad argument expansion since I had to rebase this. Please let me
  >> know if that's wrong.
  >
  > Never mind. Maybe `specs` is a list containing a single list, in which case,
  > the code is valid.
  
  It looks like a tuple of strings:
  
>>> print('specs type is %s' % type(specs))
specs type is 
>>> print('specs is %r' % (specs,))
specs is ('D1', 'D2', 'D3')
  
  I copied this pattern from somewhere in Mercurial (file patterns look like 
they're handled in a similar way), but don't remember exactly where at this 
point.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8232/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8232

To: mharbison72, #hg-reviewers, Alphare, pulkit
Cc: yuja, Alphare, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8304: cext: move variable declaration to the top of the block for C89 support

[yuja (Yuya Nishihara)]

yuja added a comment.


  >   Could we enable some kind of warning to catch this in the tests ?
  
  -Wdeclaration-after-statement for gcc and maybe clang.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: yuja, marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: D8304: cext: move variable declaration to the top of the block for C89 support

[Yuya Nishihara]

>   Could we enable some kind of warning to catch this in the tests ?

-Wdeclaration-after-statement for gcc and maybe clang.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8304: cext: move variable declaration to the top of the block for C89 support

[mharbison72 (Matt Harbison)]

Closed by commit rHG3122058df7a5: cext: move variable declaration to the top of 
the block for C89 support (authored by mharbison72).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs 
Review".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8304?vs=20841&id=20842

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

AFFECTED FILES
  mercurial/cext/revlog.c

CHANGE DETAILS

diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -153,11 +153,12 @@
 {
if (self->inlined && pos > 0) {
if (self->offsets == NULL) {
+   Py_ssize_t ret;
self->offsets = PyMem_Malloc(self->raw_length *
 sizeof(*self->offsets));
if (self->offsets == NULL)
return (const char *)PyErr_NoMemory();
-   Py_ssize_t ret = inline_scan(self, self->offsets);
+   ret = inline_scan(self, self->offsets);
if (ret == -1) {
return NULL;
};



To: mharbison72, #hg-reviewers, marmoute
Cc: marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8304: cext: move variable declaration to the top of the block for C89 support

[mharbison72 (Matt Harbison)]

mharbison72 added a comment.


  In D8304#124109 , @marmoute 
wrote:
  
  > Could we enable some kind of warning to catch this in the tests ?
  
  I don't think so.  You need `CFLAGS=-std=c89` (and maybe `-pedantic`).  But 
in C89 mode with `gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0`, it fails to build 
complaining about C++ comments and not understanding `inline`, and also issues 
a bunch of warnings about various things.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8304: cext: move variable declaration to the top of the block for C89 support

[marmoute (Pierre-Yves David)]

marmoute added a comment.
marmoute accepted this revision.


  Could we enable some kind of warning to catch this in the tests ?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers, marmoute
Cc: marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8304: cext: move variable declaration to the top of the block for C89 support

[mharbison72 (Matt Harbison)]

mharbison72 added a comment.


  This is needed on stable to fix the build.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8304/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

To: mharbison72, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8304: cext: move variable declaration to the top of the block for C89 support

[mharbison72 (Matt Harbison)]

mharbison72 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Not sure if we still care about C89 in general, but MSVC requires this style
  too.

REPOSITORY
  rHG Mercurial

BRANCH
  stable

REVISION DETAIL
  https://phab.mercurial-scm.org/D8304

AFFECTED FILES
  mercurial/cext/revlog.c

CHANGE DETAILS

diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -153,11 +153,12 @@
 {
if (self->inlined && pos > 0) {
if (self->offsets == NULL) {
+   Py_ssize_t ret;
self->offsets = PyMem_Malloc(self->raw_length *
 sizeof(*self->offsets));
if (self->offsets == NULL)
return (const char *)PyErr_NoMemory();
-   Py_ssize_t ret = inline_scan(self, self->offsets);
+   ret = inline_scan(self, self->offsets);
if (ret == -1) {
return NULL;
};



To: mharbison72, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8232: phabricator: add a helper function to convert DREVSPECs to a DREV dict list

[yuja (Yuya Nishihara)]

yuja added a comment.


  >> +def _getdrevs(ui, stack, *specs):
  >> +"""convert user supplied DREVSPECs into "Differential Revision" dicts
  >> +
  >> +See ``hg help phabread`` for how to specify each DREVSPEC.
  >> +"""
  >> +if len(*specs) > 0:
  >
  >   ^^
  >
  > Fixed bad argument expansion since I had to rebase this. Please let me
  > know if that's wrong.
  
  Never mind. Maybe `specs` is a list containing a single list, in which case,
  the code is valid.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8232/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8232

To: mharbison72, #hg-reviewers, Alphare, pulkit
Cc: yuja, Alphare, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: D8232: phabricator: add a helper function to convert DREVSPECs to a DREV dict list

[Yuya Nishihara]

> > +def _getdrevs(ui, stack, *specs):
> > +"""convert user supplied DREVSPECs into "Differential Revision" dicts
> > +
> > +See ``hg help phabread`` for how to specify each DREVSPEC.
> > +"""
> > +if len(*specs) > 0:
>   ^^
> Fixed bad argument expansion since I had to rebase this. Please let me
> know if that's wrong.

Never mind. Maybe `specs` is a list containing a single list, in which case,
the code is valid.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8232: phabricator: add a helper function to convert DREVSPECs to a DREV dict list

[yuja (Yuya Nishihara)]

yuja added a comment.


  > +def _getdrevs(ui, stack, *specs):
  > +"""convert user supplied DREVSPECs into "Differential Revision" dicts
  > +
  > +See ``hg help phabread`` for how to specify each DREVSPEC.
  > +"""
  > +if len(*specs) > 0:
  
^^
  
  Fixed bad argument expansion since I had to rebase this. Please let me
  know if that's wrong.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8232/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8232

To: mharbison72, #hg-reviewers, Alphare, pulkit
Cc: yuja, Alphare, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: D8232: phabricator: add a helper function to convert DREVSPECs to a DREV dict list

[Yuya Nishihara]

> +def _getdrevs(ui, stack, *specs):
> +"""convert user supplied DREVSPECs into "Differential Revision" dicts
> +
> +See ``hg help phabread`` for how to specify each DREVSPEC.
> +"""
> +if len(*specs) > 0:
  ^^
Fixed bad argument expansion since I had to rebase this. Please let me
know if that's wrong.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D8232: phabricator: add a helper function to convert DREVSPECs to a DREV dict list

[mharbison72 (Matt Harbison)]

Closed by commit rHG6d5db16f6195: phabricator: add a helper function to convert 
DREVSPECs to a DREV dict list (authored by mharbison72).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8232?vs=20557&id=20838

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8232/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8232

AFFECTED FILES
  hgext/phabricator.py

CHANGE DETAILS

diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1632,6 +1632,27 @@
 return meta
 
 
+def _getdrevs(ui, stack, *specs):
+"""convert user supplied DREVSPECs into "Differential Revision" dicts
+
+See ``hg help phabread`` for how to specify each DREVSPEC.
+"""
+if len(*specs) > 0:
+
+def _formatspec(s):
+if stack:
+s = b':(%s)' % s
+return b'(%s)' % s
+
+spec = b'+'.join(pycompat.maplist(_formatspec, *specs))
+
+drevs = querydrev(ui, spec)
+if drevs:
+return drevs
+
+raise error.Abort(_(b"empty DREVSPEC set"))
+
+
 def readpatch(ui, drevs, write):
 """generate plain-text patch readable by 'hg import'
 



To: mharbison72, #hg-reviewers, Alphare, pulkit
Cc: Alphare, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

mercurial@44576: 17 new changesets

[Mercurial Commits]

17 new changesets in mercurial:

https://www.mercurial-scm.org/repo/hg/rev/40f4a75938ba
changeset:   44560:40f4a75938ba
parent:  44558:ea40fea992e0
user:Martin von Zweigbergk 
date:Fri Dec 13 15:14:57 2019 -0800
summary: fix: disallow `hg fix --all --working-dir`

https://www.mercurial-scm.org/repo/hg/rev/368f85c5dfc0
changeset:   44561:368f85c5dfc0
user:Martin von Zweigbergk 
date:Thu Dec 12 16:24:43 2019 -0800
summary: fix: refactor getrevstofix() to define revisions first, then 
validate them

https://www.mercurial-scm.org/repo/hg/rev/ece43c79333e
changeset:   44562:ece43c79333e
user:Raphaël Gomès 
date:Fri Mar 13 00:41:22 2020 +0100
summary: rust-core: add missing `Debug` traits

https://www.mercurial-scm.org/repo/hg/rev/1922694d638f
changeset:   44563:1922694d638f
user:Kyle Lippincott 
date:Thu Mar 12 20:08:05 2020 -0700
summary: tests: remove doctest in narrowspec, it is broken

https://www.mercurial-scm.org/repo/hg/rev/529cb23155bc
changeset:   44564:529cb23155bc
user:Kyle Lippincott 
date:Fri Mar 13 19:25:37 2020 -0700
summary: tests: make test-doctest.t module list match reality

https://www.mercurial-scm.org/repo/hg/rev/0af56d3ee24c
changeset:   44565:0af56d3ee24c
user:Kyle Lippincott 
date:Thu Mar 12 20:18:52 2020 -0700
summary: tests: make test-doctest.t automatically find files to run tests on

https://www.mercurial-scm.org/repo/hg/rev/77d48738b8e0
changeset:   44566:77d48738b8e0
user:Kyle Lippincott 
date:Mon Jul 22 09:58:23 2019 -0700
summary: vfs: fix typo in comment (remove extra "l")

https://www.mercurial-scm.org/repo/hg/rev/ad5a10f49dfa
changeset:   44567:ad5a10f49dfa
user:Kyle Lippincott 
date:Tue Mar 17 12:59:31 2020 -0700
summary: chistedit: support histedit.summary-template in curses histedit 
plan

https://www.mercurial-scm.org/repo/hg/rev/3aab524a8480
changeset:   44568:3aab524a8480
user:Augie Fackler 
date:Wed Mar 18 12:03:27 2020 -0400
summary: phabricator: remove duplicated byteskwargs conversion

https://www.mercurial-scm.org/repo/hg/rev/5483e9c759e4
changeset:   44569:5483e9c759e4
user:Augie Fackler 
date:Tue Mar 17 17:21:34 2020 -0400
summary: tests: add test for remotefilelog interactions with hgweb

https://www.mercurial-scm.org/repo/hg/rev/9e63108123a4
changeset:   44570:9e63108123a4
user:Augie Fackler 
date:Tue Mar 17 17:26:05 2020 -0400
summary: remotefilelog: add fake heads() method that allows viewing a file 
in hgweb

https://www.mercurial-scm.org/repo/hg/rev/6a8738dc4a01
changeset:   44571:6a8738dc4a01
user:Augie Fackler 
date:Wed Mar 18 15:08:14 2020 -0400
summary: hg: make _local() behave consistently on Python 3.8 (issue6287)

https://www.mercurial-scm.org/repo/hg/rev/245aec57d76a
changeset:   44572:245aec57d76a
user:Raphaël Gomès 
date:Wed Mar 18 14:26:47 2020 +0100
summary: rust-status: add trace-level logging for Rust status fallback for 
debugging

https://www.mercurial-scm.org/repo/hg/rev/9f5e94bbc606
changeset:   44573:9f5e94bbc606
user:Martin von Zweigbergk 
date:Thu Dec 12 16:32:01 2019 -0800
summary: fix: move handling of --all into getrevstofix() for consistency

https://www.mercurial-scm.org/repo/hg/rev/5205b46bd887
changeset:   44574:5205b46bd887
user:Martin von Zweigbergk 
date:Fri Mar 13 12:16:00 2020 -0700
summary: fix: add a -s option to format a revision and its descendants

https://www.mercurial-scm.org/repo/hg/rev/a6ef1e8e2f6d
changeset:   44575:a6ef1e8e2f6d
user:Martin von Zweigbergk 
date:Fri Mar 13 12:16:20 2020 -0700
summary: fix: mark -r as advanced

https://www.mercurial-scm.org/repo/hg/rev/2ec6160449aa
changeset:   44576:2ec6160449aa
bookmark:@
tag: tip
user:Matt Harbison 
date:Thu Mar 19 14:54:10 2020 -0400
summary: tests: avoid logging a commit with a Unicode character in 
test-phabricator.t

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

D8233: phabricator: allow multiple DREVSPEC args to phabread|phabimport|phabupdate

[pulkit (Pulkit Goyal)]

This revision now requires changes to proceed.
pulkit added a comment.
pulkit requested changes to this revision.


  This one fails to apply on current tip of default. Can you rebase and resend?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8233/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8233

To: mharbison72, #hg-reviewers, marmoute, pulkit
Cc: marmoute, Kwan, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] Improve error message when a user configured editor could not be found

[Yuya Nishihara]

On Thu, 19 Mar 2020 16:52:10 +0100, mw...@posteo.de wrote:
> --- a/mercurial/ui.py Do Mrz 12 16:25:22 2020 -0700
> +++ b/mercurial/ui.py Do Mrz 19 16:06:24 2020 +0100
> @@ -14,6 +14,7 @@
>   import inspect
>   import os
>   import re
> +import shlex
>   import signal
>   import socket
>   import subprocess
> @@ -1868,7 +1869,7 @@
>   rc = self._runsystem(cmd, environ=environ, cwd=cwd, 
> out=out)
>   if rc and onerr:
>   errmsg = b'%s %s' % (
> -os.path.basename(cmd.split(None, 1)[0]),
> +shlex.split(cmd)[0],

shlex has py2/3 compatibility issue. procutil.shellsplit() can be used
instead.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

mercurial@44559: new changeset (1 on stable)

[Mercurial Commits]

New changeset (1 on stable) in mercurial:

https://www.mercurial-scm.org/repo/hg/rev/bc9a9016467d
changeset:   44559:bc9a9016467d
branch:  stable
tag: tip
parent:  44500:864e9534d3d4
user:Pierre-Yves David 
date:Wed Mar 18 21:27:45 2020 +0100
summary: byteify-string: resolve symlink before byteifying

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