git repo README.md should include instructions to install svn

[czapla]

The README.md in the github repo should include instructions to install svn as part of the Development Environment set up.

The "env:install" npm script calls the install.js script which calls svn here: ​https://github.com/WordPress/wordpress-develop/blob/master/tools/local-env/scripts/install.js#L57

[desrosj]

HI @czapla,

Thanks for this ticket, and welcome to Trac!

Are you seeing an error when SVN is not installed when running env:install? This command should be instructing Docker to execute the command within the local Docker development environment, which does have SVN installed.

If the command is not working correctly and producing an error, we'll want to figure that out.

[czapla]

Hi @desrosj !

Thanks for the welcome! My first WordPress ticket as you can tell 😊

Are you seeing an error when SVN is not installed when running env:install?

That's correct.

This command should be instructing Docker to execute the command within the local Docker development environment, which does have SVN installed.

Oh, in that case, I believe that the whole command should be passed to the shell with sh -c or the second command should be prefaced with docker-compose exec -T <container> like:

docker-compose exec -T php svn checkout -r <rest of the command...>

This is per: ​https://docs.docker.com/engine/reference/commandline/exec/#extended-description

COMMAND should be an executable, a chained or a quoted command will not work. Example: docker exec -ti my_container "echo a && echo b" will not work, but docker exec -ti my_container sh -c "echo a && echo b" will.

(These are the docs for the docker command, but it works the same for docker-compose)

[desrosj]

Just noting, this was introduced in #49720.

[alexstine]

debug.log from my local repo.

[alexstine]

I experienced an error where svn returned command not found killing npm run env:install. I've attached my debug.log.

Hope it helps.

[jnylen0]

Replying to czapla:

This command should be instructing Docker to execute the command within the local Docker development environment, which does have SVN installed.

Oh, in that case, I believe that the whole command should be passed to the shell with sh -c or the second command should be prefaced with docker-compose exec -T <container> like:

This is correct. execSync( 'docker-compose exec -T php commandA && commandB' ) will effectively run two commands, docker-compose exec -T commandA and commandB, because the && is interpreted as a command separator by the underlying shell, and never gets passed to docker-compose.

Also it seems like this may not work in Windows if it is supposed to (&& is a Linux-ism). A good way to restructure this could be as follows (needs testing):

execSync( `docker-compose exec -T php rm -rf ${test_plugin_directory}`, { stdio: 'inherit' } );
execSync( `docker-compose exec -T php svn checkout -r ${process.env.WP_IMPORTER_REVISION} https://plugins.svn.wordpress.org/wordpress-importer/trunk/ ${test_plugin_directory}`, { stdio: 'inherit' } );

[desrosj]

#53133 was marked as a duplicate.

[desrosj]

I did some digging on this today and it appears I was mistaken when I said the Docker container had SVN installed. There are a few options to fix this.

1. Add SVN to the containers and use the example from @jnylen0 above.

I don't love this. The containers are getting pretty large, so I'm hesitating to install another package when there could be a better solution. With the exception of this script, I don't think anyone will be looking to use SVN commands directly within the container after running the install script.

2. Switch to using Git (which is installed on the containers)

This would allow us to continue targeting a specific revision to use in testing. This helps when changes are made that are not released for a period of time.

3. Make the Importer available via Composer.

This would allow wordpress/importer to be specified in the composer.json file.

4. Use a URL to download and unzip the plugin.

With options 3 & 4, the ability to target specific revisions may potentially be lost as those methods would be tied to specific versions.

[jnylen0]

There is another option: bundle the importer plugin with the source tree, which eliminates this step entirely. This is the path that ​ClassicPress has taken.

This adds another infrequent maintenance task but makes things easier and quicker for contributors setting up a development environment. Also, the importer plugin is ​not updated very often.

[alexstine]

My vote would be for option 2. If the WordPress Importer is ever updated, and it needs some updates, would be easier for testing this way. Anyway, any option is better than just a complete fail.

Thanks.

[desrosj]

5.8 beta deadline is today, but this only affects build/test tools and changes are allowed during the beta cycle. I'd like to resolve this during this cycle, so leaving this in the milestone a bit longer.

Trac ticket: https://core.trac.wordpress.org/ticket/52909

[desrosj]

In 51179:

Build/Test Tools: Use Git when fetching the WordPress Importer for use in tests.

This switches to using Git in the local Docker environment install script to check out a copy of the WordPress Importer plugin for use in unit tests.

Previously, SVN was used and the commands were not correctly run within the Docker container. The container does not actually have SVN installed, and the script was only working when the machine running the command had SVN present.

Props czapla, alexstine, jnylen0, francina, desrosj.
Fixes #52909.

Merged into Core in https://core.trac.wordpress.org/changeset/51179.