Merge pull request #33 from asgrim/only-use-sudo-if-needed

Only use sudo if needed

.github/workflows/continuous-integration.yml

@@ -5,14 +5,12 @@ on:
   push:
 
 jobs:
-  ci:
-    uses: laminas/workflow-continuous-integration/.github/workflows/[email protected]
-
-  windows-tests:
+  unit-tests:
     runs-on: ${{ matrix.operating-system }}
     strategy:
       matrix:
         operating-system:
+          - ubuntu-latest
           - windows-latest
         php-versions:
           - '8.1'
@@ -28,12 +26,48 @@ jobs:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       - uses: actions/checkout@v4
       - uses: ramsey/composer-install@v3
-      - name: Run PHPUnit
+      - name: Run PHPUnit on Windows
+        if: matrix.operating-system == 'windows-latest'
         run: vendor/bin/phpunit
+      - name: Run PHPUnit on non-Windows
+        if: matrix.operating-system != 'windows-latest'
+        run: sudo vendor/bin/phpunit
+
+  coding-standards:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: 8.1
+          extensions: intl, sodium, zip
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      - uses: actions/checkout@v4
+      - uses: ramsey/composer-install@v3
+      - name: Run PHPCS
+        run: vendor/bin/phpcs
+
+  static-analysis:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: 8.1
+          extensions: intl, sodium, zip
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+      - uses: actions/checkout@v4
+      - uses: ramsey/composer-install@v3
+      - name: Run Psalm
+        run: vendor/bin/psalm
 
   build-phar:
     needs:
-      - ci
+      - unit-tests
+      - coding-standards
+      - static-analysis
     runs-on: ${{ matrix.operating-system }}
     strategy:
       matrix:

src/Command/InstallCommand.php

@@ -39,7 +39,7 @@ public function configure(): void
     public function execute(InputInterface $input, OutputInterface $output): int
     {
         if (! TargetPlatform::isRunningAsRoot()) {
-            $output->writeln('This command needs elevated privileges, and may prompt you for your password.');
+            $output->writeln('This command may need elevated privileges, and may prompt you for your password.');
         }
 
         $targetPlatform = CommandHelper::determineTargetPlatformFromInputs($input, $output);

src/Installing/UnixInstall.php

@@ -11,25 +11,43 @@
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Process\Process;
 
+use function array_unshift;
 use function file_exists;
+use function is_writable;
 use function sprintf;
 
 /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */
 final class UnixInstall implements Install
 {
     public function __invoke(DownloadedPackage $downloadedPackage, TargetPlatform $targetPlatform, OutputInterface $output): string
     {
-        (new Process(['sudo', 'make', 'install'], $downloadedPackage->extractedSourcePath))
-            ->mustRun()
-            ->getOutput();
+        $targetExtensionPath = $targetPlatform->phpBinaryPath->extensionPath();
 
         $sharedObjectName             = $downloadedPackage->package->extensionName->name() . '.so';
         $expectedSharedObjectLocation = sprintf(
             '%s/%s',
-            $targetPlatform->phpBinaryPath->extensionPath(),
+            $targetExtensionPath,
             $sharedObjectName,
         );
 
+        $makeInstallCommand = ['make', 'install'];
+
+        // If the target directory isn't writable, or a .so file already exists and isn't writable, try to use sudo
+        if (
+            ! is_writable($targetExtensionPath)
+            || (file_exists($expectedSharedObjectLocation) && ! is_writable($expectedSharedObjectLocation))
+        ) {
+            $output->writeln(sprintf(
+                '<comment>Cannot write to %s, so using sudo to elevate privileges.</comment>',
+                $targetExtensionPath,
+            ));
+            array_unshift($makeInstallCommand, 'sudo');
+        }
+
+        (new Process($makeInstallCommand, $downloadedPackage->extractedSourcePath))
+            ->mustRun()
+            ->getOutput();
+
         if (! file_exists($expectedSharedObjectLocation)) {
             throw new RuntimeException('Install failed, ' . $expectedSharedObjectLocation . ' was not installed.');
         }

test/integration/Command/InstallCommandTest.php

@@ -4,17 +4,21 @@
 
 namespace Php\PieIntegrationTest\Command;
 
+use Composer\Util\Platform;
 use Php\Pie\Command\InstallCommand;
 use Php\Pie\Container;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Console\Tester\CommandTester;
-use Symfony\Component\Process\Exception\ProcessFailedException;
 use Symfony\Component\Process\Process;
 
 use function array_key_exists;
+use function array_unshift;
+use function assert;
 use function file_exists;
 use function is_file;
+use function is_string;
+use function is_writable;
 use function preg_match;
 
 use const PHP_VERSION;
@@ -32,16 +36,14 @@ public function setUp(): void
         $this->commandTester = new CommandTester(Container::factory()->get(InstallCommand::class));
     }
 
-    public function testInstallCommandWillInstallCompatibleExtension(): void
+    public function testInstallCommandWillInstallCompatibleExtensionNonWindows(): void
     {
         if (PHP_VERSION_ID < 80300 || PHP_VERSION_ID >= 80400) {
             self::markTestSkipped('This test can only run on PHP 8.3 - you are running ' . PHP_VERSION);
         }
 
-        try {
-            (new Process(['sudo', 'ls']))->mustRun();
-        } catch (ProcessFailedException) {
-            self::markTestSkipped('Skipping as cannot run with sudo enabled');
+        if (Platform::isWindows()) {
+            self::markTestSkipped('This test can only run on non-Windows systems');
         }
 
         $this->commandTester->execute(['requested-package-and-version' => self::TEST_PACKAGE]);
@@ -62,6 +64,44 @@ public function testInstallCommandWillInstallCompatibleExtension(): void
             return;
         }
 
-        (new Process(['sudo', 'rm', $matches[1]]))->mustRun();
+        $fileToRemove = $matches[1];
+        assert(is_string($fileToRemove));
+        $rmCommand = ['rm', $fileToRemove];
+        if (! is_writable($fileToRemove)) {
+            array_unshift($rmCommand, 'sudo');
+        }
+
+        (new Process($rmCommand))->mustRun();
+    }
+
+    public function testInstallCommandWillInstallCompatibleExtensionWindows(): void
+    {
+        if (PHP_VERSION_ID < 80300 || PHP_VERSION_ID >= 80400) {
+            self::markTestSkipped('This test can only run on PHP 8.3 - you are running ' . PHP_VERSION);
+        }
+
+        if (! Platform::isWindows()) {
+            self::markTestSkipped('This test can only run on Windows systems');
+        }
+
+        $this->commandTester->execute(['requested-package-and-version' => self::TEST_PACKAGE]);
+
+        $this->commandTester->assertCommandIsSuccessful();
+
+        $outputString = $this->commandTester->getDisplay();
+        self::assertStringContainsString('Copied DLL to: ', $outputString);
+        self::assertStringContainsString('You must now add "extension=example_pie_extension" to your php.ini', $outputString);
+
+        if (
+            ! preg_match('#^Copied DLL to: (.*)$#m', $outputString, $matches)
+            || ! array_key_exists(1, $matches)
+            || $matches[1] === ''
+            || ! file_exists($matches[1])
+            || ! is_file($matches[1])
+        ) {
+            return;
+        }
+
+        (new Process(['rm', $matches[1]]))->mustRun();
     }
 }

test/integration/Installing/UnixInstallTest.php

@@ -18,9 +18,11 @@
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Console\Output\BufferedOutput;
 use Symfony\Component\Console\Output\NullOutput;
-use Symfony\Component\Process\Exception\ProcessFailedException;
 use Symfony\Component\Process\Process;
 
+use function array_unshift;
+use function is_writable;
+
 #[CoversClass(UnixInstall::class)]
 final class UnixInstallTest extends TestCase
 {
@@ -32,12 +34,6 @@ public function testUnixInstallCanInstallExtension(): void
             self::markTestSkipped('Unix build test cannot be run on Windows');
         }
 
-        try {
-            (new Process(['sudo', 'ls']))->mustRun();
-        } catch (ProcessFailedException) {
-            self::markTestSkipped('Skipping as cannot run with sudo enabled');
-        }
-
         $output         = new BufferedOutput();
         $targetPlatform = TargetPlatform::fromPhpBinaryPath(PhpBinaryPath::fromCurrentProcess());
         $extensionPath  = $targetPlatform->phpBinaryPath->extensionPath();
@@ -75,7 +71,12 @@ public function testUnixInstallCanInstallExtension(): void
         self::assertSame($extensionPath . '/pie_test_ext.so', $installedSharedObject);
         self::assertFileExists($installedSharedObject);
 
-        (new Process(['sudo', 'rm', $installedSharedObject]))->mustRun();
+        $rmCommand = ['rm', $installedSharedObject];
+        if (! is_writable($installedSharedObject)) {
+            array_unshift($rmCommand, 'sudo');
+        }
+
+        (new Process($rmCommand))->mustRun();
         (new Process(['make', 'clean'], $downloadedPackage->extractedSourcePath))->mustRun();
         (new Process(['phpize', '--clean'], $downloadedPackage->extractedSourcePath))->mustRun();
     }