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

Do not assume Drupal VM is in use because config file is there #4217

Closed
benjifisher opened this issue Jul 29, 2020 · 3 comments
Closed

Do not assume Drupal VM is in use because config file is there #4217

benjifisher opened this issue Jul 29, 2020 · 3 comments
Labels
Enhancement A feature or feature request

Comments

@benjifisher
Copy link
Contributor

benjifisher commented Jul 29, 2020

Is your feature request related to a problem? Please describe.

I am trying to set up a project using BLT where individual developers can choose between using Lando and Drupal VM. As soon as I add the file box/config.yml to the project, I get errors from recipes:multisite:init.

Describe the solution you'd like

Either add an option to blt/blt.yml describing the local environment (e.g., lando or drupalvm) or else add a boolean option describing whether to assume that Drupal VM is in use.

Describe alternatives you've considered

As a work-around, I have added these lines to blt/blt.yml:

vm:
  # Hack: set to a non-existent file in order to short-circuit
  # Acquia\Blt\Robo\Inspector\Inspector::isDrupalVmConfigPresent().
  # If you want to use Drupal VM, then override in local.blt.yml with
  # ${repo.root}/box/config.yml.
  config: ${repo.root}/box/config-does-not-exist.yml

Additional context

The comment lines above describe what I think causes the problem.

Here is what happens when I try to add a multisite to the project without that fix/hack:

$ lando blt recipes:multisite:init
This will generate a new site in the docroot/sites directory.
 Site machine name (e.g. 'example') multi2
 Local domain name [http://local.multi2.com] 
 Would you like to configure the local database credentials? (y/n) n
 Would you like to generate new virtual host entry and database for this site inside Drupal VM? (y/n) n
PHP Warning:  file_get_contents(/app/vendor/acquia/blt/scripts/drupal-vm/drupal-vm.site.yml): failed to open stream: No such file or directory in /app/vendor/acquia/blt/src/Robo/Commands/Recipes/MultisiteCommand.php on line 350

Warning: file_get_contents(/app/vendor/acquia/blt/scripts/drupal-vm/drupal-vm.site.yml): failed to open stream: No such file or directory in /app/vendor/acquia/blt/src/Robo/Commands/Recipes/MultisiteCommand.php on line 350
PHP Warning:  Invalid argument supplied for foreach() in /app/vendor/grasmash/yaml-expander/src/Expander.php on line 82

Warning: Invalid argument supplied for foreach() in /app/vendor/grasmash/yaml-expander/src/Expander.php on line 82
 Default remote drush alias [multi2.remote] 
 Default local drush alias [multi2.local] 
PHP Warning:  file_get_contents(/app/vendor/acquia/blt/scripts/drupal-vm/drupal-vm.site.yml): failed to open stream: No such file or directory in /app/vendor/acquia/blt/src/Robo/Commands/Recipes/MultisiteCommand.php on line 350

Warning: file_get_contents(/app/vendor/acquia/blt/scripts/drupal-vm/drupal-vm.site.yml): failed to open stream: No such file or directory in /app/vendor/acquia/blt/src/Robo/Commands/Recipes/MultisiteCommand.php on line 350
PHP Warning:  Invalid argument supplied for foreach() in /app/vendor/grasmash/yaml-expander/src/Expander.php on line 82

Warning: Invalid argument supplied for foreach() in /app/vendor/grasmash/yaml-expander/src/Expander.php on line 82
> blt:init:settings
Hash salt already exists.
New site generated at /app/docroot/sites/multi2
Drush aliases generated:
  * @multi2.remote
Config directory created for new site at config/multi2
$ cat drush/sites/multi2.site.yml 
{  }$

In addition to the PHP warnings, the site-alias file is empty. The PHP warnings do not come from the step that generates the prompt Would you like to generate new virtual host entry and database for this site inside Drupal VM?. They come from a later step, createSiteDrushAlias(), which checks Inspector:isDrupalVmConfigPresent().

@benjifisher benjifisher added the Enhancement A feature or feature request label Jul 29, 2020
@mikemadison13
Copy link
Contributor

can you check in the blt folder and look in local.blt.yml. Is there a vm-enable key in there? You should able to set that to false and BLT should immediately stop acting as if the VM is there.

@benjifisher
Copy link
Contributor Author

I did not have blt/local.blt.yml when I first reported this. Just to be sure, I will re-test:

$ ls blt
blt.yml  drupalvm.blt.yml
$ tail blt/blt.yml 
      - add_attributes
      - bem

vm:
  # Hack: set to a non-existent file in order to short-circuit
  # Acquia\Blt\Robo\Inspector\Inspector::isDrupalVmConfigPresent().
  # If you want to use Drupal VM, then override in local.blt.yml with
  # ${repo.root}/box/config.yml.
  # See https://github.com/acquia/blt/issues/4217.
  config: ${repo.root}/box/config.yml
$ lando blt recipes:multisite:init
This will generate a new site in the docroot/sites directory.
 Site machine name (e.g. 'example') multi2
 Local domain name [http://local.multi2.com] 
 Would you like to configure the local database credentials? (y/n) n
 Would you like to generate new virtual host entry and database for this site inside Drupal VM? (y/n) n
PHP Warning:  file_get_contents(/app/vendor/acquia/blt/scripts/drupal-vm/drupal-vm.site.yml): failed to open stream: No such file or directory in /app/vendor/acquia/blt/src/Robo/Commands/Recipes/MultisiteCommand.php on line 350

Warning: file_get_contents(/app/vendor/acquia/blt/scripts/drupal-vm/drupal-vm.site.yml): failed to open stream: No such file or directory in /app/vendor/acquia/blt/src/Robo/Commands/Recipes/MultisiteCommand.php on line 350
PHP Warning:  Invalid argument supplied for foreach() in /app/vendor/grasmash/yaml-expander/src/Expander.php on line 82

Warning: Invalid argument supplied for foreach() in /app/vendor/grasmash/yaml-expander/src/Expander.php on line 82
 Default remote drush alias [multi2.remote] 
 Default local drush alias [multi2.local] 
PHP Warning:  file_get_contents(/app/vendor/acquia/blt/scripts/drupal-vm/drupal-vm.site.yml): failed to open stream: No such file or directory in /app/vendor/acquia/blt/src/Robo/Commands/Recipes/MultisiteCommand.php on line 350

Warning: file_get_contents(/app/vendor/acquia/blt/scripts/drupal-vm/drupal-vm.site.yml): failed to open stream: No such file or directory in /app/vendor/acquia/blt/src/Robo/Commands/Recipes/MultisiteCommand.php on line 350
PHP Warning:  Invalid argument supplied for foreach() in /app/vendor/grasmash/yaml-expander/src/Expander.php on line 82

Warning: Invalid argument supplied for foreach() in /app/vendor/grasmash/yaml-expander/src/Expander.php on line 82
> blt:init:settings
Hash salt already exists.
New site generated at /app/docroot/sites/multi2
Drush aliases generated:
  * @multi2.remote
Config directory created for new site at config/multi2

The file drupalvm.blt.yml is intended to be renamed local.blt.yml when using Drupal VM instead of Lando.

As you can see, I left the comment in blt/blt.yml but reverted the hack. I get the same behavior.

I notice that vm.enable is set to false in vendor/acquia/blt/config/build.yml. Just to be on the safe side, I added enable: false to my blt/blt.yml and repeated the experiment.

If you have a look at the function I mentioned, Inspector:isDrupalVmConfigPresent(), you will see that it checks for the existence of the config file, not for the vm.enable configuration:

  public function isDrupalVmConfigPresent() {
    return file_exists($this->getConfigValue('repo.root') . '/Vagrantfile')
      && file_exists($this->getConfigValue('vm.config'));
  }

@danepowell
Copy link
Contributor

Did you overwrite vm.config in one of your config files (blt.yml)? As you noted, BLT's default config is already:

vm:
  enable: false
  config: ${repo.root}/box/config.yml

That's exactly the same as the "hack" you put into your blt.yml, thus the hack shouldn't be necessary in the first place.

I think the only thing that should change here is a guard on MultisiteCommand::createSiteDrushAlias() to ensure the alias file is present.

@benjifisher benjifisher changed the title Do not assume BLT is in use because config file is there Do not assume Drupal VM is in use because config file is there Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement A feature or feature request
Projects
None yet
Development

No branches or pull requests

3 participants