#52909 closed defect (bug) (fixed)
git repo README.md should include instructions to install svn
Reported by: |
|
Owned by: |
|
---|---|---|---|
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)
Change History (36)
#1
@
4 years ago
- Component changed from General to Build/Test Tools
- Keywords reporter-feedback added
#2
follow-up:
↓ 6
@
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, butdocker 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
#5
@
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
@
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 withdocker-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' } );
#8
@
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.
- 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.
- 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.
- Make the Importer available via Composer.
This would allow wordpress/importer
to be specified in the composer.json
file.
- 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
@
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
@
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
@
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
Trac ticket: https://core.trac.wordpress.org/ticket/52909
#13
@
4 years ago
- Owner set to desrosj
- Resolution set to fixed
- Status changed from new to closed
In 51179:
4 years ago
#14
Merged into Core in https://core.trac.wordpress.org/changeset/51179.
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.