Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#44256 closed defect (bug) (fixed)

Build can fail when `src` folder contains folders with many files

Reported by: adamsilverstein's profile adamsilverstein Owned by: pento's profile pento
Milestone: 5.1 Priority: normal
Severity: normal Version: 5.1
Component: Build/Test Tools Keywords: has-patch commit
Focuses: Cc:

Description (last modified by adamsilverstein)

For local development, I've typically symlinked plugins I am working on into the src/wp-content/plugins folder. These plugins sometimes include a large number of files (typically in a node_modules folder) that causes the copy part of the core build process to hang or fail. Since r43309 which requires a build step for core JavaScript and working out of the build folder, I am having trouble getting the process to complete. I've seen several mentions of this issue in core slack. It is possible this is also an issue for the themes folder for some developers.

In addition, since the build process wipes out the build folder with each build, I can't put my plugins folder in build instead of src.

One possible solution: exclude the plugins folder when wiping build and skip copying any non-core plugins during the copy phase. If we get more explicit about what we delete and copy, developers can have more flexibility in their local setups.

Attachments (5)

44256.diff (1.3 KB) - added by adamsilverstein 6 years ago.
44256.2.diff (1.7 KB) - added by adamsilverstein 6 years ago.
44256.3.diff (2.1 KB) - added by adamsilverstein 6 years ago.
44256.4.diff (2.2 KB) - added by pento 6 years ago.
44256.5.diff (1.9 KB) - added by pento 6 years ago.

Download all attachments as: .zip

Change History (26)

#1 @adamsilverstein
6 years ago

in 44256.diff :

  • limit copy to exclude plugins and wildcard prefixes
  • exclude plugins folder when cleaning, add a clean:plugins task that does clear plugins

Q: should we add similar explicit paths for theme copies/removal?

#2 @adamsilverstein
6 years ago

cc: @johnbillion with this patch, you should be able to leave plugins in the build folder - does this fix the issues you were experiencing? What do you think of the approach? any other ideas for how to resolve this?

#3 @pento
6 years ago

I like this approach, thanks @adamsilverstein!

I think we should be similarly explicit with themes, too.

#4 in reply to: ↑ description @netweb
6 years ago

Replying to adamsilverstein:

It is possible this is also an issue for the themes folder for some developers.

Replying to adamsilverstein:

Q: should we add similar explicit paths for theme copies/removal?

Yes please, themes using SASS, PostCSS etc for theme dev along with Grunt or Gulp would be quite common practice

I like the VCS change to only copy the root .git/.svn folders

Should the clean task also clean the upgrade and uploads folders?

#5 @adamsilverstein
6 years ago

Should the clean task also clean the upgrade and uploads folders?

Yes, good point, I will add that.

I like the VCS change to only copy the root .git/.svn folders

Yea, the original line:

'!**/.{svn,git}/**'

attempts to exclude any .svn or .git files from copying. The problem here is the prefix which causes the copy process to attempt to traverse every possible prefix path (eg every folder) which leads to the hung process.

In my testing at least on my local system these folders are ignored by default (in which case we could remove the line). It is possible that on other systems/setups that is not the case, I'm going to see if I can figure out where/why this line was added.

Last edited 6 years ago by adamsilverstein (previous) (diff)

#6 @adamsilverstein
6 years ago

in 44256.2.diff:

  • clean upgrade and uploads folders
  • explicit theme includes in build copy step
  • even more explicit paths

#7 @adamsilverstein
6 years ago

  • Description modified (diff)

#8 @adamsilverstein
6 years ago

  • Description modified (diff)

#9 @netweb
6 years ago

  • Why only copy the twenty 12/13/14 themes? What about twenty 15/16/17 themes?
  • I see what you did there with Hello Dolly, nice try, please fix the typo hello.php, not helo.php 🤣

#10 @adamsilverstein
6 years ago

Why only copy the twenty 12/13/14 themes? What about twenty 15/16/17 themes?

ooops, I didn't realize I had omitted those, will add.

I see what you did there with Hello Dolly, nice try, please fix the typo hello.php, not helo.php 🤣

Doh! that would have been a sneaky trick :)

#11 @adamsilverstein
6 years ago

in 44256.3.diff :

  • create the build and clean lists from the same array so same paths are always cleaned and copied. the copy files list has some additional exclusions added with concat
  • correct hello typo
  • include all themes

This ticket was mentioned in Slack in #core-js by adamsilverstein. View the logs.


6 years ago

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


6 years ago

#14 @johnbillion
6 years ago

Could the list of themes be changed to wp-content/themes/twenty* ?

#15 @johnbillion
6 years ago

  • Keywords has-patch needs-testing added
  • Milestone changed from 4.9.7 to 5.0

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


6 years ago

This ticket was mentioned in Slack in #buddypress by netweb. View the logs.


6 years ago

@pento
6 years ago

@pento
6 years ago

#18 @pento
6 years ago

  • Keywords commit added; needs-testing removed

In 44256.5.diff:

  • Replace the explicit list of themes with wp-content/themes/twenty*: we already use this shorthand in a few other places in Gruntfile.js.
  • Added the /** suffix to the themes and Akismet, so their files are copied.

This is working pretty well for me, let's land it and see what happens.

#19 @pento
6 years ago

  • Owner set to pento
  • Status changed from new to assigned

#20 @pento
6 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 43329:

Build Tools: grunt build should only copy Core files.

Historically, grunt build has copied all files from the src directory to the build directory. This is usually fine, but can be super slow when there are lots of custom plugins or themes in the src directory.

To rectify this, we now only copy Core plugins and themes to build.

Props adamsilverstein, pento, johnbillion.
Fixes #44256.

#21 @jorbin
6 years ago

  • Milestone changed from 5.0 to 5.1
Note: See TracTickets for help on using tickets.