Add php-tuf/composer-stager to core dependencies — for experimental Automatic Updates & Project Browser modules

Of note: We intend these Composer packages related to the AU toolchain to eventually be maintained by the core maintainers and treated essentially as a part of core. So I would encourage other RMs and FMs to do very thorough review of the code.

Just to be explicit, @Wim Leers, since the current plan is for Drupal core maintainers to completely take over ownership and maintenance, I think all the other questions in the summary are pending the details of that plan.

I updated the issue summary based on #6. It's possible that I misunderstood the implications of that comment, so if anything in the issue summary looks incorrect, please correct it.

Updating the issue summary to reflect the decision made with @effulgentsia to eliminate the dependency on symfony/filesystem.

One necessary next step here is to open issues for each of Composer Stager's dev dependencies that isn't already a core dev dependency, that explains the benefit of that dev dependency, so that we can evaluate whether the benefit is worth whatever the burden of that dependency is.

https://github.com/php-tuf/composer-stager/issues/56 is an example where we've started the discussion for infection/infection, and I think the conclusion there so far is to remove that dependency unless @TravisCarden wants to argue for keeping it.

Reviewing the code itself can happen in parallel to that, so I agree with "Needs review" as the correct issue status here.

- Composer Stager is currently in the PHP-TUF namespace/organization. Decide if we want to move it into the Drupal namespace/organization.
- Decide if we want to continue maintaining it as a GitHub repo, or whether to move it to drupal.org's GitLab, or even into the Drupal core repository and mirrored to GitHub, like we do for https://github.com/drupal/recommended-project.

Removing these from the remaining tasks, because I discussed this with other committers, and we decided to keep it in the PHP-TUF namespace and on GitHub, to make it easier for other projects (e.g., Joomla, Typo3) to co-maintain it with us if they want to.

- Decide if/how we want to empower non-Drupal projects to help contribute to / maintain Composer Stager, if it's under Drupal core's governance.

I replaced that with:

Add needed governance documents (security policy, links to Drupal core governance documents, etc.) to the repo.

Added a remaining task of performing evaluations for Composer Stager's dev dependencies. https://github.com/php-tuf/composer-stager/issues/78 is the issue for that (currently that issue only lists one dev dependency but the others will be added to it soon).

Added link to https://github.com/php-tuf/composer-stager/issues/60.

Adding a link to the summary to the Composer Stager issue to define a security policy (https://github.com/php-tuf/composer-stager/issues/79).

I added a "Code overview for reviewers" section to the issue summary to help orient reviewers.

Discussing this at DrupalCamp NJ. I think it would be worth shipping a composer.phar with Drupal core so that there is a known and valid version of the composer executable always available.

Advantages:
You can always be using a version of composer against which all the automated/manual tests have run and passed.
There is no dependency on the hosting provider to have installed a usable version of composer

There might be other ways to handle a missing or invalid composer, but this is at least simple. since it's just a phar, it's not going to be the composer/composer package which is causing issues mentioned by Dave in #3243899: [policy] Move composer/composer from a dev dependency to a production dependency

Another option... maybe trickier... would be to download the phar if needed to a temp location and execute it. The phar files do have an RSA signature available.

Also, there should be a way to run the autoupdates on the cli so that you can run as a system user that can write the files. Possibly we could update core/scripts/drupal to include some commands that actually bootstrap to do those things in a light-weight way?

Thanks for such a comprehensive summary! I hope to have time to review this “soon”. 🤞 meanwhile, I hope adding “and governance” to this title is a friendly amendment, because that’s the full scope here, no?

I think that the patch for this issue can/should change ComposerProjectTemplatesTest::MINIMUM_STABILITY from "stable" to "alpha". I think it's okay to do that, because after we branch 10.2.x, we can remove this dependency from 10.1.x and then raise ComposerProjectTemplatesTest::MINIMUM_STABILITY back to what we want to prior to tagging any 10.1 pre-releases.

@pwolanin, re #18 and #19, that's really good feedback, but I think #18 is a topic for #3346707: Add Alpha level Experimental Package Manager module and #19 is a topic for #3253158: Add Alpha level Experimental Update Manager module. Do you agree, or do you see those as something in scope for Composer Stager?

The sequence of issues is:

• Composer Stager: this issue. A PHP library agnostic of anything Drupal-specific, so that other CMSes could use it too if they want to.
• Package Manager: #3346707: Add Alpha level Experimental Package Manager module. Drupal module that depends on Composer Stager but that is independent of whether the use-case is installing (Project Browser) or updating (Automatic Updates).
• Automatic Updates: #3253158: Add Alpha level Experimental Update Manager module. Depends on Package Manager.

I'll try to post some updates here as I make progress toward a stable v2.0.0 release of Composer Stager. I finished two issues on the milestone checklist today. (I also explicitly marked (annotated) the API surface.) Of the two remaining issues, one should be fairly simple. Eliminating the dependency on Symfony Filesystem could potentially take some doing (but I can't say until I get started). I do also plan to add an issue to the milestone for some refactoring that I want to do while I can still make BC-breaking changes. I don't want that opportunity to get away, but I'm trying not to go crazy.

Tell me when you push this dependency onto stable so I can update. Thanks.

Update from my end: I'm continuing to work on the Composer Stager v2.0.0 milestone, most notably Add support for string translation for names, descriptions, and messages. Things are moving along nicely, but there's a non-trivial amount of work left.

Update: there's ongoing requirements-and-design discussion on Add support for string translation for names, descriptions, and messages. The scope has increased somewhat as a result, and there's still quite a bit of work left. But it's still moving along at a good pace without any surprises.

Update: Add support for string translation for names, descriptions, and messages is finished. There are some follow-ups and of course the other issues assigned to the v2.0.0 milestone.

Composer Stager 2.0.0-beta1 has been released. I updated the issue summary to be up to date with that.

I believe Composer Stager now implements everything that is needed for an Automatic Updates in Drupal core MVP and is therefore fully ready for reviews.

Updated patch to pull in the 2.0.0-beta1 version.

Fixed broken links in the issue summary that were due to refactoring of class names from 1.x to 2.x.

Is this really acceptable?

We have test coverage that when we tag an rc (I think) will fail if the minimum stability doesn't match - so it becomes rc-blocking to sort out. As you point out we usually only do it with Symfony because their release timelines are pretty reliable, but I think it's OK to apply the same pattern here.

#49 looks like a random failure, but this rerolls for composer.lock changes since #42, and also updates the composer-stager requirement to beta3.

Back to RTBC, optimistically assuming it'll pass tests. Note that the RTBC is for the patch itself, but putting composer-stager under core governance still requires framework and release manager review of composer-stager itself, per this issue's tags.

As you point out we usually only do it with Symfony because their release timelines are pretty reliable, but I think it's OK to apply the same pattern here.

Yeah, in this case, part of putting composer-stager into core governance, is then its release timeline is under our control. Also, there are currently no known issues blocking a stable 2.0.0 release of composer-stager. Only thing keeping it at beta right now is that it hasn't been reviewed by any Drupal core committers other than me.

I think #51 is a random failure. Queued a re-test and optimistically re-RTBC'ing.

Two different randoms.

My main concern here is that the committer team routinely struggles to keep the RTBC queue under 150 issues at a time, and this adds a separate queue on a separate system to manage, with a code base that is entirely new to most committers.

So what I would propose is we add core committers to the github repo, but in addition to the existing maintainers, not instead. We can figure out exactly what responsibilities are as time goes on, but an immediate switch from one set of maintainers to another doesn't seem like a good idea.

Right, and core maintainers will still control which versions are pinned in core-recommended at least. I'd suggest this:

• Current maintainers are 100% authorized to commit trivial fixes (like dev dependency updates).
• We gradually transition final signoff responsibility for substantive fixes onto core release and backend framework managers (but let current maintainers indicate their interest in maintaining long-term).
• Release manager signoff is required for tagging a new release of the GitHub-hosted packages.

+++ b/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
@@ -30,7 +30,7 @@ class ComposerProjectTemplatesTest extends ComposerBuildTestBase {
-  protected const MINIMUM_STABILITY = 'stable';
+  protected const MINIMUM_STABILITY = 'beta';

This is hopefully only a temporary change, like we make between alpha and RC for core for Symfony? If so we need a followup to switch it back before 10.2.0-rc1.

Also possibly we should branch 10.2.x (which we are planning to do ASAP) before we actually commit this, since needing to revert lockfile changes before RC would be messy. (And we don't want to ship it unless we're also shipping AU and/or PB, which remains to be seen.)

@catch and I both would really like to see some of the excess dev dependencies eliminated. If possible, let's please focus on dev deps that core already uses. E.g., seems what grumphp is doing could mostly be accomplished with phpcs and phpstan etc.? I saw https://github.com/php-tuf/composer-stager/issues/78 but in a way this is almost asking release managers to do too much work. Dev dependencies core already has for dev or prod are fine and don't even need their own evaluations, but I'd really like to see fewer additional dependencies proposed, and a strong case made as to why they are essential to the maintenance of the package.

Also, let's get a followup for #57, and an issue in the composer-stager queue to add a GOVERNANCE.md where we can start documenting the above stuff?

@xjm: Evaluate dev dependencies · Issue #78 · php-tuf/composer-stager has been updated with issues for each dev dependencies, including justifications for each. I've knocked off a half dozen I felt OK about eliminating without debate.

And I have created Create a governance policy · Issue #300 · php-tuf/composer-stager

I've added:

Existing maintainers would maintain commit access to the repository, but should co-ordinate any new releases or major changes with core committers.

to the issue summary since I think that represents the current plan.

Existing maintainers would maintain commit access to the repository, but should co-ordinate any new releases or major changes with core committers.

This is reasonable place to start. And there will certainly be an evolution of the responsibilities of the roles and the points in #56 indicate a direction on that.

Tagging for issue summary update now that https://github.com/php-tuf/composer-stager/releases/tag/v2.0.0-beta4 was released, which removed the PHP filesyncer per #3416542: [policy] Require rsync for automatic updates in Drupal core and punt other syncers to contrib.

https://github.com/php-tuf/composer-stager/issues/350 just got completed, making Composer Stager compatible with both Symfony 6 and 7, so that it's compatible with Drupal 10 and 11. So next steps for this issue are:

• Release Composer Stager 2.0.0-beta5, so the above fix is in a tagged release. Note that Composer Stager 2.0 is only in beta status because it hasn't been sufficiently reviewed yet. But as far as we know, it's complete, so we can release RC or stable at any time.
• Open an MR for this issue that adds Composer Stager ^2.0.0-beta5 to Drupal core's dependencies. See #50 for the latest patch that did this with an earlier version, but we don't do patches anymore.
• Update the issue summary of this issue for accuracy (e.g., remove the info about the PHP file syncer and check the rest of the issue summary for whether it accurately reflects the current state of Composer Stager).
• For anyone following this issue, and especially core committers, please review the Composer Stager codebase and open GitHub issues for any problems/concerns that you find.

Assigning to myself as I take a first pass at the merge request

This went in as part of #3346707: Add Alpha level Experimental Package Manager module

We still need to do the governance steps here. I'm not sure what's left but there's no other issue to track that in.

This dependency becomes a blocker for stable release, see #3490163: Update Composer dependencies for 11.1.0

The governance discussed is moved to #3474816: Governance for projects on Github.

A decision needs to made if the dependency evaluation stays here and the issue is tagged according to Dependency information links or if it is copied to Current PHP dependencies. Setting to NW for this.

alexpott closed this issue, so I am removing the 'Needs framework manager review' tag. I think all the followups needed are done, removing tag.

Updating title per the last comment

Attempting to pull out some next steps:

Place the Composer Stager library into core governance. This would mean, for example, all Drupal core committers having commit access to it, following Drupal core review processes for committing to it, and having a security policy in the repo that states that security issues should be reported according to Drupal's security reporting process.

Existing maintainers would maintain commit access to the repository, but should co-ordinate any new releases or major changes with core committers.

I'm not sure if all Drupal core committers have commit access, can we verify that?

For "following Drupal core review processes for committing to it", maybe that falls under https://github.com/php-tuf/composer-stager/issues/300 (Governance)? There is a basic docs/GOVERNANCE.md but that may not be very definitive/helpful. Calling this out in the README or somewhere more prominent may be worth considering.

The GitHub issue for the security policy is open, and seems to have some relatively clear next steps. I will note that there is currently a docs/SECURITY.md in the repository (the "issue summary" of that issue is slightly out of date).

The policy statement is in the Drupal Governance and the projects on GitHub should simply link to it; https://git.drupalcode.org/project/governance/-/blob/main/external-proje....

But that should be a link to the docs, not the repo, but the menu didn't get updated when that change was made. I'll make an issue to fix that sometime today.