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

Fix link command's --isolate argument with custom name #1428

Merged
merged 6 commits into from
Sep 25, 2023

Conversation

mcaskill
Copy link
Contributor

@mcaskill mcaskill commented Jul 16, 2023

Related to #1386.

Tested in laravel/valet v4.1.2 with mnapoli/silly v1.8.2 and symfony/console v6.2.5.

🟡 Unfortunately, a bug introduced in mnapoli/silly v1.8.1 breaks all arguments in runCommand() method preventing this pull request from resolving the isolation issue. See mnapoli/silly#69 for details and mnapoli/silly#70 for pull request.

🟢 Bug resolved in mnapoli/silly v1.8.3.

Passing a custom hostname to the link command with the --isolate argument will result in either a mismatched site being isolated or the site not being found:

~/Projects/example.com/www/_codebase
$ valet link example --secure --isolate
A [example] symbolic link has been created in [~/.config/valet/Sites/example].
Restarting nginx...
The [example.test] site has been secured with a fresh TLS certificate.
Found '_codebase/.valetrc' or '_codebase/.valetphprc' specifying version: php@8.1

In Site.php line 182:

  The [_codebase] site could not be found in Valet's site list.


link [--secure] [--isolate] [--] [<name>]

The reason for this is because the isolate command defaults to using the current directory's name which won't match the custom hostname.

This issue does not affect the secure command because it resolves the hostname from the current directory's full path.

This pull request passes the hostname (regardless if custom or the current directory's name) to both sub-commands to ensure consistency of operations.

@mcaskill mcaskill changed the title Fix link command's --isolate argument Fix link command's --isolate argument with custom name Jul 16, 2023
@drbyte
Copy link
Contributor

drbyte commented Jul 25, 2023

👍 Looks good to me. Thanks for all your work on this!

tests/CliTest.php Outdated Show resolved Hide resolved
mcaskill and others added 6 commits September 8, 2023 15:41
Passing a custom hostname to the `link` command with the `--isolate` argument will result in either a mismatched site being isolated or the site not being found ("The [example] site could not be found in Valet's site list.").

The reason for this is because the `isolate` command defaults to using the current directory's _name_ which won't match the custom hostname.

This issue does not affect the `secure` command because it resolves the hostname from the current directory's _full path_.

This commit passes the hostname (either custom or the current directory's name) to both sub-commands to ensure consistency of operations.

Note: A bug introduced in mnapoli/silly v1.8.1 breaks all arguments in `runCommand()` method. See mnapoli/silly#69 for details and mnapoli/silly#70 for pull request.
This matches the isolate command's usage of `Site::phpRcVersion()`.
Based on `PhpFpmTest::test_isolate_will_isolate_a_site()` and `CliTest::test_link_command_with_secure_flag_secures()`.
Silly v1.5.0 introduced improvements on command arguments and options that are used by Valet.
@mcaskill
Copy link
Contributor Author

mcaskill commented Sep 8, 2023

I've rebased the latest changes.

@mattstauffer
Copy link
Collaborator

Thank you so much @mcaskill, and thanks as always @drbyte and @driesvints!

@mattstauffer mattstauffer merged commit eb0bc01 into laravel:master Sep 25, 2023
3 checks passed
@mcaskill mcaskill deleted the improve-link-subcommands branch September 25, 2023 16:55
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.

4 participants