Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Composer patches should exit on failure when running composer commands directly #2316

Closed
danepowell opened this issue Nov 29, 2017 · 4 comments
Labels
Enhancement A feature or feature request
Milestone

Comments

@danepowell
Copy link
Contributor

danepowell commented Nov 29, 2017

Currently when BLT runs composer install as part of the build step, it sets the env var COMPOSER_EXIT_ON_PATCH_FAILURE=1, which forces composer patches to abort the install on failure. This is awesome, but it leaves people in the lurch when running composer install outside of BLT. If composer-patches fails in this context, it will throw an error but not so obviously as aborting the install.

After the next release of composer-patches, we can instead add a key to composer.json:
cweagans/composer-patches#166

Things to consider:

  • Do we need to update existing projects to use this key
  • If BLT simply includes the key in its own composer files, will it be inherited by downstream projects
  • We'll want to verify that this key is present, presumably as part of blt doctor and/or the composer validation step
  • Consider whether it makes sense to remove the env var from the build step... it should be unnecessary, but there's not much harm in leaving it, and it might be a good "belt and suspenders" safety.
@danepowell danepowell added this to the 9.1.0 milestone Nov 29, 2017
@danepowell danepowell added the Enhancement A feature or feature request label Nov 29, 2017
@danepowell
Copy link
Contributor Author

Looks like we already tried to fix this for deployments in #705

@grasmash grasmash changed the title Composer patches should exit on failure even outside of BLT Composer patches should exit on failure when running composer commands directly Dec 8, 2017
@grasmash
Copy link
Contributor

grasmash commented Dec 8, 2017

Consider whether it makes sense to remove the env var from the build step

Let's leave it in. As you said, it doesn't hurt.

If BLT simply includes the key in its own composer files, will it be inherited by downstream projects

The key is already included in composer.required.json. It should be inherited. It just wasn't doing anything because cweagans/composer-patches#166 was unmerged. Now that we require version 1.6.4, this should begin to work.

We'll want to verify that this key is present, presumably as part of blt doctor and/or the composer validation step

This should not be necessary since the configuration is inherited.

The remaining work for this issue should be:

  • test that the expected inheritance works.

@grasmash
Copy link
Contributor

grasmash commented Dec 8, 2017

I have confirmed that inheritance works as expected.

@grasmash grasmash closed this as completed Dec 8, 2017
@loopy3025
Copy link

loopy3025 commented Sep 13, 2019

I am trying to incrementally upgrad BLT from 8 to 9 then 10. On the 8-9 step, It's trying to install /files/issues/2788777-3-146.patch (Allow a site-specific profile to be installed from existing config)

Which doesn't apply to the version of Drupal it's trying to install (8.7.7). Ostensibly, BLT 9.2.8 is supposed to be compatible with drupal core ^8.7.x

So, I added this to my composer file:


        "composer-exit-on-patch-failure": false,
        "patches-ignore": {
            "drupal/core": {
                "Allow a site-specific profile to be installed from existing config": "https://www.drupal.org/files/issues/2788777-3-146.patch"
            }

Which does work when building locally, but when I try to push it through pipelines, it throws an error. I tried adding the variable declaration to my acquia-pipelines.yml file, but that seems not to have worked either.

variables:
  global:
    COMPOSER_BIN: $SOURCE_DIR/vendor/bin
    BLT_DIR: $SOURCE_DIR/vendor/acquia/blt
    COMPOSER_EXIT_ON_PATCH_FAILURE: 0

and

        - setup-app:
            type: script
            script:
              - COMPOSER_EXIT_ON_PATCH_FAILURE=0
              - source ${BLT_DIR}/scripts/pipelines/setup_app

How can I set the exit on patch to false when building with BLT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement A feature or feature request
Projects
None yet
Development

No branches or pull requests

3 participants