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

[site:install] Regression: command does not respect empty db-prefix #4189

Merged
merged 1 commit into from
Nov 7, 2019
Merged

[site:install] Regression: command does not respect empty db-prefix #4189

merged 1 commit into from
Nov 7, 2019

Conversation

jensschulze
Copy link
Contributor

Problem/Motivation

The issue that was addressed in commit e62a1fe #3930 #3931 is back.

If you submit an empty string as DB prefix (--db-prefix='') it is handled exactly as if no db-prefix was given: the command asks for a prefix regardless of the input. This is due to a regression first introduced by f1dc3aa #3917. This regression was partly reverted by 1247f20 #4049, but in both cases PHP’s implicit type casting causes problems.

You have to check the input value explicitly against null, otherwise a blank string ("no prefix") will be handled like "no value given".

if (!$something) {
    echo '$something is either null or 0 or false or an empty array or an empty string. Are you sure this is the desired behavior?';
}

How to reproduce

  • Drupal version: tested on Drupal 8.6+
  • Console version: Regression was introduced in 1.9.1
drupal -y si test \
    --langcode="en" \
    --db-type="mysql" \
    --db-host="testdbhost" \
    --db-port="3306" \
    --db-name="testdb" \
    --db-user="testuser" \
    --db-pass="testpass" \
    --db-prefix="" \
    --account-name=admin \
    --account-pass=admin \
    --account-mail=admin@example.com \
    --site-name="Test (MySQL)" \
    --site-mail="admin@example.com"

Result:

 Database Prefix []:
 >

Solution

Avoid implicit type casting and check $input->getOption('db-prefix') against null explicitly.

Empty --db-prefix must not provoke db-prefix question

Issue: #4189
@enzolutions
Copy link
Contributor

Thanks @jensschulze

@enzolutions enzolutions merged commit 73453aa into hechoendrupal:master Nov 7, 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.

2 participants