Skip to content

maint(common): consolidate builder scripts 🐸#14449

Merged
mcdurdin merged 5 commits intomasterfrom
maint/common/14065-consolidate-builder-scripts
Aug 12, 2025
Merged

maint(common): consolidate builder scripts 🐸#14449
mcdurdin merged 5 commits intomasterfrom
maint/common/14065-consolidate-builder-scripts

Conversation

@mcdurdin
Copy link
Copy Markdown
Member

@mcdurdin mcdurdin commented Aug 1, 2025

Clarifies the confusing builder.inc.sh / build-utils.sh distinction by giving the scripts more appropriate names. Most build scripts should use builder-full.inc.sh; some helper scripts can use builder-basic.inc.sh. Documented in resources/build/README.md.

Renames:

  • resources/build/builder.inc.sh to resources/build/builder-full.inc.sh
  • resources/build/build-utils.sh to resources/build/builder-basic.inc.sh

Other changes:

  • Moves Android-specific functions out of builder-basic.inc.sh and into android/build.sh.
  • Renames functions in builder-basic.inc.sh

More functions may be moved from builder-basic.inc.sh into utils.inc.sh or other scripts in the future.

Note: deb-packaging.yml currently depends on build-utils.sh, and is sourced from master branch -- while its imports come from the current branch for the build. So a temporary copy of build-utils.sh is retained in order for this branch's builds (and child branches) to pass, but should be removed in a follow-up PR once this branch lands.

Fixes: #14065
Build-bot: build all
Test-bot: skip

@keymanapp-test-bot
Copy link
Copy Markdown

keymanapp-test-bot bot commented Aug 1, 2025

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

@mcdurdin mcdurdin force-pushed the maint/common/14065-consolidate-builder-scripts branch from 4495a4a to f394245 Compare August 1, 2025 22:11
Comment on lines +111 to +122

# For CI compatibility of building Keyman for Android 18.0 with OpenJDK 11,
# this overrides JAVA_HOME for the builder script to use OpenJDK 21.
android_set_java_home() {
if [[ ! -z ${JAVA_HOME_21+x} ]]; then
builder_echo "Setting JAVA_HOME to JAVA_HOME_21 (${JAVA_HOME_21})"
export JAVA_HOME="${JAVA_HOME_21}"
fi
}

# Override JAVA_HOME
android_set_java_home
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I wonder if we should move this to the TC script wrappers?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think I prefer a general location since it's used in local dev builds

Comment on lines -314 to -333
# For CI compatibility of building Keyman for Android 18.0 with OpenJDK 11,
# this overrides JAVA_HOME for the builder script to use OpenJDK 21.
set_java_home() {
set_java_home_21
}

set_java_home_21() {
if [[ ! -z ${JAVA_HOME_21+x} ]]; then
builder_echo "Setting JAVA_HOME to JAVA_HOME_21 (${JAVA_HOME_21})"
export JAVA_HOME="${JAVA_HOME_21}"
fi
}

set_java_home_11() {
if [[ ! -z ${JAVA_HOME_11+x} ]]; then
builder_echo "Setting JAVA_HOME to JAVA_HOME_11 (${JAVA_HOME_11})"
export JAVA_HOME="${JAVA_HOME_11}"
fi
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Moved to android/build.sh

…ackaging.yml

deb-packaging.yml runs from master, so we need to continue to source
build-utils.sh until v19 release, so that we can build v18 branches.

Note that deb-packaging.yml will not successfully build on the #14449
branch until this change is merged to master.

Fixes: #14065
@darcywong00 darcywong00 modified the milestones: A19S8, A19S9 Aug 2, 2025
@mcdurdin mcdurdin linked an issue Aug 2, 2025 that may be closed by this pull request
mcdurdin added a commit that referenced this pull request Aug 5, 2025
The file build-utils.sh can be deleted in a follow-up PR once #14449
hits master.
The file build-utils.sh can be deleted in a follow-up PR once #14449
hits master.
@mcdurdin mcdurdin force-pushed the maint/common/14065-consolidate-builder-scripts branch from eb9ce59 to 0bcce97 Compare August 5, 2025 00:56
@mcdurdin mcdurdin marked this pull request as ready for review August 5, 2025 02:58
@mcdurdin mcdurdin changed the title maint(common): consolidate builder scripts maint(common): consolidate builder scripts 🐸 Aug 5, 2025
@@ -1,4 +1,4 @@
# Using the build-utils.sh builder functions
# Using the builder-basic.inc.sh builder functions
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Most build scripts should use builder-full.inc.sh; some helper scripts can use builder-basic.inc.sh. Documented in resources/build/README.md.

Should this doc also have a brief clarification?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Can do in a follow-up PR

Copy link
Copy Markdown
Contributor

@darcywong00 darcywong00 left a comment

Choose a reason for hiding this comment

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

lgtm

Base automatically changed from maint/common/14275-rename-and-cleanup-shellHelperFunctions.sh to master August 12, 2025 03:41
@mcdurdin mcdurdin merged commit a81138c into master Aug 12, 2025
29 checks passed
@mcdurdin mcdurdin deleted the maint/common/14065-consolidate-builder-scripts branch August 12, 2025 03:41
@github-project-automation github-project-automation bot moved this from Todo to Done in Keyman Aug 12, 2025
mcdurdin added a commit that referenced this pull request Aug 12, 2025
@keyman-server
Copy link
Copy Markdown
Collaborator

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

@keyman-server
Copy link
Copy Markdown
Collaborator

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

@ermshiperete
Copy link
Copy Markdown
Contributor

Do we have a v20 maintenance issue for removing the temporary copy of build-utils.sh?

@mcdurdin
Copy link
Copy Markdown
Member Author

Do we have a v20 maintenance issue for removing the temporary copy of build-utils.sh?

We do now, see #14549.

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