wecodemore / wpstarter

Easily bootstrap whole site Composer packages for WordPress.
https://wecodemore.github.io/wpstarter/
MIT License
244 stars 34 forks source link

Instead skip-db-check introduce new env keys. #83

Closed smoothie closed 2 years ago

smoothie commented 5 years ago

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

Today I had the opportunity to try wpstarter-3 and faced some problems. I "solved" it with skip-db-check. Can we add something else?

Let me explain what the problem is, before that I need to explain my setup.

Windows 10 running a vagrant (homestead // ubuntu) machine.

On Windows everything is set. Everything means I have ssh-keys, local php, git, composer (phpcs, ...), node and so on.

The machine runs php, various dbs and the http server. Nothing else. Its just plug and play.

I usually run composer install from windows.

So everything is set with windows user rights, windows line endings and so on.

If I need to connect to the db through the windows machine, I use the IP of the vagrant machine.

In every other case which usually gets served by the server itself, like .env file, I use localhost or 127.0.0.1.

So what happened?

When I ran composer install wpstarter-3 couldn't check that the db is running and threw an error message. Something like couldn't establish db connection.

Describe the solution you'd like Introduce a new .env key something like WP_DB_VIRTUALIZED_HOST or WP_DB_REMOTE_HOST. where DB starter will check for if the first connect threw an error.

Describe alternatives you've considered Current solution: skip-db-check

gmazzap commented 5 years ago

Thanks @ieim just for understanding, if you ssh into the vagrant and run composer install that works?

And if so, what's against that?

smoothie commented 5 years ago

Thank you @gmazzap!

Actually that could be a workaround for many people. In my case doesn't work out for long. Because I'd have to set stuff explicit for the machine. Stuff like phpcs and ssh-keys. There are at least two problems with it. The core issue is: I dont want to share my ssh-keys on more places then I have to. I need SSH keys due to private packages.

The vagrant box is a special case and it could be a workaround for many people but still the root problem stays the same.

As soon as somebody wants to put his database to a different server, he would have to build workarounds to make it happen. Like installing php.

gmazzap commented 5 years ago

Well, WP Starter uses the connection details in .env, so you can put the DB in any server, even remote, with or without PHP, as long the details are in .env, it will work assuming that the system that runs Composer is the same system that runs the application. And note that Composer is designed for this: running Composer in another system has no warranty to work.

The issue is that you want to install an application in a system that will not run it. This is not something common and could create several other issues: e.g. running on Windows, Composer might create directory junctions instead of symlink, and those of course will not work in the Linux machine.

Composer has several places in its code where it checks for Windows OS (https://github.com/composer/composer/search?q=isWindows&unscoped_q=isWindows) and do different things based on the result, so running Composer in Windows and then using the build application on a different system will very likely create issues.

Also, I don't undertsand what you mean by having to setup PHPCS for the Vagrant machine. If you run Composer on the VM, in a shared folder, then you can use the built folder in the IDE or in the command line from the host, that is Windows, you don't have to setup anything for VM.

I can understand the concerns regarding SSH keys. But you could setup read-only "access keys" (aka "deploy keys") on the private packages and use those for the VM, leaving your SSH keys alone.

For all these reasons, the need of using a different DB host just when running Composer look like an edge case to me.

I do see that in some case don't make sense to hit the DB at all (e.g. in a CI) and this is why skip-db-check exists.

So I'm not very keen to add support to additional keys for the purpose, because I will need to add entries in the configuration, take care of validation, add complexity in code, add documentation...

And I'm not sure it is worth it.

gmazzap commented 2 years ago

Closing. Nothing to fix here.