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

Snapshots WP CLI commands are not executed at all #59

Open
1 task done
kmgalanakis opened this issue Aug 4, 2023 · 8 comments · May be fixed by #60
Open
1 task done

Snapshots WP CLI commands are not executed at all #59

kmgalanakis opened this issue Aug 4, 2023 · 8 comments · May be fixed by #60
Labels
type:bug Something isn’t working.

Comments

@kmgalanakis
Copy link

Describe the bug

I was trying to download a snapshot via the LocalWP shell in Windows and even though I was getting no errors, the snapshot was never downloaded.

Debugging the code, I realized that the WP CLI commands are not executed and this is the output I get for the failed snapshot download WP CLI command ( wp snapshots download f3e0b62aa868982e1e53776fa769a5ab --quiet --repository=10up --include_db --include_files)

C:\Users\kmgal\Local Sites\hello-world\app\public>wp snapshots pull f3e0b62aa868982e1e53776fa769a5ab
Security Warning: Snapshots creates copies of your codebase and database. This could result in data retention policy issues, please exercise extreme caution when using production data.
Do you want to pull files? [Y/n]:
Do you want to pull the database? [Y/n]:
Are you sure you want to pull this snapshot? This is a potentially destructive operation. Please run a backup first. [y/n] y
stdClass Object
(
    [stdout] =>
    [stderr] => '""C:\Program' is not recognized as an internal or external command,
operable program or batch file.
    [return_code] => 1
)
snapshots download f3e0b62aa868982e1e53776fa769a5ab --quiet --repository=10up --include_db --include_filesSnapshot downloaded.
Importing database...
The system cannot find the path specified.
Database imported.
Preparing to replace URLs...
Home URL (defaults to home URL in snapshot: https://hello-world.local):

Steps to Reproduce

  1. In the shell of LocalWP in windows run wp snapshots pull f3e0b62aa868982e1e53776fa769a5ab

Screenshots, screen recording, code snippet

image

Environment information

  • OS: Windows 11

WordPress information

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@kmgalanakis kmgalanakis added the type:bug Something isn’t working. label Aug 4, 2023
@kmgalanakis
Copy link
Author

kmgalanakis commented Aug 4, 2023

A potential solution is to execute all the wp_cli()::runcommand() with the "launch" => false argument but after that, again on Windows, another unrelated issue occurs when attempting to import the DB

'gunzip' is not recognized as an internal or external command, operable program or batch file.

Did we choose to launch the runcommand() method in a new process for a reason?

Edit: Also switching gunzip with gzip, even though the command is executed properly from the command line, the execution via the PHP script fails.

@gsarig
Copy link

gsarig commented Aug 5, 2023

I can confirm @kmgalanakis's report, as I had similar problems where I couldn't pull or download snapshots.

In fact, in my case, using wp snapshots download <snapshot_id> falsely reported that the snapshot was downloaded, but in reality, the .wpsnapshots folder was empty. I also got a PHP warning, though, suggesting that the HOME array on $_SERVER['HOME'] was undefined:

C:\Users\MY_USER\Local Sites\helloworld\app\public>wp snapshots download <snapshot_id>

Warning: Undefined array key "HOME" in C:\Users\MY_USER\.wp-cli\packages\vendor\10up\snapshots\includes\classes\SnapshotsDirectory.php on line 298
Include database in the download? [Y/n]: y
Include files in the download? [Y/n]: y
Downloading snapshot (273.86 KB)...

Warning: Undefined array key "HOME" in C:\Users\MY_USER\.wp-cli\packages\vendor\10up\snapshots\includes\classes\SnapshotsDirectory.php on line 298

Warning: Undefined array key "HOME" in C:\Users\MY_USER\.wp-cli\packages\vendor\10up\snapshots\includes\classes\SnapshotsDirectory.php on line 298

Warning: Undefined array key "HOME" in C:\Users\MY_USER\.wp-cli\packages\vendor\10up\snapshots\includes\classes\SnapshotsDirectory.php on line 298

Warning: Undefined array key "HOME" in C:\Users\MY_USER\.wp-cli\packages\vendor\10up\snapshots\includes\classes\SnapshotsDirectory.php on line 298

Warning: Undefined array key "HOME" in C:\Users\MY_USER\.wp-cli\packages\vendor\10up\snapshots\includes\classes\SnapshotsDirectory.php on line 298

Warning: Undefined array key "HOME" in C:\Users\MY_USER\.wp-cli\packages\vendor\10up\snapshots\includes\classes\SnapshotsDirectory.php on line 298
Success: Snapshot downloaded.

It looks like on Windows, the %HOMEPATH% is the name for what Unix/Linux users call $HOME. When combined with the %HOMEDRIVE% environment variable you get a complete path to the user's home directory. (source).

So, my temporary workaround to have snapshots work was to modify \.wp-cli\packages\vendor\10up\snapshots\includes\classes\SnapshotsDirectory.php and change line 298 from this:

$directory = ! empty( $directory ) ? rtrim( $directory, '/' ) . '/' : rtrim( $_SERVER['HOME'], '/' ) . '/.wpsnapshots/';

to that:

$directory = ! empty( $directory ) ? rtrim( $directory, '/' ) . '/' : rtrim( $_SERVER['HOMEDRIVE'] .  $_SERVER['HOMEPATH'], '/' ) . '/.wpsnapshots/';

and with some refactoring to make sure that in the event of undefined values no warnings would return (but the result is the same):

$server_home = $_SERVER['HOME'] ?? '';
if(empty($server_home)) {
	$homedrive = $_SERVER['HOMEDRIVE'] ?? '';
	$homepath = $_SERVER['HOMEPATH'] ?? '';
	if( !empty($homedrive) && !empty($homepath) ) {
		$server_home = $homedrive . $homepath;	
	} 
}
$directory = ! empty( $directory ) ? rtrim( $directory, '/' ) . '/' : rtrim( $server_home, '/' ) . '/.wpsnapshots/';

Then, running the command with the --repository flag downloads the snapshot:

wp snapshots download <snapshot_id> --repository=10up

And the same goes for pull:

wp snapshots download <snapshot_id> --repository=10up

To be honest, I'm not sure why I had to pass the --repository flag after my modification, while before that I wasn't asked to do so. As it did the job, though, I didn't investigate any further.

@kmgalanakis
Copy link
Author

The reason you have to use the --repository=10up is that you haven't run wp snapshots configure 10up for the new path.

Apart from that, I am still unable to run any wp-cli command from within the package if I keep "launch" => f.

I don't know if it's only my environment but for me, the snapshot pulling is failing in so many places and I don't know if I should spend more time on that at the moment.

@kmgalanakis
Copy link
Author

I create a Windows VM and installed everything from scratch (Windows, Git, LocalWP, Snapshots). I also followed @gsarig suggestion to adjust the snapshots directory.

Now when I try to run wp snapshots pull f3e0b62aa868982e1e53776fa769a5ab, initially the process finishes with no errors but since the snapshot archive is not downloaded, by debugging the snapshot download command I get the following output.

C:\Users\KonstantinosG\LocalSites\hello-world\app\public>wp snapshots pull f3e0b62aa868982e1e53776fa769a5ab
Security Warning: Snapshots creates copies of your codebase and database. This could result in data retention policy issues, please exercise extreme caution when using production data.
Do you want to pull files? [Y/n]:
Do you want to pull the database? [Y/n]:
Are you sure you want to pull this snapshot? This is a potentially destructive operation. Please run a backup first. [y/n] y
stdClass Object
(
    [stdout] =>
    [stderr] => The system cannot find the path specified.
    [return_code] => 1
)
Snapshot downloaded.
Importing database...
The system cannot find the path specified.
Database imported.
Preparing to replace URLs...
Home URL (defaults to home URL in snapshot: https://hello-world.local):
Site URL (defaults to site URL in snapshot: https://hello-world.local):
Running replacement... This may take a while depending on the size of the database.
Search and replacing tables: wp_commentmeta, wp_comments, wp_links, wp_options, wp_postmeta, wp_posts, wp_termmeta, wp_usermeta, wp_users
URLs replaced.
Creating snapshots user...
Pulling files and replacing /wp-content. This could take a while...
tar: Error opening archive: Failed to open 'C:\\Users\\KonstantinosG/.wpsnapshots//f3e0b62aa868982e1e53776fa769a5ab/files.tar.gz'
Files pulled.
Snapshot pulled successfully.
Visit in your browser: https://hello-world.local
Make sure the following entry is in your hosts file: "127.0.0.1 hello-world.local"
Admin login: username - "snapshots", password - "password"

I don't know what is that "specified path" that cannot be found.

The tar-related error is only relevant because the actual snapshot archive hasn't been downloaded.

Apart from that, what I mentioned in this comment is still relevant to me.

Any idea how I could figure out what this path that cannot be found is?

@darylldoyle
Copy link
Contributor

I'm not 100% on this as I don't have a Windows system to test, but it could be related to paths and how we use them in the codebase. I think there are a couple of issues:

The initial issue shows this error:

stdClass Object
(
    [stdout] =>
    [stderr] => '""C:\Program' is not recognized as an internal or external command,
operable program or batch file.
    [return_code] => 1
)

This is likely to do with an unquoted path being passed as an argument, which means that the space in C:\Program Files\ is causing issues. We should ensure that wherever paths are passed to CLI commands, they're enclosed in quotes.

The second issue I'm seeing is in the comment above:

stdClass Object
(
    [stdout] =>
    [stderr] => The system cannot find the path specified.
    [return_code] => 1
)

This seems to be happening whilst the snapshot is downloading. Tracing the code back through how that works, leads me to believe it could be related to SnapshotsDirectory::get_file_path(). As you can see below, we have a hardcoded forward slash in the path name, I wonder if that could be causing the incorrect path issue, which means that the file isn't being saved locally.

public function get_file_path( string $file_name = '', ?string $id = null ) : string {
$directory = $this->get_directory( $id );
return $directory . '/' . $file_name;
}

One of the things making me think this could be the issue is this line:

tar: Error opening archive: Failed to open 'C:\\Users\\KonstantinosG/.wpsnapshots//f3e0b62aa868982e1e53776fa769a5ab/files.tar.gz'

Notice we have mixed forward and backslashes.

The other issue it could be, is the one flagged by @gsarig, where we're using $_SERVER['HOME'].

$directory = ! empty( $directory ) ? rtrim( $directory, '/' ) . '/' : rtrim( $_SERVER['HOME'], '/' ) . '/.wpsnapshots/';

WP-CLI does contain an is_windows() util that we could use to swap out the home dir:

https://github.com/wp-cli/wp-cli/blob/5c4eeaae23e80c91f2905d03d60c47877e5f039c/php/utils.php#L696-L704

@kmgalanakis, to help us narrow down the issue, could you try running the pull command again, but this time with the --debug flag set, please?

wp snapshots pull f3e0b62aa868982e1e53776fa769a5ab --debug

I'm hopeful that this will give us more insight into what's happening in those failing internal WP-CLI functions.

@gsarig
Copy link

gsarig commented Aug 8, 2023

@darylldoyle @kmgalanakis A few additional findings... I installed it on a new WSL2 instance (Debian), following these instructions.

It seems to work almost fine (at least with the caveats mentioned in the topic). When I tried to run snapshots though, I faced, more or less, the same issues: Searching and pulling was working, but when I tried to create a new snapshot, I got the following error:

Include database in snapshot? [Y/n]: y
Include files in snapshot? [Y/n]: y
Snapshot Description (e.g. Local environment):
test
Project Slug (letters, numbers, _, and - only):
test
Saving database...
Getting WordPress tables...
Exporting database...
Scrubbing users...
 - Duplicating users table into temp tables...
 - Scrubbing each user record...
 - Duplicating user meta table into temp table...
Scrubbing comments on wp_comments...
 - Duplicating comments table into temp table...
 - Duplicating comment meta table into temp table...
 - Scrubbing comments table...
Error: Unable to read file: /home/gsarig/.wpsnapshots//d05482c265d8f9a079f721a282614281/data-usermeta.sql

Hoping that it was just an issue with the double slash, I edited SnapshotsDirectory.php to remove the extra slash. Still, I got the same error:

Include database in snapshot? [Y/n]: y
Include files in snapshot? [Y/n]: y
Snapshot Description (e.g. Local environment):
test
Project Slug (letters, numbers, _, and - only):
test
Saving database...
Getting WordPress tables...
Exporting database...
Scrubbing users...
 - Duplicating users table into temp tables...
 - Scrubbing each user record...
 - Duplicating user meta table into temp table...
Scrubbing comments on wp_comments...
 - Duplicating comments table into temp table...
 - Duplicating comment meta table into temp table...
 - Scrubbing comments table...
Error: Unable to read file: /home/gsarig/.wpsnapshots/62369eb282dec6969fd5ac85fd159984/data-usermeta.sql

Given that this is now tested on Debian, even if it's WSL2 and not "real" Linux, I wonder if this is a Windows specific problem, or if it happens on Linux as well.

@gsarig
Copy link

gsarig commented Aug 8, 2023

@darylldoyle @kmgalanakis I managed to make it work on WSL2, as it only needed me to install mysql (mariadb, in particular, which comes as the default) and make a small adjustment to wp-config.php.

It isn't pretty and it doesn't update the hosts file automatically like the Windows version, but on the other hand, installing everything else (git, php, node etc) on Linux is not the pain that it is on Windows. I pulled a snapshot and pushed one as well, with no issues at all. In fact, despite its flaws, I think that I will stick to this setup as the Linux terminal is more familiar and faster than CMD. Here are the steps that I took, for anyone who would like to try it out (I hope I'm not forgetting something):


  1. Make sure that you have GUI apps support enabled (detailed instructions):
    tldr; For existing WSL, run on Powershell:
wsl --update
wsl --shutdown
  1. Install Debian from the Windows Store. I preferred Debian as the instructions to set it up are for Debian and might not work equally well on Ubuntu.
    Update: It works just fine on Ubuntu as well, following pretty much the same steps.
  2. Install all the standard tools that you would need: PHP, composer, mysql (mariadb is Debian's default - it doesn't matter), git, wp-cli and node.
  3. Install local WP following the instructions on this topic.
  4. After you finish the setup, you can start it with running the following command (it will open up the GUI window):
/opt/Local/local
  1. Create a new site using the GUI (same as on Windows)
  2. On the site's wp-config.php, change
define( 'DB_HOST', 'localhost' );

To

define( 'DB_HOST', 'localhost:/home/USERNAME/.config/Local/run/XXX/mysql/mysqld.sock' );

You can find the socket here:

image

@kmgalanakis
Copy link
Author

Hello @darylldoyle.

Apparently, the errors I was getting with launching any WP-CLI command via wp_cli()::runcommand to a new process were because of my outdated WP-CLI version. The latest Windows version of LocalWP (7.1.2+6410) packs WP-CLI version 2.7.1 while the minimum version without those issues is 2.8.0.

I've created a PR which I believe fixes all the Windows-related issues. I've tested it on a Windows VM, where I tried everything from scratch, documenting the process every step of the way.

@gsarig In my VM I only used LocalWP, Git, Grep and Gzip for Windows. All apps were installed as normal Windows apps (not via WSL). All the CLI interaction was done with the LocalWP shell.

Even though unit tests are passing, I would really like the branch to also be tested in Linux and MacOS machines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn’t working.
Projects
None yet
3 participants