Remove the ability to configure the path to Composer

Two questions about this.

1. The MR currently completely removes the configuration for composer, as an experimental module that's fine as far as core's concerned.

However package_manager is already used in production by Drupal CMS sites (or other sites using project browser/automatic updates) and those sites may be relying on having configured the composer path. Also depending on the host and who installed the site, running composer require composer/composer from the command line may not be an easy option.

So I'm wondering if this needs a two step process.

1. Keep the configured path as a final fallback if local and PATH composer can't be found. When this is the case, we could have a hook_requirements() warning telling people they need to fix the problem, but it wouldn't immediately break.

2. Later on (maybe for 12.0.0) remove the configuration entirely as the MR currently does.

2. It might be possible for automatic updates or project browser to add an update that detects whether composer is found locally or in PATH, and if it's not, runs composer require composer/composer via package manager to install a local composer. Once the local composer is installed, the site would then no longer be using the configuration any more and it would be safe to remove. If automatic updates/project browser introduces that update path before Drupal 12 support is added, it would then more or less guarantee that sites continue to work once the configuration is removed.

The above isn't necessary for core, but I think it probably needs discussion with the Drupal CMS team whether that's something that's worth attempting.

I was thinking along similar lines. It probably makes sense to remove the executables structure from config schema, so that it can stay in existing sites but is no longer considered "valid" config.

Meanwhile, the executable finder can use it as a final fallback, and trigger a deprecation warning. We can also do a separate requirements error (or warning, I guess) that having a configured path to Composer is deprecated and will not be supported in Drupal 12.

catch → credited benjifisher → .

Crediting @benjifisher for slack discussion related to this.

Reading summary and MR, I wonder if a setting fallback (instead of having to install it via composer or adjust the PATH) for finding composer would also make sense. Seems easier to migrate from the config to a setting than the other choices. I don’t see how it’s worse from a security standpoint. We already have to trust settings isn’t writable by malicious actors.

I agree with @dww and have added the settings-based fallback.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Read it over. Looks good to me! I like that we're getting the Composer binary path from within it's vendor directory versus vendor/bin/composer.

Sorry, only on my phone. Hard to closely review. But found a few concerns to start.

I have added one minor comment about the "read-only" vs "non-executable" wording.

Fixed both suggestions, which are minor enough that I think we can go directly back to RTBC here.

This removes the rsync and composer config from the schema without removing the config keys themselves - this is to support the hook_requirements() pointing to settings.php.

However it's not clear to me how someone is supposed to actually remove those config keys from a site?

We might need to add an upgrade path and skip the hook_requirements() - there should be one for when composer/rsync can't be found anyway.

Or if we otherwise want to keep that, how do people remove this from config?

Package manager isn't beta yet so manual steps are OK but a lot of sites have it installed via Drupal CMS now and we shouldn't cause them to have data integrity issues without an obvious way out.

The idea -- and I probably should have documented this in the issue summary, or open a postponed follow-up -- is that in Drupal 12, we'll have an update path to remove those keys.

So we commit this now, and give people a nice, long runway to make the requisite changes. The change record documents how to do it. But if people don't remove the keys, they will still only get requirements warnings, not errors, and if they're okay with that, then the keys will automatically go away when they update to Drupal 12.

Does that seem reasonable? Tentatively restoring RTBC here since I think that's what we'd kinda-sorta agreed on in Slack discussions, but by all means kick this back if I'm jumping the gun here. (Or, if you want me to open a follow-up for the upgrade path that removes the keys.)

I think a follow-up is good, but yeah let's open it - 12.x will be open soon.

Opened 📌 [PP-1] Remove the `package_manager.settings:executables` config structure Postponed .

catch → closed merge request !12932

Committed/pushed to 11.x and cherry-pick (with a small amount of manual merge conflict resolution) to 11.2.x, thanks!

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

Filed 🐛 Use #[IgnoreDeprecations] instead of #[Group('legacy')] - again Active for follow up