CI installs wrong Drupal version

I would prefer to configure Drupal versiosns to test in composer.json instead of CI variables.

Note that this is the existing behaviour for the default testing. See this: https://git.drupalcode.org/project/gitlab_templates/-/blob/main/scripts/...

If an entry is detected in the composer.json file, we try to respect it.

Having said that, I still don't know why it picked up Drupal 10.2.0-alpha1 instead of the latest stable as per: ^10.2

Line 40 looks like a regression to me.

Not setting require-dev of core-recommended just because require Drupal/core is set to indicate compatibility would be overkill.

That if section was intended for dealing with next major version testing where we need to override a version.

If we’re not overriding we want to try and merge the request from test run with the composer.json to end up with either an error of the release being incompatible with the version under test, or being able to set the specific core release.

I just ran into the same problem with Address: #3459145: "phpunit (max PHP version)" running with Drupal 10.1.8 → .

The composer.json requirement was "^10 || ^11" and it caused Drupal 10.1 to be used for the max PHP tests instead of 10.3. Removing the requirement fixed the build.

I was able to reproduce something similar with this code:

composer create-project drupal/recommended-project my_site_name
cd my_site_name

Then edit the "require" section of the "composer.json" file to this:

    "require": {
        "composer/installers": "^1 || ^2",
        "drupal/core": "^10 || ^11",
        "drupal/core-composer-scaffold": "10.3.x-dev",
        "cweagans/composer-patches": "~1.0",
        "drupal/core-dev": "10.3.x-dev",
        "php-parallel-lint/php-parallel-lint": "^1.2"
    },

Then composer install.

This brings Drupal 10.1.8.

It's down to this line:
"drupal/core-dev": "10.3.x-dev",

So, it makes sense that we set it to be the same as the "drupal/core" value, when this is overridden.

--

@cmlara - re #4, why do you think it's a regression? If we don't do that, we'd have "drupal/core" and "drupal/core-recommended", with different versions. Note that this is not just for the variants, we are trying to respect what the module is setting.

Ideally, modules shouldn't need to set "drupal/core" as a dependency, because in the context of Drupal, this is totally implied.

Following steps #6.

$ composer why-not drupal/core 10.3.0
drupal/core                10.3.0 requires         symfony/filesystem (^6.4)                    
drupal/recommended-project -      does not require symfony/filesystem (but v7.1.2 is installed) 
drupal/core                10.3.0 requires         symfony/finder (^6.4)                        
drupal/recommended-project -      does not require symfony/finder (but v7.1.1 is installed)

Removing lock file resolved the issue.

Package operations: 1 install, 4 updates, 0 removals
  - Downgrading symfony/finder (v7.1.1 => v6.4.8)
  - Upgrading symfony/psr-http-message-bridge (v2.3.1 => v6.4.8)
  - Installing symfony/mailer (v6.4.9)
  - Downgrading symfony/filesystem (v7.1.2 => v6.4.9)
  - Upgrading drupal/core (10.1.8 => 10.3.0)
14 package suggestions were added by new dependencies, 

Checking the run linked in the issue description, we see:

  - Locking drupal/core (10.2.0-alpha1)
  - Locking drupal/core-composer-scaffold (10.3.x-dev a1a186c)
  - Locking drupal/core-dev (10.3.x-dev 6a88909)

Which I think shows the issue we just found above.

I've put together an MR that will replace the values for the other two, but haven't tested yet (other than via composer directly).

We'd need to test with a project that sets "drupal/core" in its composer.json file.
Testing instructions: https://project.pages.drupalcode.org/gitlab_templates/info/testing-mrs/

Going to sleep so very quick response, can provide more in morning:

Ideally, modules shouldn't need to set "drupal/core" as a dependency, because in the context of Drupal, this is totally implied.
report

Disagree,
1) When we publish actual releases we add this via the fascade
2) The facade explicitly caters for modules setting this in their json.
3) Modules should call out their dependencies (I view our current exapand composer script as more of a stepping stone to a time where everyone has a properly defined composer.json, especially once the issues of allowing modules to be developed in root folder makes it into core.
4) Git based deployments (adding the module as a source repo) need this data to be in the composer.json to properly calculate an install dependencies. Composer needs to know Drupal/core comparability, the info.yml is not a replacement for this.

(I would rather we had a drupal/api-spec metapackage to replace drupal/core however we use what we have).

why do you think it's a regression?

Unless I’m mis-remembering, way back before we ever added next version testing I’m fairly sure I had this working for testing where I explicitly declared core versions in the composer.json and was able to override using the core target variables to select a specific release.

I also disagree with

Ideally, modules shouldn't need to set "drupal/core" as a dependency, because in the context of Drupal, this is totally implied.

As a module developer, I want `composer require drupal/mymodule` to fail for users who have an unsupported / old core version.

Fair enough, thanks for your explanations. If we look from a "composer-only" point of view, it makes sense indeed to declare it. It's when you bring the Drupal side of things (and the info file) that I got it mixed up.

--

So, the way the templates are working are:
- If you use defaults, it'll set all the versions in composer automatically
- If you override the _TARGET_CORE, it'll override it only for the current version, and ignore this in next/previous variants.
- If you have "drupal/core" in your "require" section, it will take that one for the current variant, and ignore it for the next/previous variants.

This MR (which still needs review) covers the edge case where "drupal/core-dev" seems to force a different "drupal/core" version that what is expected.

Visually reviewed, and (very quickly) smoke tested a theory.

The problem is that core-dev is not 'versioning limiting'. We recently had a discussion on slack about this and were reminded of https://www.drupal.org/project/drupal/issues/3163114 ✨ Match the version of drupal/core-* metapackages Needs review

The MR as proposed may stop the alpha error that occurred on this issue, however I don't see this helping a module declaring ^10 || ^11 that sets $_TARGET_CORE=^10.2 without setting ignore_project_core_version (which is only suppose to be used for next minor/major when the module is expected to show itself as incompatible and is pre-testing compatibility)

At best the MR currently appears to be treating the symptom of a larger root fault.

We have been abusing core-recommended to get an explicit version of core to install this, however that is probably not correct. Some of us moved to GitlabCI originally because DrupalCi enforced core-recommended prohibiting using a new major of a library (see 📌 Come up with a way to allow core-recommended (and tarballs?) to install Guzzle 7 for PHP 8.1 compatibility and Laminas-feed 2.19 for PHP 8.2 compatibility Closed: won't fix ). Similar issues are likely to occur in the future and we need to leave this open to developers to have flexibility in the deployment.

This does have me thinking this may not have been a regression and was just a bug from the start as I seem to recall abusing this to test modules, and it may have been after I had tested scheduled jobs that targeted explicit Core releases.

What we probably need to do is:

• Not add core-recommended unless requested (or allow an opt-out of core-recommended) (possibly out of scope for this issue, mentioning it as its closely linked to our fault here)(this moves towards my point that the expand script does too much, it needs to as it was intended to be an easy migration from DrupalCi, however eventually we should get rid of most of this and depend on the maintainer to keep their file manageable.)
• If ignore_project_core_version is set:
  • Overide drupal/core et al. to the defined constraint.
• If ignore_project_core_version is not set:
  • Override drupal/core et al. to the defined constraint.
• Validate (using composer::semver) that $_TARGET_CORE is compatible with the project drupal/core.
• If not throw an error and end the run.
• If it is compatible hard set drupal/core to $_TARGET_CORE to ensure we test using the requested release.

Needs work for issue in #15.

Thanks @cmlara. I like the suggestion here:

If ignore_project_core_version is set:
- Overide drupal/core et al. to the defined constraint.

If ignore_project_core_version is not set:
- Validate (using composer::semver) that $_TARGET_CORE is compatible with the project drupal/core.
-- If not throw an error and end the run.
-- If it is compatible hard set drupal/core et al. to $_TARGET_CORE to ensure we test using the requested release.

We can tackle the above regardless of where it is drupal/core or drupal/core-recommended (that could go on a follow-up or separate issue).

Opened !243

Was looking at ways to get rid of the expand_composer script as part of generating a custom template and might of hit a solution that could work for solving this issue without adding complex logic on our side that should be left to composer.

composer update/upgrade --with can be passed constraints, these constraints than be added to the lock file and used for additional commands.

I'm not 100% sure this is going to be BC compatible, need to debate the various scenarios to see if this may not be.
Initially I'm worried about $COMPOSER_EXTRA (and the upgrade status equivalent). Is there any flag that could be passed to composer install but not composer update

Second concern, is there a risk of commands being run after upgrade will bypass the lock and use the un-bound wildcard constraint upgrading everything from a lower constraint to a higher release. Upgrade Status is my initial concern however it does not call for updating all dependencies I believe it should be safe and will try and fit those packages into the lockfile constraints.

https://git.drupalcode.org/project/date_point/-/pipelines/243659 has "drupal/core": "^10.3 || ^11.0", in the composer file and we were getting Drupal 11 installed in the Drupal 10 pipelines, with Drupal 10 dependencies, so it was failing, like other examples above.

--

I think I got a good compromise which implements some of the suggestions in #15/#17.

See https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/236...

Basically, we are overriding the core value for ALL the variants that we control/provide, including the "current" ones, which were the only ones where we weren't doing it. By overriding the value to what the variant expects, everything else should fall into place with the right versions.

I've left the previous logic that tries to match "core-dev" to the value of "core", as it can be useful if people create variants.

Please review.

For me it's now failing with the following message

Could not use "\Drupal\Tests\Listeners\HtmlOutputPrinter" as printer: class does not exist

https://git.drupalcode.org/project/date_point/-/jobs/2377976

After I switched to MR-236 the error dissappeared.
https://git.drupalcode.org/project/date_point/-/pipelines/247639

cmlara → closed merge request !243

!236 forces a maintainer to choose between validating their module composer.json is compatible with the target version of core, or receiving the correct version of core to test. Essentially we still have the root part of this bug present just in a different scenario if we make the above changes.

I still think that the changes in the scripts/expand_composer_json.php fix that issue.

Also, with the example set in #21, we are solving a problem in two different ways:
drupal/core: ^10 || ^11 right now, in a Drupal 10 variant, Drupal 11 is brought for core but 10.3 for core-dev.

• With the above fix, with default values, the Drupal 10 variant will set the expected Drupal 10 versions and test.
• With the above fix, setting IGNORE_PROJECT_DRUPAL_CORE_VERSION to 0, the Drupal 10 variant will install Drupal 11 for core and for core-dev.

I think it fixes the inconsistency we had. There is only one point that we haven't covered from #17:

- Validate (using composer::semver) that $_TARGET_CORE is compatible with the project drupal/core.

But doing any sort of Semver checks will pass in the above scenario. ie: 10.3 satifiest ^10 || ^11 and vice-versa so I didn't see the benefit of adding the extra logic.

It is my understanding the API spec for

IGNORE_PROJECT_DRUPAL_CORE_VERSION means instruct expand_composer to overwrite core related versions without regard to existing version constraints. Used to allow next version testing where a module does not include the next major of Drupal in the allowed constraints

DRUPAL_TARGET Replaced $_TARGET_CORE for instructing composer stage which version of drupal/core to be installed along with the module under test for use in subsequent stages

Is that an inaccurate description of what these variables were created for?

Assuming the above is an accurate description and assumingcomposer.json contains:

  "require": {
    "drupal/core": "^9 || ^10"
  },
  "require-dev": {
    "drupal/core-dev": "^9 || ^10",
    "drupal/core-recomended": "^9 || ^10"
  },

DRUPAL_CORE="~10.2.0" IGNORE_PROJECT_DRUPAL_CORE_VERSION=0 php expand_composer_json.php

    "require": {
        "drupal/core": "^9 || ^10"
    },
    "require-dev": {
        "drupal/core-dev": "^9 || ^10",
        "drupal/core-recommended": "^9 || ^10"
    },

Incorrectly tests Drupal 10.3 when the test requested highest 10.2.x release(currently 10.2.7). (This is the point I was trying to make about testing with the correct version of core in #23)

DRUPAL_CORE="~10.2.0" IGNORE_PROJECT_DRUPAL_CORE_VERSION=1 php expand_composer_json.php

    "require-dev": {
        "drupal/core-recommended": "~10.2.0",
        "drupal/core-dev": "^9 || ^10"
    },

Correctly tests 10.2.7.

When we increase $_TARGET_CORE in a few weeks (or we try and test today for next-major)
DRUPAL_CORE="^11" IGNORE_PROJECT_DRUPAL_CORE_VERSION=1 php expand_composer_json.php

    "require-dev": {
        "drupal/core-recomended": "^11",
        "drupal/core-dev": "^9 || ^10"
    },

Error due to core-dev conflict. IGNORE_PROJECT_DRUPAL_CORE_VERSION can not be used to test versions of core not included in the composer.json.

Phrasing my concerns another way to hopefully make it more clear:

Since maintainers/users can control $_TARGET_CORE via the GitLab ui

Given the following composer.json

   "require": {
        "drupal/core": "9 || ^10",
    },

When $_TARGET_CORE = ~10.2.0 The "composer" job should pass and install with the latest 10.2.x release.
When $_TARGET_CORE = ^10 The "composer" job should pass and install the latest 10.x release.
When $_TARGET_CORE = ^11The "composer" job should fail as the root project composer.json is not compatible with D11.

The above should be true for composer.json variants containing either or both drupal/core-dev and core-recommended (both as required or dev constraints)

The ability to opt in to next-major testing should also not be negatively impacted, the composer stage should install the version requested (assuming all other non-core dependencies are compatible with the constraint or are lenient allowed)

With the current MR:
- If $_TARGET_CORE is ~10.2.0, then "core-dev" and "core-recommended" will be set to that, and "core" (from the module's composer.json file), will be ignored.
- If $_TARGET_CORE is ^10, then "core-dev" and "core-recommended" will be set to that, and "core" (from the module's composer.json file), will be ignored.
- If $_TARGET_CORE is ^11, then the pipeline might or might not work depending on the code.

I understand what you say here, that the maintainer put "drupal/core": "9 || ^10", in the "composer.json" file, but then, why would they set ^11 in _TARGET_CORE if they don't want the previous constraint to actually be ignored?

If users are advanced enough to start manipulating the "inner" behaviour of the CI, they should also be able to override it as needed or read the errors reported and fix accordingly.

I keep on thinking that this MR improves the current logic. It might still not be perfect, but it's solving and documenting some of the cases that are being reported. It's trying to make "drupal/core-recommended" and "drupal/core-dev" be the same, and it's allowing the CI to set up the recommended value, or even giving maintainers a way to override this behaviour if needed (but still try to set the same value in "core-dev").

, but then, why would they set ^11 in _TARGET_CORE if they don't want the previous constraint to actually be ignored?

Testing that everything is compatible with ^11, including the composer.json.

As humans we are not perfect. From time to time when working “add support for release ” issues the composer.json or info.yml will accidentally not be updated.

Automated testing helps make this become apparent quickly. Ignoring the module composer.json makes it more likely such errors will slip into a release.

We can do all the work to make the module.info and any referenced API is compatible, however if we fail to update the composer.json the module is still “incompatible” for publication.

I can’t speak for everyone however part of my release cycle process:
Tag a version via git
Navigate to GitLab UI to run a final round of tests.
When all tests pass/fail as expected publish the release on D.O.

That final rounds of tests is intended to be my final saving check, to be sure the release actually works. Depending on release I may run through several permutations. Adding a new core version always triggers my executing a manual test with the next major constraint, a test where I absolutely want no overrides to be enabled.

Removing a supported release also triggers similar to validate that a release will not install on an older version. It is helpful the info.yml works to block a new install, however that is too late to prevent WSOD’s in production for sites that already have the module installed.

IIRC we have had a couple of contrib modules have issues around the composer.json accurately reflecting compatibility that tests cycles equivalent above may have uncovered before impacting end users.

The thing is, that the root problem might not be in the templates at all, it seems to me that allowing "drupal/core-dev" 10.3 when "drupal/core" is ~11 is wrong, and that's what's triggering everything, but this is a "core-dev" packaging/composer issue in my opinion, as tools for testing 10.3 should not be "compatible" with Drupal 11, but right now composer is happy installing that combination.

I wouldn't want to add logic here (and also bring extra libraries like composer::server) that would only complicate the maintenance of the code further for a case that should be handled somewhere else.

I keep on thinking that what we have now is a good "reaction" to this issue, that fixes (or at least improves) the things that we can control.

I agree that the current MR is opinionated in the sense that it overrides whatever is set in composer.json even for the current job. To be honest, I'm ok setting IGNORE_PROJECT_DRUPAL_CORE_VERSION to 1 for current, or not setting it at all. This latter case will use whatever is in the composer.json file for "drupal/core" but it will fix "drupal/core-dev" to the needed version.

I'm fine either way in the above change (setting IGNORE_PROJECT_DRUPAL_CORE_VERSION for current vs not setting it) but I wouldn't want to overcomplicate the rest by adding extra dependencies.

I think that not setting the value perhaps is the closest thing to try what's on the composer.json, but situations like "^10 || ^11" could be more confusing as current (D10 at the time) will then test D11. That's why I'm also happy setting it, so current (D10) tests D10, as expected.

We've had some more reports via slack about this situation. While the solution is not perfect, it fixes the immediate issue, so I'm setting this to RTBC and I will commit it shortly.

I'm happy for follow-ups to be created, if needed.

Merged the MR. For now, only maintainers with "main" or "1.x-latest" in the ref value (https://project.pages.drupalcode.org/gitlab_templates/info/templates-ver...) will get this change.

It will be part of the next release. Thanks all for the back and forth. Happy for improvements to be made in follow-ups.

For now, only maintainers with "main" or "1.x-latest" in the ref value will get this change.

Can I just check my understanding ... I thought that if a commit was not yet in a tagged release, then those using '1.x-latest' would not get the change, it would only be available to those using 'main' ref. Heres a comparison of main vs 1.x-latest
Sorry if I'm wrong and this is unwanted noise on the issue.

No worries.

As long as we don’t have a 2.x, then 1.x-latest and main will be the same thing.

We just added it to the list of moving tags because it was easy, but it’s effectively the same as “main” as we don’t have short term plans for a 2.x release (breaking changes).

The documentation page needs to correct some spacing https://project.pages.drupalcode.org/gitlab_templates/info/templates-ver...

OK. So if currently 'main' and '1.x-latest' are the same, why does the comparison link I gave in #33 show that this commit is not in '1.x-latest'? Shouldn't that comparision show no differences?

As long as we don’t have a 2.x, then 1.x-latest and main will be the same thing.

main refers to main:HEAD
1.x-latest refers to the latest tagged release which may or may not be the same as main:head (and if we open a 2.x release will never be the same as main:HEAD)

This project has routinely tagged a release and not made it 1.x latest untill some time after main:head has moved forward as it has wanted to allow the newest tagged release some dwell time.

At the moment 1.x-latest is one commit behind main:head. 1.x-latest does match the most recent tagged version 1.5.7

Comment #32 is indeed slightly inaccurate on who will currently receive this.

At the moment 1.x-latest is one commit behind main:head. 1.x-latest does match the most recent tagged version 1.5.7

Yes, I agree '1.x-latest' is one commit behind main.
BUT it does match a tagged release, 1.5.7. This link below show four tags
https://git.drupalcode.org/project/gitlab_templates/-/commit/fa5f130c51b...

100% on the above comment. Thanks for clarifying @cmlara.

I’m on the phone and with the kids and my mind is not in the right place for neither the issue nor the kids.

“main” is not a tag, so yeah, it can have commits not present in “1.x-latest”.

We need to improve the documentation page so it’s clearer.

Just to be clear, I have no problem with the release process or the current tags. It is only the comments in #32/34 and #36 that I quoted above, which I am querying. They do not match what I am observing in the repo commit list, so I want to make sure I am interpretting it correctly.

No worries. I said something wrong in #32 and it's good to be questioned on these cases. I made a commit to the docs to fix some formatting and hopefully clarify a bit more what is what.

Good spot on the blank line necessary to get the bullet points formatted properly. Also a good addition to state explicitly that 'main' is the name of the branch, not a tag. Yes the page reads fine, it is nice and clear.

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