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

Introduce "composer-exit-on-patch-failure" config option. #46

Merged
merged 5 commits into from Feb 16, 2017

Conversation

Leksat
Copy link
Contributor

@Leksat Leksat commented Jun 11, 2016

#40

@cweagans
Copy link
Owner

This will cause composer validate to fail for anyone using this option. I'd prefer not to break this behavior, as some people run composer validate as part of their test suite. Having tests suddenly start failing would not be a great thing.

@Leksat
Copy link
Contributor Author

Leksat commented Jun 13, 2016

@cweagans, I can't get what you mean... For me composer validate works just fine with "composer-exit-on-patch-failure": true.

@@ -269,10 +269,12 @@ public function postInstall(PackageEvent $event) {
$extra['patches_applied'][$description] = $url;
}
catch (\Exception $e) {
$this->io->write(' <error>Could not apply patch! Skipping.</error>');
if (getenv('COMPOSER_EXIT_ON_PATCH_FAILURE')) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing support for this constant breaks backwards compatibility.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 ^^ what @bartfeenstra said. Thanks for catching that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed 😉

@cweagans
Copy link
Owner

According to the schema, that should fail. I was going to open a PR against Composer to relax the schema for config so that plugins can use it for flags like this, but I can't get composer validate to fail now that you mention it. Maybe a bug in the schema validator that Composer uses? I'll investigate more later.

@Leksat
Copy link
Contributor Author

Leksat commented Jun 14, 2016

I also restored the Could not apply patch! Skipping. message for the case if someone parses the composer output.

@danepowell
Copy link
Collaborator

Would the extra key be a better alternative to config? From the schema docs:

extra: Arbitrary extra data that can be used by plugins, for example, package of type composer-plugin may have a 'class' key defining an installer class name.

@Leksat
Copy link
Contributor Author

Leksat commented Jun 29, 2016

@cweagans, thanks for the explanation. I did not know that config options are limited. I moved the option to the extra section.

@erikhansen
Copy link

@Leksat I would love to be able to use this feature. Would you be able to resolve the merge conflict with this PR so that it can be merged in?

@dan2k3k4
Copy link
Contributor

Hey @erikhansen, I updated this PR to resolve the conflict. Please test and provide feedback. Thanks in advance!

@erikhansen
Copy link

@cweagans - Based on my testing, I'd recommend merging this PR.

@dan2k3k4 - I finally made time to test your PR and it's working great on my end. Screenshots demonstrating things working:

composer.json file

09-47-08 composer json sites git develop -8iox7

Exception being thrown and non-zero exit code
09-45-29 healthyback dev -bash 176x49-loc0b

@erikhansen
Copy link

@cweagans Any update on this getting included in the next release? Can we expect it in your 2.0 update?

@cweagans cweagans merged commit e395c7c into cweagans:master Feb 16, 2017
@cweagans
Copy link
Owner

Sorry for the delay, folks.

@dan2k3k4 dan2k3k4 deleted the exit-on-failure branch June 13, 2017 10:47
tormi added a commit to tormi/drupal that referenced this pull request May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants