You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2004/03/19 15:17:48 UTC

[PATCH] Making propchange-email.pl send diffs

> 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

Re: [PATCH] Making propchange-email.pl send diffs

Posted by Julian Foad <ju...@btopenworld.com>.
Julian Foad wrote:
> 
> 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.

I have fixed issue 952 in r9211, and committed this enhancement to propchange-email.pl.in in r9216, so you can now have diffs e-mailed for revprop changes.  The example below shows one way to do it.

- Julian


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