Bedrock support: Use home URLs instead of site URL or paths

[oriolarcas]

WP2Static assumes that the base path of some directories is the root of the Wordpress install, but in some environments like Bedrock these directories can be in a different subdirectory (like "wp/"). This assumption adds private directories to the detected URLs. This PR removes that assumption.

[leonstafford]

Hi @oriolarcas, many thanks for this PR!

I'm excited to get it merged, but also want to make sure we're not changing behaviour in an unexpected way for non-Bedrock/custom dir sites.

There are some failing tests in this PR - could you please run composer run-script test locally? You should see some errors like this:

1) WP2Static\FileHelperTest::testGetListOfLocalFilesByDir
TypeError: WP2Static\FilesHelper::getListOfLocalFilesByDir(): Argument #2 ($base_url) must be of type string, array given, called in /home/runner/work/wp2static/wp2static/tests/unit/FileHelperTest.php on line 149

/home/runner/work/wp2static/wp2static/src/FilesHelper.php:47
/home/runner/work/wp2static/wp2static/tests/unit/FileHelperTest.php:149
/home/runner/work/wp2static/wp2static/vendor/10up/wp_mock/php/WP_Mock/Tools/TestCase.php:307

@palmiak I think you're running Bedrock - what do you think about this PR?

[john-shaffer]

I did some work to try adding Bedrock to our integration tests. In order to get crawling working, I had to change https://github.com/leonstafford/wp2static/blob/1db23dc1ed9e0c13bc82d8efa5f2e1831dd463ec/src/Crawler.php#L116 to $absolute_uri = new URL( rtrim( SiteInfo::getURL( 'home' ), '/' ) . $root_relative_path );

Did you also need to make a change in that area, or is there some other way to deal with that?

[john-shaffer]

I made some other changes to get crawling and post-processing working, and added Bedrock to the integration tests.

I believe that this is the correct behavior in general and not a Bedrock-specific fix. However, this is a breaking change, and it's possible that someone has a working website that depends on the current behavior. We may want to add an option like "Use WP home URL as the site root".

[leonstafford]

@john-shaffer cool - time for a BC release version, seems all the fun in PHP these days :D

Still some failing tests on the PR though

[leonstafford]

(or does it need GH actions re-run/forced run after last push?)

[john-shaffer]

Sorry, I should have mentioned that this is still WIP

[john-shaffer]

I have some work on adding an option for this at john-shaffer@f18c15c

However, I find it impossible to predict what the impact of these changes will be on real sites.

[john-shaffer]

@leonstafford can you take a look at my review notes from earlier? If we can address those, I would be okay with merging this. I'm pretty sure that using the home URL is the correct behavior, and I guess that adding an option for the old incorrect behavior wasn't the best idea. We can do without that.

[leonstafford]

Sorry for the delay, @john-shaffer, will get to reviewing your notes this soon!

[leonstafford]

@john-shaffer sorry for delays. If you could please resolve conflicts and choose whether you want the option from your branch in, I'll merge this in soon. We can see what kind of reports if any we get in from the repo users before bundling into next major release

[SharkWipf]

Hate to be "that guy", but any update on this?
Seems like most everything here is mostly ready to go, just waiting on a reply from @john-shaffer?

[leonstafford]

Hi @SharkWipf thanks for keeping pressure on, I've got a lot of catching up to do on this repo!

[oriolarcas]

Sorry for not answering @john-shaffer, thank you for taking over the PR.