Commit bdf5cff3 authored by catch's avatar catch
Browse files

Revert "Issue #3537668 by phenaproxima: Dynamically figure out the actual path...

Revert "Issue #3537668 by phenaproxima: Dynamically figure out the actual path to Composer's binary, and make it read-only"

This reverts commit da488722.
parent 6a46e489
Loading
Loading
Loading
Loading
+0 −2
Original line number Diff line number Diff line
@@ -14,8 +14,6 @@ services:
  Drupal\package_manager\ExecutableFinder:
    public: false
    decorates: 'PhpTuf\ComposerStager\API\Finder\Service\ExecutableFinderInterface'
    calls:
      - [setLogger, ['@logger.channel.package_manager']]
  Drupal\package_manager\TranslatableStringFactory:
    public: false
    decorates: 'PhpTuf\ComposerStager\API\Translation\Factory\TranslatableFactoryInterface'
+6 −60
Original line number Diff line number Diff line
@@ -5,12 +5,8 @@
namespace Drupal\package_manager;

use Composer\InstalledVersions;
use Drupal\Component\Serialization\Json;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\File\FileSystemInterface;
use PhpTuf\ComposerStager\API\Finder\Service\ExecutableFinderInterface;
use Psr\Log\LoggerAwareInterface;
use Psr\Log\LoggerAwareTrait;

/**
 * An executable finder which looks for executable paths in configuration.
@@ -20,27 +16,19 @@
 *   at any time without warning. External code should not interact with this
 *   class.
 */
final class ExecutableFinder implements ExecutableFinderInterface, LoggerAwareInterface {

  use LoggerAwareTrait;
final class ExecutableFinder implements ExecutableFinderInterface {

  /**
   * The path where Composer is installed in the project, or FALSE if it's not.
   */
  private string|false $composerPackagePath;

  /**
   * The path of the Composer binary, or NULL if it can't be found.
   */
  private ?string $composerBinaryPath = NULL;
  private string|false|null $composerPath = NULL;

  public function __construct(
    private readonly ExecutableFinderInterface $decorated,
    private readonly ConfigFactoryInterface $configFactory,
    private readonly FileSystemInterface $fileSystem,
  ) {
    $this->composerPackagePath = InstalledVersions::isInstalled('composer/composer')
      ? InstalledVersions::getInstallPath('composer/composer')
    $this->composerPath = InstalledVersions::isInstalled('composer/composer')
      ? InstalledVersions::getInstallPath('composer/composer') . '/bin/composer'
      : FALSE;
  }

@@ -56,52 +44,10 @@ public function find(string $name): string {
    }

    // If we're looking for Composer, use the project's local copy if available.
    if ($name === 'composer') {
      $path = $this->getLocalComposerPath();

      if ($path && file_exists($path)) {
        // For extra security, try to make the file read-only rather than
        // directly executable. If that fails, it's worth warning about but is
        // not an actual problem.
        if (is_executable($path) && !$this->fileSystem->chmod($path, 0644)) {
          $this->logger?->warning('Composer was found at %path, but could not be made read-only.', [
            '%path' => $path,
          ]);
        }
        return $path;
      }
    if ($name === 'composer' && $this->composerPath && file_exists($this->composerPath)) {
      return $this->composerPath;
    }
    return $this->decorated->find($name);
  }

  /**
   * Tries to find the Composer binary installed in the project.
   *
   * @return string|null
   *   The path of the `composer` binary installed in the project's vendor
   *   dependencies, or NULL if it is not installed or cannot be found.
   */
  private function getLocalComposerPath(): ?string {
    // Composer is not installed in the project, so there's nothing to do.
    if ($this->composerPackagePath === FALSE) {
      return NULL;
    }

    // This is a bit expensive to compute, so statically cache it.
    if ($this->composerBinaryPath) {
      return $this->composerBinaryPath;
    }

    $composer_json = file_get_contents($this->composerPackagePath . '/composer.json');
    $composer_json = Json::decode($composer_json);

    foreach ($composer_json['bin'] ?? [] as $bin) {
      if (str_ends_with($bin, '/composer')) {
        $this->composerBinaryPath = $this->composerPackagePath . '/' . $bin;
        break;
      }
    }
    return $this->composerBinaryPath;
  }

}
+10 −47
Original line number Diff line number Diff line
@@ -4,14 +4,10 @@

namespace Drupal\Tests\package_manager\Unit;

use ColinODell\PsrTestLogger\TestLogger;
use Drupal\Component\Serialization\Json;
use Drupal\Core\File\FileSystemInterface;
use Drupal\package_manager\ExecutableFinder;
use Drupal\Tests\UnitTestCase;
use org\bovigo\vfs\vfsStream;
use PhpTuf\ComposerStager\API\Finder\Service\ExecutableFinderInterface;
use PHPUnit\Framework\Attributes\TestWith;

/**
 * @covers \Drupal\package_manager\ExecutableFinder
@@ -36,46 +32,27 @@ public function testCheckConfigurationForExecutablePath(): void {
    $decorated->find('composer')->shouldNotBeCalled();
    $decorated->find('rsync')->shouldBeCalled();

    $finder = new ExecutableFinder(
      $decorated->reveal(),
      $config_factory,
      $this->prophesize(FileSystemInterface::class)->reveal(),
    );
    $finder = new ExecutableFinder($decorated->reveal(), $config_factory);
    $this->assertSame('/path/to/composer', $finder->find('composer'));
    $finder->find('rsync');
  }

  /**
   * Tests that the executable finder tries to use a local copy of Composer.
   *
   * @param bool $chmod_result
   *   Whether the Composer binary will be successfully made read-only.
   */
  #[TestWith([TRUE])]
  #[TestWith([FALSE])]
  public function testComposerInstalledInProject(bool $chmod_result): void {
  public function testComposerInstalledInProject(): void {
    vfsStream::setup('root', NULL, [
      'composer-path' => [
        'bin' => [
          'composer' => 'A fake Composer executable',
        ],
        'composer.json' => Json::encode([
          'bin' => ['bin/composer'],
        ]),
        'bin' => [],
      ],
    ]);
    $composer_bin = 'vfs://root/composer-path/bin/composer';
    $this->assertTrue(chmod($composer_bin, 0755));
    $composer_path = 'vfs://root/composer-path/bin/composer';
    touch($composer_path);
    $this->assertFileExists($composer_path);

    $decorated = $this->prophesize(ExecutableFinderInterface::class);
    $decorated->find('composer')->willReturn('the real Composer');

    // The Composer binary is executable and should be made read-only.
    $file_system = $this->prophesize(FileSystemInterface::class);
    $file_system->chmod($composer_bin, 0644)
      ->willReturn($chmod_result)
      ->shouldBeCalled();

    $finder = new ExecutableFinder(
      $decorated->reveal(),
      $this->getConfigFactoryStub([
@@ -83,28 +60,14 @@ public function testComposerInstalledInProject(bool $chmod_result): void {
          'executables' => [],
        ],
      ]),
      $file_system->reveal(),
    );

    $logger = new TestLogger();
    $finder->setLogger($logger);

    $reflector = new \ReflectionProperty($finder, 'composerPackagePath');
    $reflector->setValue($finder, dirname($composer_bin, 2));
    $this->assertSame($composer_bin, $finder->find('composer'));

    // If the permissions change will fail, a warning should be logged.
    $predicate = function (array $record) use ($composer_bin): bool {
      return (
        $record['message'] === 'Composer was found at %path, but could not be made read-only.' &&
        $record['context']['%path'] === $composer_bin
    );
    };
    $this->assertSame(!$chmod_result, $logger->hasRecordThatPasses($predicate));
    $reflector = new \ReflectionProperty($finder, 'composerPath');
    $reflector->setValue($finder, $composer_path);
    $this->assertSame($composer_path, $finder->find('composer'));

    // If the executable disappears, or Composer isn't locally installed, the
    // decorated executable finder should be called.
    unlink($composer_bin);
    unlink($composer_path);
    $this->assertSame('the real Composer', $finder->find('composer'));

    $reflector->setValue($finder, FALSE);