Merge pull request #149 from hathitrust/magick-retval-bug

proper handling of return value from magick

lib/HTFeed/Image/Magick.pm

@@ -9,6 +9,9 @@ use HTFeed::Image::Shared;
 # This package contains all of the systemcalls to magick (imagemagick).
 
 # E.g. HTFeed::Image::Magick::compress("a", "b", '-compress' => 'Group4');
+# Magick returns an error code, so 0:good, >0:bad. We flip it to treat it
+# as a perl return value, such that 0: fail, 1: success.
+
 sub compress {
     my $infile  = shift;
     my $outfile = shift;
@@ -42,6 +45,7 @@ sub compress {
     );
     my $sys_ret_val = system($full_cmd);
 
+    # Flip the return value
     return !$sys_ret_val;
 }
 

lib/HTFeed/Stage/ImageRemediate.pm

@@ -877,19 +877,13 @@ sub expand_other_file_formats {
 	my $outfile    = "$path/$outname.tif";
         my $start_time = $self->{job_metrics}->time;
 
-        my $err_code = HTFeed::Image::Magick::compress(
+        my $compress_ok = HTFeed::Image::Magick::compress(
             $infile,
             $outfile,
             '-compress' => 'None'
         );
-	if ($err_code) {
-	    $self->set_error(
-		"OperationFailed",
-		operation => "imagemagick",
-		file      => $infile,
-		detail    => "decompress and ICC profile strip failed: returned $?"
-	    );
-	} else {
+
+	if ($compress_ok) {
             $self->copy_metadata($ext, $infile, $outfile);
             my $infile_size = -s $infile;
             unlink($infile);
@@ -903,6 +897,13 @@ sub expand_other_file_formats {
 	    $self->{job_metrics}->add("ingest_imageremediate_bytes_r_total", $infile_size, $labels);
 	    $self->{job_metrics}->add("ingest_imageremediate_bytes_w_total", -s $outfile, $labels);
 	    $self->{job_metrics}->inc("ingest_imageremediate_images_total", $labels);
+	} else {
+	    $self->set_error(
+		"OperationFailed",
+		operation => "imagemagick",
+		file      => $infile,
+		detail    => "decompress and ICC profile strip failed: returned $?"
+	    );
 	}
     }
 }