Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build Plugin: Simplify and improve zip contents #65232

Merged
merged 3 commits into from
Sep 12, 2024

Conversation

sirreal
Copy link
Member

@sirreal sirreal commented Sep 11, 2024

What?

Include all of /build directory.

I noticed that the build-plugin-zip script manually lists nearly all the files in the build directory. Effectively, directories are excluded from the build directory but included for the rest of the plugin.

The only difference here is the inclusion of build/vendors/react-jsx-runtime.min.js.LICENSE.txt which seems fine (or should be excluded from the build earlier).

Exclude directory entries.

Directory structure is still preserved.

As far as I can tell, directories are included in zip files when it's important to preserve their properties (such as permissions). There doesn't seem to be a reason to use special permissions in directories included in the plugin zip, so all of the directory entries can likely be excluded from the zip file.

Use long zip options

This is simply for clarity, rather than -r and -D, use the long form of the options.

Changes to zip

You can see in the diff there's a single license text file addition and a number of directory entries are excluded.

The plugin zip file size changes from 11390087 bytes on trunk to 11387680 on this branch. The zip file produced on this branch is ~2KB smaller.

Diff of zip contents
diff --git before/dev/fd/11 after/dev/fd/14
--- before/dev/fd/11
+++ after/dev/fd/14
@@ -811,6 +811,7 @@ build/vendors/react-dom.js
 build/vendors/react-dom.min.js
 build/vendors/react-jsx-runtime.js
 build/vendors/react-jsx-runtime.min.js
+build/vendors/react-jsx-runtime.min.js.LICENSE.txt
 build/vendors/react.js
 build/vendors/react.min.js
 build/viewport/index.js
@@ -837,10 +838,8 @@ build/wordcount/index.min.js
 build/wordcount/index.min.js.map
 changelog.txt
 gutenberg.php
-lib/
 lib/README.md
 lib/block-editor-settings.php
-lib/block-supports/
 lib/block-supports/background.php
 lib/block-supports/block-style-variations.php
 lib/block-supports/border.php
@@ -864,13 +863,9 @@ lib/class-wp-theme-json-gutenberg.php
 lib/class-wp-theme-json-resolver-gutenberg.php
 lib/class-wp-theme-json-schema-gutenberg.php
 lib/client-assets.php
-lib/compat/
-lib/compat/plugin/
 lib/compat/plugin/edit-site-routes-backwards-compat.php
 lib/compat/plugin/fonts.php
-lib/compat/wordpress-6.6/
 lib/compat/wordpress-6.6/admin-bar.php
-lib/compat/wordpress-6.6/block-bindings/
 lib/compat/wordpress-6.6/block-bindings/pattern-overrides.php
 lib/compat/wordpress-6.6/block-editor.php
 lib/compat/wordpress-6.6/block-template-utils.php
@@ -879,7 +874,6 @@ lib/compat/wordpress-6.6/class-gutenberg-rest-global-styles-revisions-controller
 lib/compat/wordpress-6.6/class-gutenberg-rest-templates-controller-6-6.php
 lib/compat/wordpress-6.6/class-gutenberg-token-map-6-6.php
 lib/compat/wordpress-6.6/compat.php
-lib/compat/wordpress-6.6/html-api/
 lib/compat/wordpress-6.6/html-api/class-gutenberg-html-decoder-6-6.php
 lib/compat/wordpress-6.6/html-api/class-gutenberg-html-open-elements-6-6.php
 lib/compat/wordpress-6.6/html-api/class-gutenberg-html-processor-6-6.php
@@ -891,7 +885,6 @@ lib/compat/wordpress-6.6/option.php
 lib/compat/wordpress-6.6/post.php
 lib/compat/wordpress-6.6/resolve-patterns.php
 lib/compat/wordpress-6.6/rest-api.php
-lib/compat/wordpress-6.7/
 lib/compat/wordpress-6.7/block-bindings.php
 lib/compat/wordpress-6.7/block-templates.php
 lib/compat/wordpress-6.7/blocks.php
@@ -900,7 +893,6 @@ lib/compat/wordpress-6.7/class-gutenberg-rest-templates-controller-6-7.php
 lib/compat/wordpress-6.7/class-gutenberg-token-map-6-7.php
 lib/compat/wordpress-6.7/class-wp-block-templates-registry.php
 lib/compat/wordpress-6.7/compat.php
-lib/compat/wordpress-6.7/html-api/
 lib/compat/wordpress-6.7/html-api/class-gutenberg-html-active-formatting-elements-6-7.php
 lib/compat/wordpress-6.7/html-api/class-gutenberg-html-attribute-token-6-7.php
 lib/compat/wordpress-6.7/html-api/class-gutenberg-html-decoder-6-7.php
@@ -916,8 +908,6 @@ lib/compat/wordpress-6.7/html-api/class-gutenberg-html-unsupported-exception-6-7
 lib/compat/wordpress-6.7/rest-api.php
 lib/compat/wordpress-6.7/script-modules.php
 lib/demo.php
-lib/experimental/
-lib/experimental/assets/
 lib/experimental/assets/tinymce-proxy.js
 lib/experimental/block-editor-settings-mobile.php
 lib/experimental/blocks.php
@@ -926,8 +916,6 @@ lib/experimental/class-wp-rest-block-editor-settings-controller.php
 lib/experimental/data-views.php
 lib/experimental/disable-tinymce.php
 lib/experimental/editor-settings.php
-lib/experimental/font-face/
-lib/experimental/font-face/bc-layer/
 lib/experimental/font-face/bc-layer/class-gutenberg-fonts-api-bc-layer.php
 lib/experimental/font-face/bc-layer/class-wp-fonts-provider-local.php
 lib/experimental/font-face/bc-layer/class-wp-fonts-provider.php
@@ -944,15 +932,12 @@ lib/experimental/full-page-client-side-navigation.php
 lib/experimental/kses-allowed-html.php
 lib/experimental/kses.php
 lib/experimental/l10n.php
-lib/experimental/media/
 lib/experimental/media/class-gutenberg-rest-attachments-controller.php
 lib/experimental/media/load.php
 lib/experimental/navigation-theme-opt-in.php
-lib/experimental/posts/
 lib/experimental/posts/load.php
 lib/experimental/rest-api.php
 lib/experimental/script-modules.php
-lib/experimental/sync/
 lib/experimental/sync/README.md
 lib/experimental/sync/class-gutenberg-http-signaling-server.php
 lib/experimental/synchronization.php

Noticed while working on #65064.

Testing Instructions

A good way to test is with the playground. It relies on the plugin zip compiled for this PR. Gutenberg should continue to work as expected.

@sirreal sirreal force-pushed the update/simplify-build-plugin-zip-build-files branch from 143860c to ea877be Compare September 11, 2024 09:40
@sirreal sirreal changed the title Build Plugin: Include build directory instead of listing files Build Plugin: Simplify and improve zip contents Sep 11, 2024
@sirreal sirreal marked this pull request as ready for review September 11, 2024 10:26
Copy link

github-actions bot commented Sep 11, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: sirreal <[email protected]>
Co-authored-by: gziolo <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@sirreal sirreal requested a review from gziolo September 11, 2024 10:26
Copy link

Flaky tests detected in e52b23c.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10809351596
📝 Reported issues:

@sirreal sirreal added the [Type] Build Tooling Issues or PRs related to build tooling label Sep 11, 2024
@sirreal sirreal force-pushed the update/simplify-build-plugin-zip-build-files branch from e52b23c to 6ba9c59 Compare September 11, 2024 11:18
@gziolo gziolo requested a review from ockham September 12, 2024 08:01
@gziolo
Copy link
Member

gziolo commented Sep 12, 2024

build/vendors/react-jsx-runtime.min.js.LICENSE.txt - it's fine to leave it in the folder. However, that probably means we copy too much from the package with webpack.

Copy link
Member

@gziolo gziolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with Playground and everything continues to work.

@sirreal
Copy link
Member Author

sirreal commented Sep 12, 2024

build/vendors/react-jsx-runtime.min.js.LICENSE.txt - it's fine to leave it in the folder. However, that probably means we copy too much from the package with webpack.

My thinking exactly 👍 should be addressed at an earlier stage.

@sirreal
Copy link
Member Author

sirreal commented Sep 12, 2024

I think the license file is because Terser extracts is as part of the vendors build. Other builds have terser configured not to extract comments:

'react-jsx-runtime': {
import: 'react/jsx-runtime',
global: 'ReactJSXRuntime',
},

extractComments: false,

I'm not going to dig into that, the license file isn't hurting anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants