From 05952b194e2342a96a2cb6a36b445c75221d138d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Ribari=C4=87?= Date: Sat, 20 Mar 2021 01:50:12 +0100 Subject: [PATCH 1/3] Update initConfig comment to reflect reality --- packages/env/lib/init-config.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/env/lib/init-config.js b/packages/env/lib/init-config.js index 363ed72bff9fe3..029c5307de1f56 100644 --- a/packages/env/lib/init-config.js +++ b/packages/env/lib/init-config.js @@ -19,7 +19,8 @@ const buildDockerComposeConfig = require( './build-docker-compose-config' ); /** * Initializes the local environment so that Docker commands can be run. Reads - * ./.wp-env.json, creates ~/.wp-env, and creates ~/.wp-env/docker-compose.yml. + * ./.wp-env.json, creates ~/.wp-env, ~/.wp-env/docker-compose.yml, and + * ~/.wp-env/Dockerfile. * * @param {Object} options * @param {Object} options.spinner A CLI spinner which indicates progress. From fa855a7c266a22e0d1646e2df4b15b1e0e77649c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Ribari=C4=87?= Date: Sat, 20 Mar 2021 02:50:06 +0100 Subject: [PATCH 2/3] Remove unnecessary functions --- packages/env/lib/commands/start.js | 15 --------- packages/env/lib/wordpress.js | 52 ------------------------------ 2 files changed, 67 deletions(-) diff --git a/packages/env/lib/commands/start.js b/packages/env/lib/commands/start.js index 4f3a00cc927dd7..8c13ad2d57d67e 100644 --- a/packages/env/lib/commands/start.js +++ b/packages/env/lib/commands/start.js @@ -23,8 +23,6 @@ const initConfig = require( '../init-config' ); const downloadSources = require( '../download-sources' ); const { checkDatabaseConnection, - makeContentDirectoriesWritable, - makeConfigWritable, configureWordPress, setupWordPressDirectories, } = require( '../wordpress' ); @@ -141,23 +139,10 @@ module.exports = async function start( { spinner, debug, update, xdebug } ) { : [], } ); - await Promise.all( [ - makeConfigWritable( 'development', config ), - makeConfigWritable( 'tests', config ), - ] ); - // Only run WordPress install/configuration when config has changed. if ( shouldConfigureWp ) { spinner.text = 'Configuring WordPress.'; - if ( config.coreSource === null ) { - // Don't chown wp-content when it exists on the user's local filesystem. - await Promise.all( [ - makeContentDirectoriesWritable( 'development', config ), - makeContentDirectoriesWritable( 'tests', config ), - ] ); - } - try { await checkDatabaseConnection( config ); } catch ( error ) { diff --git a/packages/env/lib/wordpress.js b/packages/env/lib/wordpress.js index da632f9703b5b6..0cbd70c4ab22b9 100644 --- a/packages/env/lib/wordpress.js +++ b/packages/env/lib/wordpress.js @@ -18,56 +18,6 @@ const copyDir = util.promisify( require( 'copy-dir' ) ); * @typedef {'development'|'tests'|'all'} WPEnvironmentSelection */ -/** - * Makes the WordPress content directories (wp-content, wp-content/plugins, - * wp-content/themes) owned by the www-data user. This ensures that WordPress - * can write to these directories. - * - * This is necessary when running wp-env with `"core": null` because Docker - * will automatically create these directories as the root user when binding - * volumes during `docker-compose up`, and `docker-compose up` doesn't support - * the `-u` option. - * - * See https://github.com/docker-library/wordpress/issues/436. - * - * @param {WPEnvironment} environment The environment to check. Either 'development' or 'tests'. - * @param {WPConfig} config The wp-env config object. - */ -async function makeContentDirectoriesWritable( - environment, - { dockerComposeConfigPath, debug } -) { - await dockerCompose.exec( - environment === 'development' ? 'wordpress' : 'tests-wordpress', - 'chown www-data:www-data wp-content wp-content/plugins wp-content/themes', - { - config: dockerComposeConfigPath, - log: debug, - } - ); -} - -/** - * Makes wp-config.php owned by www-data so that WordPress can work with the - * file. - * - * @param {WPEnvironment} environment The environment to check. Either 'development' or 'tests'. - * @param {WPConfig} config The wp-env config object. - */ -async function makeConfigWritable( - environment, - { dockerComposeConfigPath, debug } -) { - await dockerCompose.exec( - environment === 'development' ? 'wordpress' : 'tests-wordpress', - 'chown www-data:www-data wp-config.php', - { - config: dockerComposeConfigPath, - log: debug, - } - ); -} - /** * Checks a WordPress database connection. An error is thrown if the test is * unsuccessful. @@ -280,8 +230,6 @@ async function copyCoreFiles( fromPath, toPath ) { module.exports = { hasSameCoreSource, - makeContentDirectoriesWritable, - makeConfigWritable, checkDatabaseConnection, configureWordPress, resetDatabase, From 2aba6085e877cec5c2159a0842b3859824240d45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bruno=20Ribari=C4=87?= Date: Sat, 20 Mar 2021 18:23:26 +0100 Subject: [PATCH 3/3] Fix incorrect config.coreSource usage --- .../env/lib/build-docker-compose-config.js | 4 +-- .../env/test/build-docker-compose-config.js | 31 +++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/env/lib/build-docker-compose-config.js b/packages/env/lib/build-docker-compose-config.js index b17cb7bb58212e..9572e54b47a8b4 100644 --- a/packages/env/lib/build-docker-compose-config.js +++ b/packages/env/lib/build-docker-compose-config.js @@ -257,8 +257,8 @@ module.exports = function buildDockerComposeConfig( config ) { }, }, volumes: { - ...( ! config.coreSource && { wordpress: {} } ), - ...( ! config.coreSource && { 'tests-wordpress': {} } ), + ...( ! config.env.development.coreSource && { wordpress: {} } ), + ...( ! config.env.tests.coreSource && { 'tests-wordpress': {} } ), mysql: {}, 'mysql-test': {}, 'phpunit-uploads': {}, diff --git a/packages/env/test/build-docker-compose-config.js b/packages/env/test/build-docker-compose-config.js index b839d71ebcd14f..c20eff16cb9fc1 100644 --- a/packages/env/test/build-docker-compose-config.js +++ b/packages/env/test/build-docker-compose-config.js @@ -115,4 +115,35 @@ describe( 'buildDockerComposeConfig', () => { expectedVolumes ); } ); + + it( 'should create "wordpress" and "tests-wordpress" volumes if they are needed by containers', () => { + // CONFIG has no coreSource entry, so there are no core sources on the + // local filesystem, so a volume should be created to contain core + // sources. + const dockerConfig = buildDockerComposeConfig( { + env: { development: CONFIG, tests: CONFIG }, + } ); + + expect( dockerConfig.volumes.wordpress ).not.toBe( undefined ); + expect( dockerConfig.volumes[ 'tests-wordpress' ] ).not.toBe( + undefined + ); + } ); + + it( 'should NOT create "wordpress" and "tests-wordpress" volumes if they are not needed by containers', () => { + const envConfig = { + ...CONFIG, + coreSource: { + path: '/some/random/path', + local: true, + }, + }; + + const dockerConfig = buildDockerComposeConfig( { + env: { development: envConfig, tests: envConfig }, + } ); + + expect( dockerConfig.volumes.wordpress ).toBe( undefined ); + expect( dockerConfig.volumes[ 'tests-wordpress' ] ).toBe( undefined ); + } ); } );