Allow pre-release dependencies in Drupal pre-release milestones

Just make tests pass.

+++ b/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
@@ -177,7 +177,7 @@ public function testTemplateCreateProject($project, $package_dir, $docroot_dir)
-    $this->executeCommand("COMPOSER_HOME=$composer_home COMPOSER_ROOT_VERSION=$simulated_stable_version composer create-project --no-ansi $project testproject -vvv --repository $repository_path");
+    $this->executeCommand("COMPOSER_HOME=$composer_home COMPOSER_ROOT_VERSION=$simulated_stable_version composer create-project --no-ansi $project testproject -vvv --repository $repository_path --no-install && cd testproject && composer config minimum-stability dev && COMPOSER_HOME=$composer_home COMPOSER_ROOT_VERSION=$simulated_stable_version composer install");

composer create-project does two things in one go from my understanding: 1) clone the project template, 2) run composer install.

With --no-install, it skips step #2 -- installation. This gives the chance to insert an extra step #2.0 changing minimum-stability from stable to dev: composer config minimum-stability dev before installation.

BTW, there is an option --stability=STABILITY for composer create-project, but it did not work for me.

I couldn't get the --stability flag to work either, I think maybe because we specify COMPOSER_ROOT_VERSION this overrides it in some way.

Based on @jungle's work in #7, this patch now calculates the minimum stability of our all our vendored dependencies, and uses that to set the minimum stability of the template project. This means that in the usual case we will create a stable template, but if we happen to be testing RC (or even beta, alpha or dev) dependencies the template project will match, and this will allow the project to be installed.

The stable patch is ready for commit. The rc patch should prove that the tests still pass when we have RC dependencies.

Technically this works for all components, not just Symfony ones.

I have applied it over in #3161889: [META] Symfony 6 compatibility to get a -dev branch of behat/mink-browserkit-driver passing the Composer build tests.

The purpose of the testTemplateCreateProject is to determine if composer create-project drupal/recommended-project (and legacy-project) is going to work; however, this patch modifies the minimum stability of the template before testing it. This seems to imply to me that although the test will pass, the template project itself isn't going to work after the subtree splitter writes it to github, as it won't have the right minimum stability.

Have I missed a step or otherwise misunderstood the intention here?

The intent here is to get a green test run if a patch includes a non-stable dependency and there are no failures introduced by that dependency. The issue is that if, for example, we upload a patch that includes a beta or release candidate of Symfony, the project build test fails in composer create-project:

  Problem 1
    - Root composer.json requires drupal/core-recommended ^9.1 -> satisfiable by drupal/core-recommended[9.1.99].
    - drupal/core-recommended 9.1.99 requires symfony/var-dumper v5.2.0-RC2 -> found symfony/var-dumper[v5.2.0-RC2] but it does not match your minimum-stability.
  Problem 2
    - Root composer.json requires drupal/core-dev ^9.1 -> satisfiable by drupal/core-dev[9.1.99].
    - drupal/core-dev 9.1.99 requires symfony/phpunit-bridge ^5.1.4@rc -> found symfony/phpunit-bridge[v5.2.0-RC2] but it does not match your minimum-stability.

I am assuming that at the moment we never actually want to commit a pre-release version of a dependency, so we don't actually want to change the template; we just want the tests to pass.

I suppose an alternative option would be to skip the project build tests entirely if we detect a pre-release dependency? Or even, don't commit anything from this issue, and just document somewhere that when testing with pre-release dependencies you also need to skip this test?

Currently, an RC release has a minimum stability of RC, e.g.: 9.1.0-rc1

A stable release has minimum stability stable, though, e.g.: 9.1.2

So, I don't think we have a problem with using Symfony betas or other betas in Drupal beta releases; this should already be allowed. If I'm wrong about this, then perhaps we do need a patch, but it would be better to fix the metapackage generator instead of fixing up the metapackage mid-patch. I'm not sure why we see the problems e.g. in #15 since we have minimum stability dev in our dev project templates, which I would expect should be what the tests are using. If the issue is with project template version selection, then we should fix that and make sure we are starting with a dev project template for the test.

The problem will remain, though, is that once someone starts a site with drupal/recommended-project, they do not get any further updates that we might make to the templates, so we can expect that today, folks with Drupal 9 sites will by-and-by-large be using minimum stability stable. This makes it difficult for us to put non-stable dependencies in stable releases of drupal/core, because these will be rejected by minimum stability stable unless the non-stable dependency is listed in the top-level composer.json file, which of course we do not want.

If we thought that it would solve problems to allow the use of RC components in stable Drupal releases, then we could change the minimum stability of drupal/recommended-project to "rc" instead of "stable" for stable releases. This would probably have limited impact. Then, we'd also need to get the word out throughout the community that before anyone upgrades to Drupal whatever-it-is-we-decide (e.g. Drupal 9.2.0 stable, to use Symfony 5.3.0-rc*), then they all must update their top-level composer.json file to also allow minimum stability stable.

I don't think it would be a good idea to do the same thing for minimum stability beta, though, since this limitation (or lack of limitation) applies to all dependencies in the project. There is no technical reason why we could not pull back to beta or alpha as the required minimum stability to run Drupal. It just sort of begs the question what a "stable" release of Drupal is if we are saying that in order to make our release schedule work, we perpetually need to be running unstable versions of Symfony at the beginning of each release cycle. I think this is a bad idea; Drupal sort of decided to become beholden to the Symfony release schedule, so it would make sense to have our release schedule align with theirs somewhat so that we could release our next stable after their stable release.

At the moment if you put a release candidate into a dev release of core, then ComposerProjectTemplatesTest fails. Some context on #3213295: Update Symfony 5 components to 5.3-rc1. Example here: https://www.drupal.org/pift-ci-job/2062451

Bumping this to critical since it will eventually block us running on Symfony 6.

There are other use-cases for wanting to know what a -dev version is equivalent to (what the most recent tagged release on the branch was). Since release managers already have to modify Drupal::VERSION whenever they make a release, and then modify it back to *-dev afterwards, I think adding another constant right below it would be the best way to add minimal work to release managers and thereby increase the likelihood of it remaining correct as releases get made.

Therefore, how about this approach that's based on that? Since this is a different approach than #10, I'm not uploading an interdiff.

+++ b/core/lib/Drupal.php
@@ -77,6 +77,18 @@ class Drupal {
   const VERSION = '9.2.0-dev';

This context line causes #21 to not apply on Drupal 9.3. This patch only changes that line and nothing else.

+++ b/core/lib/Drupal.php
@@ -77,6 +77,18 @@ class Drupal {
+  const VERSION_FLOOR = '9.2.0-alpha1';

Right, so that's not gonna work on 9.3.x either. Since there's no release yet on 9.3.x, this patch sets it to '9.3.0-dev'.

+++ b/core/lib/Drupal.php
@@ -77,6 +77,18 @@ class Drupal {
   const VERSION = '9.3.0-dev';
 
+  /**
+   * The latest tagged release on this branch.
+   *
+   * When VERSION is not a "-dev" version, this is the same as the VERSION.
+   *
...
+   * is prior to the current state of this codebase. This is useful for knowing
+   * if the current -dev version is after an alpha, a beta, or an RC, because
+   * VERSION alone doesn't contain that information.
+   */
+  const VERSION_FLOOR = '9.3.0-dev';

--- a/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
+++ b/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php

The comment could do with describing this situation. I.e. when both are 9.x.0-dev. I.e there have been no releases for that particular minor. Otherwise the When VERSION is a "-dev" version... comment doesn't make sense.

I also wonder if this complexity is necessary.

Another approach would be to change ComposerProjectTemplatesTest to look for git tags in a minor release. If the test is not being run for a git checkout we could skip the test. And then all the complexity would be contained and there wouldn't be a confusing constant to add.

Isn't there an issue with #19 that the branch tests will fail if the release manager bumps VERSION_FLOOR as part of a release, but we have dependencies that aren't stable enough? IIRC we don't run tests before the "Back to dev" commit. What do we do if and when that happens? The release will already have been made with the set of dependencies that would fail this test, I think?

Basically, isn't this test trying to predict whether a future release will have the correct stability? The last release doesn't matter so much, as I think we want to know ahead of time whether the next release will be correct. But we can't always know what that next version will be!

The comment could do with describing this situation.

This patch only changes the comments.

Another approach would be to change ComposerProjectTemplatesTest to look for git tags in a minor release.

Hm, yeah that could be a way to do it as well. I don't know if I'll have time today to try that, so if someone else wants to, go for it.

Isn't there an issue with #19 that the branch tests will fail if the release manager bumps VERSION_FLOOR as part of a release, but we have dependencies that aren't stable enough?

That's one advantage of the VERSION_FLOOR constant over doing it based on git tags. With the constant, you could post a patch that changes that constant and run that patch on DrupalCI prior to tagging the release. With the git tags approach, you'd have to either tag and run the test locally (which is time consuming) prior to pushing, or you could post a patch on DrupalCI that overrides a local variable within ComposerProjectTemplatesTest (which might be a little harder to remember).

The last release doesn't matter so much, as I think we want to know ahead of time whether the next release will be correct.

That's not always true. For example, you might add a beta dependency to a beta1 release. You now have 2 weeks until the RC release, but on the minute after you've tagged beta1, you haven't yet updated your dependencies to rc versions. You know you need to in the coming week or so, but you don't want HEAD failing tests in the meantime. In terms of knowing whether the next release will be correct, I think the best way is per above to post a patch that changes the constant however far in advance of the upcoming release that you want to and see if it comes back green.

I worry that we're introducing extra complexity for something that will not happen very often. Wouldn't it be simpler to change the minimum stability to RC in the test now and then set it back to stable when we update the dependencies. We could make the minimum stability easy to change and have document that when it is not stable there must be a @todo with a link to an issue to change it back. This way we don't pollute \Drupal with another constant that has very little use-case.

We could also make the test fail if the stability set in the test doesn't match the minimum stability of all dependencies.

#29 sounds like a smart idea.

Another approach would be to change ComposerProjectTemplatesTest to look for git tags in a minor release.

Fixed.

We could also make the test fail if the stability set in the test doesn't match the minimum stability of all dependencies.

Fixed.

+++ b/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
@@ -26,6 +27,21 @@
+  const MINIMUM_STABILITY = 'stable';

#31 should hopefully pass, since this is correct for HEAD as-is.

These two patches just change this to 'alpha' and 'RC' respectively. The first should fail, because we already have tagged releases for beta, and the 2nd should fail, because we don't actually have non-stable dependencies (per #29).

Darn. All looks good with #37, except the "alpha" patch should have had an additional failure on 9.2.x, for specifying a lower stability than the 9.2.0-beta1 tag that's already been released, which means the getCoreStability() function isn't working as expected. Need to look into why.

The issue is that BuildTestBase::executeCommand() runs in a separate workspace directory, away from the checkout where the tests are actually running from:

    $this->commandProcess->setWorkingDirectory($this->getWorkingPath($working_dir))

As a result git tag returns nothing. The reason that assertCommandSuccessful() succeeds is that it returns the exit code of the last part of the pipe - head -1, which was successful (though it did nothing). Maybe git describe is easier/more reliable than listing all tags?

Thank you! Let's see if this now has both expected failures.

Maybe git describe is easier/more reliable than listing all tags?

+1. I didn't know about that command. I'll make that change.

To reduce noise here while I figure out how to run git commands correctly on DrupalCI, I opened #3215503: Testing issue for #3186364.

Yay, #46 works as expected. The first patch sets ComposerProjectTemplatesTest::MINIMUM_STABILITY to "stable", and that passes, because all dependencies in HEAD are stable.

The second patch sets it to "alpha", and:

• On 9.2.x it fails with the message "Failed asserting that 1 is equal to 2 or is greater than 2.", which is the first assertion in testMinimumStabilityStrictness(), which makes sure that we're not setting that constant ("alpha") as less than core's current stability level ("beta" on the 9.2.x branch).
• On 9.3.x it fails with the message "Failed asserting that two strings are identical.", which is the second assertion in testMinimumStabilityStrictness(), which makes sure that we're not setting the constant ("alpha") as different than the actual minimum stability of core's dependencies ("stable"). This also means that it also passed the first assertion that the constant ("alpha") is >= core's current stability level ("dev" on the 9.3.x branch).

Therefore, this patch will allow #3213295: Update Symfony 5 components to 5.3-rc1 to add an RC dependency and change the constant to "RC".

#46 is very encouraging. Minor comments follow:

1. +++ b/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
   @@ -66,6 +82,24 @@ public function provideTemplateCreateProject() {
   +  /**
   +   * Make sure that static::MINIMUM_STABILITY is sufficiently strict.
   +   */
   +  public function testMinimumStabilityStrictness() {
   +    // Ensure that it is not less stable than the current core stability. For
   +    // example, if we've already released a beta on the branch, ensure that we
   

   s/it/static::MINIMUM_STABILITY

   - worth the duplication vs. 'it' being ambivalent, would probably replace 'it' in later inline comments in the same method too.

2. +++ b/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
   @@ -325,4 +352,90 @@ protected function makeVendorPackage($repository_path) {
   +   */
   +  protected function getLowestDependencyStability() {
   +    $root = $this->getDrupalRoot();
   +    $process = $this->executeCommand("composer --working-dir=$root info --format=json");
   

   Tried to think of a better method name, but came up with getLeastStableStability() which is not better!

+++ b/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
@@ -325,4 +352,90 @@ protected function makeVendorPackage($repository_path) {
+      else {
+        // If the current version is developing towards an x.y.0 release, there
+        // might be tagged pre-releases. "git describe" identifies the latest
+        // one.
+        $root = $this->getDrupalRoot();
+        $process = $this->executeCommand("git -C \"$root\" describe --abbrev=0 --match=\"$version_towards-*\"");
+

Glad we're only doing this in test coverage, reminds me of old cvs_deploy and git_deploy modules.

+++ b/core/tests/Drupal/BuildTests/Composer/Template/ComposerProjectTemplatesTest.php
@@ -325,4 +352,90 @@ protected function makeVendorPackage($repository_path) {
+        $root = $this->getDrupalRoot();
+        $process = $this->executeCommand("git -C \"$root\" describe --abbrev=0 --match=\"$version_towards-*\"");

You can pass the working dir as the second argument so it might be simpler to say

$process = $this->executeCommand("git describe --abbrev=0 --match=\"$version_towards-*\"", $this->getDrupalRoot());

This doesn't work as I expected, you can only have a subdirectory of a temporary working dir, ignore this.

NW for #49.1, but otherwise this is looking good to me.

Fixes #49.1.

Looks good, thanks for working on this.

Alright proof will be in how we get on with the actual update issues, and then updating from them to stable versions, but this looks great.

Committed/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!

Follow-up: #3216787: Consider alternatives to the ComposerProjectTemplatesTest::MINIMUM_STABILITY constant