Oh, shoot. And I was trying to be incremental/careful. Thank you for
catching that!
I also like how you fixed this. The .run() method did need to be removed
because of that singular usage. Using the new generate_diff() function is a
great solution! (whereas we used to have an object, which made it unclear
on how to use it for this situation)
I've applied your patch in r1914729.
Thanks!
-g
On Sat, Dec 16, 2023 at 6:08 AM Yasuhito FUTATSUKI <futat...@poem.co.jp>
wrote:
> On 2023/12/11 12:54, Yasuhito FUTATSUKI wrote:
> > On 2023/12/10 4:22, Yasuhito FUTATSUKI wrote:
>
> >> 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'
> >> ]]]
>
> The cause was that Output.run() was broken by removal of
> Output.write_binary() on r1912978.
>
> Here is an ad hoc patch:
> [[[
> Fix PropChange.generate
>
> * tools/hook-scripts/mailer/mailer.py
> (OutputBase.run): remove, because below was the only usage.
> (PropChange.generate):
> use generate_diff() to render propchange diff instead of
> OutputBase.run()
>
> Index: tools/hook-scripts/mailer/mailer.py
> ===================================================================
> --- tools/hook-scripts/mailer/mailer.py (revision 1914700)
> +++ tools/hook-scripts/mailer/mailer.py (working copy)
> @@ -211,23 +211,7 @@
> representation."""
> raise NotImplementedError
>
> - def run(self, cmd):
> - """Override this method, if the default implementation is not
> sufficient.
> - Execute CMD, writing the stdout produced to the output
> representation."""
> - # By default we choose to incorporate child stderr into the output
> - pipe_ob = subprocess.Popen(cmd, stdout=subprocess.PIPE,
> - stderr=subprocess.STDOUT,
> - close_fds=sys.platform != "win32")
>
> - buf = pipe_ob.stdout.read(self._CHUNKSIZE)
> - while buf:
> - self.write_binary(buf)
> - buf = pipe_ob.stdout.read(self._CHUNKSIZE)
> -
> - # wait on the child so we don't end up with a billion zombies
> - pipe_ob.wait()
> -
> -
> class MailedOutput(OutputBase):
>
> def start(self, subject_line, group, params):
> @@ -598,12 +582,13 @@
> tempfile2 = tempfile.NamedTemporaryFile()
> tempfile2.write(self.repos.get_rev_prop(self.propname,
> scratch_pool))
> tempfile2.flush()
> - output.run(self.cfg.get_diff_cmd(group, {
> - 'label_from' : 'old property value',
> - 'label_to' : 'new property value',
> - 'from' : tempfile1.name,
> - 'to' : tempfile2.name,
> - }))
> + for diffs in generate_diff(self.cfg.get_diff_cmd(group, {
> + 'label_from' : 'old property value',
> + 'label_to' : 'new property value',
> + 'from' : tempfile1.name,
> + 'to' : tempfile2.name,
> + })):
> + writer.write(to_str(diffs.raw))
> output.finish()
> except MessageSendFailure:
> ret = 1
> ]]]
>
> Cheers,
> --
> Yasuhito FUTATSUKI <futat...@poem.co.jp>/<futat...@yf.bsdclub.org>
>
