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

Path errors when running wp dist-archive on Windows #76

Closed
VoHoTv opened this issue Jul 18, 2023 · 7 comments
Closed

Path errors when running wp dist-archive on Windows #76

VoHoTv opened this issue Jul 18, 2023 · 7 comments

Comments

@VoHoTv
Copy link

VoHoTv commented Jul 18, 2023

Hi there! When running the wp dist-archive command I am getting the following erorr:

"Target directory does not exist: C:\Users\USER\Local Sites\het-lokaal\app\public\wp-content\themes\lokaal/C:\Users\USER\Local Sites\het-lokaal\app\public\wp-content\themes\lokaal/C:\Users\USER\Local Sites\het-lokaal\app\public\wp-content\themes"

Is this some kind of misconfiguration on my system? Or am I missing some parameter? Thank you so much!

image

@danielbachhuber
Copy link
Member

Thanks for the report, @VoHoTv !

Is this some kind of misconfiguration on my system? Or am I missing some parameter? Thank you so much!

There are a lot of hardcoded / references in wp dist-archive 🙈

if ( 0 !== strpos( $path, '/' ) ) {
$archive_path = dirname( getcwd() . '/' . $path );
} else {
$archive_path = dirname( $path );
}
$archive_filename = null;
}
// If the path is not absolute, it is relative.
if ( 0 !== strpos( $archive_path, '/' ) ) {
$archive_path = rtrim( getcwd() . '/' . ltrim( $archive_path, '/' ), '/' );
}

I think the path breaking is building on Windows.

Maybe it's as simple as replacing the '/' references with DIRECTORY_SEPARATOR ?

Feel free to submit a pull request, if you'd like. Here is some guidance on our pull request best practices.

@danielbachhuber danielbachhuber changed the title Getting error when using the wp dist-archive command. Path errors when running wp dist-archive on Windows Jul 20, 2023
@costdev
Copy link

costdev commented Jul 20, 2023

Looks like there's a couple of things going on here:

  1. It's trying to detect / when the separator could be \.
  2. It assumes the only absolute path is one beginning with a separator, which would be true for *nix/Windows Network Shares. However, on local Windows paths is DRIVE_LETTER:\...

Take a look at what wp_normalize_path() and path_is_absolute() do. WordPress Core commonly uses these in similar scenarios.

@VoHoTv
Copy link
Author

VoHoTv commented Jul 20, 2023

@costdev Just did some tests and when I replace the current conditions which check if it's an absolute path:

"if ( 0 !== strpos( $path, '/' ) ) {}" and "if ( 0 !== strpos( $archive_path, '/' ) ) {}"

And replace it with a copy of the 2 functions you mentioned it works indeed.

@shamimipt
Copy link

Hi There, I am fetching the same issue. Is it fixed or Not?

@danielbachhuber
Copy link
Member

@shamimipt It's not fixed, no. Feel free to submit a pull request, if you'd like. Here is some guidance on our pull request best practices.

I think we'll also want to get tests running on Windows wp-cli/wp-cli-tests#155

@danielbachhuber
Copy link
Member

@VoHoTv @shamimipt Can you install the wp dist-archive main branch and see if #81 resolves your issue on Windows?

@shamimipt
Copy link

@VoHoTv Thank you so much.

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

No branches or pull requests

4 participants