Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 3 months ago

#52909 closed defect (bug) (fixed)

git repo README.md should include instructions to install svn

Reported by: czapla's profile czapla Owned by: desrosj's profile desrosj
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch
Focuses: docs Cc:

Description

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

Attachments (1)

debug.log (2.4 KB) - added by alexstine 4 years ago.
debug.log from my local repo.

Download all attachments as: .zip

Change History (36)

#1 @desrosj
4 years ago

  • Component changed from General to Build/Test Tools
  • Keywords reporter-feedback added

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.

#2 follow-up: @czapla
4 years ago

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)

This ticket was mentioned in Slack in #core by desrosj. View the logs.


4 years ago

#4 @desrosj
4 years ago

Just noting, this was introduced in #49720.

@alexstine
4 years ago

debug.log from my local repo.

#5 @alexstine
4 years ago

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

Hope it helps.

#6 in reply to: ↑ 2 @jnylen0
4 years ago

  • Keywords reporter-feedback removed

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' } );
Last edited 4 years ago by jnylen0 (previous) (diff)

#7 @desrosj
4 years ago

#53133 was marked as a duplicate.

#8 @desrosj
4 years ago

  • Milestone changed from Awaiting Review to 5.8

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.

  1. 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.

  1. Make the Importer available via Composer.

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

  1. 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.

#9 @jnylen0
4 years ago

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.

#10 @alexstine
4 years ago

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.

#11 @desrosj
4 years ago

  • Keywords needs-patch added

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.

This ticket was mentioned in PR #1387 on WordPress/wordpress-develop by desrosj.


4 years ago
#12

  • Keywords has-patch added; needs-patch removed

#13 @desrosj
4 years ago

  • Owner set to desrosj
  • Resolution set to fixed
  • Status changed from new to closed

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.

This ticket was mentioned in Slack in #core by sergey. View the logs.


4 years ago

#16 @swissspidy
3 months ago

In 59601:

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.

Merges [51179] to the 5.3 branch.
Reviewed by desrosj.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#17 @swissspidy
3 months ago

In 59608:

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.

Merges [51179] to the 5.4 branch.
Reviewed by desrosj.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#18 @swissspidy
3 months ago

In 59609:

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.

Merges [51179] to the 5.5 branch.
Reviewed by desrosj.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#19 @swissspidy
3 months ago

In 59610:

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.

Merges [51179] to the 5.6 branch.
Reviewed by desrosj.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#20 @swissspidy
3 months ago

In 59611:

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.

Merges [51179] to the 5.7 branch.
Reviewed by desrosj.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#21 @swissspidy
3 months ago

In 59612:

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.

Merges [51179] to the 5.2 branch.
Reviewed by desrosj.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#22 @swissspidy
3 months ago

In 59615:

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.

Merges [51179] to the 5.1 branch.
Reviewed by desrosj.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#23 @desrosj
3 months ago

In 59616:

Build/Test Tools: Fix bad merge in [59614].

Unprops desrosj.
See #52909.
See #62280.

#24 @desrosj
3 months ago

In 59617:

Build/Test Tools: Fix bad merge in [59614].

Unprops desrosj.
See #52909.
See #62280.

#25 @desrosj
3 months ago

In 59618:

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.

Merges [51179] to the 4.3 branch.
Reviewed by swissspidy.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#26 @desrosj
3 months ago

In 59619:

Build/Test Tools: Pin specific Importer version.

This pins the last version compatible with WordPress 4.1 to that branch.

Follow up to [59612], [59616].
Reviewed by swissspidy.

See #52909.
See #62280.

#27 @desrosj
3 months ago

In 59620:

Build/Test Tools: Pin specific Importer version.

This pins the last version compatible with WordPress 4.2 to that branch.

Follow up to [59614], [59617].
Reviewed by swissspidy.

See #52909.
See #62280.

#28 @desrosj
3 months ago

In 59621:

Build/Test Tools: Pin specific Importer version.

This pins the last version compatible with WordPress 4.3 to that branch.

Follow up to [59618].
Reviewed by swissspidy.

See #52909.
See #62280.

#29 @desrosj
3 months ago

In 59623:

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.

Merges [51179] to the 4.5 branch.
Reviewed by swissspidy.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#30 @desrosj
3 months ago

In 59624:

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.

Merges [51179] to the 4.6 branch.
Reviewed by swissspidy.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#31 @desrosj
3 months ago

In 59625:

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.

Merges [51179] to the 4.7 branch.
Reviewed by swissspidy.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#32 @desrosj
3 months ago

In 59626:

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.

Merges [51179] to the 4.8 branch.
Reviewed by swissspidy.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#33 @desrosj
3 months ago

In 59627:

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.

Merges [51179] to the 4.9 branch.
Reviewed by swissspidy.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#34 @desrosj
3 months ago

In 59628:

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.

Merges [51179] to the 5.0 branch.
Reviewed by swissspidy.

Props czapla, alexstine, jnylen0, francina, desrosj.
See #52909.
See #62280.

#35 @desrosj
3 months ago

In 59629:

Build/Test Tools: Pin specific Importer version.

This pins the last version compatible with WordPress 5.1 to that branch.

Follow up to [59615].
Reviewed by swissspidy.

See #52909.
See #62280.

Note: See TracTickets for help on using tickets.