merged branch romainneutron/FilesystemExceptions (PR #4330)

fabpot · fabpot · commit 55f682c9bead · 2012-06-19T17:08:20.000+02:00
Commits ------- a20fc68 Merge pull request #1 from SamsonIT/FilesystemExceptions 8eca661 [FileSystem] explains possible failure of symlink creation in windows b1f8744 Add Changelog BC Break note 24eb396 [Filesystem] Added few new behaviors: Discussion ---------- [Filesystem] Consistence and enhancements for Filesystem Bug fix: no Feature addition: yes Backwards compatibility break: **yes** Symfony2 tests pass: yes Fixes the following tickets: None License of the code: MIT This PR adds features and introduce a backward compatibility break. features : - whenever an action fails, a \RuntimeException is thrown - add access to the second and third arguments of ``touch`` function - add a recursive option for chmod - add a chown method - add a chgrp method The backward compatibility break happens in the mkdir method : Before this PR, a boolean is returned ; true if all directories were created, false otherwise. It now returns nothing. --------------------------------------------------------------------------- by travisbot at 2012-05-18T14:26:42Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1367000) (merged 83cdd622 into 1e15f21). --------------------------------------------------------------------------- by fabpot at 2012-05-20T02:40:28Z To be consistent, we should throw exception whenever some operation fails. --------------------------------------------------------------------------- by romainneutron at 2012-05-20T21:10:23Z I fix the consistency ; mkdir now throws an exception if a directory creation fails. This introduce a BC break, see PR message which has been updated with all features and BC break. Added chgrp and chown methods Add options for touch Add recursive option for chmod --------------------------------------------------------------------------- by travisbot at 2012-05-20T21:11:47Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1383619) (merged a4d1eeb8 into 1407f11). --------------------------------------------------------------------------- by travisbot at 2012-05-22T10:49:06Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399027) (merged 7e14b6bd into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-22T10:58:10Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399083) (merged 71852653 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-22T11:18:44Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399194) (merged 7645bad3 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-23T18:21:47Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414091) (merged b049d5b1 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-23T18:26:19Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414123) (merged 34903466 into 517ae43). --------------------------------------------------------------------------- by travisbot at 2012-05-29T16:07:26Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467173) (merged b1d1eb2e into adf07f1). --------------------------------------------------------------------------- by travisbot at 2012-05-29T16:19:38Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1467261) (merged 42015ffa into adf07f1). --------------------------------------------------------------------------- by romainneutron at 2012-06-01T14:30:45Z Any news about this PR ? --------------------------------------------------------------------------- by stloyd at 2012-06-08T09:57:39Z @romainneutron You need to [squash](http://www.silverwareconsulting.com/index.cfm/2010/6/6/Using-Git-Rebase-to-Squash-Commits) your commits, and add more proper message in squashed commit i.e.: > [Filesystem] Added few new behaviors: * whenever an action fails, a `RuntimeException` is thrown * add access to the second and third arguments of `touch()` function * add a recursive option for `chmod()` * add a `chown()` method * add a `chgrp()` method > BC break: `mkdir()` function throw exception in case of failture instead of returning Boolean value. --------------------------------------------------------------------------- by romainneutron at 2012-06-08T10:59:55Z @stloyd squash done ! --------------------------------------------------------------------------- by travisbot at 2012-06-08T11:26:20Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1565540) (merged 8f55ddb6 into f8e68e5). --------------------------------------------------------------------------- by travisbot at 2012-06-08T11:41:45Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1566247) (merged 880312b6 into f8e68e5). --------------------------------------------------------------------------- by romainneutron at 2012-06-09T11:42:24Z I've added documentation to the Filesystem component : symfony/symfony-docs#1439 --------------------------------------------------------------------------- by travisbot at 2012-06-09T16:47:20Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577754) (merged 5647ad41 into f8a09db). --------------------------------------------------------------------------- by stloyd at 2012-06-13T14:47:31Z @romainneutron You probably need to rebase your code as some changes were merge into master for `Filesystem`. --------------------------------------------------------------------------- by romainneutron at 2012-06-13T15:17:31Z @stloyd rebase OK ! by the way, do you have any idea when/if this PR will be merged ? --------------------------------------------------------------------------- by travisbot at 2012-06-13T15:20:44Z This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1611591) (merged c8b86c68 into c07e916). --------------------------------------------------------------------------- by fabpot at 2012-06-16T16:40:50Z You need to add a note about the BC breaks in the CHANGELOG file. --------------------------------------------------------------------------- by fabpot at 2012-06-16T16:43:20Z Also, instead of using `\RuntimeException`, I would create a custom exception like we have done in other components (an interface + a RuntimeException that implements the interface and extends \RuntimeException). The exception name can be something like `IOException`. --------------------------------------------------------------------------- by travisbot at 2012-06-18T10:11:20Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645757) (merged 925a8234 into 0b8b76b). --------------------------------------------------------------------------- by stloyd at 2012-06-18T10:14:52Z @fabpot Anything blocking merge of this PR ? (tests are failing because of issue in master, not releted to this PR) --------------------------------------------------------------------------- by romainneutron at 2012-06-18T10:29:20Z @fabpot @stloyd the latest push was just a rebase push for PR 4577 (symfony/symfony#4577) I'm currently fixing the Exception and changelog things, I'll push very soon --------------------------------------------------------------------------- by romainneutron at 2012-06-18T10:44:38Z @fabpot I've added the exception and the exception interface, add the changelog info --------------------------------------------------------------------------- by travisbot at 2012-06-18T10:53:34Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1645981) (merged 634d6fb9 into 0b8b76b). --------------------------------------------------------------------------- by romainneutron at 2012-06-18T11:08:43Z As reported by @stloyd the PR is failing due to an issue in the master, I re-push and trig the PR build when this issue is solved --------------------------------------------------------------------------- by travisbot at 2012-06-18T11:16:58Z This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1646006) (merged 2f65945a into 0b8b76b).
diff --git a/src/Symfony/Component/Filesystem/CHANGELOG.md b/src/Symfony/Component/Filesystem/CHANGELOG.md
@@ -4,4 +4,5 @@ CHANGELOG
 2.1.0
 -----
 
+ * 24eb396 : BC Break : mkdir() function now throws exception in case of failure instead of returning Boolean value
  * created the component
diff --git a/src/Symfony/Component/Filesystem/Exception/ExceptionInterface.php b/src/Symfony/Component/Filesystem/Exception/ExceptionInterface.php
@@ -0,0 +1,24 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Filesystem\Exception;
+
+/**
+ * Exception interface for all exceptions thrown by the component.
+ *
+ * @author Romain Neutron <imprec@gmail.com>
+ *
+ * @api
+ */
+interface ExceptionInterface
+{
+
+}
diff --git a/src/Symfony/Component/Filesystem/Exception/IOException.php b/src/Symfony/Component/Filesystem/Exception/IOException.php
@@ -0,0 +1,24 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Filesystem\Exception;
+
+/**
+ * Exception class thrown when a filesystem operation failure happens
+ *
+ * @author Romain Neutron <imprec@gmail.com>
+ *
+ * @api
+ */
+class IOException extends \RuntimeException implements ExceptionInterface
+{
+
+}
diff --git a/src/Symfony/Component/Filesystem/Filesystem.php b/src/Symfony/Component/Filesystem/Filesystem.php
@@ -11,6 +11,8 @@
 
 namespace Symfony\Component\Filesystem;
 
+use Symfony\Component\Filesystem\Exception\IOException;
+
 /**
  * Provides basic utility to manipulate the file system.
  *
@@ -28,6 +30,8 @@ class Filesystem
      * @param string $originFile The original filename
      * @param string $targetFile The target filename
      * @param array  $override   Whether to override an existing file or not
+     *
+     * @throws IOException When copy fails
      */
     public function copy($originFile, $targetFile, $override = false)
     {
@@ -40,30 +44,31 @@ public function copy($originFile, $targetFile, $override = false)
         }
 
         if ($doCopy) {
-            copy($originFile, $targetFile);
+            if (true !== @copy($originFile, $targetFile)) {
+                throw new IOException(sprintf('Failed to copy %s to %s', $originFile, $targetFile));
+            }
         }
     }
 
     /**
      * Creates a directory recursively.
      *
      * @param string|array|\Traversable $dirs The directory path
-     * @param int                       $mode The directory mode
+     * @param integer                   $mode The directory mode
      *
-     * @return Boolean true if the directory has been created, false otherwise
+     * @throws IOException On any directory creation failure
      */
     public function mkdir($dirs, $mode = 0777)
     {
-        $ret = true;
         foreach ($this->toIterator($dirs) as $dir) {
             if (is_dir($dir)) {
                 continue;
             }
 
-            $ret = @mkdir($dir, $mode, true) && $ret;
+            if (true !== @mkdir($dir, $mode, true)) {
+                throw new IOException(sprintf('Failed to create %s', $dir));
+            }
         }
-
-        return $ret;
     }
 
     /**
@@ -85,21 +90,33 @@ public function exists($files)
     }
 
     /**
-     * Creates empty files.
+     * Sets access and modification time of file.
      *
      * @param string|array|\Traversable $files A filename, an array of files, or a \Traversable instance to create
+     * @param integer                   $time  The touch time as a unix timestamp
+     * @param integer                   $atime The access time as a unix timestamp
+     *
+     * @throws IOException When touch fails
      */
-    public function touch($files)
+    public function touch($files, $time = null, $atime = null)
     {
+        if (null === $time) {
+            $time = time();
+        }
+
         foreach ($this->toIterator($files) as $file) {
-            touch($file);
+            if (true !== @touch($file, $time, $atime)) {
+                throw new IOException(sprintf('Failed to touch %s', $file));
+            }
         }
     }
 
     /**
      * Removes files or directories.
      *
      * @param string|array|\Traversable $files A filename, an array of files, or a \Traversable instance to remove
+     *
+     * @throws IOException When removal fails
      */
     public function remove($files)
     {
@@ -113,13 +130,19 @@ public function remove($files)
             if (is_dir($file) && !is_link($file)) {
                 $this->remove(new \FilesystemIterator($file));
 
-                rmdir($file);
+                if (true !== @rmdir($file)) {
+                    throw new IOException(sprintf('Failed to remove directory %s', $file));
+                }
             } else {
                 // https://bugs.php.net/bug.php?id=52176
                 if (defined('PHP_WINDOWS_VERSION_MAJOR') && is_dir($file)) {
-                    rmdir($file);
+                    if (true !== @rmdir($file)) {
+                        throw new IOException(sprintf('Failed to remove file %s', $file));
+                    }
                 } else {
-                    unlink($file);
+                    if (true !== @unlink($file)) {
+                        throw new IOException(sprintf('Failed to remove file %s', $file));
+                    }
                 }
             }
         }
@@ -128,14 +151,76 @@ public function remove($files)
     /**
      * Change mode for an array of files or directories.
      *
-     * @param string|array|\Traversable $files A filename, an array of files, or a \Traversable instance to change mode
-     * @param integer                   $mode  The new mode (octal)
-     * @param integer                   $umask The mode mask (octal)
+     * @param string|array|\Traversable $files     A filename, an array of files, or a \Traversable instance to change mode
+     * @param integer                   $mode      The new mode (octal)
+     * @param integer                   $umask     The mode mask (octal)
+     * @param Boolean                   $recursive Whether change the mod recursively or not
+     *
+     * @throws IOException When the change fail
      */
-    public function chmod($files, $mode, $umask = 0000)
+    public function chmod($files, $mode, $umask = 0000, $recursive = false)
     {
         foreach ($this->toIterator($files) as $file) {
-            @chmod($file, $mode & ~$umask);
+            if ($recursive && is_dir($file) && !is_link($file)) {
+                $this->chmod(new \FilesystemIterator($file), $mode, $umask, true);
+            }
+            if (true !== @chmod($file, $mode & ~$umask)) {
+                throw new IOException(sprintf('Failed to chmod file %s', $file));
+            }
+        }
+    }
+
+    /**
+     * Change the owner of an array of files or directories
+     *
+     * @param string|array|\Traversable $files     A filename, an array of files, or a \Traversable instance to change owner
+     * @param string                    $user      The new owner user name
+     * @param Boolean                   $recursive Whether change the owner recursively or not
+     *
+     * @throws IOException When the change fail
+     */
+    public function chown($files, $user, $recursive = false)
+    {
+        foreach ($this->toIterator($files) as $file) {
+            if ($recursive && is_dir($file) && !is_link($file)) {
+                $this->chown(new \FilesystemIterator($file), $user, true);
+            }
+            if (is_link($file) && function_exists('lchown')) {
+                if (true !== @lchown($file, $user)) {
+                    throw new IOException(sprintf('Failed to chown file %s', $file));
+                }
+            } else {
+                if (true !== @chown($file, $user)) {
+                    throw new IOException(sprintf('Failed to chown file %s', $file));
+                }
+            }
+        }
+    }
+
+    /**
+     * Change the group of an array of files or directories
+     *
+     * @param string|array|\Traversable $files     A filename, an array of files, or a \Traversable instance to change group
+     * @param string                    $group     The group name
+     * @param Boolean                   $recursive Whether change the group recursively or not
+     *
+     * @throws IOException When the change fail
+     */
+    public function chgrp($files, $group, $recursive = false)
+    {
+        foreach ($this->toIterator($files) as $file) {
+            if ($recursive && is_dir($file) && !is_link($file)) {
+                $this->chgrp(new \FilesystemIterator($file), $group, true);
+            }
+            if (is_link($file) && function_exists('lchgrp')) {
+                if (true !== @lchgrp($file, $group)) {
+                    throw new IOException(sprintf('Failed to chgrp file %s', $file));
+                }
+            } else {
+                if (true !== @chgrp($file, $group)) {
+                    throw new IOException(sprintf('Failed to chgrp file %s', $file));
+                }
+            }
         }
     }
 
@@ -145,18 +230,18 @@ public function chmod($files, $mode, $umask = 0000)
      * @param string $origin The origin filename
      * @param string $target The new filename
      *
-     * @throws \RuntimeException When target file already exists
-     * @throws \RuntimeException When origin cannot be renamed
+     * @throws IOException When target file already exists
+     * @throws IOException When origin cannot be renamed
      */
     public function rename($origin, $target)
     {
         // we check that target does not exist
         if (is_readable($target)) {
-            throw new \RuntimeException(sprintf('Cannot rename because the target "%s" already exist.', $target));
+            throw new IOException(sprintf('Cannot rename because the target "%s" already exist.', $target));
         }
 
-        if (false === @rename($origin, $target)) {
-            throw new \RuntimeException(sprintf('Cannot rename "%s" to "%s".', $origin, $target));
+        if (true !== @rename($origin, $target)) {
+            throw new IOException(sprintf('Cannot rename "%s" to "%s".', $origin, $target));
         }
     }
 
@@ -166,6 +251,8 @@ public function rename($origin, $target)
      * @param string  $originDir     The origin directory path
      * @param string  $targetDir     The symbolic link name
      * @param Boolean $copyOnWindows Whether to copy files if on Windows
+     *
+     * @throws IOException When symlink fails
      */
     public function symlink($originDir, $targetDir, $copyOnWindows = false)
     {
@@ -180,14 +267,22 @@ public function symlink($originDir, $targetDir, $copyOnWindows = false)
         $ok = false;
         if (is_link($targetDir)) {
             if (readlink($targetDir) != $originDir) {
-                unlink($targetDir);
+                $this->remove($targetDir);
             } else {
                 $ok = true;
             }
         }
 
         if (!$ok) {
-            symlink($originDir, $targetDir);
+            if (true !== @symlink($originDir, $targetDir)) {
+                $report = error_get_last();
+                if (is_array($report)) {
+                    if (defined('PHP_WINDOWS_VERSION_MAJOR') && false !== strpos($report['message'], 'error code(1314)')) {
+                        throw new IOException('Unable to create symlink due to error code 1314: \'A required privilege is not held by the client\'. Do you have the required Administrator-rights?');
+                    }
+                }
+                throw new IOException(sprintf('Failed to create symbolic link from %s to %s', $originDir, $targetDir));
+            }
         }
     }
 
@@ -235,7 +330,7 @@ public function makePathRelative($endPath, $startPath)
      *                                 - $options['override'] Whether to override an existing file on copy or not (see copy())
      *                                 - $options['copy_on_windows'] Whether to copy files instead of links on Windows (see symlink())
      *
-     * @throws \RuntimeException When file type is unknown
+     * @throws IOException When file type is unknown
      */
     public function mirror($originDir, $targetDir, \Traversable $iterator = null, $options = array())
     {
@@ -262,7 +357,7 @@ public function mirror($originDir, $targetDir, \Traversable $iterator = null, $o
             } elseif (is_file($file) || ($copyOnWindows && is_link($file))) {
                 $this->copy($file, $target, isset($options['override']) ? $options['override'] : false);
             } else {
-                throw new \RuntimeException(sprintf('Unable to guess "%s" file type.', $file));
+                throw new IOException(sprintf('Unable to guess "%s" file type.', $file));
             }
         }
     }
diff --git a/src/Symfony/Component/Filesystem/README.md b/src/Symfony/Component/Filesystem/README.md
@@ -3,29 +3,37 @@ Filesystem Component
 
 Filesystem provides basic utility to manipulate the file system:
 
-    use Symfony\Component\Filesystem\Filesystem;
+```php
+<?php
 
-    $filesystem = new Filesystem();
+use Symfony\Component\Filesystem\Filesystem;
 
-    $filesystem->copy($originFile, $targetFile, $override = false);
+$filesystem = new Filesystem();
 
-    $filesystem->mkdir($dirs, $mode = 0777);
+$filesystem->copy($originFile, $targetFile, $override = false);
 
-    $filesystem->touch($files);
+$filesystem->mkdir($dirs, $mode = 0777);
 
-    $filesystem->remove($files);
+$filesystem->touch($files, $time = null, $atime = null);
 
-    $filesystem->chmod($files, $mode, $umask = 0000);
+$filesystem->remove($files);
 
-    $filesystem->rename($origin, $target);
+$filesystem->chmod($files, $mode, $umask = 0000, $recursive = false);
 
-    $filesystem->symlink($originDir, $targetDir, $copyOnWindows = false);
+$filesystem->chown($files, $user, $recursive = false);
 
-    $filesystem->makePathRelative($endPath, $startPath);
+$filesystem->chgrp($files, $group, $recursive = false);
 
-    $filesystem->mirror($originDir, $targetDir, \Traversable $iterator = null, $options = array());
+$filesystem->rename($origin, $target);
 
-    $filesystem->isAbsolutePath($file);
+$filesystem->symlink($originDir, $targetDir, $copyOnWindows = false);
+
+$filesystem->makePathRelative($endPath, $startPath);
+
+$filesystem->mirror($originDir, $targetDir, \Traversable $iterator = null, $options = array());
+
+$filesystem->isAbsolutePath($file);
+```
 
 Resources
 ---------
diff --git a/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php b/src/Symfony/Component/Filesystem/Tests/FilesystemTest.php
