From 246f83889567707ee96eae435f480c7335f08e53 Mon Sep 17 00:00:00 2001 From: Dave Gudgeon Date: Mon, 3 Mar 2025 11:54:43 +0000 Subject: [PATCH 1/4] Update free disk space check to be based on package size --- src/Filesystem/Filesystem.php | 25 +++++++ .../Overwrite/OverwritePatchStrategy.php | 2 +- src/Patch/Task/CheckDiskSpace.php | 8 +- src/Patch/Task/CheckDiskSpaceHandler.php | 70 +++++++++++++----- .../Patch/Task/CheckDiskSpaceHandlerTest.php | 74 ++++++++++++++----- 5 files changed, 142 insertions(+), 37 deletions(-) diff --git a/src/Filesystem/Filesystem.php b/src/Filesystem/Filesystem.php index f2c6e92f..88b744b5 100644 --- a/src/Filesystem/Filesystem.php +++ b/src/Filesystem/Filesystem.php @@ -2,8 +2,11 @@ namespace Meteor\Filesystem; +use FilesystemIterator; use Meteor\Filesystem\Finder\FinderFactory; use Meteor\IO\IOInterface; +use RecursiveDirectoryIterator; +use RecursiveIteratorIterator; use RuntimeException; use Symfony\Component\Filesystem\Filesystem as BaseFilesystem; @@ -219,4 +222,26 @@ public function replaceDirectory(string $sourceDir, string $targetDir, string $r $this->io->debug(sprintf('Removing %s', $old)); $this->remove($old); } + + /** + * @param string $directory + * + * @return int + */ + public function getDirectorySize($directory) + { + $totalBytes = 0; + + $path = realpath($directory); + + if ($path !== false && $path != '' && file_exists($path)) { + foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS)) as $object) { + if (!$object->isLink()) { + $totalBytes += $object->getSize(); + } + } + } + + return $totalBytes; + } } diff --git a/src/Patch/Strategy/Overwrite/OverwritePatchStrategy.php b/src/Patch/Strategy/Overwrite/OverwritePatchStrategy.php index edfeaf21..5b90fe06 100644 --- a/src/Patch/Strategy/Overwrite/OverwritePatchStrategy.php +++ b/src/Patch/Strategy/Overwrite/OverwritePatchStrategy.php @@ -50,7 +50,7 @@ public function apply($patchDir, $installDir, array $options) $tasks[] = new LimitBackups($backupsDir, $installDir, $options['limit-backups']); } - $tasks[] = new CheckDiskSpace($installDir, $backupsDir); + $tasks[] = new CheckDiskSpace($installDir, $backupsDir, $patchFilesDir); if (!$options['skip-backup']) { $tasks[] = new BackupFiles($backupDir, $patchDir, $installDir); diff --git a/src/Patch/Task/CheckDiskSpace.php b/src/Patch/Task/CheckDiskSpace.php index 0bbbe0b8..e620abf7 100644 --- a/src/Patch/Task/CheckDiskSpace.php +++ b/src/Patch/Task/CheckDiskSpace.php @@ -14,13 +14,19 @@ class CheckDiskSpace */ public $backupsDir; + /** + * @var string + */ + public $patchFilesDir; + /** * @param string $installDir * @param string $backupsDir */ - public function __construct($installDir, $backupsDir) + public function __construct($installDir, $backupsDir, $patchFilesDir) { $this->installDir = $installDir; $this->backupsDir = $backupsDir; + $this->patchFilesDir = $patchFilesDir; } } diff --git a/src/Patch/Task/CheckDiskSpaceHandler.php b/src/Patch/Task/CheckDiskSpaceHandler.php index 712e8d07..45eccafa 100644 --- a/src/Patch/Task/CheckDiskSpaceHandler.php +++ b/src/Patch/Task/CheckDiskSpaceHandler.php @@ -12,20 +12,16 @@ class CheckDiskSpaceHandler use BackupHandlerTrait; /** - * Assuming required space is 300MB for backup and new files. Not checking the real package - * size to avoid performance issues when checking the size of thousands of files. + * Free space must be at least patch size multiplied by this number. Should + * ensure we set this large enough to make backup copies of everything in + * the patch. */ - const REQUIRED_BYTES = 314572800; - - /** - * The required free space as a percentage. - */ - const REQUIRED_FREE_SPACE_PERCENT = 10; + public const PATCH_SIZE_MULTIPLIER = 2.5; /** * The maximum number of backups to keep when running low on disk space. */ - const MAX_BACKUPS = 2; + public const MAX_BACKUPS = 2; /** * @var BackupFinder @@ -62,18 +58,24 @@ public function __construct(BackupFinder $backupFinder, Filesystem $filesystem, */ public function handle(CheckDiskSpace $task, array $config) { - if ($this->hasFreeSpace($task->installDir)) { + $spaceRequired = $this->calculateRequiredDiskSpace($task->patchFilesDir); + + if ($this->hasFreeSpace($task->installDir, $spaceRequired)) { // Plenty of space available return true; } - $this->io->warning('Patching will reduce free disk space to less than ' . self::REQUIRED_FREE_SPACE_PERCENT . '%'); + $this->io->warning(sprintf( + 'There is not enough free disk space to apply this patch. Space required: %s, Space available: %s', + $this->formatFileSize($spaceRequired), + $this->formatFileSize(disk_free_space($task->installDir)) + )); // Try removing old backups $this->removeOldBackups($task->backupsDir, $task->installDir, $config); // Check disk space again - if ($this->hasFreeSpace($task->installDir)) { + if ($this->hasFreeSpace($task->installDir, $spaceRequired)) { return true; } @@ -85,19 +87,36 @@ public function handle(CheckDiskSpace $task, array $config) return true; } + private function calculateRequiredDiskSpace($patchDirectory) + { + $patchSize = $this->filesystem->getDirectorySize($patchDirectory); + + return $patchSize * static::PATCH_SIZE_MULTIPLIER; + } + /** * @param string $installDir + * @param int $spaceRequired * * @return bool */ - private function hasFreeSpace($installDir) + private function hasFreeSpace($installDir, $spaceRequired) { - $totalSpace = disk_total_space($installDir); - $freeSpace = disk_free_space($installDir) - self::REQUIRED_BYTES; + $freeSpace = disk_free_space($installDir); + + $this->io->debug(sprintf( + 'Available disk space: %s', + $this->formatFileSize($freeSpace) + )); - $freeSpacePercent = ($freeSpace / $totalSpace) * 100; + $this->io->debug(sprintf( + 'Disk space required: %s', + $this->formatFileSize($spaceRequired) + )); - return $freeSpacePercent > self::REQUIRED_FREE_SPACE_PERCENT; + $resultingSpace = $freeSpace - $spaceRequired; + + return $resultingSpace > 0; } /** @@ -115,4 +134,21 @@ private function removeOldBackups($backupsDir, $installDir, array $config) $this->removeBackups($backups, self::MAX_BACKUPS); } + + /** + * @param type $bytes + * @param int $dec + * + * @return string + */ + private function formatFileSize($bytes, $dec = 2) + { + $size = ['B', 'kB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB']; + $factor = floor((strlen($bytes) - 1) / 3); + if ($factor == 0) { + $dec = 0; + } + + return sprintf("%.{$dec}f %s", $bytes / (1024 ** $factor), $size[$factor]); + } } diff --git a/tests/Patch/Task/CheckDiskSpaceHandlerTest.php b/tests/Patch/Task/CheckDiskSpaceHandlerTest.php index 6e279604..c2f858d2 100644 --- a/tests/Patch/Task/CheckDiskSpaceHandlerTest.php +++ b/tests/Patch/Task/CheckDiskSpaceHandlerTest.php @@ -9,6 +9,8 @@ class CheckDiskSpaceHandlerTest extends TestCase { + private const PATCH_SIZE_BYTES = 346030080; + private $backupFinder; private $filesystem; private $io; @@ -18,6 +20,11 @@ protected function setUp(): void { $this->backupFinder = Mockery::mock('Meteor\Patch\Backup\BackupFinder'); $this->filesystem = Mockery::mock('Meteor\Filesystem\Filesystem'); + + $this->filesystem->shouldReceive('getDirectorySize') + ->with('/path/to/patch') + ->andReturn(static::PATCH_SIZE_BYTES); + $this->io = new NullIO(); $this->handler = new CheckDiskSpaceHandler($this->backupFinder, $this->filesystem, $this->io); @@ -25,16 +32,20 @@ protected function setUp(): void public function testPlentyOfSpace() { - $GLOBALS['disk_total_space'] = 1048576000; $GLOBALS['disk_free_space'] = 1048576000; $config = ['name' => 'test']; - static::assertTrue($this->handler->handle(new CheckDiskSpace('install', 'install/backups'), $config)); + + $this->filesystem->shouldReceive('getDirectorySize') + ->with('/path/to/patch') + ->once() + ->andReturn(static::PATCH_SIZE_BYTES); + + static::assertTrue($this->handler->handle(new CheckDiskSpace('install', 'install/backups', '/path/to/patch'), $config)); } public function testWhenRunningLowOnSpace() { - $GLOBALS['disk_total_space'] = 1048576000; $GLOBALS['disk_free_space'] = 419430400; $config = ['name' => 'test']; @@ -43,13 +54,17 @@ public function testWhenRunningLowOnSpace() ->with('install/backups', 'install', $config) ->andReturn([]); - static::assertFalse($this->handler->handle(new CheckDiskSpace('install', 'install/backups'), $config)); + $this->filesystem->shouldReceive('getDirectorySize') + ->with('/path/to/patch') + ->once() + ->andReturn(static::PATCH_SIZE_BYTES); + + static::assertFalse($this->handler->handle(new CheckDiskSpace('install', 'install/backups', '/path/to/patch'), $config)); } public function testRemovesOldBackupsWhenRunningLowOnSpace() { - $GLOBALS['disk_total_space'] = 1048576000; - $GLOBALS['disk_free_space'] = 419430400; + $GLOBALS['disk_free_space'] = static::PATCH_SIZE_BYTES; $config = ['name' => 'test']; @@ -70,7 +85,7 @@ public function testRemovesOldBackupsWhenRunningLowOnSpace() ->with('backups/3') ->andReturnUsing(function () { // Free up some space - $GLOBALS['disk_free_space'] += 104857600; + $GLOBALS['disk_free_space'] += static::PATCH_SIZE_BYTES; }) ->once(); @@ -90,13 +105,12 @@ public function testRemovesOldBackupsWhenRunningLowOnSpace() }) ->once(); - static::assertTrue($this->handler->handle(new CheckDiskSpace('install', 'install/backups'), $config)); + static::assertTrue($this->handler->handle(new CheckDiskSpace('install', 'install/backups', '/path/to/patch'), $config)); } public function testDoesNotRemoveMostRecentBackups() { - $GLOBALS['disk_total_space'] = 1048576000; - $GLOBALS['disk_free_space'] = 419430400; + $GLOBALS['disk_free_space'] = static::PATCH_SIZE_BYTES; $config = ['name' => 'test']; @@ -113,13 +127,12 @@ public function testDoesNotRemoveMostRecentBackups() $this->filesystem->shouldReceive('remove') ->never(); - static::assertFalse($this->handler->handle(new CheckDiskSpace('install', 'install/backups'), $config)); + static::assertFalse($this->handler->handle(new CheckDiskSpace('install', 'install/backups', '/path/to/patch'), $config)); } public function testRemovesOldBackupsWhenRunningLowOnSpaceButNotEnoughIsFreedUp() { - $GLOBALS['disk_total_space'] = 1048576000; - $GLOBALS['disk_free_space'] = 104857600; + $GLOBALS['disk_free_space'] = 2000; $config = ['name' => 'test']; @@ -160,13 +173,38 @@ public function testRemovesOldBackupsWhenRunningLowOnSpaceButNotEnoughIsFreedUp( }) ->once(); - static::assertFalse($this->handler->handle(new CheckDiskSpace('install', 'install/backups'), $config)); + static::assertFalse($this->handler->handle(new CheckDiskSpace('install', 'install/backups', '/path/to/patch'), $config)); } -} -function disk_total_space($directory) -{ - return $GLOBALS['disk_total_space'] ?? 1048576000; + public function testWarningOutputWhenNotEnoughSpace() + { + $GLOBALS['disk_free_space'] = 2000000; + + $config = ['name' => 'test']; + + $backups = [ + new Backup('backups/2', []), + new Backup('backups/1', []), + ]; + + $this->backupFinder->shouldReceive('find') + ->with('install/backups', 'install', $config) + ->andReturn($backups) + ->once(); + + $io = Mockery::mock(\Meteor\IO\IOInterface::class, [ + 'askConfirmation' => null, + 'debug' => null, + ]); + + $this->handler = new CheckDiskSpaceHandler($this->backupFinder, $this->filesystem, $io); + + $io->shouldReceive('warning') + ->once() + ->with('There is not enough free disk space to apply this patch. Space required: 825.00 MB, Space available: 1.91 MB'); + + static::assertFalse($this->handler->handle(new CheckDiskSpace('install', 'install/backups', '/path/to/patch'), $config)); + } } function disk_free_space($directory) From 01305449f812734346f861c932e6a55695a0034a Mon Sep 17 00:00:00 2001 From: Dave Gudgeon Date: Thu, 27 Feb 2025 10:16:56 +0000 Subject: [PATCH 2/4] Upgrade to upload and download artifact v4 --- .github/workflows/test.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 56bb890b..a55026bb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -106,10 +106,11 @@ jobs: files: "bin/meteor.phar" fail: true - name: Store the artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: meteor.phar path: bin/meteor.phar + overwrite: true package-for-testing: name: Create test package @@ -125,7 +126,7 @@ jobs: url: "https://gist.githubusercontent.com/DenisYaschuk/d3ade2d88d058cf9c971cf9d1f580a0f/raw/871ee04ee0ee01a6a2e0f97e67ce0206f78e3179/migrations-update.php" target: tests/mock_project/package - name: Copy meteor to mock project - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: meteor.phar path: "tests/mock_project/package" @@ -156,10 +157,11 @@ jobs: target: "tests/mock_project/package/output/github-action-test_2.0/github-action-test_2.0" - name: Store the mock project artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: mock_project path: tests/mock_project + overwrite: true functional-tests: name: Functional Tests @@ -184,7 +186,7 @@ jobs: php-version: ${{ matrix.php-version }} - name: Retrieve the mock project - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: mock_project path: mock_project From abbca49c811c71ffbe091f09d88c2e3c2c9fcb4d Mon Sep 17 00:00:00 2001 From: Dave Gudgeon Date: Mon, 10 Mar 2025 19:59:14 +0000 Subject: [PATCH 3/4] Changes following review --- src/Filesystem/Filesystem.php | 2 +- src/IO/ConsoleIO.php | 13 ++++++++++ src/IO/IOInterface.php | 8 ++++++ src/IO/NullIO.php | 8 ++++++ src/Patch/Task/CheckDiskSpaceHandler.php | 25 +++---------------- tests/IO/ConsoleIOTest.php | 22 ++++++++++++++++ .../Patch/Task/CheckDiskSpaceHandlerTest.php | 12 ++++++++- 7 files changed, 67 insertions(+), 23 deletions(-) diff --git a/src/Filesystem/Filesystem.php b/src/Filesystem/Filesystem.php index 88b744b5..87ce7ec6 100644 --- a/src/Filesystem/Filesystem.php +++ b/src/Filesystem/Filesystem.php @@ -234,7 +234,7 @@ public function getDirectorySize($directory) $path = realpath($directory); - if ($path !== false && $path != '' && file_exists($path)) { + if ($path !== false && $path != '' && is_dir($path)) { foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS)) as $object) { if (!$object->isLink()) { $totalBytes += $object->getSize(); diff --git a/src/IO/ConsoleIO.php b/src/IO/ConsoleIO.php index 4fb3cc50..1de563b9 100644 --- a/src/IO/ConsoleIO.php +++ b/src/IO/ConsoleIO.php @@ -77,6 +77,19 @@ public function isInteractive() return $this->input->isInteractive(); } + /** + * {@inheritdoc} + */ + public function formatFileSize($bytes, $dec = 2) + { + $suffix = ['B', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB', 'EiB', 'ZiB', 'YiB']; + + $base = 1024; + $class = min((int)log($bytes , $base) , count($suffix) - 1); + + return sprintf("%1.{$dec}f" , $bytes / pow($base, $class)) . ' ' . $suffix[$class]; + } + /** * {@inheritdoc} */ diff --git a/src/IO/IOInterface.php b/src/IO/IOInterface.php index 4ffe4297..51a0080b 100644 --- a/src/IO/IOInterface.php +++ b/src/IO/IOInterface.php @@ -11,6 +11,14 @@ interface IOInterface */ public function isInteractive(); + /** + * @param int $bytes + * @param int $dec + * + * @return string + */ + public function formatFileSize($bytes, $dec = 2); + /** * Gets argument by name. * diff --git a/src/IO/NullIO.php b/src/IO/NullIO.php index d8583f06..9c6aeaa8 100644 --- a/src/IO/NullIO.php +++ b/src/IO/NullIO.php @@ -15,6 +15,14 @@ public function isInteractive() return false; } + /** + * {@inheritdoc} + */ + public function formatFileSize($bytes, $dec = 2) + { + return ''; + } + /** * {@inheritdoc} */ diff --git a/src/Patch/Task/CheckDiskSpaceHandler.php b/src/Patch/Task/CheckDiskSpaceHandler.php index 45eccafa..fccbc217 100644 --- a/src/Patch/Task/CheckDiskSpaceHandler.php +++ b/src/Patch/Task/CheckDiskSpaceHandler.php @@ -67,8 +67,8 @@ public function handle(CheckDiskSpace $task, array $config) $this->io->warning(sprintf( 'There is not enough free disk space to apply this patch. Space required: %s, Space available: %s', - $this->formatFileSize($spaceRequired), - $this->formatFileSize(disk_free_space($task->installDir)) + $this->io->formatFileSize($spaceRequired), + $this->io->formatFileSize(disk_free_space($task->installDir)) )); // Try removing old backups @@ -106,12 +106,12 @@ private function hasFreeSpace($installDir, $spaceRequired) $this->io->debug(sprintf( 'Available disk space: %s', - $this->formatFileSize($freeSpace) + $this->io->formatFileSize($freeSpace) )); $this->io->debug(sprintf( 'Disk space required: %s', - $this->formatFileSize($spaceRequired) + $this->io->formatFileSize($spaceRequired) )); $resultingSpace = $freeSpace - $spaceRequired; @@ -134,21 +134,4 @@ private function removeOldBackups($backupsDir, $installDir, array $config) $this->removeBackups($backups, self::MAX_BACKUPS); } - - /** - * @param type $bytes - * @param int $dec - * - * @return string - */ - private function formatFileSize($bytes, $dec = 2) - { - $size = ['B', 'kB', 'MB', 'GB', 'TB', 'PB', 'EB', 'ZB', 'YB']; - $factor = floor((strlen($bytes) - 1) / 3); - if ($factor == 0) { - $dec = 0; - } - - return sprintf("%.{$dec}f %s", $bytes / (1024 ** $factor), $size[$factor]); - } } diff --git a/tests/IO/ConsoleIOTest.php b/tests/IO/ConsoleIOTest.php index 2e83e5bd..443c9d2a 100644 --- a/tests/IO/ConsoleIOTest.php +++ b/tests/IO/ConsoleIOTest.php @@ -153,4 +153,26 @@ public function testNewLine() static::assertSame("\n", $this->getOutput()); } + + /** + * @dataProvider formatFileSizeProvider + */ + public function testFormatFileSize($bytes, $dec, $expected) + { + $actual = $this->io->formatFileSize($bytes, $dec); + + static::assertEquals($expected, $actual); + } + + public function formatFileSizeProvider() + { + return [ + ['1024', 2, '1.00 KiB'], + ['1234', 2, '1.21 KiB'], + ['123456789', 2, '117.74 MiB'], + ['1048576', 2, '1.00 MiB'], + ['1048576', 0, '1 MiB'], + ['100456789012', 2, '93.56 GiB'], + ]; + } } diff --git a/tests/Patch/Task/CheckDiskSpaceHandlerTest.php b/tests/Patch/Task/CheckDiskSpaceHandlerTest.php index c2f858d2..4f49a205 100644 --- a/tests/Patch/Task/CheckDiskSpaceHandlerTest.php +++ b/tests/Patch/Task/CheckDiskSpaceHandlerTest.php @@ -195,13 +195,23 @@ public function testWarningOutputWhenNotEnoughSpace() $io = Mockery::mock(\Meteor\IO\IOInterface::class, [ 'askConfirmation' => null, 'debug' => null, + 'formatFileSize' => '', ]); $this->handler = new CheckDiskSpaceHandler($this->backupFinder, $this->filesystem, $io); $io->shouldReceive('warning') ->once() - ->with('There is not enough free disk space to apply this patch. Space required: 825.00 MB, Space available: 1.91 MB'); + ->with('There is not enough free disk space to apply this patch. Space required: 825.00 MiB, Space available: 1.91 MiB'); + + $io->shouldReceive('formatFileSize') + ->with(static::PATCH_SIZE_BYTES * CheckDiskSpaceHandler::PATCH_SIZE_MULTIPLIER) + ->andReturn('825.00 MiB'); + + $io->shouldReceive('formatFileSize') + ->with($GLOBALS['disk_free_space']) + ->andReturn('1.91 MiB'); + static::assertFalse($this->handler->handle(new CheckDiskSpace('install', 'install/backups', '/path/to/patch'), $config)); } From ea2638e2ffd2f9c9f73ae9186418dcedd4644f23 Mon Sep 17 00:00:00 2001 From: Dave Gudgeon Date: Mon, 10 Mar 2025 20:04:26 +0000 Subject: [PATCH 4/4] cs fix --- src/IO/ConsoleIO.php | 4 ++-- tests/Patch/Task/CheckDiskSpaceHandlerTest.php | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/IO/ConsoleIO.php b/src/IO/ConsoleIO.php index 1de563b9..470a1a6a 100644 --- a/src/IO/ConsoleIO.php +++ b/src/IO/ConsoleIO.php @@ -85,9 +85,9 @@ public function formatFileSize($bytes, $dec = 2) $suffix = ['B', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB', 'EiB', 'ZiB', 'YiB']; $base = 1024; - $class = min((int)log($bytes , $base) , count($suffix) - 1); + $class = min((int) log($bytes, $base), count($suffix) - 1); - return sprintf("%1.{$dec}f" , $bytes / pow($base, $class)) . ' ' . $suffix[$class]; + return sprintf("%1.{$dec}f", $bytes / pow($base, $class)) . ' ' . $suffix[$class]; } /** diff --git a/tests/Patch/Task/CheckDiskSpaceHandlerTest.php b/tests/Patch/Task/CheckDiskSpaceHandlerTest.php index 4f49a205..c94d135e 100644 --- a/tests/Patch/Task/CheckDiskSpaceHandlerTest.php +++ b/tests/Patch/Task/CheckDiskSpaceHandlerTest.php @@ -212,7 +212,6 @@ public function testWarningOutputWhenNotEnoughSpace() ->with($GLOBALS['disk_free_space']) ->andReturn('1.91 MiB'); - static::assertFalse($this->handler->handle(new CheckDiskSpace('install', 'install/backups', '/path/to/patch'), $config)); } }