Patchwork [2,of,2] py3: roll up threading.Thread constructor args into **kwargs

login
register
mail settings
Submitter Matt Harbison
Date Oct. 26, 2018, 2:41 a.m.
Message ID <434a43635f1df4375081.1540521692@Envy>
Download mbox | patch
Permalink /patch/36279/
State New
Headers show

Comments

Matt Harbison - Oct. 26, 2018, 2:41 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1540520002 14400
#      Thu Oct 25 22:13:22 2018 -0400
# Node ID 434a43635f1df4375081cd6820c255ecf8ea22ad
# Parent  d69cf134bd50c0891d205000c00aed4e028c9f1d
py3: roll up threading.Thread constructor args into **kwargs

The constructor doesn't have a `verbose` keyword argument in py3.
Boris Feld - Oct. 26, 2018, 8:05 a.m.
LGTM.

I couldn't find any code that was passing those arguments and as the
class is local to the `_windowsworker` function, can we remove the
support for those arguments?

On 26/10/2018 04:41, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1540520002 14400
> #      Thu Oct 25 22:13:22 2018 -0400
> # Node ID 434a43635f1df4375081cd6820c255ecf8ea22ad
> # Parent  d69cf134bd50c0891d205000c00aed4e028c9f1d
> py3: roll up threading.Thread constructor args into **kwargs
>
> The constructor doesn't have a `verbose` keyword argument in py3.
>
> diff --git a/mercurial/worker.py b/mercurial/worker.py
> --- a/mercurial/worker.py
> +++ b/mercurial/worker.py
> @@ -250,10 +250,9 @@ def _posixexitstatus(code):
>  
>  def _windowsworker(ui, func, staticargs, args):
>      class Worker(threading.Thread):
> -        def __init__(self, taskqueue, resultqueue, func, staticargs,
> -                     group=None, target=None, name=None, verbose=None):
> -            threading.Thread.__init__(self, group=group, target=target,
> -                                      name=name, verbose=verbose)
> +        def __init__(self, taskqueue, resultqueue, func, staticargs, *args,
> +                     **kwargs):
> +            threading.Thread.__init__(self, *args, **kwargs)
>              self._taskqueue = taskqueue
>              self._resultqueue = resultqueue
>              self._func = func
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Matt Harbison - Oct. 30, 2018, 1:32 a.m.
On Fri, 26 Oct 2018 04:05:00 -0400, Boris FELD <boris.feld@octobus.net>  
wrote:

> LGTM.
>
> I couldn't find any code that was passing those arguments and as the
> class is local to the `_windowsworker` function, can we remove the
> support for those arguments?

If I remove *args and **kwargs, it works on py2, but not on py3:

+  AttributeError: module 'mercurial.fileset' has no attribute 'match'\r  
(esc)
+  \r (esc)
+  Traceback (most recent call last):\r (esc)
+    File "c:/Users/Matt/projects/hg_py3/hg", line 43, in <module>\r (esc)
+      dispatch.run()\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\dispatch.py",  
line 97, in run\r (esc)
+      status = dispatch(req)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\dispatch.py",  
line 221, in dispatch\r (esc)
+      ret = _runcatch(req) or 0\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\dispatch.py",  
line 364, in _runcatch\r (esc)
+      return _callcatch(ui, _runcatchfunc)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\dispatch.py",  
line 372, in _callcatch\r (esc)
+      return scmutil.callcatch(ui, func)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\scmutil.py", line  
166, in callcatch\r (esc)
+      return func()\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\dispatch.py",  
line 355, in _runcatchfunc\r (esc)
+      return _dispatch(req)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\dispatch.py",  
line 1001, in _dispatch\r (esc)
+      cmdpats, cmdoptions)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\dispatch.py",  
line 744, in runcommand\r (esc)
+      ret = _runcommand(ui, options, cmd, d)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\dispatch.py",  
line 1010, in _runcommand\r (esc)
+      return cmdfunc()\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\dispatch.py",  
line 998, in <lambda>\r (esc)
+      d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)\r  
(esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\util.py", line  
1644, in check\r (esc)
+      return func(*args, **kwargs)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\hgext\\fix.py", line 174, in  
fix\r (esc)
+      for rev, path, newdata in results:\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\worker.py", line  
324, in _windowsworker\r (esc)
+      raise t.exception\r (esc)
+    File "c:\\Program Files\\Python37\\lib\\threading.py", line 917, in  
_bootstrap_inner\r (esc)
+      self.run()\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\worker.py", line  
272, in run\r (esc)
+      for res in self._func(*self._staticargs + (args,)):\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\hgext\\fix.py", line 156, in  
getfixes\r (esc)
+      newdata = fixfile(ui, opts, fixers, ctx, path, basectxs[rev])\r  
(esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\hgext\\fix.py", line 449, in  
fixfile\r (esc)
+      if fixer.affects(opts, fixctx, path):\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\hgext\\fix.py", line 597, in  
affects\r (esc)
+      return scmutil.match(fixctx, [self._fileset], opts)(path)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\scmutil.py", line  
791, in match\r (esc)
+      return matchandpats(ctx, pats, opts, globbed, default,  
badfn=badfn)[0]\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\scmutil.py", line  
782, in matchandpats\r (esc)
+      default, listsubrepos=opts.get('subrepos'), badfn=badfn)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\context.py", line  
1363, in match\r (esc)
+      icasefs=icasefs)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\match.py", line  
182, in match\r (esc)
+      badfn=badfn)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\match.py", line  
103, in _buildkindpatsmatcher\r (esc)
+      listsubrepos=listsubrepos, badfn=badfn)\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\match.py", line  
54, in _expandsets\r (esc)
+      matchers.append(ctx.matchfileset(pat, badfn=badfn))\r (esc)
+    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\context.py", line  
183, in matchfileset\r (esc)
+      return fileset.match(self, expr, badfn=badfn)\r (esc)
+  AttributeError: module 'mercurial.fileset' has no attribute 'match'\r  
(esc)
+  [1]
    $ cat *.changed
-  BAR
-  FOO1
-  FOO2
-
-  $ cd ..
+  bar
+  foo1
+  foo2
+
+  $ cd ..

ERROR: test-fix.t output changed

> On 26/10/2018 04:41, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1540520002 14400
>> #      Thu Oct 25 22:13:22 2018 -0400
>> # Node ID 434a43635f1df4375081cd6820c255ecf8ea22ad
>> # Parent  d69cf134bd50c0891d205000c00aed4e028c9f1d
>> py3: roll up threading.Thread constructor args into **kwargs
>>
>> The constructor doesn't have a `verbose` keyword argument in py3.
>>
>> diff --git a/mercurial/worker.py b/mercurial/worker.py
>> --- a/mercurial/worker.py
>> +++ b/mercurial/worker.py
>> @@ -250,10 +250,9 @@ def _posixexitstatus(code):
>>
>>  def _windowsworker(ui, func, staticargs, args):
>>      class Worker(threading.Thread):
>> -        def __init__(self, taskqueue, resultqueue, func, staticargs,
>> -                     group=None, target=None, name=None, verbose=None):
>> -            threading.Thread.__init__(self, group=group, target=target,
>> -                                      name=name, verbose=verbose)
>> +        def __init__(self, taskqueue, resultqueue, func, staticargs,  
>> *args,
>> +                     **kwargs):
>> +            threading.Thread.__init__(self, *args, **kwargs)
>>              self._taskqueue = taskqueue
>>              self._resultqueue = resultqueue
>>              self._func = func
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - Oct. 30, 2018, 11:28 a.m.
On Mon, 29 Oct 2018 21:32:14 -0400, Matt Harbison wrote:
> On Fri, 26 Oct 2018 04:05:00 -0400, Boris FELD <boris.feld@octobus.net>  
> wrote:
> 
> > LGTM.
> >
> > I couldn't find any code that was passing those arguments and as the
> > class is local to the `_windowsworker` function, can we remove the
> > support for those arguments?
> 
> If I remove *args and **kwargs, it works on py2, but not on py3:
[...]
> +    File "c:\\Users\\Matt\\projects\\hg_py3\\mercurial\\context.py", line  
> 183, in matchfileset\r (esc)
> +      return fileset.match(self, expr, badfn=badfn)\r (esc)
> +  AttributeError: module 'mercurial.fileset' has no attribute 'match'\r  
> (esc)

Perhaps, getfixes() isn't thread safe, and the worker would have to be turned
off on Windows by threadsafe=False.

Patch

diff --git a/mercurial/worker.py b/mercurial/worker.py
--- a/mercurial/worker.py
+++ b/mercurial/worker.py
@@ -250,10 +250,9 @@  def _posixexitstatus(code):
 
 def _windowsworker(ui, func, staticargs, args):
     class Worker(threading.Thread):
-        def __init__(self, taskqueue, resultqueue, func, staticargs,
-                     group=None, target=None, name=None, verbose=None):
-            threading.Thread.__init__(self, group=group, target=target,
-                                      name=name, verbose=verbose)
+        def __init__(self, taskqueue, resultqueue, func, staticargs, *args,
+                     **kwargs):
+            threading.Thread.__init__(self, *args, **kwargs)
             self._taskqueue = taskqueue
             self._resultqueue = resultqueue
             self._func = func