-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
PHP 7.4 #163
PHP 7.4 #163
Conversation
@sean-e-dietrich there is a related ticket docksal/docksal#1245. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sean-e-dietrich since Drush 8 is not currently compatible with PHP 7.4 we should not install it all.
Please comment out lines related to Drush 8 and add a comment with an explanation.
7.4/Dockerfile
Outdated
@@ -296,7 +290,7 @@ $HOME/.composer/vendor/phpcompatibility/phpcompatibility-paragonie/PHPCompatibil | |||
composer clear-cache; \ | |||
\ | |||
# Drush modules | |||
drush dl registry_rebuild --default-major=7 --destination=$HOME/.drush >/dev/null; \ | |||
#drush dl registry_rebuild --default-major=7 --destination=$HOME/.drush >/dev/null; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a comment to explain why this is commented out
@lmakarov @achekulaev made some adjustments to the tests and then commented out the drush 8 commands. |
tests/test.bats
Outdated
@@ -229,7 +234,8 @@ _healthcheck_wait () | |||
|
|||
# Check Drush version | |||
run docker exec -u docker "$NAME" bash -lc 'drush --version | grep "^ Drush Version : ${DRUSH_VERSION} $"' | |||
[[ ${status} == 0 ]] | |||
# Currently check on all other version than 7.4 | |||
[[ "${VERSION}" != "7.4" ]] && [[ ${status} == 0 ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this passing now? If VERSION is 7.4 then this code will return 1
and should fail the test, but it does not.
cc @achekulaev
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lmakarov i don't disable or turn off the installing of drush launcher just drush8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sean-e-dietrich this check results in the following for PHP 7.4:
[[ "7.4" != "7.4" ]] && [[ 1 == 0 ]]
That exists with 1
and should fail the test, but that does not happen for whatever reason in bats, which does not make sense to me.
Doing the comparison in a sub-shell works as expected in bats:
( [[ "7.4" != "7.4" ]] && [[ 1 == 0 ]] )
It looks like the short form comparison in bats is equivalent to the long form:
if [[ "${VERSION}" != "7.4" ]]; then [[ ${status} == 0 ]]; fi
Though, logically (and in a regular bash script), they produce different results.
I pushed a fix for this.
Closes #162