Problem/Motivation

The core-recommended metapackage currently pins all production dependencies to an exact version. For example:

   "require": {
        "drupal/core": "9.1.3",
        "asm89/stack-cors": "1.3.0",
        "composer/semver": "3.2.2",
        "doctrine/annotations": "1.11.1",
        "doctrine/lexer": "1.2.1",
        "doctrine/reflection": "1.2.2",
        "egulias/email-validator": "2.1.22",
        "guzzlehttp/guzzle": "6.5.5",
        "guzzlehttp/promises": "1.4.0",
        "guzzlehttp/psr7": "1.7.0",
        "laminas/laminas-diactoros": "2.5.0",
        [...]
    }

This decision was discussed at length in #3076600: Create drupal/core-recommended metapackage. See comment #17 on the original issue especially. The strict pinning is intended to prevent sites from accidentally updating their dependencies to a version that's not compatible with core due to a regression. The hotfixes also had to be backported to 8.5.x because of the highly critical security vulnerability in SA-CORE-2019-003 that was disclosed around the same time. So drupal/core-recommended was intended to prevent such a scenario.

Since the core-recommended metapackage was made available, we've discovered a few issues with the approach:

  1. Occasionally, a third-party dependency will disclose a security issue without coordinating with the Drupal Security Team. When this happens, a site owner managing their site with Composer might need to install the security update for the dependency before a core security release has been issued. This has happened twice recently with Archive_Tar in SA-CORE-2020-013 and SA-CORE-2020-001.

  2. Core only issues security releases for dependency updates if the dependency is exploitable in core. For example, Symfony issued a CVE for a remote code execution vulnerability in September 2020 for the http-client component, which is not used by Drupal, but their release also included an update to http-kernel for compatibility. As a result, many sites' security scanners and CI builds began failing because the core-recommended version of http-kernel was also flagged as insecure, and affected site owners couldn't easily update the component themselves because of core-recommended's restrictions.

    So, we had to issue a hotfix patch release in 9.0.5 and 8.9.5 right after our normal patch release.

  3. Sometimes, when a site has contrib or custom code with separate Composer dependencies, the strict pinning in core-recommended can prevent Composer from resolving dependency conflicts if there's an issue with core's pinned versions somewhere in the dependency tree.

While preventing a composer update --with-dependencies from breaking a site has advantages, overall, it is probably worse for a composer-managed site to not be able to install a necessary security update. In the 14 months since 8.8.0 was released and drupal/core-recommended became the recommended way to maintain a site, we've had more critical issues related to dependency version restrictions than we ever did with upstream dependency conflicts in the four years prior.

Since May 2022, Drupal.org provides core testing environments that update core dependencies to the latest version allowed by the current constraints, and these tests run nightly for all PHP versions on supported branches. This means that if an upstream dependency update does turn out to be incompatible with core, we are likely to find out about it within 24 hours.

Proposed resolution

  • One possible solution is to loosen the constraints in drupal/core-recommended from an exact version to ~x.y.z, so that patch-level updates are allowed. This would allow site owners to apply typical security updates for their dependencies without a core release being required.

    This wouldn't help if the third-party security release were issued in a new minor (which happens occasionally), and core releases would still be required for the tarball vendor version, but the impact would be lower. (Also we'll hopefully eventually phase out tarball installations; e.g. core automatic updates may not even support tarball sites.)

    It also wouldn't help with specific patch releases that introduce regressions, but new minor versions are the most likely to be disruptive. (E.g., it would have prevented sites from updating to Twig 1.38.)

  • It'd also be easier to manage if Composer had any concept of security releases and a composer audit command or similar so that the site owner could automatically apply security updates only. There are workarounds like the FriendsOfPHP security advisory list and Roave Security Advisories that core could adopt. (It might also be worthwhile to provide an official Drupal SA list like webflo's.) That way, sites could run composer updates that were restricted to security releases only, reducing the risk that a site would update to an arbitrary new release of a dependency that introduced a regression.

  • We could also renew advocating upstream for security release and auditing features in Composer 2.

  • Other suggestions?

Remaining tasks

Discuss. Maybe split out some child issues.

As per #10, this change might require changes to the core packaging script:

I wonder if we'll need to update the core packaging script to use --prefer-lowest on the composer install command as doing that would be exactly the same as the currently locked package versions.

User interface changes

N/A

API changes

TBD

Data model changes

Probably not.

Release notes snippet

The drupal/core-recommended metapackage now allows patch-level updates for dependencies. This means that site owners using drupal/core-recommended can now install most Composer dependency security updates themselves, without needing to wait for an upstream release of Drupal core that updates the affected package. Site owners should test patch-level updates before deploying them. Instructions for managing dependency updates with the updated drupal/core-recommended metapackage.

Note that egulias/email-validator has a wider constraint due to the name of its most recent supported version, so site owners may wish to add a specific constraint to avoid updates to version 3.3 or higher in the future.

PSA draft

https://docs.google.com/document/d/1Ht_ymWXGqXxS_V9wboyGQZ4P8dY9_sAPmIAn...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm created an issue. See original summary.

xjm’s picture

Issue summary: View changes
xjm’s picture

Title: Strict constraints in drupal/core-recommended prevent Composer-managed sites with applying their own security updates when a core update is not available » Strict constraints in drupal/core-recommended prevent Composer-managed sites from applying their own security updates when a core update is not available

xjm credited Mixologic.

xjm credited alexpott.

xjm credited larowlan.

xjm’s picture

Crediting folks who participated in semi-recent Slack discussions about this.

xjm’s picture

Title: Strict constraints in drupal/core-recommended prevent Composer-managed sites from applying their own security updates when a core update is not available » Strict constraints in drupal/core-recommended make it harder for Composer-managed sites from applying their own security updates when a core update is not available

@Mixologic pointed out that we have the workaround of removing the dep on drupal/core-recommended and switching to drupal/core until core provides a compatible update, but that's some highly manual frustration and not what users expect.

Also see: https://github.com/drupal/core-recommended/pull/9

cilefen’s picture

I agree. We have to update everything or not pin versions. Version ranges, along with min/max testing as you suggested, would be a solution that doesn't require new technologies.

alexpott’s picture

Status: Active » Needs review
FileSize
6.59 KB

Here's a patch to change core recommend to use a tilde and a minimum of what is in the lock file. Let's see what breaks.

I wonder if we'll need to update the core packaging script to use --prefer-lowest on the composer install command as doing that would be exactly the same as the currently locked package versions.

Also what do we want to do about PinnedDevDependencies?

Status: Needs review » Needs work

The last submitted patch, 10: 3198340-10.patch, failed testing. View results

alexpott’s picture

Status: Needs work » Needs review
FileSize
2.15 KB
8.74 KB

Fixing the tests.

Mixologic’s picture

It might be worth examining the design goal of drupal/core-recommended

drupal/core-recommended was intended to be a *removeable* shield that prevents upstream semver breaking changes in drupal core's 3rd party dependencies.

The tradeoff is that in order to update to something beyond what is pinned (like for upstream security or perhaps a separate dependency need), one must either remove the drupal/core-recommended shield and return to using drupal/core, or temporarily require the desired package as the pinned version. (i.e.
composer req pear/archive_tar:"1.4.12 as 1.4.11")

Some additional side effects that we've experience from this include:

  1. Confusion about how to update a site - updating drupal/core vs drupal/core-recommended etc.
  2. Confusion about what to use as a dependency (a starter template should rightfully require drupal/core-recommended, however, contrib modules should *not* use that because it makes it impossible to remove the seatbelt.) - theres about 12 contrib projects now that have it as a dep on drupal.org.
  3. Widespread lack of knowledge/documentation about how one is supposed to remove it, and the consequences, and benefits therein. i.e. the whole reason we hear about 'pain' with it in the first place.
  4. This only has the ability to protect a site against *just* drupal/core's 3rd party deps. It offers no protection against a contrib/custom project's 3rd party composer package introducing a semver incompatible change and breaking sites on an update.

So we have a few choices on how we move forward here.

  1. The proposed resolution of relaxing the constraints
  2. Improve the tooling and documentation
  3. Removing drupal/core-recommended entirely
  4. Expanding upon how drupal/core-recommended is built and making it more agile.

Option A: Relaxing constraints

This path is based on assumptions about the nature of future breaking changes and security releases, which we derive from our past experiences and somewhat on instinct. The idea is that we believe that semver breaks are less likely to happen in a patch release, and the security releases are not likely to happen in a minor release. I question whether past experiences give us any indication as to the nature of future issues.

  • If/When there *is* a secrelease in a minor instead of a patch, we are still going to have the same pain we have now, except even less people will have encountered this, meaning even less institutional knowledge and ideas about what to do when it occurs.
  • If/When a semver break happens in a patch, we're still going to potentially have broken sites until a core release can be created to fix that semver break.

Option B: Improve Tooling and Docs

Much of the 'pain' people are reporting is because they are unaware that there is a set of steps they can take when they need to get beyond what is locked in drupal/core-recommended.

We can/should provide a local drupal command/composer command that enables or disables the 'seatbelts'. This should basically do the following:

composer require drupal/core:~9.1.1  
composer remove drupal/core-recommended

Or whatever the appropriate composer steps are exactly.

Whether we relax the constraints or stay with the status quo, we should provide this tooling. So that there is a clear singular answer to "how do I get an upstream secrelease that I need?" because there will still be this same issue if there is a secrelease in a minor.

As an aside, it could be that the confusion comes from the name itself: in this scenario, the recommended steps are to *remove* drupal/core-recommended, which does sound counter intuitive.

Option C: Expansion of the Contraption

Another idea that we discussed in slack is to make use of the fact that composer allows for 4 digit version numbers, and make an automated release of drupal/core-recommended every time an upstream dependency changes and tests pass.

E.g. When 9.2.0 of drupal/core comes out, 9.2.0.0 of drupal/core-recommended is generated from cores' max dependencies.

The next time an upstream dependency puts out a release, we kick off a test using that new release and if it passes, we generate 9.2.0.1 of drupal/core-recommendedwith the new version of the release pinned.

People can keep updating drupal/core-recommended, and will get the newer versions of secreleases without having to wait on a core release cycle.

This is a bit more contraptiony than we have now, and would require infra to build this into our current CI/CD for core.
It also assumes some weird off label things like composer works with 4 digits. It never says it *should* or is supposed to, just that it does. So that rug could disappear in composer 3 etc.

But it is a path we could take, and we could have it build as often as hourly.

Option D: Remove drupal/core-recommended

There are some side effects of having this in the first place. And if we winnow down the scenarios where we provide safety, and we acknowledge that we're only really as safe as our tests, it starts to be where maybe this entire contraption isn't worth it after all and we should deprecate it and going back to allowing deps to float upwards by just using drupal/core ?

To summarize my own personal opinions on this, Im sour on Option A. because I think its based on too much hunch and instinct about what the future holds. Just doing option B is my preference, but if we do A we should do B also. Option C has appeal, as it improves the existing seatbelts, but, they also still have some existing issues that we cant fix going this route, and there are some 'doing brand new, atypical things' risk associated with it. Option D seems better to me than A, because by that point we've removed enough of the protection that Im unsure whether the maintenance of it is worth it still.

xjm’s picture

Adding note to the IS that the option of loosening the constraints might require a change to the packaging script. Also all the tags.

xjm credited Mile23.

xjm credited Warped.

xjm’s picture

Additional Slack participants who contributed to the discussion mentioned in #13.

xjm’s picture

Something that I keep wondering about is what other platform-type projects do. I feel like we are inventing solutions (with core-recommended being such an invented solution in the first place, and also especially all the evil-genius-scientist ideas that were discussed related to #17 option C).

Does anyone have any ideas for examples of other projectsʻ update management for these sorts of issues?

Ideally we would use native composer behavior and best practices for sites to update only the minimum dependency changes required for security releases. This is also something that our automatic updates tooling will need to do once we start building it, so tagging for the initiative.

Mixologic’s picture

core-recommended is kinda like seismic upgrades for a building that had a bad earthquake experience. We don't actually know if we live on a faultline, or if more earthquakes are coming. But we put it into place just in case.

It is definitely something we invented to solve a problem that everybody else has, but nobody else had ever attempted to solve.

It may be that the problem isn't worth solving - when we built it, we thought it was. Perhaps history and time has shown us that the issue of potential upstream api breakages/bugs that make it to your production site and leave you in some indeterminate, broken state aren't really the existential threat we thought they were.

After some more slack discussion, I've come around to the idea that if the release managers deem the risks acceptable, then straddling the 'best of both worlds' -> protecting from minor updates in deps where the breaks have typically happened, and allowing security patches, (Option A from above) Is likely fine.

As far as native composer behavior and best practices go .. well, I dont want to go full sdboyer and write 70 pages (https://medium.com/@sdboyer/so-you-want-to-write-a-package-manager-4ae9c...), but I don't believe that there are best practices because composer lacks support for some of the things we wish it did. Package and dependency management is not even really a solved problem space, and all the various package managers out there have their pros/cons etc.

Composer itself has no notion of whether a package should be classified as a security upgrade or not, thus does not have any features to alter its behavior based on that. We will, again, have to invent something on our own to solve that problem. Maybe that will get moved upstream. So, whenever is plausible, yes, we should use the industry standard/best practices. But very often we find ourselves on the edge of unexplored wilderness without a map, and we still need to proceed.

Anyhow, Now Im voting for a Combo of A/B. For B we can open a meta to improve things in many ways and figure out what we do and dont want to do.

catch’s picture

A seems like a decent compromise that doesn't prevent us from doing D later if we decide that's what we really want to do after-all. It's always going to be a balance between preventing updates to untested breaking changes vs. the possibility of prevent an update to a security release - only allowing patch releases theoretically should minimise both.

C seems like a lot of extra work and complexity to me.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

alexpott’s picture

Here's a reroll on 10.x and 9.4.x.

effulgentsia’s picture

+1 to the approach of relaxing the patch-level while locking the minor-level, which is basically option A from #13, and I think that comment explains the pros and cons well.

It's a tough thing to balance, but this package is named "drupal/core-recommended", so it should represent our recommendation on where that balance should be. I think that the risk of a security vulnerability is usually greater than the risk of a regression introduced in a patch release, so relaxing the constraints to make it the default behavior to get the higher patch release is I think consistent with what we should be recommending, and therefore what "drupal/core-recommended" should do.

On the other hand, I think the risk of a regression introduced in a new minor update is usually significant enough that sites should generally not get new potentially incompatible minor releases unintentionally. Especially when a security fix itself bumps the minor version, it's probably because that release includes something that the maintainers had a reason for not wanting to release as a patch release. I do think it's important for site owners to be able to get such a release, especially if it includes a security fix, but if Drupal core has not yet issued a new release that verifies compatibility, I think the site owner should run a different command to get these potentially incompatible updates. So I think something along the lines of #13B is worth doing as well, but I think that can be a followup to this issue rather than having to hold this issue up.

+++ b/composer/Generator/Builder/DrupalCoreRecommendedBuilder.php
@@ -56,7 +56,7 @@ protected function initialPackageMetadata() {
-      "description" => "Locked core dependencies; require this project INSTEAD OF drupal/core.",
+      "description" => "Minimum core dependencies; require this project INSTEAD OF drupal/core.",

I think we should describe this as something different than "minimum". Perhaps something like "Core dependencies locked to verified compatible minor versions"?

xjm’s picture

"Core and its dependencies with known-compatible minor versions" might be clearer.

Mile23’s picture

Relaxing the version to ^major.minor makes it the same as drupal/core. See: https://git.drupalcode.org/project/drupal/-/blob/9.4.x/core/composer.jso...

If that's the desired outcome, then we should add a replaces tag to drupal/core and have it replace drupal/core-recommended, and we should not generate any more drupal/core-recommended metapackages for any future releases. Then people won't be able to update using drupal/core-recommended.

Then if anyone asks, we can say: Use drupal/core.

Alternately we could just have drupal/core-recommended always be a copy of drupal/core with an extra._readme that says: Hey, you really should switch to drupal/core.

The reason we built this thing in the first place goes back to #13: There was a concern that people might break their sites using untested versions of dependencies after an update. That would lead to, you know... unwanted surprise. We'd do that because we don't trust our dependency constraints.

In an ideal world, we do #2874198: Create and run dependency regression tests for core and min/max dependency testing for various PHP versions. This allows us to begin to trust our tests, so that we can have an open-ended dependency constraint like drupal/core.

In a less-ideal world, projects with proper CI/CD pipelines will be our testing infrastructure and they'll tell us what's going on.

mmjvb’s picture

Relaxing the version to ^major.minor makes it the same as drupal/core.

Indeed, that would make drupal/core-recommended obsolete (13d). Probably why nobody suggests it.

13a suggests to allow bug fixes: ~major.minor.patch
That is the right thing to do, assuming dependencies correctly follow semantic versioning. Agree that it doesn't help when they don't, suggest to persuade them to follow semantic versioning and investigate alternatives when they don't.

Mile23’s picture

Loosening patchlevel is the same as locking the minor, which got the +1 in #25.

That's what's being suggested in #13a.

#13a and #13d are basically the same thing.

Whether we loosen the constraints of drupal/core-recommended (thus making it superfluous, being equivalent to drupal/core), or we remove it (leaving only drupal/core), we need to test against dependency regressions, and also have a min/max test for each supported PHP version.

That way we deliver the same level of QA as the current drupal/core-recommended.

mfb’s picture

The proposal under discussion is ~major.minor.patch which locks the minor version and allows any patch version greater than or equal to the specified patch version; the ^major.minor in drupal/core locks the major version and allows any minor version greater than or equal to specified minor version. So these are not the same thing. But yes dependency regression tests.. for both drupal/core-recommended and drupal/core would be very nice to have

mmjvb’s picture

#13a and #13d are basically the same thing.

No, they are not. #13d is locking major, #13a is locking major.minor. Big difference!!!

Whether we loosen the constraints of drupal/core-recommended (thus making it superfluous, being equivalent to drupal/core)

#13a is not the same as #13d. #13a does loosen but DOES NOT make it superfluous or equivalent to drupal/core !!!

longwave’s picture

Over in #3243899: [policy] Move composer/composer from a dev dependency to a production dependency @mxr576 brings up the point that if we are to introduce Composer as a production dependency for Automatic Updates, then with the present policy we will also need new releases of Drupal whenever Composer releases a security update, even if in some cases the security issue isn't exposed by Drupal's use of Composer.

effulgentsia’s picture

with the present policy we will also need new releases of Drupal whenever Composer releases a security update, even if in some cases the security issue isn't exposed by Drupal's use of Composer

Per https://www.drupal.org/core/maintainers/create-core-security-release/dep..., if it's not exposed by Drupal's use, we would not need to issue a security release for it. We would update drupal/core and drupal/core-recommended with Drupal's next normal patch or minor release though. However, if it is exposed by Drupal's use (and depending on the vulnerability, it well might be), then yes, we'd need to issue a Drupal security advisory and release for it, but that's no different than with any of our other dependencies, many of which have required more security updates than Composer has had.

xjm’s picture

Adding to #33, we do issue off-schedule (non-security) patch releases for core dep updates even if core isn't vulnerable, as a best effort when the release managers are available, so that folks can make their scanners hush. :) E.g. last week we had the Feb. bugfix release Wednesday and then a release to update Twig Friday. But the rest is accurate.

Mile23’s picture

Just to clarify:

The reason drupal/core-recommended exists is to turn drupal/core's lock file into a metapackage, so that there is a known-good lock file's worth of dependencies that have been tested before release.

That's so when you're a total Drupal newb and you're making your very first Drupal for the very first time, and you say composer create-project drupal/recommended-project, it will work. That way, our Drupal newb won't give up in exasperation and decide WordPress is better or whatever.

That's the reason drupal/core-recommended exists.

Need a dependency version that hasn't been tested with core? Here's drupal/core-recommended to tell you that it hasn't been tested with core. That new dependency probably works, but it's not Tested For Compatibility so therefore we disallow.

This is what I mean when I say that loosening drupal/core-recommended's constraints is the same as just requiring drupal/core. We might as well not have it.

So there are two options:

1) Make drupal/core-recommended go away, so that no one has to concern themselves with a double-locked set of dependencies. I endorse this position and did back in the Composer Initiative days, as well.

2) Close this issue as 'works as designed,' because all of the things in the original post that are listed as negatives are the reason this thing exists in the first place. The workaround is to not use it, and require drupal/core instead. In fact, the things in the original issue summary are not actually negatives at all, other than drupal/core-recommended and Composer telling you something useful that you didn't know before, causing you to work on something that needed the effort.

quietone’s picture

Every time I think about this I always arrive at the same preference - to leave drupal/core-recommended as is and do #13 Option B.

Maybe I am taking a simplistic view but it seems those who choose to use core-recommended have done so for stability and simplicity and, well it is called recommended. (Why would you not use the Drupal package recommended by Drupal?) Loosening the constraints is counter to the stability I think they want. Therefore, I do think that the README for core-recommended can be improved to explain the consequences in regard to dependency security and patch updates as well as pointing to documentation about what they can do, which can be part of #13 Option B.

However, I am not sure who the users of core-recommended are expected to be. Both #13 and #35 suggest that is meant to be temporary, as a path toward using drupal/core but the README says otherwise, that " It is recommended that all Composer-managed Drupal sites use this project".

mmjvb’s picture

With core-recommended being too restrictive, and core not restrictive enough, core-recommended should allow bug fixes, developers apparently have a need for core-restricted (current core-recommended) and people not conservative go for drupal/core. Consider the name core-recommended fine for the general practice of Composer built software to allow them. Consider it inappropriate in its current form.

Not sure it would be possible for a Composer plugin to change the requirements of existing metapackage. Obviously, very easy to provide your own metapackage allowing bug fixes. Or even package depending improvements to the constraints. Not everyone is following semantic versioning release management. Some only provide minor and major or even have different meaning about bug.

Consider it strange core-recommended is not configurable, even when properly named as strict. Strict sounds abstract, implementations should be allowed with differences, through configuration, of course.

Agree with works as designed, design sucks.

alexpott’s picture

so that there is a known-good lock file's worth of dependencies that have been tested before release.

We have that already - that's the root composer.lock file.

I think we should be making something that's useful and not a trap. If you choose core-recommended then the moment you are likely to learn about it's problems is exactly the moment you don't want to have to learn about the problem's of using the drupal/core package. That is, your CI will be red because the security scanner is failing and you have a release a client has been desperate to have for weeks to get out the same day. This is exactly NOT the time to relaxing all the restrictions in your lock file to accommodate a patch level update. Yes you could change to drupal/core and use the --no-update option and then only update the package you need to. Or core-recommended could lock to minors instead of patches and be more useful.

Mile23’s picture

"We have that already - that's the root composer.lock file."

That's not true, though. That's in core only, or doing dev work. If you require drupal/core-recommended, or drupal/core into a production project, you'll end up generating a new lock file for your project, and depending on how drupal/core sets its dependency constraints (and a million other things as well), you'll likely end up with an untested version.

So the dilemma here is:

If there *must* be a feature where you can say, "I want all the known-good tested dependencies," then we need drupal/core-recommended to be tightly constrained.

But if we use that by default for new projects, then we run into exactly the problem from #38: It could be annoying as all get out in a bunch of circumstances which quite likely to occur.

So that leaves us with some options:

1) Don't require that feature (which implies testing as many dependency versions as possible before release in order to ensure quality).

2) Leave everything the way it is and just deal.

3) Maybe even don't use drupal/core-recommended in the project templates (drupal/recommended-project), and use drupal/core instead, making drupal/core-recommended an opt-in feature.

Loosening the constraints of drupal/core-recommended means we don't need it, and we should instead focus on the constraints within drupal/core.

mfb’s picture

But currently you do get particular versions because drupal/core-recommended locks the patch version, right? So the proposal seems potentially worth trying as a middle ground between drupal/core and the current incarnation of drupal/core-recommended - only the minor version would be locked so it's easier to get security updates (aside from any security updates that come only in a new minor version).

Admittedly, locking the minor version isn't something I have experience with though - most of my experience is thru libraries with very loose constraints and dealing with all the fun fallout/inconsistencies of supporting multiple versions of other libraries...

alexpott’s picture

@Mile23 if you want to use the minor version constrained to get the exact tested dependencies with core you could use composer update --prefer-lowest - I would not recommend doing that though ;)

mmjvb’s picture

update --prefer-lowest is not guaranteeing strict minimum versions. Similar to --prefer-stable is not making sure you get stable releases.

It leaves it up to Composer to resolve the versions, having a preference for minimum version, but not pinning.

There may be situations where strict minimum is desired. See https://github.com/dereuromark/composer-prefer-lowest
For Drupal production sites that is clearly a bad idea. Having bug fixes is far more important than minimum tested version. Using drupal/core requirements also bad idea, far too liberal to be the recommended constraints. However, symfony packages may be the exception due to frequent minor releases.

Unfortunately, poor usage of the update command causes undesired changes to the lock file. If only there was an option like --when-needed to stop updates when you can and make updates when you need to. Recommend using --no-update and install rather than update.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

xeM8VfDh’s picture

My $0.02

CVE-2022-31043 is a new guzzle vulnerability that requires updating to 6.5.7, but core-recommended is pinned to 6.5.6:

"name": "drupal/core-recommended",
"require": {
...
"guzzlehttp/guzzle": "6.5.6",
...
}

So now I cannot simply fix the vulnerability with "composer update", I have to wait for core to publish a release. This is crazy. It should be changed to:

"name": "drupal/core-recommended",
"require": {
...
"guzzlehttp/guzzle": "^6.5.6",
...
}

Pinning should be done to a minor release, at the very least (so ^6.5), and maybe even to the major release (^6). As it stands now, we can't get security updates quickly, which is no good.

cilefen’s picture

drupal/core-recommended pins dependencies. If that doesn't work for you, you need not use it.

xeM8VfDh’s picture

For the time being, I and all Pantheon users cannot *not* use it. It's part of Pantheons workflow. I'm not currently concerned with whether you recommend using it or not. It IS vulnerable right now, and it IS code produced and hosted by the Drupal core team. Don't you think that merits at least fixing the pinned version to 6.5.7? Am I taking crazy pills??

P.S. Pantheon users are not an insignificant portion of Drupal users, so perhaps its worth thinking more deeply than "just stop using this". Not to mention the fact that there are lots of comments above about this very issue of too-strict pinning. I appreciate your quick responses, but I feel like you are ignoring the actual vulnerability here and missing the forest for the trees.

cilefen’s picture

@xeM8VfDh: This is the issue about "what to do with core-recommended", so this is where you should talk about that.

As I mentioned on #3225966: Consider loosening our constraint to allow sites to install Guzzle 6 or 7, or otherwise handle PHP 8.1 deprecations for Guzzle 6, where we should stop commenting because it is the kind of conversation that should take place in Slack, the release managers cannot build releases instantaneously. We are talking in circles. We are aware of the Guzzle patch. This issue is about relaxing the constraints in core-recommended a little.

If you want more control over dependencies or cannot wait for security releases that bump third-party dependencies in core-recommended, do: https://www.drupal.org/docs/develop/using-composer/using-composer-to-ins....

xeM8VfDh’s picture

You're right, we are talking past each other. I will wait for 9.4.0 and see if that resolves the issues.

cilefen’s picture

And I understand you said that Pantheon doesn't allow not using core-recommended. All hosting is a little different and we choose them for various reasons. Then please be patient and wait for the release. Drupal is enormous and releases take time.

cilefen’s picture

Drupal 9.4.0 will not improve core-recommended, because this issue is still open. If Pantheon requires it, then you'll just have to be patient and wait for Drupal to release security versions. No one I know was aware Guzzle was going to drop a security release on a Friday morning.

cilefen’s picture

Back to the topic of this issue:

I’ve read this thread. I don’t know of an other PHP project with a pinned set of dependencies like core-recommended. I agree with a few people here that the best way forward is to deprecate core-recommend and to just continue to do a solid job of setting ranges in drupal/core and testing them.

jrearick’s picture

I like the idea of removing the pinning. However, it does feel like a risky move. Perhaps we don’t need to think of it as an all-or-nothing thing. Perhaps core-recommended can loosen up on some dependencies that have a proven track record of not releasing broken patch versions. The dependencies that release rarely or are known to break things can still be strictly pinned.

These dependencies do get updated every once in a while, so I would imagine we have historical data on if there have been instances of dependencies breaking things that we had to fix before updating a pinned version? Perhaps quantifying the risk will help make this decision easier.

xeM8VfDh’s picture

I stand corrected, 9.3.16 did indeed fix core-recommended issue. Thanks for entertaining my confusion @cilefen.

As for this issue here, I support moving to reasonable, non-strictly-pinned versions, like "^6.5" instead of "6.5.6".

xjm’s picture

There are two useful things about drupal/core-recommended:

  1. Preventing accidental minor-version updates when applying security updates. Symfony and Composer, like Drupal core, backport security fixes to the previous minor, so it makes sense to have a best practice sites can use for that.
  2. Especially now that we allow Guzzle 7 in D9, preventing accidental major version updates for Guzzle is important to not break stuff.

A stated objective originally was to provide the dependency versions core is tested with to sites. We have max testing now, so within 24h we will know if an upstream release breaks core. So that mitigates that earlier problem.

Using ~ instead of ^ would help address points 1 and 2, now that max testing helps with the third thing.

I'm not opposed to just deprecating it, either. But it creates so much work and overhead for the release managers in its current state, and makes it more difficult for advanced users to keep their sites secure without doing anything to help less experienced site owners stay secure. Speaking as the only person on the hook dealing with this problem every single time there's an upstream release, I gotta say, the current situation is not sustainable. Timely core major, minor, and security fixes for real core security issues, or noisy security advisories for every single upstream Composer security update ever. Pick one.

xeM8VfDh’s picture

@xjm, that sounds reasonable to me

xjm’s picture

Issue summary: View changes
xjm’s picture

Updated patches, plus a patch to make rerolling this easier. Steps to reroll:

  1. git apply --index composer-3198340-change-only.patch
  2. COMPOSER_ROOT_VERSION=10.0.x-dev composer update drupal/core (replace version constant as appropriate)

I didn't provide an interdiff because of the reroll fuss, but I changed the description as I suggested in #34:

-      "description" => "Locked core dependencies; require this project INSTEAD OF drupal/core.",
+      "description" => "Core and its dependencies with known-compatible minor versions. Require this project INSTEAD OF drupal/core.",
catch’s picture

I and all Pantheon users cannot *not* use it. It's part of Pantheons workflow.

You can use a custom upstream with Pantheon and manage updates yourself.

Fully agreed with changing core-recommendedto minor releases, it was always going to involve trade-offs but it's clear we're trading off the wrong things.

The test failures on updated dependencies in #57 don't look obviously related to me.

xjm’s picture

The failures on the updated deps environments are also happening in HEAD, so they need not block this issue.

xjm’s picture

#58 is signoff from the release managers also.

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Given the hassle with the recent Guzzle releases I think this trade-off is worthwhile. Now we have max testing we should be able to figure out any breakages quickly, and if we get a serious problem or repeat offender we can consider re-pinning specific packages while leaving others to freely upgrade their minor versions.

The patch looks great, ultimately just a one line change with test support.

xjm’s picture

Issue summary: View changes

CR draft: https://www.drupal.org/node/3285240

Added a release note to snippet to the issue summary.

xjm’s picture

This will probably require some documentation updates in the handbook as well.

xjm’s picture

Issue summary: View changes
Mile23’s picture

Given that there's now a max-dependencies build that's already finding issues (#3285230: Migrate's DownloadFunctionalTest:: testExceptionThrow() is failing on guzzlehttp/psr7 2.3.0), which will enable maintainers to quickly manage upper constraints if necessary...

And given that there are a ton of people who have drupal/core-recommended in their project and don't realize they can change it (and never will)...

And given that release managers need sleep from time to time...

Rock on with your loose tilde constraints!

Looking at the patch, I was curious whether stuff like "symfony/http-foundation": "~v4.4.42" is a valid semver, so I applied the patch, and did this:

cd composer/Metapackage/CoreRecommended
composer install
[ ..stuff happens.. ]
composer show symfony/http-foundation | grep versions
versions : * v4.4.42

It would appear so... Plus we have Drupal\BuildTests\Composer\ComposerValidateTest which does validate the file.

+1

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/composer.json
@@ -52,7 +52,8 @@
+            "phpstan/extension-installer": true

This shouldn't be in 9.x - it's a 10.x only thing

alexpott’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
432 bytes
8.79 KB

Fixing #66 - giving given it is such a minor thing putting this back to rtbc

xjm’s picture

Thanks @Mile23 and @alexpott.

#20 and #65 count as subsystem maintainer signoff, I think. (We don't formally have subsystem maintainers for our Composer integration, but those are two of the people who worked a lot on it.)

I think "Needs security review" is also now covered, because all the committers who've +1ed this are on the Security Team. Many other team members (non-committers helping with comms and process on these upstream releases) have also expressed ardent support for this change.

That just leaves going over the handbook to add a section about this along the lines of what's in the CR, and to look for any references to the strict pinning that need to be updated.

catch’s picture

Made a start on updating the docs. It might be worth a dedicated paragraph about this somewhere, but wasn't sure where that should be.

https://www.drupal.org/node/2718229/revisions/view/12669926/12670510

https://www.drupal.org/node/2700999/revisions/view/12669873/12670508

  • catch committed 9db4e31 on 10.0.x
    Issue #3198340 by alexpott, xjm, Mile23, cilefen, mmjvb, catch,...
  • catch committed d894465 on 9.5.x
    Issue #3198340 by alexpott, xjm, Mile23, cilefen, mmjvb, catch,...
catch’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.0.x and cherry-picked to 9.5.x, thanks!

Leaving tagged for documentation updates since I only did the bare minimum.

mmjvb’s picture

What about backports to 9.4 and 9.3 ?

effulgentsia’s picture

This is great! I'm glad this got in. I have some policy questions now that this is in:

  1. When an upstream security patch is released, will core continue to make a new off-schedule security release that bumps the minimum up relatively soon after (though not necessarily with the same level of urgency as until now)? Or will that new release wait until the next regular security or bug-fix window? The reason I ask is with respect to Automatic Updates: currently the code in that initiative/module only updates when there's a new core version, so I'm wondering if that's still ok, or if that initiative needs to also monitor for upstream updates.
  2. If our nightly testing uncovers a regression with a new upstream update, are we thinking that we'll make an off-schedule bug-fix release that fixes that relatively soon after rather than waiting till the next bug-fix release window? That way, we minimize the time window in which a new site doing a fresh composer create-project drupal/recommended-project gets something that's broken?
  3. Currently, our DrupalCI environments doing max testing are doing a full-max that's allowed by core's composer.json. That can include minor and major upstream updates, not solely patch-level updates. Should we also add one or more additional environments to do max-testing within the constraints of core-recommended (i.e., max patch-level, but pinned minors)? So that we have something that's testing exactly what a composer create-project drupal/recommended-project would give you?
longwave’s picture

I think #73.3 is something we must do.

Given the example in the change record of composer/semver:~3.2.6 it's entirely possible that 3.2.7 could be released at the same time as, or even after, e.g. 3.3.1 or 4.0.0. Our current max testing will get the highest version only - but that may not cause a regression, but it's still possible that 3.2.7 does, and we would not catch that.

xjm’s picture

Status: Fixed » Needs work

Going to NW for them so we don't forget and have it auto-close on us.

xjm’s picture

Also this.

Mile23’s picture

#73.3:

"[Max test] can include minor and major upstream updates, not solely patch-level updates."

See #2769841: Prefer caret over tilde in composer.json where we decided that the caret was more important than this concern. We even added a test to enforce it. :-)

"Should we also add one or more additional environments to do max-testing within the constraints of core-recommended (i.e., max patch-level, but pinned minors)? So that we have something that's testing exactly what a composer create-project drupal/recommended-project would give you?"

Given that we now have loose constraints on drupal/core-recommended, we can't say we know what create-project will give any arbitrary user, regardless of whether we test it or not.

Running a max test on drupal/core-recommended will only tell you the same information you'd get from a max test on drupal/core.

Running a normal test on drupal/core-recommended will only tell you the same information you'd get from any other DrupalCI run.

The only way to be sure would be to have a process that finds all the in-between releases and runs a separate test on that matrix of differences for all dependencies, run against drupal/core.

Is that something we want to build? Or is once a day good enough?

See for instance: #2874198: Create and run dependency regression tests for core

mfb’s picture

Running a max test on drupal/core-recommended will only tell you the same information you'd get from a max test on drupal/core.

I don't understand why.. :) I was thinking that max test on drupal/core gives you psr/log "^1.1", i.e. the latest 1.x version (which could be e.g. 1.2.0), while max test on drupal/core-recommended gives you psr/log "~1.1.4", i.e. the latest 1.1.x version (which could be e.g. 1.1.5 but not 1.2.0).

alexpott’s picture

I think we should consider doing this for 9.4.x too because having this in alpha / beta / rc is not really that important since people don't tend to move use core-recommended and unstable versions. Here's a patch for 9.4.x

catch’s picture

Version: 9.5.x-dev » 9.4.x-dev
Status: Needs work » Needs review
Issue tags: +Needs release manager review

Moving to 9.4.x for discussion and tagging with release manager review. Would have been better a month ago, but I think it's better late in 9.4.x than never, it'll make the next six months a lot less painful and it's quite low risk overall.

longwave’s picture

Note that doing this in 9.4.x would already bump doctrine/reflection in core-recommended, although I believe the risk there is almost non-existent seeing as we have deprecated use of it:

$ composer outdated -p
Info from https://repo.packagist.org: #StandWithUkraine
Color legend:
- patch or minor release available - update recommended
- major release available - update possible
composer/composer        2.2.13  2.2.14  Composer helps you declare, manage and install dependencies of PHP projects. It ensures you have the right stack everywhere.
doctrine/reflection      1.2.2   1.2.3   The Doctrine Reflection project is a simple library used by the various Doctrine projects which adds some additional functionality on top of the reflection functional...

(composer/composer is a dev dependency only)

mmjvb’s picture

On an rc2 of 9.4 started 11 jun:

docker@cli:/var/www$ composer outdated --patch-only

Color legend:
- patch or minor release available - update recommended
- major release available - update possible
doctrine/reflection 1.2.2   1.2.3   The Doctrine Reflection project is a simple library used...
Package doctrine/reflection is abandoned, you should avoid using it. Use roave/better-reflection instead.
psr/container       1.1.1   1.1.2   Common Container Interface (PHP FIG PSR-11)
symfony/debug       v4.4.41 v4.4.41 Provides tools to ease debugging PHP code
Package symfony/debug is abandoned, you should avoid using it. Use symfony/error-handler instead.
docker@cli:/var/www$

Including new minors

docker@cli:/var/www$ composer outdated -m
Color legend:
- patch or minor release available - update recommended
- major release available - update possible
doctrine/reflection              1.2.2   1.2.3   The Doctrine Reflection project is a simple...
Package doctrine/reflection is abandoned, you should avoid using it. Use roave/better-reflection instead.
laminas/laminas-escaper          2.9.0   2.10.0  Securely and safely escape HTML, HTML attri...
laminas/laminas-stdlib           3.7.1   3.10.1  SPL extensions, array utilities, error hand...
psr/container                    1.1.1   1.1.2   Common Container Interface (PHP FIG PSR-11)
symfony/debug                    v4.4.41 v4.4.41 Provides tools to ease debugging PHP code
Package symfony/debug is abandoned, you should avoid using it. Use symfony/error-handler instead.
symfony/polyfill-ctype           v1.25.0 v1.26.0 Symfony polyfill for ctype functions
symfony/polyfill-iconv           v1.25.0 v1.26.0 Symfony polyfill for the Iconv extension
symfony/polyfill-intl-idn        v1.25.0 v1.26.0 Symfony polyfill for intl's idn_to_ascii an...
symfony/polyfill-intl-normalizer v1.25.0 v1.26.0 Symfony polyfill for intl's Normalizer clas...
symfony/polyfill-mbstring        v1.25.0 v1.26.0 Symfony polyfill for the Mbstring extension
symfony/polyfill-php80           v1.25.0 v1.26.0 Symfony polyfill backporting some PHP 8.0+ ...
docker@cli:/var/www$
mmjvb’s picture

On 9.3.16 created just now:

docker@cli:/var/www/d93$ composer outdated --patch-only
Color legend:
- patch or minor release available - update recommended
- major release available - update possible
composer/semver                    3.2.6    3.2.9    Semver library that offers utilities, v...
doctrine/lexer                     1.2.1    1.2.3    PHP Doctrine Lexer parser library that ...
doctrine/reflection                1.2.2    1.2.3    The Doctrine Reflection project is a si...
Package doctrine/reflection is abandoned, you should avoid using it. Use roave/better-reflection instead.
laminas/laminas-stdlib             3.6.1    3.6.4    SPL extensions, array utilities, error ...
psr/container                      1.1.1    1.1.2    Common Container Interface (PHP FIG PSR...
symfony/console                    v4.4.34  v4.4.42  Eases the creation of beautiful and tes...
symfony/debug                      v4.4.31  v4.4.41  Provides tools to ease debugging PHP code
Package symfony/debug is abandoned, you should avoid using it. Use symfony/error-handler instead.
symfony/dependency-injection       v4.4.34  v4.4.42  Allows you to standardize and centraliz...
symfony/deprecation-contracts      v2.5.0   v2.5.1   A generic function and convention to tr...
symfony/error-handler              v4.4.34  v4.4.41  Provides tools to manage errors and eas...
symfony/event-dispatcher           v4.4.34  v4.4.42  Provides tools that allow your applicat...
symfony/event-dispatcher-contracts v1.1.11  v1.1.12  Generic abstractions related to dispatc...
symfony/http-client-contracts      v2.5.0   v2.5.1   Generic abstractions related to HTTP cl...
symfony/http-foundation            v4.4.34  v4.4.42  Defines an object-oriented layer for th...
symfony/http-kernel                v4.4.35  v4.4.42  Provides a structured process for conve...
symfony/mime                       v5.4.0   v5.4.9   Allows manipulating MIME messages
symfony/process                    v4.4.35  v4.4.41  Executes commands in sub-processes
symfony/routing                    v4.4.34  v4.4.41  Maps an HTTP request to a set of config...
symfony/serializer                 v4.4.35  v4.4.42  Handles serializing and deserializing d...
symfony/service-contracts          v2.5.0   v2.5.1   Generic abstractions related to writing...
symfony/translation                v4.4.34  v4.4.41  Provides tools to internationalize your...
symfony/translation-contracts      v2.5.0   v2.5.1   Generic abstractions related to transla...
symfony/validator                  v4.4.35  v4.4.41  Provides tools to validate values
symfony/var-dumper                 v5.4.0   v5.4.9   Provides mechanisms for walking through...
symfony/yaml                       v4.4.34  v4.4.37  Loads and dumps YAML files
twig/twig                          v2.14.11 v2.14.13 Twig, the flexible, fast, and secure te...
docker@cli:/var/www/d93$

Including new minors

docker@cli:/var/www/d93$ composer outdated -m
Color legend:
- patch or minor release available - update recommended
- major release available - update possible
composer/semver                    3.2.6    3.3.2   Semver library that offers utilities, ve...
doctrine/lexer                     1.2.1    1.2.3   PHP Doctrine Lexer parser library that c...
doctrine/reflection                1.2.2    1.2.3   The Doctrine Reflection project is a sim...
Package doctrine/reflection is abandoned, you should avoid using it. Use roave/better-reflection instead.
egulias/email-validator            3.1.2    3.2     A library for validating emails against ...
laminas/laminas-diactoros          2.8.0    2.11.0  PSR HTTP Message implementations
laminas/laminas-escaper            2.9.0    2.10.0  Securely and safely escape HTML, HTML at...
laminas/laminas-feed               2.15.0   2.17.0  provides functionality for consuming RSS...
laminas/laminas-stdlib             3.6.1    3.10.1  SPL extensions, array utilities, error h...
psr/container                      1.1.1    1.1.2   Common Container Interface (PHP FIG PSR-11)
symfony/console                    v4.4.34  v4.4.42 Eases the creation of beautiful and test...
symfony/debug                      v4.4.31  v4.4.41 Provides tools to ease debugging PHP code
Package symfony/debug is abandoned, you should avoid using it. Use symfony/error-handler instead.
symfony/dependency-injection       v4.4.34  v4.4.42 Allows you to standardize and centralize...
symfony/deprecation-contracts      v2.5.0   v2.5.1  A generic function and convention to tri...
symfony/error-handler              v4.4.34  v4.4.41 Provides tools to manage errors and ease...
symfony/event-dispatcher           v4.4.34  v4.4.42 Provides tools that allow your applicati...
symfony/event-dispatcher-contracts v1.1.11  v1.1.12 Generic abstractions related to dispatch...
symfony/http-client-contracts      v2.5.0   v2.5.1  Generic abstractions related to HTTP cli...
symfony/http-foundation            v4.4.34  v4.4.42 Defines an object-oriented layer for the...
symfony/http-kernel                v4.4.35  v4.4.42 Provides a structured process for conver...
symfony/mime                       v5.4.0   v5.4.9  Allows manipulating MIME messages
symfony/polyfill-ctype             v1.23.0  v1.26.0 Symfony polyfill for ctype functions
symfony/polyfill-iconv             v1.23.0  v1.26.0 Symfony polyfill for the Iconv extension
symfony/polyfill-intl-idn          v1.23.0  v1.26.0 Symfony polyfill for intl's idn_to_ascii...
symfony/polyfill-intl-normalizer   v1.23.0  v1.26.0 Symfony polyfill for intl's Normalizer c...
symfony/polyfill-mbstring          v1.23.1  v1.26.0 Symfony polyfill for the Mbstring extension
symfony/polyfill-php80             v1.23.1  v1.26.0 Symfony polyfill backporting some PHP 8....
symfony/process                    v4.4.35  v4.4.41 Executes commands in sub-processes
symfony/routing                    v4.4.34  v4.4.41 Maps an HTTP request to a set of configu...
symfony/serializer                 v4.4.35  v4.4.42 Handles serializing and deserializing da...
symfony/service-contracts          v2.5.0   v2.5.1  Generic abstractions related to writing ...
symfony/translation                v4.4.34  v4.4.41 Provides tools to internationalize your ...
symfony/translation-contracts      v2.5.0   v2.5.1  Generic abstractions related to translation
symfony/validator                  v4.4.35  v4.4.41 Provides tools to validate values
symfony/var-dumper                 v5.4.0   v5.4.9  Provides mechanisms for walking through ...
symfony/yaml                       v4.4.34  v4.4.37 Loads and dumps YAML files
twig/twig                          v2.14.11 v2.15.1 Twig, the flexible, fast, and secure tem...
docker@cli:/var/www/d93$
longwave’s picture

Oh, psr/container is interesting.

We will always lock to ~1.1.1 because that is the last version that supports PHP 7.3. But in practice most people are on PHP 7.4 or above, so they get 1.1.2 instead.

I think it is too late to make this change in a patch release of 9.3, this can only be in 9.4.0

alexpott’s picture

I agree with @longwave - I don't think that we should be changing this in a patch release. I've opened #3285572: Update dependencies to latest patch releases for 9.4.x / 9.5.x to do the patch level updates to 9.4.x. This will need a reroll when that one goes in.

DamienMcKenna’s picture

FWIW +1 to getting this into 9.4.

xjm’s picture

Re: #73.1, I think whether or not we release an off-schedule patch update to bump constraints, and the timeline on which we do that, will depend on the specific issue and the risk to contrib. All off-schedule patch releases, including for non-core-exploitable security updates, are done on a best-effort basis at the release managers' discretion.

Similarly, for #73.2, if there's an upstream regression, whether or not we issue an off-schedule patch release for it will depend on the nature and severity of the regression, and is at the release managers' discretion.

Re: having more test environments, the current number of environments was already negotiated with the DA, and the DA expressed that they need limitations along current lines on how many variants there are and how often they are run. If we add too many different testing environments, it creates a lot of overhead for the DA to maintain them, it adds significantly to the cost of core testing, and it prompts contrib to spend all the DA's money by configuring massive numbers of testing environments for projects that don't have core's need for them. So I don't think #73.3 is an option.

In cases where a dependency is releasing parallel updates to multiple minor version at the same time (e.g., Symfony 6.1.0 and 6.0.9 were released together), the chances of 6.0.9 breaking a hypothetical site where that was allowed, but 6.1.0 not breaking sites also, are nonzero but are not nearly as high as a new minor version breaking sites. Part of the original scope here is that the release managers consider it acceptable risk that that might happen. If it does, we'll get a critical issue about it and decide how to handle it at the time. It's not part of our compromise with the DA to provide every-version testing, nor even every-minor-version testing.

I'm hypothetically OK with backporting this to 9.4 before 9.4.0 tomorrow -- I half-wanted to do that anyway -- but I don't think we should backport it if we can't sort it before 9.4.0 tomorrow. I need to look more closely at the version updates part of it.

xjm’s picture

Status: Needs review » Needs work

So I think if we do backport this to 9.4, we need not only a release note but also a core blog post or possibly a PSA about the change. I'm all for it if we can get that done, but 9.4.0 is supposed to be tagged tomorrow so the timeline is kind of short.

Meanwhile, the patch-version updates are in, so the 9.4.x patch would need a reroll.

xjm’s picture

Oh it's definitely not allowed in 9.3; that branch is security-only now.

alexpott’s picture

Status: Needs work » Needs review
FileSize
280 bytes
8.79 KB

Here's a reroll now that the dependency updates have landed.

alexpott’s picture

I think we could use the change record as the basis for PSA, release note whatever is required. Here's a suggestion. Edited to add section based on text from @xjm about converting tarballs.

Patch-level updates for drupal/core-recommended

The drupal/core-recommended metapackage now allows patch-level updates for dependencies. This means that site owners using drupal/core-recommended can now install most Composer dependency security updates themselves, without needing to wait for an upstream release of Drupal core that updates the affected package.

Beginning in Drupal 9.4, the core-recommended package's dependency constraints are set to a patch version with a ~. For example:

        "composer/semver": "~3.2.6",

This constraint will allow the package to be updated from 3.2.6 to 3.2.7 or 3.2.8 (etc.), but not to 3.3.0 or higher. To update the dependency, sites using core-recommended will simply need to run:

composer update composer/semver

Impact on sites built with tarballs

Drupal 9.4+ sites built with tarballs will no longer be receiving the same level of security support for core dependencies. They should convert to using drupal/core-recommended in order to be able to apply such security updates.

Handling conflicts with upstream updates

Rarely, an upstream Composer dependency's patch-level update may introduce a regression. When this happens, Drupal core will usually declare a conflict with the affected package version; however, these changes might not be immediately available in a core release. Site owners should test their updates before deploying.

If a dependency update does cause a regression, site owners can also add a top-level requirement for the known-good version. For example, if a site encountered regressions with composer/semver 3.2.8, but 3.2.7 worked appropriately, the site owner can run:

composer require composer/semver 3.2.7

Then, if a later update in 3.2.9 resolved the issue, the site could run:

composer remove --no-update composer/semver
composer update composer/semver
xjm’s picture

Issue summary: View changes

Moving the PSA text draft from #91 to the IS for editing.

catch’s picture

Proposed PSA looks good to me, and I'm +1 to this going into 9.4.x for all the reasons given. We might end up with regressions from upstream patch releases we need to deal with, but these are nicer problems to have than our current ones hopefully.

xjm’s picture

Issue summary: View changes

Upon reflection, I think the audience for a PSA is different than the audience for a CR, so I brought it up a level in a separate PSA draft: https://docs.google.com/document/d/1Ht_ymWXGqXxS_V9wboyGQZ4P8dY9_sAPmIAn...

It still links the change record for the implementation details and updated dependency management instructions.

xjm’s picture

#93 and #88 are release management signoff for this change. If for some reason the security time declines to issue a PSA tomorrow we would need to revert the change. But for now let's assume we are going forward with it since it is better to have this in before 9.4.0 rather than after.

xjm’s picture

I just noticed that the email validator package has a minor version contstraint with no patch version specified:
egulias/email-validator ~3.2

This is not an issue with our constraints -- for whatever reason, they recently released a new minor as "3.2" instead of "3.2.0", despite using semver previously:
https://github.com/egulias/EmailValidator/tags

Regardless, I believe the tilde constraint would still allow an update to 3.3.0+ when such is released.

I'm not sure what, if anything, we can do about that.

cilefen’s picture

catch’s picture

Status: Needs review » Reviewed & tested by the community

Let's do the 9.4.x backport and try to deal with anything that comes up (including e-mail validator if that does turn out to cause issues, but seems like it will be fine).

xjm’s picture

So #97 is in the docs, but the docs for tilde say:

The ~ operator is best explained by example: ~1.2 is equivalent to >=1.2 <2.0.0, while ~1.2.3 is equivalent to >=1.2.3 <1.3.0. As you can see it is mostly useful for projects respecting semantic versioning. A common usage would be to mark the minimum minor version you depend on, like ~1.2 (which allows anything up to, but not including, 2.0).

Tilde Version Range

I'm not sure which takes precedent.

We could manually test this, but it would require faking stuff locally with a path repository that had a tag named 3.1.0 and also 3.2 and 3.3.0, and see whether ~3.2 allows an update to 3.3.0. We could also accept this as a minor risk we'll put up with, add a note about it to the CR and release note, and backport it anyway, since we can't do anything but file an upstream issue regardless.

cilefen’s picture

A local test shows that ~3.2 will upgrade to 3.3.0.

xjm’s picture

Issue summary: View changes

Amending the release note with a suggestion from @catch that should cover this for the release notes. We agreed that it's not ideal but the alternative of being stuck with exact versions is still worse.

  • xjm committed 99d3dd4 on 9.4.x
    Issue #3198340 by alexpott, xjm, cilefen, Mile23, mmjvb, catch, longwave...
xjm’s picture

Status: Reviewed & tested by the community » Needs work

OK, backported to 9.4, and adjusted the CR to reflect this.

Thanks everyone!

Still leaving open for the hypothetical documentation update.

xjm’s picture

Also tagging for the release highlights as this represents a site maintainability improvement for many users.

longwave’s picture

I think we could deal with #96-101 in a followup, by ensuring anything locked to x.y is set in core-recommended as ~x.y.0.

Composer should understand that ~3.2.0 still allows 3.2: https://semver.madewithlove.com/?package=egulias%2Femail-validator&const...

xjm’s picture

Issue tags: +Needs followup
mmjvb’s picture

Confirming that ~3.2.0 allows for 3.2. Composer uses internally 3.2.0.0.

docker@cli:/var/www$ composer show egulias/email-validator -a 2>&1 |grep versions
versions : dev-master, 3.x-dev, * 3.2, 3.1.2, 3.1.1, 3.1.0, 3.0.1, 3.0.0, 2.1.x-dev, 2.1.25, 2.1.24, 2.1.23, 2.1.22, 2.1.21, 2.1.20, 2.1.19, 2.1.18, 2.1.17, 2.1.16, 2.1.15, 2.1.14, 2.1.13, 2.1.12, 2.1.11, 2.1.10, 2.1.9, 2.1.8, 2.1.7, 2.1.6, 2.1.5, 2.1.4, 2.1.3, 2.1.2, 2.1.1, 2.1.0, 2.0.1, 2.0.0, 1.2.x-dev, 1.2.17, 1.2.16, 1.2.15, 1.2.14, 1.2.13, 1.2.12, 1.2.11, 1.2.10, 1.2.9, 1.2.8, 1.2.7, 1.2.6, 1.2.5, 1.2.4, 1.2.3, 1.2.2, 1.2.1, 1.2.0, 1.1.1, 1.1.0, 1.0.0, dev-dependabot/composer/guzzlehttp/guzzle-7.4.4, dev-bug-321, dev-message-id-validator
docker@cli:/var/www$ composer show egulias/email-validator -a ~3.2 2>&1 |grep versions
versions : 3.x-dev, * 3.2
docker@cli:/var/www$ composer show egulias/email-validator -a ~3.2.0 2>&1 |grep versions
versions : * 3.2
docker@cli:/var/www$ composer show egulias/email-validator -a ~3.1.0 2>&1 |grep versions
versions : 3.1.2, 3.1.1, 3.1.0
docker@cli:/var/www$
longwave’s picture

heddn’s picture

I've opened up #3291780: guzzlehttp/guzzle 7.4.5 requires guzzlehttp/psr7 ^2.3 which breaks HEAD just now. I understand the intention is specifically patch level. But I was hoping that this would mean I could update to Drupal 9.4.0 and get a patched version of guzzle. In the case mentioned in that issue, I still can't because guzzle bumped their transitive dependency by a minor version.

xeM8VfDh’s picture

I ran into the same issue @heddn, but I believe this will resolve it (though there is a core update now to resolve it)

composer update guzzlehttp/guzzle guzzlehttp/psr7

mmjvb’s picture

Indeed:

docker@cli:/var/www$ composer show drupal/core|grep versions
versions : * 9.4.1
docker@cli:/var/www$ composer show |grep guzzle
guzzlehttp/guzzle                              6.5.8      Guzzle is a PHP HTTP cl...
guzzlehttp/promises                            1.5.1      Guzzle promises library
guzzlehttp/psr7                                1.9.0      PSR-7 message implement...
docker@cli:/var/www$ composer update guzzlehttp/*
Loading composer repositories with package information
Info from https://repo.packagist.org: #StandWithUkraine
Updating dependencies
Nothing to modify in lock file
Installing dependencies from lock file (including require-dev)
Nothing to install, update or remove
Package doctrine/reflection is abandoned, you should avoid using it. Use roave/better-reflection instead.
Package symfony/debug is abandoned, you should avoid using it. Use symfony/error-handler instead.
Generating autoload files
87 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
docker@cli:/var/www$
docker@cli:/var/www$ composer outdated guzzlehttp/*
Color legend:
- patch or minor release available - update recommended
- major release available - update possible
guzzlehttp/guzzle 6.5.8 7.4.5 Guzzle is a PHP HTTP client library
guzzlehttp/psr7   1.9.0 2.4.0 PSR-7 message implementation that also provides common...
docker@cli:/var/www$ composer outdated guzzlehttp/* -p
docker@cli:/var/www$ composer outdated guzzlehttp/* -m
docker@cli:/var/www$
xjm’s picture

Status: Needs work » Fixed

Marking back to fixed since the followup was filed.

Status: Fixed » Closed (fixed)

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