[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#497743: patch review



On Wed, Sep 10, 2008 at 10:49:39AM +0200, Xavier Lüthi wrote:
> Perhaps I'm wrong, but to my knowledge and following the man page of
> File::Temp, the function to use in order to create a temporary
> folder is "tempdir" and not "mktempdir" (line 65 in your debdiff
> file).

you're entirely right; somehow all my attempts at doing Perl seem
bound to utter failure :)

Attached is the new diff.

Cheers,

--Seb
Only in .: amlabel-cdrw.8
Only in ./debian: cdrw-taper
Only in ./debian: cdrw-taper.debhelper.log
Only in ./debian: cdrw-taper.substvars
diff -u -r ../cdrw-taper-0.4.bak/debian/changelog ./debian/changelog
--- ../cdrw-taper-0.4.bak/debian/changelog	2008-09-03 15:50:40.000000000 -0700
+++ ./debian/changelog	2008-09-04 09:46:39.000000000 -0700
@@ -1,3 +1,12 @@
+cdrw-taper (0.4-2.1) unstable; urgency=low
+
+  * Non-maintainer upload.
+  * Use File:Temp to generate a temporary file (Closes: #496380).
+  * Use either . or /usr/share/cdrw-taper to find taperlib.pm
+    (Closes: #497743).
+
+ -- Sebastien Delafond <seb@debian.org>  Wed, 03 Sep 2008 16:32:21 -0700
+
 cdrw-taper (0.4-2) unstable; urgency=low
 
   * QA upload.
Only in ./debian: changelog~
diff -u -r ../cdrw-taper-0.4.bak/debian/control ./debian/control
--- ../cdrw-taper-0.4.bak/debian/control	2008-09-03 15:50:40.000000000 -0700
+++ ./debian/control	2008-09-03 16:26:25.000000000 -0700
@@ -8,7 +8,7 @@
 
 Package: cdrw-taper
 Architecture: all
-Depends: amanda-server (>= 1:2.4.4p1-1), perl-base (>= 5.6.0-16), mkisofs (>= 2.0), cdrecord (>= 2.0)
+Depends: amanda-server (>= 1:2.4.4p1-1), perl-base (>= 5.6.0-16), mkisofs (>= 2.0), cdrecord (>= 2.0), libfile-temp-perl
 Suggests: dvd+rw-tools (>= 5.5.4.3.4)
 Description: taper replacement for amanda to support backups to CD-RW or DVD+RW
  The Amanda CDRW-Taper is a drop-in replacement for the taper component of the
Only in ./debian: files
Only in ./doc: changelog
Only in ../cdrw-taper-0.4.bak/doc: CHANGES
diff -u -r ../cdrw-taper-0.4.bak/src/amlabel-cdrw ./src/amlabel-cdrw
--- ../cdrw-taper-0.4.bak/src/amlabel-cdrw	2008-09-03 15:50:40.000000000 -0700
+++ ./src/amlabel-cdrw	2008-09-10 09:08:21.000000000 -0700
@@ -8,11 +8,13 @@
 # intended for use with cdrw-taper
 
 use strict;
+use File::Temp qw/tempdir/;
 
 # Path to taperlib.pm
-push @INC, "/usr/lib/amanda";
-
-require "taperlib.pm";
+my $dir = $0;
+$dir =~ s/[^\/]*$//;
+push @INC, $dir;
+require "/usr/share/cdrw-taper/taperlib.pm";
 
 ##
 ## No user editable settings below here
@@ -69,16 +71,17 @@
     error("Won't label non-erasable media!");
 }
 
-# Write the label file to a temporary directory
-mkdir("/tmp/amlabel-cdrw.$$", 0755) || error("Cannot make directory /tmp/amlabelcd.$$: $!");
-open LABEL, ">/tmp/amlabel-cdrw.$$/AMANDA_LABEL" or error("Cannot create label: $!");
+# Write the label file to a temporary file
+my $tmpDir = tempdir("amlabel-cdrw-XXXXXXXXXXXX", CLEANUP => 1);
+my $amandaLabel = "$tmpDir/AMANDA_LABEL";
+open LABEL, ">$amandaLabel" or error("Cannot create label: $!");
 print LABEL "$NEW_LABEL\n";
 close LABEL;
 
 if ($mediaInfo->getType() eq "CDRW") {
     # Exit silently on errors. mkisofs/cdrecord already generate
     # appropriate messages
-    my $result = system("$taper->{MKISOFS} -J -R -pad -quiet /tmp/amlabel-cdrw.$$ | $taper->{CDRECORD} dev=$writeDev -data blank=fast -");
+    my $result = system("$taper->{MKISOFS} -J -R -pad -quiet '$amandaLabel' | $taper->{CDRECORD} dev=$writeDev -data blank=fast -");
     error("Error writing CD-RW") if ($result / 256 != 0);
 } else { # DVD
     my $result;
@@ -88,20 +91,12 @@
 	    error("Error formatting ".$mediaInfo->getType());
         }
     }
-    $result = system("$taper->{GROWISOFS} -Z $mountDev -J -R -pad -quiet /tmp/amlabel-cdrw.$$");
+    $result = system("$taper->{GROWISOFS} -Z $mountDev -J -R -pad -quiet '$amandaLabel'");
     if ($result / 256 != 0) {
 	error("Error writing ".$mediaInfo->getType());
     }
 }
 
-# Clean up temporary files
-if (-e "/tmp/amlabel-cdrw.$$/AMANDA_LABEL") {
-    unlink "/tmp/amlabel-cdrw.$$/AMANDA_LABEL";
-}
-if (-d "/tmp/amlabel-cdrw.$$") {
-    rmdir "/tmp/amlabel-cdrw.$$";
-}
-
 if ($WRITE_TAPELIST) {
     # Finally, append the new entry to the media list
     open(ML, ">>$taper->{AMANDA_CONF}->{tapelist}")
@@ -114,9 +109,6 @@
 
 # print an error message and exit
 sub error {
-  # Clean up temporary files
-  unlink "/tmp/amlabel-cdrw.$$/AMANDA_LABEL" if -e "/tmp/amlabel-cdrw.$$/AMANDA_LABEL";
-  rmdir "/tmp/amlabel-cdrw.$$" if -d "/tmp/amlabel-cdrw.$$";
   print STDERR "amlabel-cdrw: $_[0]\n" if $_[0];
   exit 1;
 }
Only in ./src: amlabel-cdrw~

Reply to: