ScaffoldFilePermissionsValidator should use ComposerInspector instead of ComposerUtility

I think this one will need input from 📌 Add new InstalledPackagesList which does not rely on Composer API to get package info Fixed , as we need list of installed packages.

@travis.carden the proposed change in question

So @ted.bow if it was okay to call composerInspector->getConfig() while passing a working directory for the core's composer json to fetch the extra config ?

This now blocks 📌 Do not allow drupal/core-composer-scaffold to be used by packages other than core Fixed .

@ted.bow could you leave a comment here about the discussion you had with @travis.carden.

📌 Add new InstalledPackagesList which does not rely on Composer API to get package info Fixed landed, this should now be able to move forward.

While we should still get that input from @tedbow, meanwhile this MR can be rebased on top of 📌 Add new InstalledPackagesList which does not rely on Composer API to get package info Fixed ! 👍

@omkar.podey add an issue summary here? thanks

Regarding #6, I discussed this with @tedbow.

I did a little experimentation to see if a Composer plugin could alter the file mapping defined by drupal/core, that gets written to installed.json and composer.lock.

The answer is yes, it could be done, if the plugin did something similar to what cweagans/composer-patches used to do. I don't, however, think that's especially likely to happen.

The real problem is that we do not properly account for the overridability of scaffold file mappings supported by the scaffold plugin, and we never have. I've opened a post-MVP issue to deal with that ✨ When determining scaffold file mappings, determine them consistently and completely, accounting for overrides Active .

For that reason, @tedbow and I agreed that it's okay for us to change the validator to call $composerInspector->getConfig('extra.drupal-scaffold.file-mapping', $installed_packages['drupal/core']->path). Doing that will get the file mapping defined by core, which is not guaranteed to be complete or accurate for the actual project -- but it also doesn't make the current situation any worse. And it helps get us off of ComposerUtility.

Yeah, that sucks...composer.lock isn't there in core's directory (and rightfully so), and therefore ComposerInspector::validate() throws an exception when it tries to validate that. The only real way out of this pickle is for us to stop requiring the presence of composer.lock, but that means we can't get the types of installed packages until Composer merges and tags https://github.com/composer/composer/pull/11340. Maddening.

Here's what I would suggest: in another issue, remove the requirement for composer.lock to exist, and use library as the default package type when creating InstalledPackage objects. getInstalledPackagesList() should try to load package types from composer.lock, but if composer.lock doesn't exist, we should just let the package type default to library. (We can get away with this since InstalledPackage's constructor is private.)

Then, when we require a Composer version that provides the type key, we can change InstalledPackage to require a type key.

Traced into some of the failures and the reason why it's going to keep failing is because the changed scaffold file mapping has to be defined in path/to/stage/dir/vendor/drupal/core/composer.json. Not installed.json, which is where ComposerUtility would read it from.

Hopefully the fixture manipulator is smart enough to do that...

Okay so it isn't possible to manipulate composer.json's of packages right now, But i got to discuss it with @Wim.leers and we decided to update \Drupal\fixture_manipulator\FixtureManipulator::modifyPackage to actually manipulate the composer.json of the package.

I'll make a new method for it, for now , referencing \Drupal\fixture_manipulator\FixtureManipulator::addConfig as it also manipulates composer.json

Discussed in detail during meeting just now. Conclusion: this is hard-blocked on direction from @tedbow in 📌 Update FixtureManipulator to work with InstalledPackagesList, real composer show command Fixed to avoid wasted work.

📌 Update FixtureManipulator to work with InstalledPackagesList, real composer show command Fixed landed, this is unblocked!

Wim Leers → closed merge request !711

This patch will pass once 📌 Remove FixtureManipulator::modifyPackage() last usage Fixed lands.

The last submitted patch, 22: 3342227-21.patch, failed testing. View results →

+++ b/src/Validator/ScaffoldFilePermissionsValidator.php
@@ -27,13 +27,17 @@ final class ScaffoldFilePermissionsValidator implements EventSubscriberInterface
-   * Constructs a SiteDirectoryPermissionsValidator object.
+   * Constructs a ScaffoldFilePermissionsValidator object.

😅

I have no objections to this code. Committing when tests pass.

The last submitted patch, 22: 3342227-21.patch, failed testing. View results →

No it didn't. We're just positively plagued by these pestilential random failures.

And, committed!

Only one (or two) things left to do, and ComposerUtility is gone.

That means 📌 Do not allow drupal/core-composer-scaffold to be used by packages other than core Fixed is now unblocked! 👍

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