WordPress.org

Make WordPress Core

Opened 5 months ago

Last modified 2 months ago

#49635 new enhancement

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

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

Description

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 (5)

#1 @azaozz
5 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
5 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.


2 months ago

#4 follow-up: @SergeyBiryukov
2 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
2 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 2 months ago by azaozz (previous) (diff)
Note: See TracTickets for help on using tickets.