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

If composer.json is invalid, the upgrader can't find composer.phar #161

Open
madmatt opened this issue Mar 26, 2019 · 2 comments
Open

If composer.json is invalid, the upgrader can't find composer.phar #161

madmatt opened this issue Mar 26, 2019 · 2 comments

Comments

@madmatt
Copy link
Member

madmatt commented Mar 26, 2019

Steps to reproduce

  1. Have an invalid composer.json - any error will do. Running composer help should fail (e.g. add a comma at the end of an array where it shouldn't be seen)
  2. Run upgrade-code recompose

Expected result: Error that implies the composer.json is broken, or something else

Actual result:

$ upgrade-code recompose
In ComposerExec.php line 97:                                           
  Could not find the composer executable.  

Investigation

Looks like this is because of SilverStripe\Upgrader\Composer\ComposerExec::testExecPath() - it attempts to run composer about and fails if there's any error. Instead, it should fail one way for a file that can't be found, and fail differently if composer runs but returns an error (maybe just pull the error through)?

Given the likelihood that this code works with invalid composer files (fairly high I would imagine), it seems like this is worth doing.

@dnsl48
Copy link

dnsl48 commented Apr 2, 2019

Could change for composer validate potentially?
Giving it impact/low as having a broken composer.json doesn't feel to be a too common problem in the first place. Please, feel free to change it if you think otherwise.

@madmatt
Copy link
Member Author

madmatt commented Apr 3, 2019

I'd argue that during an upgrade is when you're most likely to have a broken composer.json, because you're removing modulesew. Probably impact/low is okay though given at least now Google should find this thread when someone searches for that issue + silverstripe upgrade :-)

Another option could just be to include the output of composer validate as part of the error message?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants