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

Support acquia/blt 13.5.2 updates #3

Closed
hkorik opened this issue Aug 24, 2022 · 10 comments · May be fixed by #7
Closed

Support acquia/blt 13.5.2 updates #3

hkorik opened this issue Aug 24, 2022 · 10 comments · May be fixed by #7

Comments

@hkorik
Copy link
Contributor

hkorik commented Aug 24, 2022

The code merged in with version 13.5.2 of acquia/blt breaks the blt blt:update command due to an inheriting class code that extends the robo class. The method now requires to indicate the string type declaration added to the method signature.

This change also revealed a bug with the getCiEnv() method's format in retrieving the environment variable. The isset($_ENV['TRAVIS']) would not retrieve the global variable and requires to be replaced with getenv().

Breaking change:
https://github.com/acquia/blt/pull/4582/files#diff-da4db5d78e93095cb66e19a12ef185565c317720d574ae00f0218399f4cd1973R64

Error output:

PHP Fatal error:  Declaration of Acquia\BltTravis\Blt\Plugin\EnvironmentDetector\TravisDetector::getCiSettingsFile() must be compatible with Acquia\Blt\Robo\Common\EnvironmentDetector::getCiSettingsFile(): string in .../vendor/acquia/blt-travis/src/Blt/Plugin/EnvironmentDetector/TravisDetector.php on line 12

Proposed resolution:
Update the method declaration type in the inherited class within acquia/blt-travis codebase as performed in the base class.

@hkorik hkorik changed the title Acquia EnvironmentDetecter Type Declaration Update Support acquia/blt 13.5.2 updates Sep 8, 2022
@hkorik
Copy link
Contributor Author

hkorik commented Sep 8, 2022

First part of issue addressed with #5 . Second addressed in #4 .

@kmonty
Copy link

kmonty commented Sep 13, 2022

This is a really nasty error and completely breaks using BLT both on Travis and on localdev (Lando). The only remedy downgrading to BLT 13.4.

Hopefully Acquia can cut a new release of this.

@kmonty
Copy link

kmonty commented Sep 13, 2022

Noting you can require "acquia/blt-travis": "dev-master#f761463" and work-around this issue without downgrading the main acquia/blt package.

@jedgar1mx
Copy link

I was able to prevent the issue by adding "acquia/drupal-environment-detector": "1.4.1" to my composer.json

@jedgar1mx
Copy link

I notice that there is an error on acquia-pipelines as well.
TypeError: Acquia\BltTravis\Blt\Plugin\EnvironmentDetector\TravisDetector::getCiSettingsFile(): Return value must be of type string, none returned in Acquia\BltTravis\Blt\Plugin\EnvironmentDetector\TravisDetector::getCiSettingsFile() (line 16 of /mnt/tmp/local.prod/source/vendor/acquia/blt-travis/src/Blt/Plugin/EnvironmentDetector/TravisDetector.php).
Even with the latest dev-master

@jedgar1mx
Copy link

I was trying to update to the latest blt since I'm still on 13.5.1. But I keep getting errors.

Fatal error: Declaration of Acquia\BltTravis\Blt\Plugin\EnvironmentDetector\TravisDetector::getCiSettingsFile() must be compatible with Acquia\Blt\Robo\Common\EnvironmentDetector::getCiSettingsFile(): string in /app/vendor/acquia/blt-travis/src/Blt/Plugin/EnvironmentDetector/TravisDetector.php on line 12

PHP Fatal error: Declaration of Acquia\BltTravis\Blt\Plugin\EnvironmentDetector\TravisDetector::getCiSettingsFile() must be compatible with Acquia\Blt\Robo\Common\EnvironmentDetector::getCiSettingsFile(): string in /app/vendor/acquia/blt-travis/src/Blt/Plugin/EnvironmentDetector/TravisDetector.php on line 12

@m4olivei
Copy link

@jedgar1mx I'm using the latest master and the error is fixed there. Specifically it was fixed in #5.

@danepowell / @hkorik / @mikemadison13 Can we get a tagged release here? Riding on dev-master is not ideal. Thanks so much.

@danepowell
Copy link
Contributor

https://github.com/acquia/blt-travis/releases/tag/v1.1.0

Sounds like this can be closed.

@jedgar1mx
Copy link

Travis-CI works but I'm still encountering errors but these are on acquia-pipelines.

TypeError: Acquia\BltTravis\Blt\Plugin\EnvironmentDetector\TravisDetector::getCiSettingsFile(): Return value must be of type string, none returned in Acquia\BltTravis\Blt\Plugin\EnvironmentDetector\TravisDetector::getCiSettingsFile() (line 16 of /mnt/tmp/local.prod/source/vendor/acquia/blt-travis/src/Blt/Plugin/EnvironmentDetector/TravisDetector.php).

 [warning] Drush command terminated abnormally.

 [Acquia\Blt\Robo\Tasks\DrushTask]  Exit code 1  Time 0.217s

 [error]  Failed to install Drupal!
For troubleshooting guidance and support, see https://docs.acquia.com/blt/support/ 

Travis-CI works but I get

@jedgar1mx
Copy link

I tried both master and latest release

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 a pull request may close this issue.

5 participants