We recently added code to get more information on file copy errors, by catching exceptions. However, not all copy failures throw exceptions - we also need to check the return value from copy().

CommentFileSizeAuthor
#1 file.inc_.patch682 bytesgargsuchi

Comments

gargsuchi’s picture

gargsuchi commented
StatusFileSize
new file.inc_.patch682 bytes

Please find attached a patch file for the same.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 1: file.inc_.patch, failed testing.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented 
  1. +++ plugins/destinations/file.inc
@@ -390,7 +390,12 @@ class MigrateFileUri extends MigrateFile {
+      $copied = copy($this->sourcePath, $destination);
+      if ($copied == FALSE) {

    Why the $copied variable, rather than calling copy() in the if? 

  2. +++ plugins/destinations/file.inc
@@ -390,7 +390,12 @@ class MigrateFileUri extends MigrateFile {
+        $migration->saveMessage(t('Error copying the file %file"',
+          array('%file' => $this->sourcePath)));

    Let's get the destination in the message as well.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs work » Fixed

Committed (with some modification), thanks!

  • mikeryan committed dd50f19 on 7.x-2.x 
    Issue #2537206 by gargsuchi,mikeryan: copy() return not checked
mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Issue tags: +Migrate 2.9

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.