Re: [PATCH 7 of 7 stable] packaging: fix buildrpm whitespace

2019-10-30 Thread Mads Kiilerich 

On 10/30/19 9:14 PM, Augie Fackler wrote:

On Mon, Oct 28, 2019 at 12:37:15AM +0100, Mads Kiilerich wrote:

# HG changeset patch
# User Mads Kiilerich 
# Date 1572203819 -3600
#  Sun Oct 27 20:16:59 2019 +0100
# Branch stable
# Node ID c84f1465c44ebc539b803b876206712e0ebd78b4
# Parent  8c18adcd0177f3ca35f7f20f52f27f5a13ac9f90
packaging: fix buildrpm whitespace

Queued, many thanks - I have been meaning to try and fix our in-tree
RPM building for a while. Let's plan to switch this to py3 by default
for 5.2.1?



I can see Debian packaging already did switched to py3. So perhaps just 
do the same already now for 5.2.0? Change the buildrpm default to py3 
and provide a --python2 option? While it is late and risky for 5.2.0, it 
seems even more risky to do it for 5.2.1.


I don't know if it makes sense to switch over the old Fedora targets. 
Fedora 31 is out now, 28 was EOL 5 months ago, and 29 is EOL in a month. 
Perhaps just delete them and rely on the generic `make rpm` target instead?


The docker targets are a bit more tricky to make more durable and 
low-maintenance. They can't as easily be generic. But it could perhaps 
be done with something like `make docker-fedora FEDORA=31` .


For centos, we should perhaps keep 5 and 6 on py2 for now ... also 
because we don't yet have means for building our own py3 for these RPMs. 
Building Python is a bit more tricky than building Mercurial, and it 
would require more testing.


/Mads

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

D7151: setup: allow py3 install without env vars

2019-10-30 Thread Kwan (Ian Moody) 
Closed by commit rHG3733533c22a4: setup: allow py3 install without env vars 
(authored by Kwan).
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/D7151?vs=17372=17415

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

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

AFFECTED FILES
  setup.py

CHANGE DETAILS

diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -6,32 +6,27 @@
 
 import os
 
-supportedpy = '~= 2.7'
-if os.environ.get('HGALLOWPYTHON3', ''):
-# Mercurial will never work on Python 3 before 3.5 due to a lack
-# of % formatting on bytestrings, and can't work on 3.6.0 or 3.6.1
-# due to a bug in % formatting in bytestrings.
-# We cannot support Python 3.5.0, 3.5.1, 3.5.2 because of bug in
-# codecs.escape_encode() where it raises SystemError on empty bytestring
-# bug link: https://bugs.python.org/issue25270
-#
-# TODO: when we actually work on Python 3, use this string as the
-# actual supportedpy string.
-supportedpy = ','.join(
-[
-'>=2.7',
-'!=3.0.*',
-'!=3.1.*',
-'!=3.2.*',
-'!=3.3.*',
-'!=3.4.*',
-'!=3.5.0',
-'!=3.5.1',
-'!=3.5.2',
-'!=3.6.0',
-'!=3.6.1',
-]
-)
+# Mercurial will never work on Python 3 before 3.5 due to a lack
+# of % formatting on bytestrings, and can't work on 3.6.0 or 3.6.1
+# due to a bug in % formatting in bytestrings.
+# We cannot support Python 3.5.0, 3.5.1, 3.5.2 because of bug in
+# codecs.escape_encode() where it raises SystemError on empty bytestring
+# bug link: https://bugs.python.org/issue25270
+supportedpy = ','.join(
+[
+'>=2.7',
+'!=3.0.*',
+'!=3.1.*',
+'!=3.2.*',
+'!=3.3.*',
+'!=3.4.*',
+'!=3.5.0',
+'!=3.5.1',
+'!=3.5.2',
+'!=3.6.0',
+'!=3.6.1',
+]
+)
 
 import sys, platform
 import sysconfig
@@ -89,39 +84,6 @@
 printf(error, file=sys.stderr)
 sys.exit(1)
 
-# We don't yet officially support Python 3. But we want to allow developers to
-# hack on. Detect and disallow running on Python 3 by default. But provide a
-# backdoor to enable working on Python 3.
-if sys.version_info[0] != 2:
-badpython = True
-
-# Allow Python 3 from source checkouts.
-if os.path.isdir('.hg') or 'HGPYTHON3' in os.environ:
-badpython = False
-
-if badpython:
-error = """
-Python {py} detected.
-
-Mercurial currently has beta support for Python 3 and use of Python 2.7 is
-recommended for the best experience.
-
-Please re-run with Python 2.7 for a faster, less buggy experience.
-
-If you would like to beta test Mercurial with Python 3, this error can
-be suppressed by defining the HGPYTHON3 environment variable when invoking
-this command. No special environment variables or configuration changes are
-necessary to run `hg` with Python 3.
-
-See https://www.mercurial-scm.org/wiki/Python3 for more on Mercurial's
-Python 3 support.
-""".format(
-py='.'.join('%d' % x for x in sys.version_info[0:2])
-)
-
-printf(error, file=sys.stderr)
-sys.exit(1)
-
 if sys.version_info[0] >= 3:
 DYLIB_SUFFIX = sysconfig.get_config_vars()['EXT_SUFFIX']
 else:



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

D7152: phabricator: use True primitive instead of b'true' for phabupdate actions

2019-10-30 Thread Kwan (Ian Moody) 
Closed by commit rHGe57bf37eaeb5: phabricator: use True primitive instead of 
btrue for phabupdate actions (authored by Kwan).
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/D7152?vs=17373=17416

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

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

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
@@ -1613,7 +1613,7 @@
 
 actions = []
 for f in flags:
-actions.append({b'type': f, b'value': b'true'})
+actions.append({b'type': f, b'value': True})
 
 drevs = querydrev(repo, spec)
 for i, drev in enumerate(drevs):



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

D7180: tests: fix typo "includfe"

2019-10-30 Thread martinvonz (Martin von Zweigbergk) 
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  tests/test-match.py

CHANGE DETAILS

diff --git a/tests/test-match.py b/tests/test-match.py
--- a/tests/test-match.py
+++ b/tests/test-match.py
@@ -316,7 +316,7 @@
 
 # We're using includematcher instead of patterns because it behaves 
slightly
 # better (giving narrower results) than patternmatcher.
-def testVisitdirIncludeIncludfe(self):
+def testVisitdirIncludeInclude(self):
 m1 = matchmod.match(b'', b'', include=[b'path:dir/subdir'])
 m2 = matchmod.match(b'', b'', include=[b'rootfilesin:dir'])
 dm = matchmod.differencematcher(m1, m2)
@@ -430,7 +430,7 @@
 
 # We're using includematcher instead of patterns because it behaves 
slightly
 # better (giving narrower results) than patternmatcher.
-def testVisitdirIncludeIncludfe(self):
+def testVisitdirIncludeInclude(self):
 m1 = matchmod.match(b'', b'', include=[b'path:dir/subdir'])
 m2 = matchmod.match(b'', b'', include=[b'rootfilesin:dir'])
 im = matchmod.intersectmatchers(m1, m2)
@@ -644,7 +644,7 @@
 
 # We're using includematcher instead of patterns because it behaves 
slightly
 # better (giving narrower results) than patternmatcher.
-def testVisitdirIncludeIncludfe(self):
+def testVisitdirIncludeInclude(self):
 m1 = matchmod.match(b'', b'', include=[b'path:dir/subdir'])
 m2 = matchmod.match(b'', b'', include=[b'rootfilesin:dir'])
 um = matchmod.unionmatcher([m1, m2])



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

D7144: status: use unfiltered repo if we're getting status of working copy

2019-10-30 Thread martinvonz (Martin von Zweigbergk) 
martinvonz added a comment.


  In D7144#105248 , @mharbison72 
wrote:
  
  > The comment about maybe affecting extensions made me think of df463ca0adef 
.  
I never figured out what the issue was.
  
  I suspect that's because largefiles creates a repo subclass and that subclass 
does not have the `__getattr__` and `__setattr__` overrides that `repoview` 
does, so setting `lfstatus`or `_largefilesenabled` on that subclass won't 
propagate it to the parent.
  
  In D7144#105352 , @marmoute 
wrote:
  
  > In D7144#105048 , @martinvonz 
wrote:
  >
  >> In D7144#105039 , @marmoute 
wrote:
  >>
  >>>   I have a couple of questions:
  >>>
  >>> […]
  >>>
  >>> - This could cause issue to extensions, can we abtract the unfiltering 
though a function so that extension can rewrap it ?
  >>
  >> To make sure I address it properly, what's the issue(s) you're thinking of?
  >
  > Imagine you are an extension writer adding extra information on status (eg: 
tell the user if his stack is behind and he needs to rebase). If you get an 
unfiltered repository, any operation you run (eg: a revset to check the graph, 
will go horribly wrong). So we need need at minimum an easy way to disable the 
unfiltering if an extension needs to do it.
  >
  > In D7144#105066 , @martinvonz 
wrote:
  >
  >> In D7144#105065 , @marmoute 
wrote:
  >>
  >>> Another question is " Why are we loading the changelog if we don't need 
it ?". If the code were not accessing the changelog, we could not pay the 
filtering code (and save other parsing cost).
  >>
  >> We load the changelog when we do `repo['.']`. I had added a hack 
(https://www.mercurial-scm.org/repo/hg/file/4565a0afc289/mercurial/localrepo.py#l1539)
 to try to avoid that, but it was no longer effective. I think using the 
unfiltered repo is a better solution anyway.
  >
  > If the performance gain are clear, using an unfiltered repository seems 
like a reasonable gain/risk move. However I would prefer this to remain 
temporary. Keeping the repository filtering aligned to the current semantic is 
important to avoid surprise and obscure bug in the future. We need an overall 
effort to reduce the filtering impact anyway (independantly from this series).
  > In the case here, I wonder if we have and easy way to prevent the filtering 
computation kick in, as it is clearly unnecessary.
  >
  > - can you share a profile of what takes time with the filtering version
  > - Do you know which part is triggering the filtering ?
  
  I'll get back to you about the other things later, but I think the following 
profile answers these last questions:
  
| 100.0%  0.19s  dispatch.py:_callcatchline 36:  dispatch.run()
| 100.0%  0.19s  scmutil.py: callcatch line 433:  return 
scmutil.callcatch(ui...
| 100.0%  0.19s  dispatch.py:_runcatchfunc line 177:  return func()
| 100.0%  0.19s  hg.py:  repositoryline 414:  return 
_dispatch(req)
 \ 52.6%  0.10s  localrepo.py:   statusline 6831:  
opts.get(b'subrepos'),
   \ 31.6%  0.06s  localrepo.py:   __getitem__ line 3248:  return 
self[node1].status(
 | 31.6%  0.06s  repoview.py:changelog line 1547:  rev = 
self.changelog.rev(ch...
 | 31.6%  0.06s  repoview.py:filterrevsline 278:  revs = 
filterrevs(unfi, sel...
 | 31.6%  0.06s  repoview.py:computehidden line 217:  
repo.filteredrevcache[filte...
   \ 15.8%  0.03s  repoview.py:hideablerevsline 87:  hidden = 
hideablerevs(repo)
 | 15.8%  0.03s  obsolete.py:getrevs   line 39:  obsoletes = 
obsolete.getrev...
 | 15.8%  0.03s  obscache.py:  line 905:  
repo.obsstore.caches[name] ...
 | 15.8%  0.03s  obscache.py:_computeobsoletesetline  469:  
wrapped = lambda repo: _com...
 | 10.5%  0.02s  phases.py:  getrevset line 434:  notpublic = 
repo._phasecach...
   \  5.3%  0.01s  scmutil.py: __get__ line 106:  return 
super(_basefilecache...
 |  5.3%  0.01s  localrepo.py:   _phasecacheline 1636:  
entry.obj = self.func(obj)
 |  5.3%  0.01s  phases.py:  __init__line 1432:  return 
phases.phasecache(se...
 |  5.3%  0.01s  phases.py:  _readrootsline 235:  
self.phaseroots, self.dirty...
   \  5.3%  0.01s  phases.py:  loadphaserevsline 243:  
self.loadphaserevs(repo)  #...
 |  5.3%  0.01s  phases.py:  _getphaserevsnativeline  
325:  res = self._getphaserevsnat...
 |  5.3%  0.01s  changelog.py:   rev   line 301:  
pycompat.maplist(repo.chang...
   \

[PATCH 3 of 3 STABLE] mail: black wants to add this blank line

2019-10-30 Thread Augie Fackler 
# HG changeset patch
# User Augie Fackler 
# Date 1572467958 14400
#  Wed Oct 30 16:39:18 2019 -0400
# Branch stable
# Node ID 8ab6c4d652ee5ec4ec3e386261067a236385a53f
# Parent  4ec0fed0eeeb1e8c1a7cd7041325d4733e26016a
mail: black wants to add this blank line

I can't figure out how this got overlooked on previous runs, but here
we are. It looks like the culprit change is already public?

diff --git a/mercurial/mail.py b/mercurial/mail.py
--- a/mercurial/mail.py
+++ b/mercurial/mail.py
@@ -445,6 +445,7 @@ if pycompat.ispy3:
 ep = email.parser.BytesParser()
 return ep.parsebytes(data)
 
+
 else:
 
 Generator = email.generator.Generator
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 1 of 3 STABLE] contrib: fix up example fix configuration for our move to released black

2019-10-30 Thread Augie Fackler 
# HG changeset patch
# User Augie Fackler 
# Date 1572466659 14400
#  Wed Oct 30 16:17:39 2019 -0400
# Branch stable
# Node ID 0dc1b2cc8e1784b059728c9d9decaf58a50937ae
# Parent  2278f79e85e96a5ca389f77c47d92a3dc9348443
contrib: fix up example fix configuration for our move to released black

diff --git a/contrib/examples/fix.hgrc b/contrib/examples/fix.hgrc
--- a/contrib/examples/fix.hgrc
+++ b/contrib/examples/fix.hgrc
@@ -5,11 +5,5 @@ clang-format:pattern = (**.c or **.cc or
 rustfmt:command = rustfmt {rootpath}
 rustfmt:pattern = set:**.rs
 
-# We use black, but currently with
-# https://github.com/psf/black/pull/826 applied. For now
-# contrib/grey.py is our fork of black. You need to pip install
-# git+https://github.com/python/black/@d9e71a75ccfefa3d9156a64c03313a0d4ad981e5
-# to have the dependencies for grey.
-#
-# black:command = python3.7 contrib/grey.py --config=black.toml -
-# black:pattern = set:**.py - hgext/fsmonitor/pywatchman/** - 
mercurial/thirdparty/** - "contrib/python-zstandard/** - contrib/grey.py"
+black:command = black --config=black.toml -
+black:pattern = set:**.py - hgext/fsmonitor/pywatchman/** - 
mercurial/thirdparty/** - "contrib/python-zstandard/**"
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 2 of 3 STABLE] hghave: verify we have a black that is new enough for our format

2019-10-30 Thread Augie Fackler 
# HG changeset patch
# User Augie Fackler 
# Date 1572467385 14400
#  Wed Oct 30 16:29:45 2019 -0400
# Branch stable
# Node ID 4ec0fed0eeeb1e8c1a7cd7041325d4733e26016a
# Parent  0dc1b2cc8e1784b059728c9d9decaf58a50937ae
hghave: verify we have a black that is new enough for our format

We require what is currently the absolute latest black, so let's be paranoid.

diff --git a/tests/hghave.py b/tests/hghave.py
--- a/tests/hghave.py
+++ b/tests/hghave.py
@@ -1,5 +1,6 @@
 from __future__ import absolute_import, print_function
 
+import distutils.version
 import os
 import re
 import socket
@@ -982,7 +983,8 @@ def has_emacs():
 
 @check('black', 'the black formatter for python')
 def has_black():
-# use that to actual black as soon as possible
 blackcmd = 'black --version'
-version_regex = b'black, version \d'
-return matchoutput(blackcmd, version_regex)
+version_regex = b'black, version ([0-9a-b.]+)'
+version = matchoutput(blackcmd, version_regex)
+sv = distutils.version.StrictVersion
+return version and sv(version.group(1)) >= sv('19.10b0')
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3 STABLE] formatting: drop `grey`, our custom black version

2019-10-30 Thread Augie Fackler 
On Tue, Oct 29, 2019 at 12:08:58PM +0100, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David 
> # Date 1572343645 -3600
> #  Tue Oct 29 11:07:25 2019 +0100
> # Branch stable
> # Node ID 17c367aaedbc1f72e6d5b19fb1de8828a1dca3ab
> # Parent  0e9acc6539a0d23b2a7428ad86cc4fa79e47d7a5
> # EXP-Topic switch-to-black
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #  hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 
> 17c367aaedbc
> formatting: drop `grey`, our custom black version

queued, thanks

I'll send some follow-ups to try and prevent some footguns, I think.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 2 of 2 STABLE V3] py3: fix patchbomb to accept non-ASCII header value for email preview

2019-10-30 Thread Augie Fackler 
On Wed, Oct 30, 2019 at 10:02:23PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara 
> # Date 1572439788 -32400
> #  Wed Oct 30 21:49:48 2019 +0900
> # Branch stable
> # Node ID 3b53b4da70668d5abd27993778fe2da0606aca45
> # Parent  e7add06518eeebeb236a97ab051bb55e95c591b8
> py3: fix patchbomb to accept non-ASCII header value for email preview

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

Re: [PATCH 7 of 7 stable] packaging: fix buildrpm whitespace

2019-10-30 Thread Augie Fackler 
On Mon, Oct 28, 2019 at 12:37:15AM +0100, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich 
> # Date 1572203819 -3600
> #  Sun Oct 27 20:16:59 2019 +0100
> # Branch stable
> # Node ID c84f1465c44ebc539b803b876206712e0ebd78b4
> # Parent  8c18adcd0177f3ca35f7f20f52f27f5a13ac9f90
> packaging: fix buildrpm whitespace

Queued, many thanks - I have been meaning to try and fix our in-tree
RPM building for a while. Let's plan to switch this to py3 by default
for 5.2.1?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7144: status: use unfiltered repo if we're getting status of working copy

2019-10-30 Thread marmoute (Pierre-Yves David) 
marmoute added a comment.


  
  
  In D7144#105048 , @martinvonz 
wrote:
  
  > In D7144#105039 , @marmoute 
wrote:
  >
  >>   I have a couple of questions:
  >>
  >> […]
  >>
  >> - This could cause issue to extensions, can we abtract the unfiltering 
though a function so that extension can rewrap it ?
  >
  > To make sure I address it properly, what's the issue(s) you're thinking of?
  
  Imagine you are an extension writer adding extra information on status (eg: 
tell the user if his stack is behind and he needs to rebase). If you get an 
unfiltered repository, any operation you run (eg: a revset to check the graph, 
will go horribly wrong). So we need need at minimum an easy way to disable the 
unfiltering if an extension needs to do it.
  
  In D7144#105066 , @martinvonz 
wrote:
  
  > In D7144#105065 , @marmoute 
wrote:
  >
  >> Another question is " Why are we loading the changelog if we don't need it 
?". If the code were not accessing the changelog, we could not pay the 
filtering code (and save other parsing cost).
  >
  > We load the changelog when we do `repo['.']`. I had added a hack 
(https://www.mercurial-scm.org/repo/hg/file/4565a0afc289/mercurial/localrepo.py#l1539)
 to try to avoid that, but it was no longer effective. I think using the 
unfiltered repo is a better solution anyway.
  
  If the performance gain are clear, using an unfiltered repository seems like 
a reasonable gain/risk move. However I would prefer this to remain temporary. 
Keeping the repository filtering aligned to the current semantic is important 
to avoid surprise and obscure bug in the future. We need an overall effort to 
reduce the filtering impact anyway (independantly from this series).
  
  In the case here, I wonder if we have and easy way to prevent the filtering 
computation kick in, as it is clearly unnecessary.
  
  - can you share a profile of what takes time with the filtering version
  - Do you know which part is triggering the filtering ?

REPOSITORY
  rHG Mercurial

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

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

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

D7176: rebase: allow rebasing obsolete commit without successor

2019-10-30 Thread marmoute (Pierre-Yves David) 
marmoute added a comment.
marmoute added subscribers: khanchi97, marmoute.


  That is not necessarly True, If the changeset is pruned, but successors of a 
public changeset, rebasing it would create phase-divergence. @khanchi97 fixed 
similar cases in September.
  
  Overall, we should migrate the rebase code to use the `precheck` logic in 
rewrite util (and make sure things are well covered there).
  
  One can read more about current Sushil work in this area here: 
https://www.mercurial-scm.org/wiki/CEDPrecheckPlan.

REPOSITORY
  rHG Mercurial

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

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

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

Re: [PATCH 2 of 2 STABLE V3] py3: fix patchbomb to accept non-ASCII header value for email preview

2019-10-30 Thread Denis Laxalde 
Yuya Nishihara a écrit :
> # HG changeset patch
> # User Yuya Nishihara 
> # Date 1572439788 -32400
> #  Wed Oct 30 21:49:48 2019 +0900
> # Branch stable
> # Node ID 3b53b4da70668d5abd27993778fe2da0606aca45
> # Parent  e7add06518eeebeb236a97ab051bb55e95c591b8
> py3: fix patchbomb to accept non-ASCII header value for email preview

Looks good to me, thanks!

> Since mail.headencode() is disabled by -n/--test, non-ASCII header value
> has to be allowed.
> 
> Spotted by Denis Laxalde.
> 
> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> --- a/hgext/patchbomb.py
> +++ b/hgext/patchbomb.py
> @@ -960,7 +960,10 @@ def email(ui, repo, *revs, **opts):
>  hdr = pycompat.strurl(hdr)
>  change = True
>  if isinstance(val, bytes):
> -val = pycompat.strurl(val)
> +# header value should be ASCII since it's encoded by
> +# mail.headencode(), but -n/--test disables it and raw
> +# value of platform encoding is stored.
> +val = encoding.strfromlocal(val)
>  if not change:
>  # prevent duplicate headers
>  del m[hdr]
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 2 of 2 STABLE] py3: use native strings when forming email headers in patchbomb

2019-10-30 Thread Denis Laxalde 
Yuya Nishihara a écrit :
> > Nevertheless, I still think this change here is worth. I'm fine if it
> > goes into default or if it waits after the release. Just let me know the
> > plan.
> 
> I also want to remove this TODO, but I don't think inserting many
> strfromlocal()s is the best way to do that. Maybe we'll need some helper
> to cope with py3's new mail API?

I agree, all these strfromlocal() are not nice. At a first glance, I'd
say we should use native strings whenever possible, but there probably
are details here and there...
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D7179: revset: simplify checkstatus() by using any()

2019-10-30 Thread martinvonz (Martin von Zweigbergk) 
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/revset.py

CHANGE DETAILS

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -689,19 +689,15 @@
 if fname not in c.files():
 return False
 else:
-for f in c.files():
-if m(f):
-break
-else:
+if not any(m(f) for f in c.files()):
 return False
 files = repo.status(c.p1().node(), c.node())[field]
 if fname is not None:
 if fname in files:
 return True
 else:
-for f in files:
-if m(f):
-return True
+if any(m(f) for f in files):
+return True
 
 return subset.filter(matches, condrepr=(b'', field, pat))
 



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

D7178: [RFC] rust-matchers: add `Matcher` trait and implement `AlwaysMatcher`

2019-10-30 Thread kevincox (Kevin Cox) 
kevincox added a comment.


  I see a lot of the functions are here to give optimization hints. In order to 
make someone non-familiar with the code able to understand it each method 
should state the contracts that it is making. I am having a really hard time 
reconciling how the different functions interact and which methods have 
precedence over each other.

INLINE COMMENTS

> matchers.rs:28
> +fn exact_match(, _filename: impl AsRef) -> bool;
> +fn file_set() -> Option> {
> +None

Please document.

> matchers.rs:31
> +}
> +fn roots() -> Option> {
> +None

Please document.

> martinvonz wrote in matchers.rs:104
> Should this be `is_always()` per Rust naming conventions?

always isn't an adjective so I don't think that really makes sense. How about 
`matches_everything`.

> matchers.rs:104
> +/// optimization might be possible.
> +fn always() -> bool {
> +false

It is probably worth noting that false-negatives are fine but false-positives 
will lead to incorrect behaviour.

REPOSITORY
  rHG Mercurial

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

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

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

[PATCH 2 of 2 STABLE V3] py3: fix patchbomb to accept non-ASCII header value for email preview

2019-10-30 Thread Yuya Nishihara 
# HG changeset patch
# User Yuya Nishihara 
# Date 1572439788 -32400
#  Wed Oct 30 21:49:48 2019 +0900
# Branch stable
# Node ID 3b53b4da70668d5abd27993778fe2da0606aca45
# Parent  e7add06518eeebeb236a97ab051bb55e95c591b8
py3: fix patchbomb to accept non-ASCII header value for email preview

Since mail.headencode() is disabled by -n/--test, non-ASCII header value
has to be allowed.

Spotted by Denis Laxalde.

diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
--- a/hgext/patchbomb.py
+++ b/hgext/patchbomb.py
@@ -960,7 +960,10 @@ def email(ui, repo, *revs, **opts):
 hdr = pycompat.strurl(hdr)
 change = True
 if isinstance(val, bytes):
-val = pycompat.strurl(val)
+# header value should be ASCII since it's encoded by
+# mail.headencode(), but -n/--test disables it and raw
+# value of platform encoding is stored.
+val = encoding.strfromlocal(val)
 if not change:
 # prevent duplicate headers
 del m[hdr]
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 1 of 2 STABLE V3] tests: check patchbomb with a non-ascii commit message

2019-10-30 Thread Yuya Nishihara 
# HG changeset patch
# User Denis Laxalde 
# Date 1571998245 -7200
#  Fri Oct 25 12:10:45 2019 +0200
# Branch stable
# Node ID e7add06518eeebeb236a97ab051bb55e95c591b8
# Parent  8fda98a6842709c26b61747c7420b28cd874755e
tests: check patchbomb with a non-ascii commit message

This fails on Python 3 but gets fixed in the next changeset.

diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
--- a/tests/test-patchbomb.t
+++ b/tests/test-patchbomb.t
@@ -445,7 +445,9 @@ with a specific bundle type
 
 utf-8 patch:
   $ "$PYTHON" -c 'fp = open("utf", "wb"); fp.write(b"h\xC3\xB6mma!\n"); 
fp.close();'
-  $ hg commit -A -d '4 0' -m 'utf-8 content'
+  $ hg commit -A -d '4 0' \
+  >   --encoding "utf-8" \
+  >   -m `"$PYTHON" -c 'import sys; getattr(sys.stdout, "buffer", 
sys.stdout).write(b"\xc3\xa7a")'`
   adding description
   adding utf
 
@@ -454,16 +456,16 @@ no mime encoding for email --test:
   this patch series consists of 1 patches.
   
   
-  displaying [PATCH] utf-8 content ...
+  displaying [PATCH] ?a ...
   MIME-Version: 1.0
   Content-Type: text/plain; charset="iso-8859-1"
   Content-Transfer-Encoding: quoted-printable
-  Subject: [PATCH] utf-8 content
-  X-Mercurial-Node: 909a00e13e9d78b575aeee23dddbada46d5a143f
+  Subject: [PATCH] ?a
+  X-Mercurial-Node: f81ef97829467e868fc405fccbcfa66217e4d3e6
   X-Mercurial-Series-Index: 1
   X-Mercurial-Series-Total: 1
-  Message-Id: <909a00e13e9d78b575ae.240@test-hostname>
-  X-Mercurial-Series-Id: <909a00e13e9d78b575ae.240@test-hostname>
+  Message-Id: 
+  X-Mercurial-Series-Id: 
   User-Agent: Mercurial-patchbomb/* (glob)
   Date: Thu, 01 Jan 1970 00:04:00 +
   From: quux
@@ -474,18 +476,18 @@ no mime encoding for email --test:
   # User test
   # Date 4 0
   #  Thu Jan 01 00:00:04 1970 +
-  # Node ID 909a00e13e9d78b575aeee23dddbada46d5a143f
+  # Node ID f81ef97829467e868fc405fccbcfa66217e4d3e6
   # Parent  ff2c9fa2018b15fa74b33363bda9527323e2a99f
-  utf-8 content
-  
-  diff -r ff2c9fa2018b -r 909a00e13e9d description
+  ?a
+  
+  diff -r ff2c9fa2018b -r f81ef9782946 description
   --- /dev/nullThu Jan 01 00:00:00 1970 +
   +++ b/descriptionThu Jan 01 00:00:04 1970 +
   @@ -0,0 +1,3 @@
   +a multiline
   +
   +description
-  diff -r ff2c9fa2018b -r 909a00e13e9d utf
+  diff -r ff2c9fa2018b -r f81ef9782946 utf
   --- /dev/nullThu Jan 01 00:00:00 1970 +
   +++ b/utfThu Jan 01 00:00:04 1970 +
   @@ -0,0 +1,1 @@
@@ -497,19 +499,19 @@ mime encoded mbox (base64):
   this patch series consists of 1 patches.
   
   
-  sending [PATCH] utf-8 content ...
+  sending [PATCH] ?a ...
 
   $ cat mbox
   From quux ... ... .. ..:..:..  (re)
   MIME-Version: 1.0
   Content-Type: text/plain; charset="utf-8"
   Content-Transfer-Encoding: base64
-  Subject: [PATCH] utf-8 content
-  X-Mercurial-Node: 909a00e13e9d78b575aeee23dddbada46d5a143f
+  Subject: [PATCH] ?a
+  X-Mercurial-Node: f81ef97829467e868fc405fccbcfa66217e4d3e6
   X-Mercurial-Series-Index: 1
   X-Mercurial-Series-Total: 1
-  Message-Id: <909a00e13e9d78b575ae.240@test-hostname>
-  X-Mercurial-Series-Id: <909a00e13e9d78b575ae.240@test-hostname>
+  Message-Id: 
+  X-Mercurial-Series-Id: 
   User-Agent: Mercurial-patchbomb/* (glob)
   Date: Thu, 01 Jan 1970 00:04:00 +
   From: Q  (no-py3 !)
@@ -518,15 +520,15 @@ mime encoded mbox (base64):
   Cc: bar
   
   IyBIRyBjaGFuZ2VzZXQgcGF0Y2gKIyBVc2VyIHRlc3QKIyBEYXRlIDQgMAojICAgICAgVGh1IEph
-  biAwMSAwMDowMDowNCAxOTcwICswMDAwCiMgTm9kZSBJRCA5MDlhMDBlMTNlOWQ3OGI1NzVhZWVl
-  MjNkZGRiYWRhNDZkNWExNDNmCiMgUGFyZW50ICBmZjJjOWZhMjAxOGIxNWZhNzRiMzMzNjNiZGE5
-  NTI3MzIzZTJhOTlmCnV0Zi04IGNvbnRlbnQKCmRpZmYgLXIgZmYyYzlmYTIwMThiIC1yIDkwOWEw
-  MGUxM2U5ZCBkZXNjcmlwdGlvbgotLS0gL2Rldi9udWxsCVRodSBKYW4gMDEgMDA6MDA6MDAgMTk3
-  MCArMDAwMAorKysgYi9kZXNjcmlwdGlvbglUaHUgSmFuIDAxIDAwOjAwOjA0IDE5NzAgKzAwMDAK
-  QEAgLTAsMCArMSwzIEBACithIG11bHRpbGluZQorCitkZXNjcmlwdGlvbgpkaWZmIC1yIGZmMmM5
-  ZmEyMDE4YiAtciA5MDlhMDBlMTNlOWQgdXRmCi0tLSAvZGV2L251bGwJVGh1IEphbiAwMSAwMDow
-  MDowMCAxOTcwICswMDAwCisrKyBiL3V0ZglUaHUgSmFuIDAxIDAwOjAwOjA0IDE5NzAgKzAwMDAK
-  QEAgLTAsMCArMSwxIEBACitow7ZtbWEhCg==
+  biAwMSAwMDowMDowNCAxOTcwICswMDAwCiMgTm9kZSBJRCBmODFlZjk3ODI5NDY3ZTg2OGZjNDA1
+  ZmNjYmNmYTY2MjE3ZTRkM2U2CiMgUGFyZW50ICBmZjJjOWZhMjAxOGIxNWZhNzRiMzMzNjNiZGE5
+  NTI3MzIzZTJhOTlmCj9hCgpkaWZmIC1yIGZmMmM5ZmEyMDE4YiAtciBmODFlZjk3ODI5NDYgZGVz
+  Y3JpcHRpb24KLS0tIC9kZXYvbnVsbAlUaHUgSmFuIDAxIDAwOjAwOjAwIDE5NzAgKzAwMDAKKysr
+  IGIvZGVzY3JpcHRpb24JVGh1IEphbiAwMSAwMDowMDowNCAxOTcwICswMDAwCkBAIC0wLDAgKzEs
+  MyBAQAorYSBtdWx0aWxpbmUKKworZGVzY3JpcHRpb24KZGlmZiAtciBmZjJjOWZhMjAxOGIgLXIg
+  ZjgxZWY5NzgyOTQ2IHV0ZgotLS0gL2Rldi9udWxsCVRodSBKYW4gMDEgMDA6MDA6MDAgMTk3MCAr
+  MDAwMAorKysgYi91dGYJVGh1IEphbiAwMSAwMDowMDowNCAxOTcwICswMDAwCkBAIC0wLDAgKzEs
+  MSBAQAoraMO2bW1hIQo=
   
   
   >>> import base64
@@ -541,18 +543,18 @@ mime encoded mbox (base64):
   # User test
   # Date 4 0
   #  Thu Jan 01 00:00:04 1970 +
-  # Node ID 909a00e13e9d78b575aeee23dddbada46d5a143f
+  #

D7178: [RFC] rust-matchers: add `Matcher` trait and implement `AlwaysMatcher`

2019-10-30 Thread martinvonz (Martin von Zweigbergk) 
martinvonz added inline comments.
martinvonz added subscribers: spectral, martinvonz.

INLINE COMMENTS

> matchers.rs:19-22
> +Recursive,
> +Empty,
> +Set(HashSet),  // Should we implement a `NonEmptyHashSet`?
> +This,

Could you add a comment explaining what each value means? (I *think* 
`Recursive` means to match all files recursively here, `This` means to visit 
this directory and all subdirectories, `Empty` means to not visit any 
subdirectories, and `Set` means to visit exactly that set.)

> matchers.rs:20-21
> +Recursive,
> +Empty,
> +Set(HashSet),  // Should we implement a `NonEmptyHashSet`?
> +This,

Is `Empty` needed as an optimization? Or could we just use an empty set?

> matchers.rs:25
> +
> +pub trait Matcher {
> +/// Returns whether `filename` is in `file_set`

It may be better to remove everything we're not sure we'll need to start with. 
That way it's easier for you to add new matchers and we won't have to clean up 
later if we never end up needing some of these things. Things I think we can 
remove to start with:

`roots()`
`prefix()`
`anypats()`

> matchers.rs:27
> +/// Returns whether `filename` is in `file_set`
> +fn exact_match(, _filename: impl AsRef) -> bool;
> +fn file_set() -> Option> {

Drop the `_` prefix from `_filename`?

> matchers.rs:28
> +fn exact_match(, _filename: impl AsRef) -> bool;
> +fn file_set() -> Option> {
> +None

`Option>` is usually suspicious, IMO. Can we just return an 
empty set instead of `None`?

> matchers.rs:32
> +fn roots() -> Option> {
> +None
> +}

I think `{''}` (in Python syntax) makes more sense here.

> matchers.rs:35
> +/// Returns whether `filename` is matched by this matcher
> +fn match_fn(, _filename: impl AsRef) -> bool {
> +true

Drop the `_` prefix here too. Does the compiler really warn about unused 
arguments even in trait definitions?

> matchers.rs:35
> +/// Returns whether `filename` is matched by this matcher
> +fn match_fn(, _filename: impl AsRef) -> bool {
> +true

Call it `matches()` instead?

> matchers.rs:36
> +fn match_fn(, _filename: impl AsRef) -> bool {
> +true
> +}

Default to `false` like the Python version does?

> matchers.rs:39-49
> +/// Decides whether a directory should be visited based on whether it
> +/// has potential matches in it or one of its subdirectories. This is
> +/// based on the match's primary, included, and excluded patterns.
> +///
> +/// Returns `VisitDir::Recursive` if the given directory and all
> +/// subdirectories should be visited.
> +/// Otherwise returns `VisitDir::This` or `VisitDir::No` indicating 
> whether

Maybe we can try without this one? It doesn't seem like we need `visit_dir()` 
now that we have `visit_children_set()` and it seems to have a superset of the 
functionality. I asked @spectral about that and they thought we should only 
need the latter, except that it may be slower in some cases, since it needs to 
return a set of children to visit. However, it seems (to me) that that cost 
should be made up for by the speed gained by not visiting uninteresting 
subdirectories. So I would suggest not adding `visit_dir()` to start with and 
we can add it later if it seems useful in some case. We should also see if we 
can switch over existing uses of `visit_dir()` to use `visit_children_set()` in 
the Python code.

> matchers.rs:104
> +/// optimization might be possible.
> +fn always() -> bool {
> +false

Should this be `is_always()` per Rust naming conventions?

> matchers.rs:112-122
> +/// Matcher will match the paths in `files_set()` recursively: 
> optimization
> +/// might be possible
> +fn prefix() -> bool {
> +false
> +}
> +/// None of `.always()`, `.is_exact()`, and `.prefix()` is true:
> +/// optimizations will be difficult.

I think `is_always()` and `is_exact()` are very often useful for optimizations, 
but I'm not sure about these two.

> matchers.rs:114
> +/// might be possible
> +fn prefix() -> bool {
> +false

Similarly, should this be `is_prefix()`?

> matchers.rs:119
> +/// optimizations will be difficult.
> +fn any_pats() -> bool {
> +// TODO rename? It's confusing

And here

> matchers.rs:130-132
> +fn exact_match(, _filename: impl AsRef) -> bool {
> +false
> +}

Seems like a good default implementation. Put this in the trait instead?

REPOSITORY
  rHG Mercurial

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

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

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