When determining scaffold file mappings, determine them consistently and completely, accounting for overrides

Created on 22 February 2023, about 2 years ago

Problem/Motivation

First of all, let me say this is not MVP for Package Manager.

There are a couple of places in Package Manager where we need to load file mappings used by the Drupal scaffold plugin. The design of the plugin allows these to be heavily overridden, using the following flow:

  1. The initial file mapping comes from drupal/core (see the extra.drupal-scaffold.file-mapping section of its composer.json).
  2. Then, the extra.drupal-scaffold.allowed-packages list, if it exists in the root composer.json, is consulted. In the listed order, any file mappings from those packages' composer.json files are applied.
  3. Finally, the root composer.json's extra.drupal-scaffold.file-mapping section, if it exists, is merged in. This is how the root composer.json ends up having the final say about what will be scaffolded, and where, and how.

Right now, we are not at all accounting for this chain of overrides when we're trying to figure out what files will be scaffolded, and where. We simply read what drupal/core is defining, and run with that. But, given what we've just learned above, that's not at all guaranteed to be accurate.

Proposed resolution

We need some way to figure out what the actual file mappings at runtime will be, via the Composer inspector service.

We can't try to invoke the plugin directly at the API level; because it's a Composer plugin, its code won't be available to Drupal's autoloader. πŸ‘Ž

We don't want to try and compute it ourselves -- that would be duplicating quite a bit of internal logic from the scaffold plugin. πŸ‘Ž

What I think should happen is that core should add a new command to the scaffold plugin which computes the runtime file mapping, and outputs it as JSON. Then, the Composer inspector should call that command and parse the output to deliver the final, reliable file mapping.

✨ Feature request
Status

Active

Version

3.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts

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

Merge Requests

Comments & Activities

  • Issue created by @phenaproxima
  • πŸ‡ΊπŸ‡ΈUnited States tedbow Ithaca, NY, USA
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί
  • πŸ‡§πŸ‡ͺBelgium wim leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    See the last paragraph at #3357969-9: For unattended updates run each stage life cycle phase in a different request β†’ β€” if I'm correct, that means this issue is a blocker for improved performance: on sites with many contributed/custom modules installed, this would ensure that NONE of the contrib/custom modules would need to be copied from active to stage, thus cutting down significantly on the amount of time spent on file I/O as reported at #3304108-13: Determine if rsync is faster than the PHP file copier for Composer stager β†’ and #3304108-16: Determine if rsync is faster than the PHP file copier for Composer stager β†’ .

    Tagging because this can potentially make individual core updates faster and because this would help Drupal core remain fast even on sites with enormous codebases such as hundreds of installed contributed modules.

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

    This is now a core issue as package manager is in there now.

    This is a major bug because it break automatic updates and you need a way to append stuff to .htaccess if you have custom rules in there because otherwise an automatic update will override your .htaccess changes.

    Okay there is a work around. You can do vendor/bin/drush cset package_manager.settings include_unknown_files_in_project_root 1

    But this is still a major bug. We should support copying files that are used in scaffold without having to set anything additional. And as there is no settings form and the error message doesn't point you to this it would be extremely hard for someone to solve this if they can't just ping @catch.

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

    We need to make changes in \Drupal\package_manager\PathExcluder\UnknownPathExcluder::getScaffoldFiles() and it needs to account for scaffolding options like

      "[web-root]/sites/default/default.settings.php": {
        "mode": "replace",
        "path": "assets/sites/default/default.settings.php",
        "overwrite": true
      },
      "[web-root]/sites/default/settings.php": {
        "mode": "replace",
        "path": "assets/sites/default/settings.php",
        "overwrite": false
      },
      "[web-root]/robots.txt": {
        "mode": "append",
        "prepend": "assets/robots-prequel.txt",
        "append": "assets/robots-append.txt"
      },
    
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    And we also need to account for additional packages that can scaffold. The root composer and Drupal core are always permitted to scaffold.

    Example: Permit scaffolding from the project `upstream/project`
    ```
      "name": "my/project",
      ...
      "extra": {
        "drupal-scaffold": {
          "allowed-packages": [
            "upstream/project"
          ],
          ...
        }
      }
    ```
    
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    I've written some dumb code in the \Drupal\package_manager\PathExcluder\UnknownPathExcluder - but I think the plan to add a command to the scaffold plugin to produce a list of files is a good one. But this is a starter for 10, so to say.

  • Merge request !11472Some dumb code to start the process off β†’ (Open) created by alexpott
  • Pipeline finished with Running
    23 days ago
    Total: 677s
    #448223
Production build 0.71.5 2024