fixed rotateByCopy to avoid any rename()

framework/CHANGELOG.md

@@ -59,6 +59,7 @@ Yii Framework 2 Change Log
 - Bug #3909: `Html::to()` should not prefix base URL to URLs that already contain scheme (qiangxue)
 - Bug #3934: yii.handleAction() in yii.js does not correctly detect if a hyperlink contains useful URL or not (joni-jones, qiangxue)
 - Bug #3968: Messages logged in shutdown functions are not handled (qiangxue)
+- Bug #3989: Fixed yii\log\FileTarget::$rotateByCopy to avoid any rename (cebe)
 - Bug #3996: Traversing `Yii::$app->session` may cause a PHP error (qiangxue)
 - Bug #4020: OCI column detection did not work so gii and other things failed (Sanya1991)
 - Bug: Fixed inconsistent return of `\yii\console\Application::runAction()` (samdark)

framework/log/FileTarget.php

@@ -18,7 +18,7 @@ use yii\helpers\FileHelper;
  * [[maxFileSize]] (in kilo-bytes), a rotation will be performed, which renames
  * the current log file by suffixing the file name with '.1'. All existing log
  * files are moved backwards by one place, i.e., '.2' to '.3', '.1' to '.2', and so on.
- * The property [[maxLogFiles]] specifies how many files to keep.
+ * The property [[maxLogFiles]] specifies how many history files to keep.
  *
  * @author Qiang Xue <qiang.xue@gmail.com>
  * @since 2.0
@@ -53,7 +53,13 @@ class FileTarget extends Target
     public $dirMode = 0775;
     /**
      * @var boolean Whether to rotate primary log by copy and truncate
-     * which is more compatible with log tailers. Defaults to false.
+     * which is more compatible with log tailers. Defaults to `false`.
+     *
+     * This property can also be useful on windows systems where the
+     * [rename()](http://www.php.net/manual/en/function.rename.php) function has some problems.
+     * See the [comment by Martin Pelletier](http://www.php.net/manual/en/function.rename.php#102274) in
+     * the PHP documentation for details. By setting rotateByCopy to `true` you can work
+     * around this problem.
      */
     public $rotateByCopy = false;
 
@@ -93,6 +99,9 @@ class FileTarget extends Target
             throw new InvalidConfigException("Unable to append to log file: {$this->logFile}");
         }
         @flock($fp, LOCK_EX);
+        // clear stat cache to ensure getting the real current file size and not a cached one
+        // this may result in rotating twice when cached file size is used on subsequent calls
+        clearstatcache();
         if (@filesize($this->logFile) > $this->maxFileSize * 1024) {
             $this->rotateFiles();
             @flock($fp, LOCK_UN);
@@ -114,8 +123,9 @@ class FileTarget extends Target
     protected function rotateFiles()
     {
         $file = $this->logFile;
-        for ($i = $this->maxLogFiles; $i > 0; --$i) {
-            $rotateFile = $file . '.' . $i;
+        for ($i = $this->maxLogFiles; $i >= 0; --$i) {
+            // $i == 0 is the original log file
+            $rotateFile = $file . ($i === 0 ? '' : '.' . $i);
             if (is_file($rotateFile)) {
                 // suppress errors because it's possible multiple processes enter into this section
                 if ($i === $this->maxLogFiles) {
@@ -133,10 +143,5 @@ class FileTarget extends Target
                 }
             }
         }
-        if (is_file($file)) {
-            @rename($file, $file . '.1'); // suppress errors because it's possible multiple processes enter into this section
-        }
-        // clear stat cache after moving files to later file size check is not cached
-        clearstatcache();
     }
 }

tests/unit/framework/log/FileTargetTest.php

+<?php
+/**
+ * @author Carsten Brandt <mail@cebe.cc>
+ */
+
+namespace yiiunit\framework\log;
+
+use yii\helpers\FileHelper;
+use yii\log\Dispatcher;
+use yii\log\Logger;
+use yii\log\Target;
+use Yii;
+use yiiunit\TestCase;
+
+/**
+ * @group log
+ */
+class FileTargetTest extends TestCase
+{
+    protected function setUp()
+    {
+        parent::setUp();
+        $this->mockApplication();
+    }
+
+    public function booleanDataProvider()
+    {
+        return [
+            [true],
+            [false]
+        ];
+    }
+
+    /**
+     * @dataProvider booleanDataProvider
+     */
+    public function testRotate($rotateByCopy)
+    {
+        $logFile = Yii::getAlias('@yiiunit/runtime/log/filetargettest.log');
+        FileHelper::removeDirectory(dirname($logFile));
+        mkdir(dirname($logFile), 0777, true);
+
+        $logger = new Logger();
+        $dispatcher = new Dispatcher([
+            'logger' => $logger,
+            'targets' => [
+                'file' => [
+                    'class' => 'yii\log\FileTarget',
+                    'logFile' => $logFile,
+                    'levels' => ['warning'],
+                    'maxFileSize' => 1024, // 1 MB
+                    'maxLogFiles' => 1, // one file for rotation and one normal log file
+                    'logVars' => [],
+                    'rotateByCopy' => $rotateByCopy
+                ]
+            ]
+        ]);
+
+        // one file
+
+        $logger->log(str_repeat('x', 1024), Logger::LEVEL_WARNING);
+        $logger->flush(true);
+
+        clearstatcache();
+
+        $this->assertTrue(file_exists($logFile));
+        $this->assertFalse(file_exists($logFile . '.1'));
+        $this->assertFalse(file_exists($logFile . '.2'));
+        $this->assertFalse(file_exists($logFile . '.3'));
+        $this->assertFalse(file_exists($logFile . '.4'));
+
+        // exceed max size
+        for($i = 0; $i < 1024; $i++) {
+            $logger->log(str_repeat('x', 1024), Logger::LEVEL_WARNING);
+        }
+        $logger->flush(true);
+
+        // first rotate
+
+        $logger->log(str_repeat('x', 1024), Logger::LEVEL_WARNING);
+        $logger->flush(true);
+
+        clearstatcache();
+
+        $this->assertTrue(file_exists($logFile));
+        $this->assertTrue(file_exists($logFile . '.1'));
+        $this->assertFalse(file_exists($logFile . '.2'));
+        $this->assertFalse(file_exists($logFile . '.3'));
+        $this->assertFalse(file_exists($logFile . '.4'));
+
+        // second rotate
+
+        for($i = 0; $i < 1024; $i++) {
+            $logger->log(str_repeat('x', 1024), Logger::LEVEL_WARNING);
+        }
+        $logger->flush(true);
+
+        clearstatcache();
+
+        $this->assertTrue(file_exists($logFile));
+        $this->assertTrue(file_exists($logFile . '.1'));
+        $this->assertFalse(file_exists($logFile . '.2'));
+        $this->assertFalse(file_exists($logFile . '.3'));
+        $this->assertFalse(file_exists($logFile . '.4'));
+    }
+}
\ No newline at end of file