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>

Reply via email to