Add enviroment variable to enable extra composer plugins

What about if we set allow_plugins to true for everyone so this becomes a non-issue? Thats not desireable in general but in CI we already run untrusted code. Our environments are isolated from anything important.

FYI I've left before_script unused so that folks can add steps without overwriting script.

What about if we set allow_plugins to true for everyone so this becomes a non-issue? Thats not desireable in general but in CI we already run untrusted code.

I'm 50/50 on that, I won't fight against it, however I'm not sure I would advocate it either..

On one hand it seems reasonable to make the CI system easier. On the other hand, there is something to be said for making sure module developers understand that in the real world that isn't the case.

I've ran across my fair share of modules that expect the composer.json to be honored for patches and other aspects when the plugins explicitly document they will not do so. The CI acts as a good 'last check' on the developer to say "oh I should go document this in the README while I instruct the CI system to honor that change"

FYI I've left before_script unused so that folks can add steps without overwriting script.

At first from my drupal spoons experience I wasn't sure this was going to work, however it appears on visual review our newer templates would handle this just fine This seems like a reasonable method and I no longer see a need to add a variable as this issue requests.

Think we can close this out though I'll leave it open for a couple days for any other opinions or response feedback.

+1 to just allowing all plugins. See #3334914: Testing is broken because simplesamlphp/composer-module-installer contains a Composer plugin which is blocked → for the corresponding drupalci issue, that had to be reverted due to old php/composer versions, I think that's not an issue here?

> On one hand it seems reasonable to make the CI system easier. On the other hand, there is something to be said for making sure module developers understand that in the real world that isn't the case.

plugins are a standard composer system and compose will ask if you want to allow that plugin, that is not possible on CI, and as #1 said, there is no security concern here.

It is possible to customize this process, but you need to duplicate the whole composer base job to add something between the php script and composer install, which means that future changes will not be applied and your configuration might break.

I opened a merge request, will try if that works in https://git.drupalcode.org/project/simplesamlphp_auth/-/merge_requests/26.

Yes, seems to work fine, https://git.drupalcode.org/project/simplesamlphp_auth/-/jobs/140243 worked now. I didn't test any other PHP configurations or so though.

I had forgotten I had opened this issue.

I believe this issue might actually be outdated now with changes that were done with the templates after this issue was opened (or I had not noticed at the time this was possible.)

A module can add an additional plugin through its composer.json without any template changes

Example:

    "config": {
        "allow-plugins": {
            "infection/extension-installer": true
        }

Sample:
https://git.drupalcode.org/project/s3fs/-/merge_requests/35/diffs#3957b9...

The only question left open would be if we want to just enable all plugins, however since it is easy to add additional plugins to the composer.json I'm not sure there is a real benefit to do so, and worry it might just create more issues long term (such as needing to disable plugins, or development plugins unexpectedly changing the CI environment.)

@Berdir Can you run the test against PHP 7.4? That was the image that broke with "allow-plugins" => TRUE see #3381247: Composer config failure when running functional tests →

I think the issue might still be present since we use the same physical images as DrupalCi, and this would be added before a composer update could run so I suspect we are still on an older version of composer in GitlabCi.

Yeah, I see, the merge of the composer.json of the project does that. Which I'd say is actually kind of funny as it IMHO goes entirely against your argument in #3:

> On the other hand, there is something to be said for making sure module developers understand that in the real world that isn't the case.

In the real world, you can't put arbitrary root-level-only stuff in your composer.json and expect that that to work. Seems weird to me to have that in there just for CI.

The risks to me seem minimal, not sure why you'd want to disable plugins and you can't have "optional" plugins, any unknown plugin is a hard fail right now on CI. And if you really want to do something unusual, you could still use this trick in the opposite direction and change the allow plugin configuration to whatever you want?

One problem is that merging this *will* break your module, as it would then only have that and no longer be a recursive merge, but I'd assume there are very few cases doing that atm?

That was a crosspost, yeah, looks like 7.4 is still broken, that's unfortunate.

The suggested approach seems to be working. Per #10, that does seem stranger to me than just allowing all plugins, I guess we'd need to add steps too upgrade composer within the script if the version is below what we need, but I'm unblocked.

If we decide to close this as a won't fix we probably should ensure that behavior is well documented, because I wouldn't have thought that this is possible.

As this is something that would need to be on the parent project, I agree with you that it doesn't make sense to have it in the "composer.json" of the module as the settings will be ignored in any non-our-drupal-gitlab-ci environments. Having said that, it's an easy workaround.

Workaround 2 could be to extend the "composer" step. We've left empty the "before_script" deliberately so people can easily extend it without copy/pasting the whole "composer" step. So you could do:

composer:
  before_script:
    - composer config allow-plugins.infection/extension-installer true

Given that we have two relatively easy workarounds, I don't really think that we need to change anything in the project. I'd probably close this issue, but happy to listen to arguments for not doing it.

We could extend the documentation page with another use case of overriding tasks. For example, in this section https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... →

Closing the issue as mentioned in the earlier comment. Please reopen if you think we need to do something and why.

This issue has been "closed (works as designed)" so @berdir would you be able to close MR54 that you opened, please? I'm tidying up the Merge Request queue and trying to close the ones that are done, so we can more easily see the active issues.

berdir → closed merge request !54

Thank you @berdir