Skip to content

maint(windows): consider GO_FAST env variable in add_zip_files 🗜️#14338

Merged
ermshiperete merged 5 commits intomasterfrom
maint/windows/addzip
Jul 23, 2025
Merged

maint(windows): consider GO_FAST env variable in add_zip_files 🗜️#14338
ermshiperete merged 5 commits intomasterfrom
maint/windows/addzip

Conversation

@ermshiperete
Copy link
Copy Markdown
Contributor

@ermshiperete ermshiperete commented Jul 10, 2025 •
edited
Loading

This changes makes use of the logic that used to be in win/zip.inc.sh: if the environment variable GO_FAST is set to 1, we use a low compression level, resulting in faster builds. If it is set to 0 we use maximum compression, resulting in slower builds. If GO_FAST is not set we don't specify the compression level and rely on the defaults.

Also make use of the new functionality by replacing the use of the wzzip function.

Note: GO_FAST gets set in some existing scripts.

Test-bot: skip

@github-project-automation github-project-automation bot moved this to Todo in Keyman Jul 10, 2025
@keymanapp-test-bot
Copy link
Copy Markdown

keymanapp-test-bot bot commented Jul 10, 2025 •
edited
Loading

User Test Results

Test specification and instructions

User tests are not required

@keymanapp-test-bot keymanapp-test-bot bot added this to the A19S7 milestone Jul 10, 2025
@github-actions github-actions bot added windows/ developer/ oem/ oem/fv/ oem/fv/windows/ windows/config/ resources/ maint Maintenance work -- continuous integration, build scripts, infrastructure labels Jul 10, 2025
@ermshiperete ermshiperete changed the title maint(windows): consider GO_FAST env variable maint(windows): consider GO_FAST env variable in add_zip_files 🗜️ Jul 10, 2025
rc-swag
rc-swag approved these changes Jul 10, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@rc-swag rc-swag left a comment

Choose a reason for hiding this comment

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

lgtm
Is it correct the GO_FAST ability has been added but it isn't actually used in the build script?

@darcywong00
Copy link
Copy Markdown
Contributor

darcywong00 commented Jul 11, 2025

Looked up some of the history of GO_FAST
#13812 (comment)

Seems like the use case to make test builds faster

@ermshiperete ermshiperete mentioned this pull request Jul 11, 2025
Merged
@ermshiperete ermshiperete force-pushed the maint/windows/addzip branch from 29a625d to b79595d Compare July 11, 2025 06:52
@darcywong00
Copy link
Copy Markdown
Contributor

darcywong00 commented Jul 11, 2025

Is there another usage at

keyman/windows/src/engine/inst/build.sh

Line 23 in b79595d

. "$KEYMAN_ROOT/resources/build/win/zip.inc.sh"
?

@ermshiperete
Copy link
Copy Markdown
Contributor Author

ermshiperete commented Jul 14, 2025

Is there another usage at

keyman/windows/src/engine/inst/build.sh

Line 23 in b79595d

. "$KEYMAN_ROOT/resources/build/win/zip.inc.sh"

?

I overlooked that file reference. It doesn't do anything with the file, so I just removed the reference. Thanks!

rc-swag
rc-swag approved these changes Jul 14, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@rc-swag rc-swag left a comment

Choose a reason for hiding this comment

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

lgtm

@keyman-server keyman-server modified the milestones: A19S7, A19S8 Jul 20, 2025
@ermshiperete
maint(windows): consider GO_FAST env variable
501c666 
This changes makes use of the logic that used to be in `win/zip.inc.sh`:
if the environment variable `GO_FAST` is set to 1, we use a low
compression level, resulting in faster builds. If it is set to 0 we
use maximum compression, resulting in slower builds. If `GO_FAST` is
not set we don't specify the compression level and rely on the defaults.

Test-bot: skip
@ermshiperete
maint(windows): replace use of wzzip with add_zip_files
4a2cf53 
This makes use of the new functionality refactored in the previous
commit by replacing the use of the `wzzip` function with `add_zip_files`.

Test-bot: skip
@ermshiperete
maint(windows): remove another (unused) include of win/zip.inc.sh
e4c1867 
Addresses code review comment.
@ermshiperete ermshiperete force-pushed the maint/windows/addzip branch from 57aaa35 to e4c1867 Compare July 21, 2025 08:27
@ermshiperete ermshiperete changed the base branch from maint/common/zipfile to master July 21, 2025 08:27
@ermshiperete ermshiperete force-pushed the maint/windows/addzip branch from 41cd44a to 71af4a9 Compare July 22, 2025 15:30
@ermshiperete ermshiperete merged commit 193fb75 into master Jul 23, 2025
29 checks passed
@ermshiperete ermshiperete deleted the maint/windows/addzip branch July 23, 2025 05:55
@github-project-automation github-project-automation bot moved this from Todo to Done in Keyman Jul 23, 2025
@keyman-server
Copy link
Copy Markdown
Collaborator

keyman-server commented Jul 23, 2025

Changes in this pull request will be available for download in Keyman version 19.0.88-alpha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcdurdin mcdurdin mcdurdin approved these changes

@rc-swag rc-swag rc-swag approved these changes

@darcywong00 darcywong00 Awaiting requested review from darcywong00

Assignees

No one assigned

Labels

developer/ maint Maintenance work -- continuous integration, build scripts, infrastructure oem/fv/windows/ oem/fv/ oem/ resources/ windows/config/ windows/engine/ windows/

Projects

Keyman
Archived in project

Milestone

A19S8

Development

Successfully merging this pull request may close these issues.

5 participants

@ermshiperete @darcywong00 @keyman-server @mcdurdin @rc-swag