Add new --dev flag to allow building and cleaning /src again.

[omarreiss]

This ticket introduces a --dev flag to the build, copy, watch and clean tasks, meant specifically for development. This makes it possible to develop from /src again.

When running grunt build --dev we only run grunt build:js and grunt build:css. When running a regular grunt build we also include grunt build:files, which also copies over the PHP. This means we've also separated the different build concerns into different build tasks that could be run separately (extra win! 🎉).

We first tried an approach which used symlinking. However, this gave too many issues on too many systems so we reverted back to an approach which allows building into /src

This ticket was inspired by issues that were raised after #43055 got committed, mostly in the #core-committers channel on Slack. The main issues I've seen are the following:

• Some people have lots of plugins in their wp-content folder. Building means copying all the plugin files over to /build. For some this crashes.
• Developing with grunt watch can give issues on some development environments that run in VirtualBox (like VVV), where changes aren't being picked up. Having to rebuild manually after each change is a hassle.
• XDebug breakpoints set in src/ wouldn't be picked up.

I believe these issues should mostly be mitigated or resolved through this ticket. More testing needed.

[netweb]

Looking at ​https://www.npmjs.com/package/grunt-contrib-symlink#usage-tips-on-microsoft-windows:

Usage tips on Microsoft Windows

Make sure your command prompt has administrative privileges, otherwise the task will not work.

This is more than likely going to be an issue for the majority of Windows users, I expect it will be particularly troublesome if users are using the PowerShell terminal over cmd.exe also.

[flixos90]

Tested this in VVV while working on #44466, and it works great for me!

Although we typically tend to commit things when we are fairly sure they are solid, for the sake of getting actual usage and testing, I'd suggest to commit iteratively here. What is currently available could go in right now, and if/as issues come up, we can solve them gradually.

[netweb]

Replying to flixos90:

Although we typically tend to commit things when we are fairly sure they are solid, for the sake of getting actual usage and testing (and generally improving the workflow right away), I'd suggest to commit iteratively here. What is currently available could go in right now, and if/as issues come up, we can solve them gradually.

Let's get some Windows testing reported before committing this, @azaozz @SergeyBiryukov if you could please :)

[omarreiss]

This is more than likely going to be an issue for the majority of Windows users, I expect it will be particularly troublesome if users are using the PowerShell terminal over cmd.exe also.

@netweb I've made it so that it gracefully falls back to copying files when that happens. When symlinking causes errors, I also output the following warning:
Error: failed to create a symlink on your system. Will use copy instead.

When running grunt with --verbose it will also output the exact error message.

I think this should be relatively safe to commit, also since the build:dev task is an addition to what is already there and doesn't really alter anything else.

[netweb]

Replying to omarreiss:

This is more than likely going to be an issue for the majority of Windows users, I expect it will be particularly troublesome if users are using the PowerShell terminal over cmd.exe also.

@netweb I've made it so that it gracefully falls back to copying files when that happens. When symlinking causes errors, I also output the following warning:
Error: failed to create a symlink on your system. Will use copy instead.

Great 💯

When running grunt with --verbose it will also output the exact error message.

I think this should be relatively safe to commit, also since the build:dev task is an addition to what is already there and doesn't really alter anything else.

I'd still like to see some feedback from core committers that use Windows before this is committed please.

[flixos90]

I think this should be relatively safe to commit, also since the ​build:dev task is an addition to what is already there and doesn't really alter anything else.

I agree with this. Let's make contributors' lives better as soon as we can, and iterate to the point where we are able to accomplish that for everyone.

@netweb I definitely agree that we need to dive into the issue and make it work on Windows too. But committing what we have at the moment will not make it harder for you to do what you were able to do before. Therefore I suggest doing a first commit now, but not closing the ticket with it.

[netweb]

Replying to flixos90:

I think this should be relatively safe to commit, also since the ​build:dev task is an addition to what is already there and doesn't really alter anything else.

I agree with this. Let's make contributors' lives better as soon as we can, and iterate to the point where we are able to accomplish that for everyone.

@netweb I definitely agree that we need to dive into the issue and make it work on Windows too. But committing what we have at the moment will not make it harder for you to do what you were able to do before. Therefore I suggest doing a first commit now, but not closing the ticket with it.

Sorry but I disagree, we should ensure that the core committers who use Windows can use this first please, recent build tool changes have already caught out a lead developer (see #44262) because there was no Windows testing.

[SergeyBiryukov]

Tested build-with-symlinks.patch​ on Windows -- grunt build:dev task works as expected, requires to be run as an administrator though for symlinks to be created properly (that's probably also to be expected).

[omarreiss]

Rebased

[omarreiss]

Makes clean task a little bit less eager, cleans the symlinks before cleaning the files, shows better errors when symlinking fails.

[SergeyBiryukov]

Found an issue with the approach: when using symlinks, wp-load.php is included from the src/ directory, which means ABSPATH also points to src/, leading to a warning in the admin:

WARNING: S:\home\wordpress.test\develop\src\wp-includes\formatting.php:5434 - readfile(S:\home\wordpress.test\develop\src/wp-includes/js/wp-emoji-loader.js): failed to open stream: No such file or directory

The warning comes from _print_emoji_detection_script():

<?php readfile( ABSPATH . WPINC . '/js/wp-emoji-loader.js' ); ?>

At a glance, these functions are also affected:

• rich_edit_exists():

  $wp_rich_edit_exists = file_exists( ABSPATH . WPINC . '/js/tinymce/tinymce.js' );
  

• get_post_embed_html():

  $output .= file_get_contents( ABSPATH . WPINC . '/js/wp-embed.js' );
  

• print_embed_scripts():

  readfile( ABSPATH . WPINC . '/js/wp-embed-template.js' );
  

ABSPATH is set as:

if ( ! defined( 'ABSPATH' ) ) {
	define( 'ABSPATH', dirname( __FILE__ ) . '/' );
}

Apparently __FILE__ always resolves symlinks. $_SERVER['SCRIPT_FILENAME'] could help, but it points to the main script (e.g. wp-admin/index.php), not the currently included file, so dirname( $_SERVER['SCRIPT_FILENAME'] ) does not give the correct path.

Creating a wp-config.php file in the build/ directory does not help either, it's simply ignored.

Patching wp-load.php is the only workaround I can think of at the moment:

if ( ! defined( 'ABSPATH' ) ) {
	define( 'ABSPATH', preg_replace( '#src$#', 'build', dirname( __FILE__ ) ) . '/' );
}

[SergeyBiryukov]

• It looks like files inside wp-admin are symlinked, but not the files in subfolders, e.g. wp-admin/includes, which are copied instead. Same for wp-includes, e.g. wp-includes/customize or wp-includes/rest-api.
• The error in build:dev says: "Failed to delete symlinks". Seems like a copy/paste from clean-all, should probably be "Failed to create symlinks".

[SergeyBiryukov]

Replying to SergeyBiryukov:

It looks like files inside wp-admin are symlinked, but not the files in subfolders, e.g. wp-admin/includes, which are copied instead. Same for wp-includes, e.g. wp-includes/customize or wp-includes/rest-api.

Nevermind, missed that subfolders are symlinks themselves.

[omarreiss]

Makes sure wp-load.php isn't symlinked because the file needs to be physically in the build folder.

[SergeyBiryukov]

I tried copying wp-load.php to build directory last night, but it didn't work in my testing.

build-with-symlinks4.patch​ doesn't seem to work either. The file does physically exist in the build/ folder now, but the files that include it (pretty much any core PHP file) still include the one from the src/ folder, as that's what require( dirname( __FILE__ ) . '/wp-load.php' ); resolves to.

[omarreiss]

I've updated the description of this ticket to describe the new approach. What isn't covered yet by build-into-src.diff is that the index.php in /src still assumes WordPress can't be run from /src anymore. We need to come up with a different solution to signal devs they need to build.

[atimmer]

In 44359:

Build tools: Allow building WordPress to src.

After the JavaScript reorganization in [43309], it was no longer possible to test WordPress from the src folder. That meant a build step was required to test PHP modifications. That is suboptimal as even a simple copy is slower than a web server just serving the new file.

We achieve building to src by setting a WORKING_DIR constant in the Gruntfile that is build by default, but changes to src when the --dev flag is present on any Grunt command. We provide sensible defaults so some commands, such as copying version.php, always build to build.

Because testing from build is no longer required, we change the messages present in index.php and wp-admin/index.php to be more broadly about building WordPress.

We also change the webpack config to have more straightforward behavior based on the buildTarget argument. It only determines the build target now and has no implicit behavior anymore. grunt build still works as it worked before, to make sure that the build server produces the same wordpress.zip we are used to.

We do all this instead of a symlink setup because symlinks don't work on every platform.

Props omarreiss, netweb, flixos90, SergeyBiryukov.
Fixes #44492.

[atimmer]

In 44361:

Build tools: Fix the travis:js build.

After [44359] it is impossible to not use ES6 syntax for some logic in the Gruntfile.js, so adjust the esversion setting for the Gruntfile.js to 6. Because the previous setting in .jshintrc was not compatible with setting esversion, set the esversion in the .jshintrc explicitly.

See #44492.

[iandunn]

It seems like .gitignore should now have an entry for src/wp-admin/js/, is that correct?

[SergeyBiryukov]

In 44365:

Build Tools: After [44359], add src/wp-admin/js to .gitignore.

Props iandunn.
See #44492.

[SergeyBiryukov]

In 44366:

Build Tools: Remove unnecessary sprintf() in translatable strings.

Add a missing sprintf() for npm install substring.

See #44492.

[atimmer]

Thanks @SergeyBiryukov, I hadn't gotten around to it yet but now I don't have to 🙂

Thanks for spotting it @iandunn.

[pento]

In 44508:

Build/Tests: Default to running unit tests from src.

This commit also defaults WP-CLI commands to running against src, too.

Props atimmer, pento.
See #44492.
Fixes #45863.

[pento]

In 44509:

Build/Tests: Default to running unit tests from src.

This is the actual commit, unlike [44508], which was not.

Props atimmer.
See #44492.
Fixes #45863.

[atimmer]

@pento I don't have the capacity to look into this before Friday. As this is build tooling it shouldn't block beta1, right?

[ocean90]

In 45688:

I18N: Use RTL stylesheets when running from /src.

To run WordPress from /src you have to use the --dev flag which also builds the RTL stylesheets thus the admin notice and force to LTR is no longer required.

See #44492.
Fixes #44865.

[ocean90]

In 48311:

Administration: Make all color schemes available when running from /src.

To run WordPress from /src you have to use the --dev flag which also builds the color scheme stylesheets thus the restriction is no longer required.

See #44492.
Fixes #50558.