On 2023/12/10 4:22, Yasuhito FUTATSUKI wrote:
> Hi,
>
> On 2023/12/09 0:04, Daniel Sahlberg wrote:
>
>> Den fre 8 dec. 2023 kl 05:40 skrev Yasuhito FUTATSUKI <
>> futat...@yf.bsdclub.org>:
>
> <snip>
>
>>> Then, how about this patch? It at least mailer-t1.sh passed both
>>> on python=python2.7 and on python=python3.9.
>>>
>>> [[[
>>> Fix inconsistency in path argment on Config.which_group()
>>>
>>> Previously, we call Config.which_group() with path as bytes in
>>> Commit.__init__() and with path as str in Lock.__init__() and
>>> in PropChange.__init__(), but Config.which_group handled path
>>> argment as bytes. To fix it we only use str as path argment on
>>> Config.wich_group().
>>>
>>> * tools/hook-scripts/mailer/mailer.py
>>> (Config.which_groups): Treat path as str.
>>> (Commit.__init__): convert bytes path into str before calling above.
>>>
>>> Found by: Ruediger Pluem (rpluem {_AT_} apache.org)
>>>
>>> Index: tools/hook-scripts/mailer/mailer.py
>>> ===================================================================
>>> --- tools/hook-scripts/mailer/mailer.py (revision 1913728)
>>> +++ tools/hook-scripts/mailer/mailer.py (working copy)
>>> @@ -488,7 +488,7 @@
>>> # collect the set of groups and the unique sets of params for the
>>> options
>>> self.groups = { }
>>> for path, change in self.changelist:
>>> - for (group, params) in self.cfg.which_groups(path, log):
>>> + for (group, params) in self.cfg.which_groups(to_str(path), log):
>>> # turn the params into a hashable object and stash it away
>>> param_list = sorted(params.items())
>>> # collect the set of paths belonging to this group
>>> @@ -1486,9 +1486,9 @@
>>> "Return the path's associated groups."
>>> groups = []
>>> for group, pattern, exclude_pattern, repos_params, search_logmsg_re
>>> in self._group_re:
>>> - match = pattern.match(to_str(path))
>>> + match = pattern.match(path)
>>> if match:
>>> - if exclude_pattern and exclude_pattern.match(to_str(path)):
>>> + if exclude_pattern and exclude_pattern.match(path):
>>> continue
>>> params = repos_params.copy()
>>> params.update(match.groupdict())
>>> ]]]
>>>
>>> Cheers,
>>> --
>>> Yasuhito FUTATSUKI <futat...@yf.bsdclub.org>
>>>
>>
>> This looks good to me! Thanks for the detailed explaination!
>
> Thank you for the review. However, it turned out that even with this
> patch, mailer.py did not work for post-revprop-change hook.
> It caused exception like
>
> [[[
> svn: E165001: post-revprop-change hook failed (exit code 1) with output:
> Traceback (most recent call last):
> File
> "/home/futatuki/tmp/svn-test/mailer_test/repo-smtpoutput/hooks/mailer.py",
> line 1593, in <module>
> ret = svn.core.run_app(main, cmd, config_fname, repos_dir,
> File "/usr/local/lib/python3.9/site-packages/svn/core.py", line 324, in
> run_app
> return func(application_pool, *args, **kw)
> File
> "/home/futatuki/tmp/svn-test/mailer_test/repo-smtpoutput/hooks/mailer.py",
> line 148, in main
> return messenger.generate(output, pool)
> File
> "/home/futatuki/tmp/svn-test/mailer_test/repo-smtpoutput/hooks/mailer.py",
> line 601, in generate
> output.run(self.cfg.get_diff_cmd(group, {
> File
> "/home/futatuki/tmp/svn-test/mailer_test/repo-smtpoutput/hooks/mailer.py",
> line 224, in run
> self.write_binary(buf)
> AttributeError: 'SMTPOutput' object has no attribute 'write_binary'
> ]]]
>
> On 1.14.x branch, the patch can be applied clearly, and it worked
> for post-revprop-change hook.
On 1.14.x without this patch, it mailer.py does not work for revprop-change,
lock, and unlock if at least one group definithion with "for_paths" in
mailer.conf, and the patch fixes it.
(My non-generalized test script did not checked the case that mailer.py
contains for_paths definitions. That is why I missed it.)
So I'll commit it and nominate for backport, although it is too late
1.14.3.
Cheers,
--
Yasuhito FUTATSUKI <futat...@poem.co.jp>/<futat...@yf.bsdclub.org>
