[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[Patch] mailer.py

From: Mathias Weinert <mathias.weinert_at_gfa-net.de>
Date: 2005-12-06 17:37:23 CET

Hi there,

I was just starting to test mailer.py and want to provide some
patches for it (and some questions :-) ). I am using it under
a cygwin environment (with WinXP SP2), using subversion
1.3.0-rc4.

1. The script is not very robust against wrong command line
arguments. You get for example an assertion if you call it with
a repos path with a trailing slash (like bash completion
provides it):
assertion "is_canonical (base, blen)" failed: file "subversion/libsvn_subr/path.c", line 114
Aborted (core dumped)

But after thinking a while about this I think this might be
no problem as the script will mostly be used by subversion hooks
which will always provide the right parameters. On the other hand
everybody testing the script before implementing it in a hook
will possibly run into such problems.
In order to prevent this assertion I changed one line in the
script: See hunk 6 in patch at bottom of this mail

2. It took a while for me to accept that propchange and
propchange2 are meant to handle revision properties ;-). So why
not calling these commands revpropchange and revpropchange2? As
it's probably too late to change the command names now I would
at least apply the following patch for the command line help:
See hunks 1 and 5 in patch at bottom of this mail

3. I always run into an error when calling the command 'commit'
when there are changes in other areas and show_nonmatching_paths
is set to yes. I always get:

[...]
Diffs of changes in other areas also in this revision:
Traceback (most recent call last):
  File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 1280, in ?
    sys.argv[3:3+expected_args])
  File "/usr/local/lib/svn-python/svn/core.py", line 217, in run_app
    return apply(func, (_core.application_pool,) + args, kw)
  File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 75, in main
    messenger.generate()
  File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 364, in generate
    group, params, paths, subpool)
  File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 621, in generate_content
    renderer.render(data)
  File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 906, in render
    self._render_diffs(data.other_diffs)
  File "/d/Subversion/subversion-1.3.0-rc4/tools/hook-scripts/mailer/mailer.py", line 941, in _render_diffs
    if not diff.diff and not diff.diff_url:
AttributeError: DiffGenerator instance has no attribute 'diff'

The cause of this is a typo bug at the generation of other
diffs: See hunk 2 in patch at bottom of this mail (looks like
a copy-and-paste-error)

4. Just a small thing when calling the command 'commit' when
there are changes in other areas and show_nonmatching_paths is
set to yes. In this case the line 'Diffs of changes in other
areas also in this revision:' is always printed even if there
are no changes that match the generate_diffs option. To avoid
this I moved the writing of this line into _render_diffs (I am
not a Python programer and I am sure there must be a more
elegant solution to this): See hunks 3 and 4 in patch at bottom
of this mail

5. If you call mailer.py propchange2 with the wrong action it
is waiting for user input. I don't know what this is good for
but am interested in it. Could you please explain me this?

6. A question of understanding the configuration file: Let's
say I am only interested in getting mails about changed .html
files. So I create a group with 'for_paths = .*\.html'. When
doing this all .html files are processed with this group and
all other paths are processed with default. Is there a way
to explicitly say "do nothing with all other paths" -
especially if I have some other groups (.c files, .java
files, ...)? Or do I have to ensure that the default section
doesn't contain a to_addr for example?

Thanks in advance - and keep on developing this wonderful
piece of software (subversion as a total, for sure ;-) )!

Mathias

[[[
* tools/hook-scripts/mailer/mailer.py
  - now also accepting repos name with trailing slash
  - change of command line help parameter names: REVPROPNAME instead of PROPNAME
  - typo bug removed from generate_content()
  - TextCommitRenderer.render() now only showing text regarding diffs in other areas if these diffs are shown theirselves
]]]

--- tools/hook-scripts/mailer/mailer.py.orig 2005-06-24 19:50:11.000000000 +0200
+++ tools/hook-scripts/mailer/mailer.py 2005-12-06 17:28:28.977720100 +0100
@@ -8,8 +8,8 @@
 # $LastChangedRevision: 15162 $
 #
 # USAGE: mailer.py commit REPOS REVISION [CONFIG-FILE]
-# mailer.py propchange REPOS REVISION AUTHOR PROPNAME [CONFIG-FILE]
-# mailer.py propchange2 REPOS REVISION AUTHOR PROPNAME ACTION \
+# mailer.py propchange REPOS REVISION AUTHOR REVPROPNAME [CONFIG-FILE]
+# mailer.py propchange2 REPOS REVISION AUTHOR REVPROPNAME ACTION \
 # [CONFIG-FILE]
 # mailer.py lock REPOS AUTHOR [CONFIG-FILE]
 # mailer.py unlock REPOS AUTHOR [CONFIG-FILE]
@@ -598,7 +598,7 @@
 
   if len(paths) != len(changelist) and show_nonmatching_paths == 'yes':
     other_diffs = DiffGenerator(changelist, paths, False, cfg, repos, date,
- group, params, diffsels, diffurls, pool),
+ group, params, diffsels, diffurls, pool)
   else:
     other_diffs = [ ]
 
@@ -900,10 +900,11 @@
 
     w('\nLog:\n%s\n' % data.log)
 
- self._render_diffs(data.diffs)
+ self._render_diffs(data.diffs,'')
     if data.other_diffs:
- w('\nDiffs of changes in other areas also in this revision:\n')
- self._render_diffs(data.other_diffs)
+ self._render_diffs(data.other_diffs,
+ '\nDiffs of changes in other areas also '
+ 'in this revision:\n')
 
   def _render_list(self, header, data_list):
     if not data_list:
@@ -934,12 +935,16 @@
         w(' - copied%s from r%d, %s%s\n'
           % (text, d.base_rev, d.base_path, is_dir))
 
- def _render_diffs(self, diffs):
+ def _render_diffs(self, diffs, first_diff_text):
     w = self.output.write
 
+ is_first_diff = True
     for diff in diffs:
       if not diff.diff and not diff.diff_url:
         continue
+ if is_first_diff:
+ w('%s' % first_diff_text)
+ is_first_diff = False
       if diff.kind == 'D':
         w('\nDeleted: %s\n' % diff.base_path)
         if diff.diff_url:
@@ -1222,8 +1227,8 @@
     scriptname = os.path.basename(sys.argv[0])
     sys.stderr.write(
 """USAGE: %s commit REPOS REVISION [CONFIG-FILE]
- %s propchange REPOS REVISION AUTHOR PROPNAME [CONFIG-FILE]
- %s propchange2 REPOS REVISION AUTHOR PROPNAME ACTION [CONFIG-FILE]
+ %s propchange REPOS REVISION AUTHOR REVPROPNAME [CONFIG-FILE]
+ %s propchange2 REPOS REVISION AUTHOR REVPROPNAME ACTION [CONFIG-FILE]
        %s lock REPOS AUTHOR [CONFIG-FILE]
        %s unlock REPOS AUTHOR [CONFIG-FILE]
 
@@ -1253,7 +1258,7 @@
     usage()
 
   cmd = sys.argv[1]
- repos_dir = sys.argv[2]
+ repos_dir = os.path.normpath(sys.argv[2])
   try:
     expected_args = cmd_list[cmd]
   except KeyError:

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Dec 6 17:39:23 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.