WordPress.org

Make WordPress Core

Opened 3 weeks ago

Closed 7 days ago

Last modified 7 days ago

#47078 closed defect (bug) (fixed)

Building may fail to generate *-rtl.css files in wp-includes/css

Reported by: azaozz Owned by: azaozz
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch commit
Focuses: Cc:

Description

Happens when running grunt (build) after grunt --dev was run. In this case there are *.min.css, *-rtl.css and *-rtl.min.css files in both src/wp-admin/css/ and src/wp-includes/css directories. That seems to trip RTLCSS as it tries to process minified and already processed files.

Attachments (2)

47078.diff (1.2 KB) - added by azaozz 3 weeks ago.
47078.1.diff (1.0 KB) - added by azaozz 12 days ago.

Download all attachments as: .zip

Change History (10)

@azaozz
3 weeks ago

#1 @azaozz
3 weeks ago

In 47078.diff:

  • Exclude minified and already processed files when running the rtlcss:core task.
  • Also delete wp-includes/css/dist directory when running clean:css task.
  • Add svnignore *-rtl.css on wp-includes/css.

#2 @desrosj
2 weeks ago

  • Keywords has-patch needs-testing added

I was able to reproduce this and 47078.diff fixes the problem, but the wp-includes/css/dist folder is never recreated when running either command (grunt --dev or grunt (build)).

#3 @azaozz
12 days ago

but the wp-includes/css/dist folder is never recreated

Right, it is created but then "cleaned" right after. Happens because wp-includes/css/dist is created by webpack which runs in build:js. Then we get to build:css which does clean:css and removes /dist.

Looking a but "deeper", we don't really need to do any of that clean:js, clean:css stuff in /build. All WP files are deleted there anyway. Perhaps we should make these run only for grunt --dev and of course for grunt clean --dev, grunt clean:css --dev etc. We should also split the clean tasks from the build tasks and do the cleaning before we run webpack.

For now lets fix this ticket by excluding the src/wp-includes/css/dist directory from processing with rtlcss.

@azaozz
12 days ago

#4 @azaozz
12 days ago

In 47078.1.diff: exclude minified and already processed files, and files from external packages when running the rtlcss task.

#5 follow-up: @desrosj
8 days ago

  • Keywords commit added; needs-testing removed

47078.1.diff works perfectly for me, @azaozz.

#6 @azaozz
7 days ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In 45317:

Build tools: fix generating *-rtl.css files in wp-includes/css after grunt --dev was used.

Fixes #47078.

#7 in reply to: ↑ 5 @azaozz
7 days ago

Replying to desrosj:

Thanks for testing! :)

I'm actually not sure if this needs merging to the 5.2 branch. Seems sufficient to fix in trunk. Feel free to reopen and merge there if needed.

#8 @azaozz
7 days ago

  • Milestone changed from 5.2.1 to 5.3
Note: See TracTickets for help on using tickets.