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

Move ~/.valet to ~/.config/valet #449

Merged
merged 7 commits into from Aug 30, 2018

Conversation

Projects
None yet
4 participants
@eberkund
Contributor

eberkund commented Oct 1, 2017

Closes #447

@adamwathan

This comment has been minimized.

Member

adamwathan commented Oct 3, 2017

It doesn't look like this takes into account SSL certificates yet, yeah? I'm pretty sure just moving the certificates to a new folder will break any existing secured sites; we would need some story for remembering what sites are secured, unsecuring them all, moving the config, then resecuring the previously secured sites.

I think this sort of logic might already be handled when we switch TLDs with Valet, so might be worth checking out what we do there.

@eberkund

This comment has been minimized.

Contributor

eberkund commented Oct 3, 2017

Do you mean because the nginx.conf file would have the old paths so it is easiest to just unsecure and resecure in order to update the paths there?

@adamwathan

This comment has been minimized.

Member

adamwathan commented Oct 3, 2017

Yeah exactly 👍🏻

@drbyte

This comment has been minimized.

Contributor

drbyte commented Oct 3, 2017

Agreed, that "should" fix them.

@eberkund

This comment has been minimized.

Contributor

eberkund commented Oct 3, 2017

Added,

Nginx::rewriteSecureNginxFiles();
@adamwathan

This comment has been minimized.

Member

adamwathan commented Oct 17, 2017

Just checked out this branch locally and tried to run valet install to make sure it all works but getting errors.

> valet install
Stopping nginx...
PHP Deprecated:  Non-static method Valet\Nginx::rewriteSecureNginxFiles() should not be called statically in /Users/adamwathan/.composer/vendor/laravel/valet/cli/Valet/Configuration.php on line 50

Deprecated: Non-static method Valet\Nginx::rewriteSecureNginxFiles() should not be called statically in /Users/adamwathan/.composer/vendor/laravel/valet/cli/Valet/Configuration.php on line 50
PHP Fatal error:  Uncaught Error: Using $this when not in object context in /Users/adamwathan/.composer/vendor/laravel/valet/cli/Valet/Nginx.php:133
Stack trace:
#0 /Users/adamwathan/.composer/vendor/laravel/valet/cli/Valet/Configuration.php(50): Valet\Nginx::rewriteSecureNginxFiles()
#1 /Users/adamwathan/.composer/vendor/laravel/valet/cli/Valet/Configuration.php(26): Valet\Configuration->createConfigurationDirectory()
#2 /Users/adamwathan/.composer/vendor/laravel/valet/cli/includes/facades.php(28): Valet\Configuration->install()
#3 /Users/adamwathan/.composer/vendor/laravel/valet/cli/valet.php(40): Facade::__callStatic('install', Array)
#4 [internal function]: Silly\Application->{closure}()
#5 /Users/adamwathan/.composer/vendor/laravel/valet/vendor/php-di/invoker/src/Invoker.php(82): call_user_func_array(Object(Closure), Array)
#6 /Users/adamwathan/.composer/vendor/laravel/valet/vendor/mnapoli/silly/src/Application.php(95): Invoker\Invoker->call(Object(Closure), Array)
#7 [internal function]: Silly\Application->Silly\{closure in /Users/adamwathan/.composer/vendor/laravel/valet/cli/Valet/Nginx.php on line 133

Fatal error: Uncaught Error: Using $this when not in object context in /Users/adamwathan/.composer/vendor/laravel/valet/cli/Valet/Nginx.php:133
Stack trace:
#0 /Users/adamwathan/.composer/vendor/laravel/valet/cli/Valet/Configuration.php(50): Valet\Nginx::rewriteSecureNginxFiles()
#1 /Users/adamwathan/.composer/vendor/laravel/valet/cli/Valet/Configuration.php(26): Valet\Configuration->createConfigurationDirectory()
#2 /Users/adamwathan/.composer/vendor/laravel/valet/cli/includes/facades.php(28): Valet\Configuration->install()
#3 /Users/adamwathan/.composer/vendor/laravel/valet/cli/valet.php(40): Facade::__callStatic('install', Array)
#4 [internal function]: Silly\Application->{closure}()
#5 /Users/adamwathan/.composer/vendor/laravel/valet/vendor/php-di/invoker/src/Invoker.php(82): call_user_func_array(Object(Closure), Array)
#6 /Users/adamwathan/.composer/vendor/laravel/valet/vendor/mnapoli/silly/src/Application.php(95): Invoker\Invoker->call(Object(Closure), Array)
#7 [internal function]: Silly\Application->Silly\{closure in /Users/adamwathan/.composer/vendor/laravel/valet/cli/Valet/Nginx.php on line 133

Happy to take another look whenever you get a chance to test this PR locally and make any changes necessary to get it working, thanks!

if ($this->files->isDir($oldPath)) {
rename($oldPath, VALET_HOME_PATH);
Nginx::rewriteSecureNginxFiles();

This comment has been minimized.

@drbyte

drbyte Oct 17, 2017

Contributor

This Nginx::rewriteSecureNginxFiles(); line can be removed, since running valet install will already fire it a couple steps later.

@drbyte

This comment has been minimized.

Contributor

drbyte commented Oct 17, 2017

The other remaining problem is updating the /usr/local/etc/dnsmasq.conf to remove the old entry for .valet, after adding the new one for .config/valet

If the old invalid entry isn't removed, then dnsmasq won't restart properly.

@drbyte

This comment has been minimized.

Contributor

drbyte commented Oct 18, 2017

One way to clean up the dnsmasq.conf file might be something like this in Dnsmasq.php:

    function appendCustomConfigImport($customConfigPath)
    {
+        // cleanup old ~/.valet references
+        $contents = preg_replace('/^conf-file=.*\/\.valet\/.*$/m', '', $this->files->get($this->configPath));
+        $this->files->putAsUser($this->configPath, $contents);
+
+        // Add dnsmasq config for valet
        if (! $this->customConfigIsBeingImported($customConfigPath)) {
            $this->files->appendAsUser(
                $this->configPath,
                PHP_EOL.'conf-file='.$customConfigPath.PHP_EOL
            );
        }
    }
@drbyte

This comment has been minimized.

Contributor

drbyte commented Oct 18, 2017

Caveat: The automatic pruning of non-existing directories, which is executed on every valet command via Configuration::prune() will delete the /Users/me/.valet/Sites entry from config.json's paths array, thus making all valet link links no longer recognized.

To fix, one must simply re-link any project directory using valet link [link-name] which will automatically add the .config/valet/Sites directory back to the paths array.

@eberkund

This comment has been minimized.

Contributor

eberkund commented Oct 22, 2017

Okay thanks for the feedback, I'm working on a fix.

Caveat: The automatic pruning of non-existing directories, which is executed on every valet command via Configuration::prune() will delete the /Users/me/.valet/Sites entry from config.json's paths array, thus making all valet link links no longer recognized.

Doesn't that get recreated here?

$this->createSitesDirectory();

@drbyte

This comment has been minimized.

Contributor

drbyte commented Oct 22, 2017

Doesn't that get recreated here?

Not in my testing. None of my linked directories worked until I re-linked one, which re-added the entry to the paths array.

How have you been testing these changes?

@eberkund

This comment has been minimized.

Contributor

eberkund commented Oct 22, 2017

How have you been testing these changes?

I cloned the repo, installed the dependencies and I'm running the valet commands with ./valet

I see what you mean, I have ~/Sites but not ~/.config/Sites in my config.json. I'll take a look at that.

@eberkund

This comment has been minimized.

Contributor

eberkund commented Nov 27, 2017

I updated the code to prepend the ~/.config/valet/Sites folder to the sites when it upgrades.

Also the upgrading guide says you might need to relink when you upgrade: https://laravel.com/docs/5.5/valet#upgrading

Either is okay with me.

@drbyte

This comment has been minimized.

Contributor

drbyte commented Dec 13, 2017

This has been working great for me since October

👍 for merge.

@drbyte

This comment has been minimized.

Contributor

drbyte commented Aug 24, 2018

This still continues to work smoothly for me.

I like the cleaner home-directory, and that my configs are with other package configs.

@mattstauffer mattstauffer merged commit 7eb1aec into laravel:master Aug 30, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

mattstauffer added a commit to tightenco/lambo that referenced this pull request Sep 4, 2018

cretueusebiu added a commit to cretueusebiu/valet-windows that referenced this pull request Sep 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment