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

Update ORCA to alpha18. #3712

Merged
merged 23 commits into from
Jul 17, 2019
Merged

Update ORCA to alpha18. #3712

merged 23 commits into from
Jul 17, 2019

Conversation

danepowell
Copy link
Contributor

@danepowell danepowell commented Jun 13, 2019

https://github.com/acquia/orca/releases/tag/v1.0.0-alpha17

This is awesome because we should get full deprecation testing now.

Blocked by:

@danepowell danepowell changed the title Update ORCA to alpha15. Update ORCA to alpha17. Jun 25, 2019
@danepowell
Copy link
Contributor Author

Alpha17 out now, should also fix phpstan neon issues: https://github.com/acquia/orca/releases/tag/v1.0.0-alpha17

@danepowell
Copy link
Contributor Author

@TravisCarden do you have a timeline for releasing a new ORCA version that uses the shared Coding Standards library rather than ORCA's built-in standards? I think this will be necessary to get the static analysis tests passing here, since we developed to the shared standards rather than ORCA's (they are slightly different).

@TravisCarden
Copy link
Contributor

@danepowell I'm working on a release as we speak. I'm hoping to get it out today. I'll let you know.

@TravisCarden
Copy link
Contributor

All right, @danepowell. v1.0.0-alpha18, using our shared coding standards library, is out now!

@danepowell
Copy link
Contributor Author

danepowell commented Jul 9, 2019

Just one final bug to squash before we can merge this.

@TravisCarden I think ORCA is erroneously throwing a deprecation error here: https://travis-ci.com/acquia/blt/jobs/214613419#L2026

Notice that BLT uses symfony/config 3.4.29, and in that version the function BLT is calling is not deprecated, so I don't think ORCA should be throwing this error.

This is just a guess, but I wonder if ORCA itself uses Symfony Config 4+ and that's why it's throwing an error (even though the SUT in this case uses Symfony Config 3).

@TravisCarden
Copy link
Contributor

You're right, @danepowell: ORCA uses Symfony Config 4.3. I wonder if that constitutes a bug in phpstan. In any case, I'll see what I can do about it.

@danepowell
Copy link
Contributor Author

danepowell commented Jul 12, 2019

I confirmed locally that if you use ORCA's phpstan binary, it causes this false positive. If you use BLT's phpstan binary, it works fine (even for the same version of phpstan).

I think what's happening here is that phpstan is autoloading from its own directory, which means that it autoloads the wrong version of symfony for deprecation testing. You can confirm this by unceremoniously killing ORCA's symfony config: rm -rf vendor/symfony/config/

After that tests will pass.

@TravisCarden
Copy link
Contributor

@danepowell I've worked around the issue in acquia/orca#33. I can't merge the change yet because it will cause my builds to fail until you unpin from mglaman/phpstan-drupal:0.11.7 and cut a new release. So you can test this per usual:

diff --git a/.travis.yml b/.travis.yml
index ce2dc757..ba35f8ae 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -58,6 +58,7 @@ before_install:
   # Install ORCA.
   - git clone --branch ${ORCA_VERSION} --depth 1 https://github.com/acquia/orca.git ../orca
   - curl https://patch-diff.githubusercontent.com/raw/acquia/orca/pull/30.patch | git -C ../orca apply
+  - curl https://patch-diff.githubusercontent.com/raw/acquia/orca/pull/33.patch | git -C ../orca apply
   - ../orca/bin/travis/before_install.sh

 install:

My local testing indicates that you'll still have at least one problem to work out, but it works like before on the other packages I tested with, and I think based on that and the failure itself (see below) that the problem will have to be addressed in BLT. In other words, the ball is in your court. :)

$ orca phpstan --sut=acquia/blt
> '/Users/travis.carden/Projects/acquia/orca-build/vendor/bin/phpstan' 'analyse' '--configuration=/Users/travis.carden/Projects/acquia/orca/src/Task/DeprecatedCodeScanner/phpstan.neon' '--autoload-file=/Users/travis.carden/Projects/acquia/orca-build/docroot/autoload.php' '/Users/travis.carden/Projects/acquia/orca-build/vendor/acquia/blt'
  70/181 [▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░░░░░]  38%
Fatal error: During class fetch: Uncaught PHPStan\Broker\ClassAutoloadingException: Class Composer\Plugin\PluginInterface not found and could not be autoloaded. in /Users/travis.carden/Projects/acquia/orca-build/vendor/phpstan/phpstan/src/Broker/Broker.php:358
Stack trace:
#0 [internal function]: PHPStan\Broker\Broker->PHPStan\Broker\{closure}('Composer\\Plugin...')
#1 /Users/travis.carden/Projects/acquia/blt/src/Composer/Plugin.php(23): spl_autoload_call('Composer\\Plugin...')
#2 /Users/travis.carden/Projects/acquia/orca-build/vendor/composer/ClassLoader.php(444): include('/Users/travis.c...')
#3 /Users/travis.carden/Projects/acquia/orca-build/vendor/composer/ClassLoader.php(322): Composer\Autoload\includeFile('/Users/travis.c...')
#4 [internal function]: Composer\Autoload\ClassLoader->loadClass('Acquia\\Blt\\Comp...')
#5 [internal function]: spl_autoload_call('Acquia\\Blt\\Comp...')
#6 /Users/travis.carden/Projects/acquia/orca-build/vendor/phpstan/phpstan/src/Broker/Broker.php(363): class_exists('Acquia\\Blt\\Comp...')
 in /Users/travis.carden/Projects/acquia/blt/src/Composer/Plugin.php on line 23
PHP Fatal error:  During class fetch: Uncaught PHPStan\Broker\ClassAutoloadingException: Class Composer\Plugin\PluginInterface not found and could not be autoloaded. in /Users/travis.carden/Projects/acquia/orca-build/vendor/phpstan/phpstan/src/Broker/Broker.php:358
Stack trace:
#0 [internal function]: PHPStan\Broker\Broker->PHPStan\Broker\{closure}('Composer\\Plugin...')
#1 /Users/travis.carden/Projects/acquia/blt/src/Composer/Plugin.php(23): spl_autoload_call('Composer\\Plugin...')
#2 /Users/travis.carden/Projects/acquia/orca-build/vendor/composer/ClassLoader.php(444): include('/Users/travis.c...')
#3 /Users/travis.carden/Projects/acquia/orca-build/vendor/composer/ClassLoader.php(322): Composer\Autoload\includeFile('/Users/travis.c...')
#4 [internal function]: Composer\Autoload\ClassLoader->loadClass('Acquia\\Blt\\Comp...')
#5 [internal function]: spl_autoload_call('Acquia\\Blt\\Comp...')
#6 /Users/travis.carden/Projects/acquia/orca-build/vendor/phpstan/phpstan/src/Broker/Broker.php(363): class_exists('Acquia\\Blt\\Comp...')
 in /Users/travis.carden/Projects/acquia/blt/src/Composer/Plugin.php on line 23

@danepowell danepowell force-pushed the orca-15 branch 3 times, most recently from 0e0a5ce to 7edc9e4 Compare July 17, 2019 17:29
@danepowell danepowell changed the title Update ORCA to alpha17. Update ORCA to alpha18. Jul 17, 2019
@danepowell
Copy link
Contributor Author

Upstream issue on the off-chance that there's some better way to work around this than to directly require composer/composer into the fixture: phpstan/phpstan#2305

@danepowell danepowell merged commit 498f73e into acquia:10.x Jul 17, 2019
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.

4 participants