Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@wordpress/env: Set owner of wp-content to www-data #20406

Merged
merged 1 commit into from
Feb 28, 2020
Merged

@wordpress/env: Set owner of wp-content to www-data #20406

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions packages/env/lib/env.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ module.exports = {
await module.exports.stop( { spinner, debug } );

await checkForLegacyInstall( spinner );

const config = await initConfig( { spinner, debug } );

spinner.text = 'Downloading WordPress.';
Expand Down Expand Up @@ -114,6 +115,14 @@ module.exports = {
log: config.debug,
} );

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 ) {
Expand Down Expand Up @@ -355,6 +364,35 @@ async function copyCoreFiles( fromPath, toPath ) {
} );
}

/**
* 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 {string} environment The environment to check. Either 'development' or 'tests'.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion. I've had the feedback before to document this slightly differently, I think since this was merged:
https://github.com/WordPress/gutenberg/pull/18920/files#diff-78efab576563261e860347a820aa0997R234

You can inline a bit like this or I think consider a typedef if alignment gets difficult.

Suggested change
* @param {string} environment The environment to check. Either 'development' or 'tests'.
* @param {'development'|'tests'} environment The environment to check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! This exists in a few other places and I can spot a few other JSDoc mistakes so I'll tackle this separately.

* @param {Config} 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,
}
);
}

/**
* Performs the given action again and again until it does not throw an error.
*
Expand Down