Make WordPress Core

Opened 13 months ago

Last modified 3 weeks ago

#49635 new enhancement

Build tools: Remove generated and package sourced files from committed code.

Reported by: peterwilsoncc Owned by:
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Focuses: Cc:


Currently several PHP files either auto-generated or copied during the build process are committed in to the repositories src directory, this includes:

  • src/wp-includes/assets/script-loader-packages.php
  • wp-includes/blocks/*.php

As including built files can lead to errors during the commit process, it would be dandy if these files could be removed from the repository and added to the list of ignored files.

A consideration is if this can be and needs to be done in such a way to avoid requiring JavaScript and CSS be built in order to run the PHP unit tests. I'm sure there are other complexities too.

Related #48154

Change History (8)

#1 @azaozz
13 months ago

I'm sure there are other complexities too.

Yeah, been thinking about this for a while. One problem is that once a file is added to SVN and committed, it never goes away completely. If it is deleted, it will not be present in the new(er) revisions, but it still exists in the older revisions from before it was deleted.

This causes a problem when a file is deleted from the repository and ignored, and then recreated when building. If a previous revision is restored, and there is already an ignored file with the same name at the same location, it will cause a conflict in SVN.

A way to work around this would be to rename the newly ignored files. But renaming files in /build (in production) may trigger fatal errors in some plugins... The other option is to just ignore these problems as they would happen relatively rarely and are usually pretty easy to fix.

Any other ideas/suggestions/thoughts? :)

#2 @bookdude13
13 months ago

As @azaozz mentioned in #48154 comment 67:

script-loader-packages.php is slightly different depending on whether Webpack was run in "build" or "dev" mode.

Thoughts on renaming that script-loader-packages-prod.php and script-loader-packages-dev.php to differentiate, and then .gitignore the current script-loader-packages.php? Not sure if that solves the underlying problem, but an idea.

This ticket was mentioned in Slack in #core-committers by sergey. View the logs.

10 months ago

#4 follow-up: @SergeyBiryukov
10 months ago

It was brought up in Slack that the wp-includes/assets/script-loader-packages.php file is:

As of [47471], I think it should be removed from the develop repo.

#5 in reply to: ↑ 4 @azaozz
10 months ago

Replying to SergeyBiryukov:

As of [47471], I think it should be removed from the develop repo.

Yes, wp-includes/assets/script-loader-packages.php can be SVN deleted, then added to svn:ignore and .gitignore. The same applies to wp-includes/blocks/*.php.

The problems (as mentioned in comment:1) are:

  1. These files will be missing or will not get updated in /src when doing npm run build or grunt build or similar. This might trigger a PHP fatal when testing but seems pretty rare edge case.
  2. After the delete + ignore steps are committed, reverting to an earlier revision (where the files are still under version control) may result in SVN conflicts that could also trigger PHP fatal(s) in /src if left unfixed.

Both of these seem like pretty rare edge cases, so can probably just "bite the bullet" and do it? :)

Last edited 10 months ago by azaozz (previous) (diff)

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

4 weeks ago

#7 @peterwilsoncc
3 weeks ago

  • Milestone changed from Awaiting Review to 5.8

Putting this on the 5.8 milestone for biting said bullet, the longer the issue remains the more problems it will cause.

For ease, I suspect this is best done just prior to branching so the two supported branches at the time are in sync without the files.

#8 @desrosj
3 weeks ago

Related: #50460.

Note: See TracTickets for help on using tickets.