> On Wed, Mar 17, 2004 at 04:06:41PM +0000, Julian Foad wrote:
>
>>Whenever I see a prop-change email (usually for svn:log) I always wonder
>>what has been changed, and so I would like it to show the previous value or
>>preferably a diff.
This patch allows propchange-email.pl to accept a diff file to put in the message instead of just a copy of the new property value. If the diff file is not supplied, it works as before. A diff file can be generated by the pre-revprop-change hook when issue 952 is fixed, or by the post-revprop-change hook if the pre- hook saved the previous value.
There is onle line of Perl that I need help with:
+ (my $status, @svnlines) = &safe_read_from_pipe('/bin/cat', $diff_file);
+ ### Should read directly from the file instead of invoking 'cat'.
Can someone tell me a better way to read in the contents of file $diff-file?
The second patch attached is a diff of the hook templates, giving an example of how to use this new feature. I can make a patch for the source code that writes these templates, if you want.
Ben Reser wrote:
> Dumb question, but why don't you just do the actual diff in the
> pre-revprop? You have access to the new proval as STDIN? Then just
> write out the diff to a temp file that the post hook reads and sends
> along.
I hadn't noticed that it was supposed to be available on stdin, so thanks for pointing that out. It wasn't working but I have a fix now. But I already had access to the before and after values, in the post-change hook, so I could have done the diff there. However, I thought it would be better (more portable etc.) for most of the processing to be done within the single propchange-email.pl program. I have changed my mind now, and re-written this patch to do it that way, and made it backward-compatible in that propchange-email.pl has the old behaviour unless the hook specifically passes it a file containing the diff.
> Also I'd add the author name to the filename. It's possible that two
> people would try to edit the same property at the same time. GMTA. But
> unlikely that the same person is going to be editing the same property
> at the same time. It's still possible but pretty unlikely.
Yup. I originally thought that the user name would sometimes contain unacceptable characters, but now I think that is unlikely. The property name will hardly ever be anything but svn:log, so is of little use in distinguishing between simultaneous accesses. It also always contains a colon which would require substitution to make it work on Windows; not too difficult but an extra hassle. Therefore I have changed to using the rev number and author but not the property name.
In my first attempt, the file name had to be generated independently by the hook scripts and by propchange-email.pl, so I wanted to avoid anything remotely complicated like substitution of characters, but now I have a cleaner solution where the file name is passed from the hook to the Perl program, so the hooks can use whatever file name they like.
- Julian
Allow propchange-email.pl to accept a diff file to put in the message instead
of just a copy of the property value. If the diff file is not supplied, it
works as before.
* tools/hook-scripts/propchange-email.pl.in
Add a new option "-d diff_file" that specifies a file whose content is used
instead of the property value.
Index: tools/hook-scripts/propchange-email.pl.in
===================================================================
--- tools/hook-scripts/propchange-email.pl.in (revision 9143)
+++ tools/hook-scripts/propchange-email.pl.in (working copy)
@@ -86,6 +86,7 @@
my $rev;
my $author;
my $propname;
+my $diff_file;
# Use the reference to the first project to populate.
my $current_project = $project_settings_list[0];
@@ -94,6 +95,7 @@
# project. If a key exists but has a false value (''), then the
# command line option is allowed but requires special handling.
my %opt_to_hash_key = ('--from' => 'from_address',
+ '-d' => '',
'-h' => 'hostname',
'-l' => 'log_file',
'-m' => '',
@@ -124,13 +126,24 @@
else
{
# Here handle -m.
- unless ($arg eq '-m')
+ if ($arg eq '-m')
{
- die "$0: internal error: should only handle -m here.\n";
+ $current_project = &new_project;
+ $current_project->{match_regex} = $value;
+ push(@project_settings_list, $current_project);
+ }
+ elsif ($arg eq '-d')
+ {
+ if ($diff_file)
+ {
+ die "$0: command line option `$arg' can only be used once.\n";
+ }
+ $diff_file = $value;
+ }
+ else
+ {
+ die "$0: internal error: should not be handling `$arg' here.\n";
}
- $current_project = &new_project;
- $current_project->{match_regex} = $value;
- push(@project_settings_list, $current_project);
}
}
elsif ($arg =~ /^-/)
@@ -210,12 +223,22 @@
}
######################################################################
-# Harvest data using svn.
+# Harvest data.
-# Get the new property value svn.
-my $repos_url = 'file://' . &abs_path($repos);
-my @svnlines = &read_from_process($svn, 'propget', '--revprop', '-r', $rev,
- $propname, $repos_url);
+my @svnlines;
+# Get the diff file if it was provided, otherwise the property value.
+if ($diff_file)
+ {
+ (my $status, @svnlines) = &safe_read_from_pipe('/bin/cat', $diff_file);
+ ### Should read directly from the file instead of invoking 'cat'.
+ }
+else
+ {
+ # Get the property value using svn.
+ my $repos_url = 'file://' . &abs_path($repos);
+ @svnlines = &read_from_process($svn, 'propget', '--revprop', '-r', $rev,
+ $propname, $repos_url);
+ }
# Figure out what directories have changed using svnlook. This is
# merely so we can determine what project might care about receiving
@@ -232,7 +255,10 @@
push(@body, "Revision: $rev\n");
push(@body, "Property Name: $propname\n");
push(@body, "\n");
-push(@body, "New Property Value:\n");
+unless ($diff_file)
+ {
+ push(@body, "New Property Value:\n");
+ }
push(@body, map { /[\r\n]+$/ ? $_ : "$_\n" } @svnlines);
push(@body, "\n");
@@ -351,7 +377,7 @@
sub usage
{
warn "@_\n" if @_;
- die "usage: $0 REPOS REVNUM USER PROPNAME [[-m regex] [options] [email_addr ...]] ...\n",
+ die "usage: $0 REPOS REVNUM USER PROPNAME [-d diff_file] [[-m regex] [options] [email_addr ...]] ...\n",
"options are\n",
" --from email_address Email address for 'From:' (overrides -h)\n",
" -h hostname Hostname to append to author for 'From:'\n",
@@ -360,6 +386,9 @@
" -r email_address Email address for 'Reply-To:'\n",
" -s subject_prefix Subject line prefix\n",
"\n",
+ "The message will contain a copy of the diff_file if it is provided,\n",
+ "otherwise a copy of the (assumed to be new) property value.\n",
+ "\n",
"This script supports a single repository with multiple projects,\n",
"where each project receives email only for changes to properties\n",
"in revisions which otherwise modified that project. A project is\n",
Allow propchange-email.pl to accept a diff file to put in the message instead
of just a copy of the new property value. If the diff file is not supplied,
it works as before.
* pre-revprop-change.tmpl
Generate a diff of the property value.
* post-revprop-change.tmpl
Pass the saved diff file to propchange-email.pl.
### For a test, r5170 has a typo: "te"->"the".
Index: pre-revprop-change.tmpl
===================================================================
--- pre-revprop-change.tmpl
+++ pre-revprop-change.tmpl (working copy)
@@ -45,5 +45,16 @@
USER="$3"
PROPNAME="$4"
-if [ "$PROPNAME" = "svn:log" ]; then exit 0; fi
-exit 1
+# Don't allow changes to revision properties other than svn:log
+if [ "$PROPNAME" != "svn:log" ]; then exit 1; fi
+
+# Generate a diff of the change, to be used by propchange-email.pl
+ID="r$REV-$USER"
+DIFF_FILE="$ID.diff"
+svnlook log -r "$REV" "$REPOS" > "$ID.old"
+cat - > "$ID.new"
+echo >> "$ID.new" # Add an extra newline, like svn and svnlook do.
+diff -u -L "$PROPNAME (old)" -L "$PROPNAME (new)" "$ID.old" "$ID.new" > "$DIFF_FILE"
+rm "$ID.old" "$ID.new"
+
+exit 0
Index: post-revprop-change.tmpl
===================================================================
--- post-revprop-change.tmpl
+++ post-revprop-change.tmpl (working copy)
@@ -37,4 +37,6 @@
USER="$3"
PROPNAME="$4"
-propchange-email.pl "$REPOS" "$REV" "$USER" "$PROPNAME" watchers@example.org
+DIFF_FILE="r$REV-$USER.diff"
+propchange-email.pl "$REPOS" "$REV" "$USER" "$PROPNAME" -d "$DIFF_FILE" watchers@example.org
+rm "$DIFF_FILE"
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Mar 20 19:46:25 2004