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

Re: [PATCH] Made commit-email.pl work on both Windows and Unix systems.

From: Michael Wallendahl <mwallend_at_spikus.com>
Date: 2005-01-25 19:42:20 CET

Michael Wallendahl wrote:

> I am resubmitting my patch with a proper subject line and minor
> edits. I also rewrote my log file to follow standards.
> I believe that having commit-email.pl work out of the box on Windows
> systems will aid in its adoption.

I am following the suggestions in the HACKING file and resubmitting my
patch as it has been a while without any responses.

The commit-email.pl Perl script is an excellent tool and I believe that
Windows admins would derive an even greater benefit if it ran on Windows
out of the box. Attached are the necessary modifications to the script
so that it runs on Windows as well as Unix systems.

The "cost" of these modifications is that the script now requires a few
additional Perl packages which may or may not already be installed on
the Unix system (they were found on my FreeBSD 5.3 test box). These
packages are included with the free Windows ActivePerl distribution.
The additional packages are File::Temp and Net::SMTP.

Does anyone have any feedback on why this patch should or should not be
included?

Thanks for your time,

-Mike

Made commit-email.pl work on both Windows and Unix systems:

Three major changes had to be made in order for the script to run under
Windows as well as Unix:

1. Eliminate hard coded /tmp path for temporary directory.
2. Remove dependency on external sendmail program.
3. Rework the 'safe_read_from_pipe' subroutine to not fork under Windows
   systems.

* commit-email.pl:

  Removed hard coded /tmp path by including temporary directory code from
  'contrib/client-side/svn_load_dirs.pl' submitted by Ian Brockbank. Requires
  File::Temp and Cwd. Adds $orig_dir, package Temp::Delete, and installs
  signal handlers to clean up temporary directories if necessary.

  Removed dependency on 'sendmail' by using Net::SMTP package. Included code
  submitted by Benjamin Garret and Jeff Cave to dev@ list on April 9, 2004.
  Unix systems may be able to just set $mailserver to "localhost". Requires
  Net::SMTP. Adds $mailserver, $mailerDebugLevel, $anonymousFromAddressPrefix.
  Removed check logic for 'sendmail' program as it is not needed now.

  Included rewritten 'safe_read_from_pipe' subroutine found in contrib/client-
  side/svn_load_dirs.pl submitted by Ian Brockbank. This allows Windows
  systems to call the subroutine while retaining existing functionality for
  compatible systems.
  
  Created new $scriptDebug variable. If true, script prints out debug
  messages to STDERR during run.
  
  Updated copyright year to include 2005.
  
  Set $mail_from to "subversion" if $author not defined (e.g. by an anonymous
  commit). Still can be overridden by '--from' option as before. Set
  $anonymousFromAddressPrefix to "" if you want previous script behavior.
  Included code submitted by Benjamin Garret and Jeff Cave to dev@ list on
  April 9, 2004.

Index: commit-email.pl.in
===================================================================
--- commit-email.pl.in (revision 12791)
+++ commit-email.pl.in (working copy)
@@ -13,7 +13,7 @@
 # $LastChangedRevision$
 #
 # ====================================================================
-# Copyright (c) 2000-2004 CollabNet. All rights reserved.
+# Copyright (c) 2000-2005 CollabNet. All rights reserved.
 #
 # This software is licensed as described in the file COPYING, which
 # you should have received as part of this distribution. The terms
@@ -37,32 +37,51 @@
 use strict;
 use Carp;
 
+use File::Temp 0.12 qw(tempdir tempfile);
+use Cwd;
+use Net::SMTP;
+
 ######################################################################
 # Configuration section.
 
-# Sendmail path.
-my $sendmail = "/usr/sbin/sendmail";
+# Set to 1 if you want general script debug info printed to stderr.
+my $scriptDebug = 0;
 
-# Svnlook path.
+# Svnlook path. For example:
+# "C:\\Program Files\\Subversion\\bin\\svnlook.exe"
+# "/usr/local/bin/svnlook"
 my $svnlook = "@SVN_BINDIR@/svnlook";
 
+# Mail Server name and Debug Level.
+# Set $mailDebugLevel to 1 for mailer debug info to stderr.
+my $mailserver = "localhost";
+my $mailerDebugLevel = 0;
+
+# The default sender address prefix to use when an anonymous commit
+# is found. Set to "" for previous behavior.
+my $anonymousFromAddressPrefix = "subversion";
+
 # By default, when a file is deleted from the repository, svnlook diff
 # prints the entire contents of the file. If you want to save space
 # in the log and email messages by not printing the file, then set
 # $no_diff_deleted to 1.
 my $no_diff_deleted = 0;
+
 # By default, when a file is added to the repository, svnlook diff
 # prints the entire contents of the file. If you want to save space
 # in the log and email messages by not printing the file, then set
 # $no_diff_added to 1.
 my $no_diff_added = 0;
 
+# End Configuration Section.
+######################################################################
+
 # Since the path to svnlook depends upon the local installation
 # preferences, check that the required programs exist to insure that
 # the administrator has set up the script properly.
 {
   my $ok = 1;
- foreach my $program ($sendmail, $svnlook)
+ foreach my $program ($svnlook)
     {
       if (-e $program)
         {
@@ -82,7 +101,6 @@
   exit 1 unless $ok;
 }
 
-
 ######################################################################
 # Initial setup/command-line handling.
 
@@ -212,11 +230,38 @@
 }
 
 ######################################################################
+# Create a temporary directory for svn to work in.
+
+my $tmp_dir = tempdir( "svn_commit_email_XXXXXXXXXX", TMPDIR => 1 );
+print STDERR "Temporary Directory created: '$tmp_dir'\n" if $scriptDebug;
+
+# Put in a signal handler to clean up any temporary directories.
+sub catch_signal {
+ my $signal = shift;
+ warn "$0: caught signal $signal. Quitting now.\n";
+ exit 1;
+}
+
+$SIG{HUP} = \&catch_signal;
+$SIG{INT} = \&catch_signal;
+$SIG{TERM} = \&catch_signal;
+$SIG{PIPE} = \&catch_signal;
+
+# Create an object that when DESTROY'ed will delete the temporary
+# directory. The CLEANUP flag to tempdir should do this, but they
+# call rmtree with 1 as the last argument which takes extra security
+# measures that do not clean up the .svn directories.
+my $tmp_dir_cleanup = Temp::Delete->new; # Package Temp::Delete defined
+ # at end of file.
+
+my $orig_dir = cwd; # Save the original working dir so that we can chdir
+ # back to it at end of script for cleanup of $temp_dir.
+
+######################################################################
 # Harvest data using svnlook.
 
-# Change into /tmp so that svnlook diff can create its .svnlook
-# directory.
-my $tmp_dir = '/tmp';
+# Change into a temporary directory so that svnlook diff can create its
+# .svnlook directory.
 chdir($tmp_dir)
   or die "$0: cannot chdir `$tmp_dir': $!\n";
 
@@ -404,11 +449,20 @@
       {
         $subject = "r$rev - $dirlist";
       }
+
     if ($subject_prefix =~ /\w/)
       {
         $subject = "$subject_prefix $subject";
       }
+
     my $mail_from = $author;
+ if ((! defined $mail_from) || $mail_from eq "" )
+ {
+ if (! ($anonymousFromAddressPrefix eq ""))
+ {
+ $mail_from = $anonymousFromAddressPrefix;
+ }
+ }
 
     if ($from_address =~ /\w/)
       {
@@ -418,7 +472,7 @@
       {
         $mail_from = "$mail_from\@$hostname";
       }
-
+
     my @head;
     push(@head, "To: $to\n");
     push(@head, "From: $mail_from\n");
@@ -455,22 +509,26 @@
 
     push(@head, "\n");
 
- if ($sendmail =~ /\w/ and @email_addresses)
+
+ # Send the message out;
+ print STDERR "Begin sending mail.\n" if $scriptDebug;
+ my $mailer = Net::SMTP->new($mailserver, Timeout => 60,
+ Debug => $mailerDebugLevel)
+ or (die "$0: cannot open server '$mailserver' for writing: $!\n");
+
+ $mailer->mail($mail_from);
+ foreach my $emailAddress (@email_addresses)
       {
- # Open a pipe to sendmail.
- my $command = "$sendmail -f$mail_from $userlist";
- if (open(SENDMAIL, "| $command"))
- {
- print SENDMAIL @head, @body;
- close SENDMAIL
- or warn "$0: error in closing `$command' for writing: $!\n";
- }
- else
- {
- warn "$0: cannot open `| $command' for writing: $!\n";
- }
+ $mailer->to($emailAddress);
       }
+ $mailer->data();
 
+ $mailer->datasend(@head);
+ $mailer->datasend(@body);
+ $mailer->dataend();
+ $mailer->quit;
+ print STDERR "Finished sending mail.\n" if $scriptDebug;
+
     # Dump the output to logfile (if its name is not empty).
     if ($log_file =~ /\w/)
       {
@@ -479,6 +537,7 @@
             print LOGFILE @head, @body;
             close LOGFILE
               or warn "$0: error in closing `$log_file' for appending: $!\n";
+ print STDERR "Wrote to '$log_file'.\n" if $scriptDebug;
           }
         else
           {
@@ -542,18 +601,49 @@
       croak "$0: safe_read_from_pipe passed no arguments.\n";
     }
 
- my $pid = open(SAFE_READ, '-|');
- unless (defined $pid)
+ my $openfork_available = "MSWin32" ne $^O;
+ if ($openfork_available) # We can fork on this system.
     {
- die "$0: cannot fork: $!\n";
+ my $pid = open(SAFE_READ, "-|");
+ unless (defined $pid)
+ {
+ die "$0: cannot fork: $!\n";
+ }
+ unless ($pid)
+ {
+ # child
+ open(STDERR, ">&STDOUT")
+ or die "$0: cannot dup STDOUT: $!\n";
+ exec(@_)
+ or die "$0: cannot exec '@_': $!\n";
+ }
     }
- unless ($pid)
+ else # Running on Windows. No fork.
     {
- open(STDERR, ">&STDOUT")
- or die "$0: cannot dup STDOUT: $!\n";
- exec(@_)
- or die "$0: cannot exec `@_': $!\n";
+ my @commandline = ();
+ my $command;
+
+ while ($command = shift)
+ {
+ # Munge the command to protect it from the command line
+ $command =~ s/\"/\\\"/g; # Replace all " with \"
+ if ($command =~ m/\s/) { $command = "\"$command\""; }
+ if ($command eq "") { $command = "\"\""; }
+ if ($command =~ m/\n/)
+ {
+ warn "$0: carriage return detected in command - may not work\n";
+ }
+ push(@commandline, $command);
+ }
+
+ print STDERR "Running @commandline\n" if $scriptDebug;
+
+ # Now do the pipe.
+ open(SAFE_READ, "@commandline |")
+ or die "$0: cannot pipe to command: $!\n";
     }
+
+ # parent
   my @output;
   while (<SAFE_READ>)
     {
@@ -598,3 +688,22 @@
       return @output;
     }
 }
+
+# This package exists just to delete the temporary directory.
+package Temp::Delete;
+
+sub new
+{
+ bless {}, shift;
+}
+
+sub DESTROY
+{
+ print STDERR "Cleaning up '$tmp_dir'.\n" if $scriptDebug;
+
+ # Have to back out of the tmp_dir before we can remove
+ # it (at least on Windows).
+ chdir($orig_dir)
+ or warn "$0: cannot chdir '$orig_dir': $!\n";
+ File::Path::rmtree([$tmp_dir], 0, 0);
+}

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jan 25 19:48:07 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.