Metapackage Generator Breaks Under Composer --no-dev

Created on 13 May 2022, about 2 years ago
Updated 19 March 2023, about 1 year ago

Problem/Motivation

Based on #3272110-20: Drupal 9 and 10's Drupal\Component composer.json files are totally out of date β†’

@andypost noticed that the change for that issue used symfony/finder, but also noted that it's a dev requirement, so not quite so concerning from the perspective of #1451320: Evaluate Symfony2's Finder Component to simplify file handling β†’

I was curious if this would break the change, when we don't install dev.

It did, but it also broke core's ability to generate meta packages.

This happens because drupal/drupal says this:

    "autoload": {
        "psr-4": {
            "Drupal\\Core\\Composer\\": "core/lib/Drupal/Core/Composer"
        }
    },
    "autoload-dev": {
        "psr-4": {
            "Drupal\\Composer\\": "composer"
        }
    },

Steps to reproduce

rm -rf vendor/
composer install
COMPOSER_ROOT_VERSION=9.5.x-dev composer update --no-dev 'drupal/core*'

Note the --no-dev.

During the no-dev update, we see errors like this:

Class Drupal\Composer\Composer is not autoloadable, can not call post-update-cmd script

This comes from #3076234: Relocate Scaffold plugin outside of 'core' directory β†’ to try and isolate the scaffold plugin.

Merge drupal/drupal’s autoload-dev into autoload, and the whole process works.

Proposed resolution

Since drupal/drupal is supposed to be a project only for core development, merge its autoload-dev into autoload.

This will allow Composer to generate the autoload for either dev or no-dev.

Remaining tasks

Decide if this matters.

Mitigate based on needs of #1451320: Evaluate Symfony2's Finder Component to simplify file handling β†’ and other similar dependency questions.

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Fixed

Version

10.1 ✨

Component
ComposerΒ  β†’

Last updated 3 days ago

No maintainer
Created by

πŸ‡ΊπŸ‡ΈUnited States Mile23 Seattle, WA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Marking because this doesn't seem to break anything. And the great explanation in #5 seems to show this is a bug for only drupal/drupal.

    The worst-case outcome for someone who (erroneously) uses drupal/drupal for production after this change is that they have an extra psr-4 namespace declared for the composer/ directory, which will be present, because they're using the repo. Whether that's a problem is something to discuss, but I don't think it is within drupal/drupal.

    This is my only concern but if they are using drupa/drupal that seems like user error?

  • Status changed to Needs work over 1 year ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    There's no MR yet.

    Also... given drupal/core is dev only, why do we need to support running this with --no-dev?

  • Status changed to Needs review over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States Mile23 Seattle, WA

    Recent fail seemingly unrelated:

    Testing Drupal\Tests\text\Functional\TextFieldTest
    ....F                                                               5 / 5 (100%)
    
    Time: 02:12.704, Memory: 4.00 MB
    
    There was 1 failure:
    
    1) Drupal\Tests\text\Functional\TextFieldTest::testTextfieldWidgetsAllowedFormats
    Failed asserting that '\n
                uzTtfezy\n
          \n
      \n
        oKFzcW4S_label\n
                  Hello World\n
              \n
    ' contains "".
    
    /var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122
    /var/www/html/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
    /var/www/html/core/modules/text/tests/src/Functional/TextFieldTest.php:261
    /var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
    

    Rebased and pushed again for another test.

    @catch: The downside of leaving this bug in core is that a developer might composer install without dev for some reason and make a patch that breaks the metapackages. Then a maintainer might end up not noticing that. It seems vaguely unlikely, but it also seems like it would be a real pain to figure out if it broke something else.

  • Status changed to RTBC over 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Doesn't seem like it could break anything right?

    So it could help those who do what @Mile23 said in #11

    Least that's how I'm reading it.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    I think approach is fine and it don't think ot matters that this is only supposed to be used for development. Only putting in 10.1.x because not sure this is important enough to backport.

    Committed 6a9c88c and pushed to 10.1.x. Thanks!

  • Status changed to Fixed about 1 year ago
    • alexpott β†’ committed 6a9c88c8 on 10.1.x
      Issue #3280415 by Mile23, smustgrave, neclimdul, catch: Metapackage...
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.69.0 2024