chore(windows): move from Makefile to build.sh #11461

mcdurdin · 2024-05-16T07:12:18Z

Now builds from a clean repo:

windows/src/build.sh configure build test publish

Cleanup of various build scripts and dependencies.

TODO:

Move Developer-related build script changes to chore(developer): build.sh for developer #11421
Re-review all build scripts for consistency
CI linkage - release build
CI linkage - test build
user test spec
CI linkage - test osk render build --> this will come in a follow-up PR (lower priority)
convert remaining makefiles for manual tests and support projects --> this will come in a follow-up PR (lower priority)

User Testing

TEST_WINDOWS: Please perform a basic regression test of Keyman for Windows to ensure that all functions continue to operate as expected.

keymanapp-test-bot · 2024-05-16T07:12:23Z

User Test Results

Test specification and instructions

✅ TEST_WINDOWS (PASSED): I tested this issue with the attached "Keymandeveloper-18.0.39-alpha-test-11461" build on the Windows 11 OS environment: Here is my observation. (notes)

Test Artifacts

Android
Developer
iOS
- Keyman for iOS (simulator image)
- FirstVoices Keyboards for iOS (simulator image)
- TestFlight internal PR build version - 18.0.39 (0.11461.11053)
Keyboards
- Test Keyboards
Linux
macOS
- Keyman for macOS
Web
- KeymanWeb Test Home
Windows

mcdurdin · 2024-05-20T01:29:21Z

Note: split out a number of common changes into separate PRs #11477, #11478, and moved Developer-related fixes into #11421.

dinakaranr · 2024-05-20T13:58:44Z

Test Results

TEST_WINDOWS (PASSED): I tested this issue with the attached "Keymandeveloper-18.0.39-alpha-test-11461" build on the Windows 11 OS environment: Here is my observation.
I have followed the acceptance scenario from the help file. I executed the below scenarios

Installed the "keyman-18.0.39.exe" file.
Open the configuration dialog by clicking the keyman icon from the toolbox tray.
Installed keyboards that are required for testing.

SUITE_BASELINE: Keyman for Windows Base Line Tests: Passed
SUITE_CAPSLOCK: Passed
CapsAlwaysOff: Passed
SHIFT: CapsOnOnly/ShiftFreesCaps: Passed
SUITE_TSF_APP: Passed
SUITE_IMX_KEYBOARDS: Passed
SUITE_STORE_AND_CONTEXT: Passed

I followed the test scenario described above test suite. I executed as per the test case instructions and outputs were correctly displayed in "Text Editor". It works well. Thank you.

The build agents use a fixed location for signtool.exe, but this is not the case for other devs, so we need to grab our VS environment in order to locate it.

rc-swag · 2024-05-22T00:52:38Z

windows/src/engine/inst/build.sh

+# after all other builds complete
+
+builder_describe_outputs \
+  publish       /windows/src/desktop/inst/keymanengine.msm


This should either be
$KEYMAN_ROOT/windows/release/${VERSION}/keymanengine.msm
of the local version before copy_installer
keymanengine.msm
Either case nothing is copying it to where this line suggests.
Also should see comment below

Good catch. Fixed.

rc-swag · 2024-05-22T01:01:02Z

windows/src/engine/inst/build.sh

+function copy-installer() {
+  builder_heading copy-installer
+  mkdir -p "$KEYMAN_ROOT/windows/release/${VERSION}"
+  cp keymanengine.msm "$KEYMAN_ROOT/windows/release/${VERSION}/keymanengine.msm"


Do we no longer include version in the name `keymanengine-${VERSION}.msm

Yeah, another good catch. The Keyman Desktop .wxs has:

keyman/windows/src/desktop/inst/keymandesktop.wxs

Line 61 in 53b583e

<Merge Id="keymanengine" Language='1033' SourceFile='..\..\engine\inst\keymanengine.msm' />

Which is why the build worked. Fixed this one too.

rc-swag · 2024-05-22T01:08:14Z

windows/src/engine/keyman32/build.sh

+
+function do_publish() {
+  wrap-signcode //d "Keyman Engine for Windows" "$WINDOWS_PROGRAM_ENGINE/keyman32.dll"
+  wrap-symstore "$WINDOWS_PROGRAM_ENGINE/keyman32.dll" //t keyman-engine-windows


Just querying this change in behaviour
for my local publish this gets skipped as $KEYMAN_SYMSTOREPATH is not is set an so wrap-symstore exits early. Previously the Defines.mak file had this line.

# KEYMAN_SYMSTOREPATH defaults to sibling folder "symbols". If it is not present, # then we won't attempt to write symbols to the store. !IFNDEF KEYMAN_SYMSTOREPATH KEYMAN_SYMSTOREPATH=$(KEYMAN_ROOT)\..\symbols !ENDIF

I will add a check to wrap-symstore.

It's already there, see:

keyman/resources/build/win/environment.inc.sh

Lines 117 to 121 in 8b2070c

wrap-symstore() {

if [[ -z "${KEYMAN_SYMSTOREPATH+x}" ]]; then

builder_warn "\$KEYMAN_SYMSTOREPATH is not set. Skipping symstore for $@"

return 0

fi

I guess that is slightly different behaviour. It's probably better not to default to ../symbols!

rc-swag

lgtm

keyman-server · 2024-05-23T08:07:57Z

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

Relates to #11461. The FV build is now a part of the Windows publish build. At the same time, addresses a number of typos in the build script, which had not been tested because it was not linked to the Windows build.

keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label May 16, 2024

keymanapp-test-bot bot added this to the A18S2 milestone May 16, 2024

github-actions bot added windows/ developer/ common/ common/resources/ Build infrastructure oem/ oem/fv/ oem/fv/windows/ windows/config/ windows/engine/ chore developer/ide/ labels May 16, 2024

mcdurdin mentioned this pull request May 19, 2024

chore(developer): build.sh for developer #11421

Merged

5 tasks

keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed and removed user-test-missing User tests have not yet been defined for the PR labels May 19, 2024

mcdurdin force-pushed the chore/windows/11316-build.sh branch from 8ece62c to 3957148 Compare May 19, 2024 23:09

mcdurdin force-pushed the chore/developer/11317-build.sh branch from 781d154 to 8b2070c Compare May 19, 2024 23:35

mcdurdin added 4 commits May 20, 2024 08:16

chore(windows): remove legacy build Makefiles

9f7eff1

chore(windows): move insthelper up to engine folder

d06f4b1

chore(windows): add compiled resources to .gitignore

f9a321f

chore(windows): add new build scripts for Windows builds

c06dd48

mcdurdin force-pushed the chore/windows/11316-build.sh branch from 3957148 to c06dd48 Compare May 20, 2024 01:28

github-actions bot added common/ and removed developer/ common/ common/resources/ Build infrastructure developer/ide/ labels May 20, 2024

github-actions bot added developer/ developer/ide/ labels May 20, 2024

mcdurdin marked this pull request as ready for review May 20, 2024 05:41

mcdurdin requested review from rc-swag and ermshiperete as code owners May 20, 2024 05:41

mcdurdin linked an issue May 20, 2024 that may be closed by this pull request

chore(windows): Consolidate build scripts on windows #11316

Closed

keymanapp-test-bot bot removed the user-test-required User tests have not been completed label May 20, 2024

fix(windows): use run_in_vs_env to ensure signtool.exe is on path

d43b931

The build agents use a fixed location for signtool.exe, but this is not the case for other devs, so we need to grab our VS environment in order to locate it.

mcdurdin requested a review from jahorton as a code owner May 21, 2024 07:13

github-actions bot added developer/ developer/ide/ and removed developer/ developer/ide/ labels May 21, 2024

rc-swag reviewed May 22, 2024

View reviewed changes

fix(windows): correct filename and path for keymanengine.msm

c67369e

github-actions bot added developer/ developer/ide/ and removed developer/ developer/ide/ labels May 23, 2024

rc-swag approved these changes May 23, 2024

View reviewed changes

Base automatically changed from chore/developer/11317-build.sh to master May 23, 2024 07:22

mcdurdin merged commit 0259522 into master May 23, 2024
25 checks passed

mcdurdin deleted the chore/windows/11316-build.sh branch May 23, 2024 07:23

mcdurdin mentioned this pull request May 24, 2024

fix(windows): add FirstVoices Keyboards build to Windows release build #11524

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(windows): move from Makefile to build.sh #11461

chore(windows): move from Makefile to build.sh #11461

mcdurdin commented May 16, 2024 •

edited

Loading

keymanapp-test-bot bot commented May 16, 2024 •

edited

Loading

mcdurdin commented May 20, 2024

dinakaranr commented May 20, 2024

rc-swag May 22, 2024

mcdurdin May 23, 2024

rc-swag May 22, 2024

mcdurdin May 23, 2024

rc-swag May 22, 2024 •

edited

Loading

mcdurdin May 23, 2024

mcdurdin May 23, 2024 •

edited

Loading

rc-swag left a comment

keyman-server commented May 23, 2024

	wrap-symstore() {
	if [[ -z "${KEYMAN_SYMSTOREPATH+x}" ]]; then
	builder_warn "\$KEYMAN_SYMSTOREPATH is not set. Skipping symstore for $@"
	return 0
	fi

chore(windows): move from Makefile to build.sh #11461

chore(windows): move from Makefile to build.sh #11461

Conversation

mcdurdin commented May 16, 2024 • edited Loading

User Testing

keymanapp-test-bot bot commented May 16, 2024 • edited Loading

User Test Results

Test Artifacts

mcdurdin commented May 20, 2024

dinakaranr commented May 20, 2024

Test Results

rc-swag May 22, 2024

Choose a reason for hiding this comment

mcdurdin May 23, 2024

Choose a reason for hiding this comment

rc-swag May 22, 2024

Choose a reason for hiding this comment

mcdurdin May 23, 2024

Choose a reason for hiding this comment

rc-swag May 22, 2024 • edited Loading

Choose a reason for hiding this comment

mcdurdin May 23, 2024

Choose a reason for hiding this comment

mcdurdin May 23, 2024 • edited Loading

Choose a reason for hiding this comment

rc-swag left a comment

Choose a reason for hiding this comment

keyman-server commented May 23, 2024

mcdurdin commented May 16, 2024 •

edited

Loading

keymanapp-test-bot bot commented May 16, 2024 •

edited

Loading

rc-swag May 22, 2024 •

edited

Loading

mcdurdin May 23, 2024 •

edited

Loading