WordPress.org

Make WordPress Core

#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 21 months ago.
47078.1.diff (1.0 KB) - added by azaozz 21 months ago.

Download all attachments as: .zip

Change History (12)

@azaozz
21 months ago

#1 @azaozz
21 months 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
21 months 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
21 months 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
21 months ago

#4 @azaozz
21 months 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
21 months ago

  • Keywords commit added; needs-testing removed

47078.1.diff works perfectly for me, @azaozz.

#6 @azaozz
21 months 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
21 months 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
21 months ago

  • Milestone changed from 5.2.1 to 5.3

#9 @johnbillion
20 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

These files need to be ignored in .gitignore too.

#10 @johnbillion
20 months ago

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

In 45410:

Build/Test Tools: Ignore generated RTL CSS files from Git version control.

Fixes #47078

Note: See TracTickets for help on using tickets.