WordPress.org

Make WordPress Core

Opened 15 months ago

Closed 15 months ago

Last modified 10 months ago

#44256 closed defect (bug) (fixed)

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

Reported by: adamsilverstein Owned by: 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 15 months ago.
44256.2.diff (1.7 KB) - added by adamsilverstein 15 months ago.
44256.3.diff (2.1 KB) - added by adamsilverstein 15 months ago.
44256.4.diff (2.2 KB) - added by pento 15 months ago.
44256.5.diff (1.9 KB) - added by pento 15 months ago.

Download all attachments as: .zip

Change History (26)

#1 @adamsilverstein
15 months 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
15 months 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
15 months ago

I like this approach, thanks @adamsilverstein!

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

#4 in reply to: ↑ description @netweb
15 months 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
15 months 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 15 months ago by adamsilverstein (previous) (diff)

#6 @adamsilverstein
15 months ago

in 44256.2.diff:

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

#7 @adamsilverstein
15 months ago

  • Description modified (diff)

#8 @adamsilverstein
15 months ago

  • Description modified (diff)

#9 @netweb
15 months 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
15 months 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
15 months 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.


15 months ago

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


15 months ago

#14 @johnbillion
15 months ago

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

#15 @johnbillion
15 months 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.


15 months ago

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


15 months ago

@pento
15 months ago

@pento
15 months ago

#18 @pento
15 months 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
15 months ago

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

#20 @pento
15 months 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
10 months ago

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