search

PHPCSStandards / composer-installer

Composer installer for PHP_CodeSniffer coding standards
https://packagist.org/packages/dealerdirect/phpcodesniffer-composer-installer
MIT License
559 stars 36 forks source link

Move loadInstalledPaths from init to onDependenciesChangedEvent #51

Closed gapple closed 6 years ago

gapple commented 6 years ago

Proposed Changes

The plugin executes phpcs when initialized, which may cause it to include the autoloader while it's in an inconsistent state.

For example, if a package that includes a static file is removed during an update (e.g. a compat library that provides global PHP functions):

  1. Composer removes the outdated package from the file system
  2. The phpcodesniffer-composer-installer plugin is initialized and attempts to use the autoloader files on disk
  3. The outdated autoloader files on disk require the file from the removed package, and the composer update / install fails with a fatal error.

This PR moves the initialization of installedPaths to the method which responds to the POST_INSTALL_CMD and POST_UPDATES_CMD events, so that it is only initialized just before it's actually needed.

Related Issues

Fixes #49

Potherca commented 6 years ago

The failing build on Travis is caused by an issue with the security-checker.

It uses code that is only PHP7.1+ compatible.

gapple commented 6 years ago

I've tagged security checker to the last release that supported PHP 5, and the Travis build is now successful.

Would it be better (and possible) to manage the version of security-checker via require-dev in composer.json?

Potherca commented 6 years ago

I would be all for it but that is up to @frenck as I am no longer part of the organisation.

(I could ask @frenck to add me as a contributor... then I could make more contributions).

frenck commented 6 years ago

This seems to be a sane fix for the issue at hand. I will test this one asap.

frenck commented 6 years ago

@gapple About the security checker as a dependency, I'm kinda in the middle of this. IMHO, security package checking is part of a CI and not part of a development workflow (e.g., GitHub is now moving towards providing that kind of things).

So I'm fine with both solutions tbh.

This has been changed by @jrfnl in #58.