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 Workflow: Avoid double zip for plugin files #32780

Closed
wants to merge 4 commits into from

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Jun 17, 2021

Description

When uploading files as an artifact to a GHA workflow, GH automatically creates a zip file that includes the files. If the artifact file in question is a zip file itself (such as gutenberg.zip), this results in a double zip (in this case: gutenberg-plugin.zip, which includes gutenberg.zip). This is a frequent source of confusion.

This PR fixes this issue by uploading the collection of Gutenberg plugin files that are normally included in gutenberg.zip, and by relying on GHA to create the zip.

The downside is that any GH job that downloads the artifact will get that collection of files. For our release-draft-creation step, this means that it'll have to "manually" create the zip asset (to be attached to the release draft) itself.

How has this been tested?

TBD

@ockham ockham added the [Type] Build Tooling Issues or PRs related to build tooling label Jun 17, 2021
@ockham ockham self-assigned this Jun 17, 2021
@desrosj
Copy link
Contributor

desrosj commented Jun 17, 2021

Just wanted to note that we should compare performance here and consider any difference as a trade off. I've found that uploading individual files drastically increases the time it takes to upload artifacts.

@ockham
Copy link
Contributor Author

ockham commented Jun 17, 2021

Just wanted to note that we should compare performance here and consider any difference as a trade off. I've found that uploading individual files drastically increases the time it takes to upload artifacts.

Ah, wasn't aware, thanks!

According to https://github.com/WordPress/gutenberg/pull/32780/checks?check_run_id=2849536885:

Build Release Artifact
succeeded 3 hours ago in 4m 24s

(for this PR)

I had a look at a few other recent PRs. I see that the build workflow takes around 3.5 minutes for quite a few of them, but I also see 4.5 minutes, and even 7.5 minutes. I wonder if maybe the npm cache is the dominant factor here 🤔

Anyway, I'll optimize a bit more: I'll add an option to bin/build-plugin-zip.sh to allow skipping the zipping of the files, so we don't have to zip and unzip during the build job.

@ockham
Copy link
Contributor Author

ockham commented Jun 17, 2021

We could probably squeeze out a bit more performance if we moved the npm ci step out of ./bin/build-plugin-zip.sh and into the workflow, and use the usual npm caching feature there 🤔

@desrosj
Copy link
Contributor

desrosj commented Jun 17, 2021

To compare speed, look specifically at the upload step. It looks like the workflow run linked above took ~25s to upload. On other workflow runs, it looks like a zipped file takes ~1 second before this change. Some examples:

Not saying the difference disqualifies this suggestion, but it's something to consider.

Regarding npm ci instead of npm install, I am in favor of using npm ci wherever possible. The only difference between the two is that an error will occur when a required dependency is not listed in the lock file for npm ci. We guard against uncommitted differences in the lock file in the static analysis workflow: https://github.com/WordPress/gutenberg/runs/2851719820?check_suite_focus=true#step:8:1. I meant to open a PR for this but did not get around to it. While we're at it, we should include inline documentation when npm install is used to document why for each instance.

@ockham
Copy link
Contributor Author

ockham commented Jun 17, 2021

To compare speed, look specifically at the upload step. It looks like the workflow run linked above took ~25s to upload. On other workflow runs, it looks like a zipped file takes ~1 second before this change. Some examples:

Not saying the difference disqualifies this suggestion, but it's something to consider.

Ah, I forgot that we get timings for individual steps! Thanks for pointing that out. That's a rather significant difference indeed 😕

@ockham ockham force-pushed the update/build-workflow-avoid-double-zip branch from 7fb8dc2 to ddd66f8 Compare January 10, 2022 12:18
@ockham
Copy link
Contributor Author

ockham commented Jan 10, 2022

Rebased. I'll have another look at timings. If it's still as significant as reported by @desrosj, I'm inclined to close this PR.

@ockham
Copy link
Contributor Author

ockham commented Jan 12, 2022

Rebased. I'll have another look at timings. If it's still as significant as reported by @desrosj, I'm inclined to close this PR.

29 seconds now on this PR: https://github.com/WordPress/gutenberg/runs/4761132221?check_suite_focus=true 😕
Still only 1 second on recent merges to trunk: https://github.com/WordPress/gutenberg/runs/4789229322?check_suite_focus=true

Doesn't seem worth it. I'll close.

@ockham ockham closed this Jan 12, 2022
@ockham ockham deleted the update/build-workflow-avoid-double-zip branch January 12, 2022 14:20
@ockham ockham restored the update/build-workflow-avoid-double-zip branch January 12, 2022 14:23
@ockham
Copy link
Contributor Author

ockham commented Jan 12, 2022

Ah, forgot to try updating actions/upload-artifact to a more current version -- maybe it's been optimized...

@ockham ockham reopened this Jan 12, 2022
@ockham ockham force-pushed the update/build-workflow-avoid-double-zip branch from 9dc4077 to cfc886e Compare January 12, 2022 14:32
@github-actions
Copy link

Size Change: -594 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-editor/index.min.js 140 kB +118 B (0%)
build/block-library/blocks/code/theme-rtl.css 131 B +5 B (+4%)
build/block-library/blocks/code/theme.css 131 B +5 B (+4%)
build/block-library/blocks/gallery/editor-rtl.css 965 B -1 B (0%)
build/block-library/blocks/gallery/editor.css 967 B -3 B (0%)
build/block-library/blocks/navigation/style-rtl.css 1.82 kB +11 B (+1%)
build/block-library/blocks/navigation/style.css 1.81 kB +11 B (+1%)
build/block-library/blocks/navigation/view.min.js 2.82 kB +4 B (0%)
build/block-library/blocks/page-list/editor-rtl.css 401 B +16 B (+4%)
build/block-library/blocks/page-list/editor.css 402 B +15 B (+4%)
build/block-library/blocks/post-comments/style-rtl.css 521 B +12 B (+2%)
build/block-library/blocks/post-comments/style.css 521 B +12 B (+2%)
build/block-library/blocks/tag-cloud/style-rtl.css 214 B +68 B (+47%) 🚨
build/block-library/blocks/tag-cloud/style.css 215 B +69 B (+47%) 🚨
build/block-library/common-rtl.css 908 B -2 B (0%)
build/block-library/common.css 905 B -3 B (0%)
build/block-library/editor-rtl.css 10 kB -4 B (0%)
build/block-library/editor.css 10 kB -3 B (0%)
build/block-library/index.min.js 164 kB -1.17 kB (-1%)
build/block-library/style-rtl.css 10.8 kB +78 B (+1%)
build/block-library/style.css 10.8 kB +78 B (+1%)
build/block-library/theme-rtl.css 672 B +3 B (0%)
build/block-library/theme.css 676 B +2 B (0%)
build/blocks/index.min.js 46.3 kB +24 B (0%)
build/components/index.min.js 214 kB +31 B (0%)
build/core-data/index.min.js 13.3 kB +14 B (0%)
build/edit-post/index.min.js 29.6 kB -25 B (0%)
build/edit-site/index.min.js 37.2 kB -11 B (0%)
build/edit-widgets/index.min.js 16.5 kB -12 B (0%)
build/editor/index.min.js 38.3 kB +125 B (0%)
build/editor/style-rtl.css 3.71 kB -37 B (-1%)
build/editor/style.css 3.7 kB -37 B (-1%)
build/rich-text/index.min.js 11 kB +7 B (0%)
build/server-side-render/index.min.js 1.58 kB +10 B (+1%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 960 B
build/admin-manifest/index.min.js 1.1 kB
build/annotations/index.min.js 2.75 kB
build/api-fetch/index.min.js 2.21 kB
build/autop/index.min.js 2.12 kB
build/blob/index.min.js 459 B
build/block-directory/index.min.js 6.28 kB
build/block-directory/style-rtl.css 1.01 kB
build/block-directory/style.css 1.01 kB
build/block-editor/default-editor-styles-rtl.css 378 B
build/block-editor/default-editor-styles.css 378 B
build/block-editor/style-rtl.css 14.6 kB
build/block-editor/style.css 14.6 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 65 B
build/block-library/blocks/archives/style.css 65 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 111 B
build/block-library/blocks/audio/style.css 111 B
build/block-library/blocks/audio/theme-rtl.css 125 B
build/block-library/blocks/audio/theme.css 125 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 470 B
build/block-library/blocks/button/editor.css 470 B
build/block-library/blocks/button/style-rtl.css 560 B
build/block-library/blocks/button/style.css 560 B
build/block-library/blocks/buttons/editor-rtl.css 292 B
build/block-library/blocks/buttons/editor.css 292 B
build/block-library/blocks/buttons/style-rtl.css 275 B
build/block-library/blocks/buttons/style.css 275 B
build/block-library/blocks/calendar/style-rtl.css 207 B
build/block-library/blocks/calendar/style.css 207 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 90 B
build/block-library/blocks/code/style.css 90 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 406 B
build/block-library/blocks/columns/style.css 406 B
build/block-library/blocks/comment-template/style-rtl.css 127 B
build/block-library/blocks/comment-template/style.css 127 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/cover/editor-rtl.css 546 B
build/block-library/blocks/cover/editor.css 547 B
build/block-library/blocks/cover/style-rtl.css 1.22 kB
build/block-library/blocks/cover/style.css 1.22 kB
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 417 B
build/block-library/blocks/embed/style.css 417 B
build/block-library/blocks/embed/theme-rtl.css 124 B
build/block-library/blocks/embed/theme.css 124 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 255 B
build/block-library/blocks/file/style.css 255 B
build/block-library/blocks/file/view.min.js 322 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/style-rtl.css 1.6 kB
build/block-library/blocks/gallery/style.css 1.6 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 B
build/block-library/blocks/group/editor-rtl.css 159 B
build/block-library/blocks/group/editor.css 159 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 114 B
build/block-library/blocks/heading/style.css 114 B
build/block-library/blocks/html/editor-rtl.css 332 B
build/block-library/blocks/html/editor.css 333 B
build/block-library/blocks/image/editor-rtl.css 810 B
build/block-library/blocks/image/editor.css 809 B
build/block-library/blocks/image/style-rtl.css 507 B
build/block-library/blocks/image/style.css 511 B
build/block-library/blocks/image/theme-rtl.css 124 B
build/block-library/blocks/image/theme.css 124 B
build/block-library/blocks/latest-comments/style-rtl.css 284 B
build/block-library/blocks/latest-comments/style.css 284 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B
build/block-library/blocks/latest-posts/editor.css 137 B
build/block-library/blocks/latest-posts/style-rtl.css 528 B
build/block-library/blocks/latest-posts/style.css 527 B
build/block-library/blocks/list/style-rtl.css 94 B
build/block-library/blocks/list/style.css 94 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 493 B
build/block-library/blocks/media-text/style.css 490 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 649 B
build/block-library/blocks/navigation-link/editor.css 650 B
build/block-library/blocks/navigation-link/style-rtl.css 94 B
build/block-library/blocks/navigation-link/style.css 94 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 299 B
build/block-library/blocks/navigation-submenu/editor.css 299 B
build/block-library/blocks/navigation-submenu/view.min.js 343 B
build/block-library/blocks/navigation/editor-rtl.css 1.93 kB
build/block-library/blocks/navigation/editor.css 1.94 kB
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 273 B
build/block-library/blocks/paragraph/style.css 273 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/style-rtl.css 446 B
build/block-library/blocks/post-comments-form/style.css 446 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 721 B
build/block-library/blocks/post-featured-image/editor.css 721 B
build/block-library/blocks/post-featured-image/style-rtl.css 153 B
build/block-library/blocks/post-featured-image/style.css 153 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 305 B
build/block-library/blocks/post-template/style.css 305 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/post-title/style-rtl.css 80 B
build/block-library/blocks/post-title/style.css 80 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 198 B
build/block-library/blocks/pullquote/editor.css 198 B
build/block-library/blocks/pullquote/style-rtl.css 389 B
build/block-library/blocks/pullquote/style.css 388 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 234 B
build/block-library/blocks/query-pagination/style.css 231 B
build/block-library/blocks/query/editor-rtl.css 131 B
build/block-library/blocks/query/editor.css 132 B
build/block-library/blocks/quote/style-rtl.css 187 B
build/block-library/blocks/quote/style.css 187 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 397 B
build/block-library/blocks/search/style.css 398 B
build/block-library/blocks/search/theme-rtl.css 64 B
build/block-library/blocks/search/theme.css 64 B
build/block-library/blocks/separator/editor-rtl.css 99 B
build/block-library/blocks/separator/editor.css 99 B
build/block-library/blocks/separator/style-rtl.css 245 B
build/block-library/blocks/separator/style.css 245 B
build/block-library/blocks/separator/theme-rtl.css 172 B
build/block-library/blocks/separator/theme.css 172 B
build/block-library/blocks/shortcode/editor-rtl.css 474 B
build/block-library/blocks/shortcode/editor.css 474 B
build/block-library/blocks/site-logo/editor-rtl.css 744 B
build/block-library/blocks/site-logo/editor.css 744 B
build/block-library/blocks/site-logo/style-rtl.css 181 B
build/block-library/blocks/site-logo/style.css 181 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 84 B
build/block-library/blocks/site-title/editor.css 84 B
build/block-library/blocks/social-link/editor-rtl.css 177 B
build/block-library/blocks/social-link/editor.css 177 B
build/block-library/blocks/social-links/editor-rtl.css 670 B
build/block-library/blocks/social-links/editor.css 669 B
build/block-library/blocks/social-links/style-rtl.css 1.32 kB
build/block-library/blocks/social-links/style.css 1.32 kB
build/block-library/blocks/spacer/editor-rtl.css 332 B
build/block-library/blocks/spacer/editor.css 332 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 471 B
build/block-library/blocks/table/editor.css 472 B
build/block-library/blocks/table/style-rtl.css 481 B
build/block-library/blocks/table/style.css 481 B
build/block-library/blocks/table/theme-rtl.css 188 B
build/block-library/blocks/table/theme.css 188 B
build/block-library/blocks/template-part/editor-rtl.css 560 B
build/block-library/blocks/template-part/editor.css 559 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 571 B
build/block-library/blocks/video/editor.css 572 B
build/block-library/blocks/video/style-rtl.css 173 B
build/block-library/blocks/video/style.css 173 B
build/block-library/blocks/video/theme-rtl.css 124 B
build/block-library/blocks/video/theme.css 124 B
build/block-library/reset-rtl.css 474 B
build/block-library/reset.css 474 B
build/block-serialization-default-parser/index.min.js 1.09 kB
build/block-serialization-spec-parser/index.min.js 2.79 kB
build/components/style-rtl.css 15.5 kB
build/components/style.css 15.5 kB
build/compose/index.min.js 11.2 kB
build/customize-widgets/index.min.js 11.4 kB
build/customize-widgets/style-rtl.css 1.5 kB
build/customize-widgets/style.css 1.49 kB
build/data-controls/index.min.js 631 B
build/data/index.min.js 7.49 kB
build/date/index.min.js 31.9 kB
build/deprecated/index.min.js 485 B
build/dom-ready/index.min.js 304 B
build/dom/index.min.js 4.5 kB
build/edit-navigation/index.min.js 16 kB
build/edit-navigation/style-rtl.css 3.76 kB
build/edit-navigation/style.css 3.76 kB
build/edit-post/classic-rtl.css 546 B
build/edit-post/classic.css 547 B
build/edit-post/style-rtl.css 7.16 kB
build/edit-post/style.css 7.16 kB
build/edit-site/style-rtl.css 6.83 kB
build/edit-site/style.css 6.82 kB
build/edit-widgets/style-rtl.css 4.17 kB
build/edit-widgets/style.css 4.17 kB
build/element/index.min.js 3.29 kB
build/escape-html/index.min.js 517 B
build/format-library/index.min.js 6.58 kB
build/format-library/style-rtl.css 571 B
build/format-library/style.css 571 B
build/hooks/index.min.js 1.63 kB
build/html-entities/index.min.js 424 B
build/i18n/index.min.js 3.71 kB
build/is-shallow-equal/index.min.js 501 B
build/keyboard-shortcuts/index.min.js 1.8 kB
build/keycodes/index.min.js 1.39 kB
build/list-reusable-blocks/index.min.js 1.72 kB
build/list-reusable-blocks/style-rtl.css 838 B
build/list-reusable-blocks/style.css 838 B
build/media-utils/index.min.js 2.92 kB
build/notices/index.min.js 925 B
build/nux/index.min.js 2.08 kB
build/nux/style-rtl.css 747 B
build/nux/style.css 743 B
build/plugins/index.min.js 1.84 kB
build/primitives/index.min.js 924 B
build/priority-queue/index.min.js 582 B
build/react-i18n/index.min.js 671 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.65 kB
build/reusable-blocks/index.min.js 2.22 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/shortcode/index.min.js 1.49 kB
build/token-list/index.min.js 639 B
build/url/index.min.js 1.9 kB
build/viewport/index.min.js 1.05 kB
build/warning/index.min.js 248 B
build/widgets/index.min.js 7.15 kB
build/widgets/style-rtl.css 1.16 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.04 kB

compressed-size-action

@ockham
Copy link
Contributor Author

ockham commented Jan 12, 2022

Ah, forgot to try updating actions/upload-artifact to a more current version -- maybe it's been optimized...

35 seconds now 🙄 https://github.com/WordPress/gutenberg/runs/4790668381?check_suite_focus=true

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