Problem/Motivation

Working on #3357969: For web server dependent unattended updates run the entire life cycle in a separate process that will not be affected by hosting time limits

I found that \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::testDrushUpdate requires drush via
$this->runComposer('composer require drush/drush', 'project');
Since it doesn't use "COMPOSER_MIRROR_PATH_REPOS=1" it actually symlinks drush. Looking that project made for the build test using $destroyBuild = FALSE, I noticed that none of drush's dependencies are actually in the vendor directory.

If I change to use $this->runComposer('COMPOSER_MIRROR_PATH_REPOS=1 composer require drush/drush', 'project');

The testDrushUpdate fails with

Fatal error: Uncaught Error: Class "Consolidation\Config\Util\ConfigOverlay" not found in /private/tmp/build_workspace_b88551db587cca7e09d6b4b3f191d747eW80mu/project/vendor/drush/drush/src/Config/DrushConfig.php:17

This is one of drush's dependencies and is not vendor directory for the build test project.

I assume it is only working now because somehow symlinking allows loading classes from the actual core clone

Steps to reproduce

Proposed resolution

Don't symlink the drush package in the build test and make sure it's dependencies are also present

Remaining tasks

User interface changes

API changes

Data model changes

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

tedbow created an issue. See original summary.

tedbow’s picture

Assigned: Unassigned » phenaproxima
tedbow’s picture

Pushed commit that fixed the test locally. will see if drupalci is fixed

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

Looks good if tests pass.

phenaproxima’s picture

Title: Drush build test symlinks drush and only passes because it loads classes not in build test project » Drush build test symlinks Drush, and does not install any of its dependencies
phenaproxima’s picture

Assigned: phenaproxima » tedbow
Status: Reviewed & tested by the community » Needs work

Kicking back for failure on 10.0.x. :(

tedbow’s picture

Assigned: tedbow » phenaproxima
Status: Needs work » Needs review

Ok think the build tests will pass now on both branches

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

  • phenaproxima committed c83c648d on 3.0.x authored by tedbow
    Issue #3368741 by tedbow: Drush build test symlinks Drush, and does not...
phenaproxima’s picture

Status: Reviewed & tested by the community » Fixed
wim leers’s picture

Assigned: phenaproxima » Unassigned
Status: Fixed » Needs work
    // Do not run development Composer plugin.
    $this->runComposer("composer config allow-plugins.dealerdirect/phpcodesniffer-composer-installer false", $template_dir);

Why?

        if (isset($package_info['require'])) {
          unset(
            $package_info['require']['symfony/polyfill-php72'],
            $package_info['require']['symfony/polyfill-php73'],
            $package_info['require']['symfony/polyfill-php74'],
            $package_info['require']['symfony/polyfill-php80'],
            $package_info['require']['symfony/polyfill-php81'],
          );
          $packages[$name][$version]['require'] = $package_info['require'];
        }

Why? These are not dependencies of https://github.com/drush-ops/drush/blob/12.x/composer.json?

I cannot find the answers in the code nor in this issue 😅

phenaproxima’s picture

Status: Needs work » Fixed

To answer your questions:

  1. Because, unless we disable this, install fails because it (the plugin) tries to run an executable before it's available. At least, that's what I understand from discussing with @tedbow, who traced into it. Probably a bug in the plugin itself. For our build test purposes, we don't need that plugin anyway, so this is an acceptable workaround given the importance of solving this problem.
  2. Those packages are dependencies of some packages, but they are (somehow - don't ask me why) not installed on PHP versions that don't need them (i.e., you don't need a PHP 7.2 polyfill on PHP 8). Since Drupal 10 requires PHP 8.1, those polyfills are not actually present in the local codebase and therefore unable to be installed, so we just make sure no package depends on them, since that would break Composer's ability to build the code base successfully. I remember having to do similar stuff in some earlier iterations of the build tests.
wim leers’s picture

Thanks!

So I was hoping that what's written in #14 would be shipped with the code in the form of comments, because otherwise it'll be really hard to track down once this gets into core 😅 (i.e. to answer when will it be safe to remove this?)

I am just trying to make things simpler for our future selves!

tedbow’s picture

Status: Fixed » Needs work
Issue tags: +Needs follow-up

@Wim Leers thanks for asking these questions. Sounds at least like we need follow-up to at least add some code comments. Likely we would get these questions on our core review

tedbow’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs follow-up

@phenaproxima thanks for adding comments in the new MR. I think they looks good

  • phenaproxima committed a62d80d8 on 3.0.x
    Issue #3368741 by tedbow, phenaproxima: Drush build test symlinks Drush...
phenaproxima’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.