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

mcdurdin · 2024-05-11T23:25:39Z

Now builds from a clean repo:

developer/src/build.sh configure build test publish

Splits kmbrowserhost into kmdbrowserhost for Developer; this means that Developer Browser Host now inherits the Developer settings rather than the Keyman for Windows settings, and simplifies distribution and management. The only difference between the two is in the startup code so this seems like a good split.
Cleanup of various build scripts and dependencies.

TODO:

Re-review all build scripts for consistency
Windows Makefiles --> in follow-up PR chore(windows): move from Makefile to build.sh #11461
CI linkage - release build
CI linkage - test build
user test spec

User Testing

TEST_DEVELOPER: Please perform a basic regression test of Keyman Developer to ensure that all functions continue to operate as expected.

keymanapp-test-bot · 2024-05-11T23:25:45Z

User Test Results

Test specification and instructions

✅ TEST_DEVELOPER (PASSED): for all other use cases.

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.11421.11027)
Keyboards
- Test Keyboards
Linux
macOS
- Keyman for macOS
Web
- KeymanWeb Test Home
Windows
- Keyman for Windows - build failure
- FirstVoices Keyboards for Windows - build failure
- Text Editor (32 bit) - build failure
- Text Editor (64 bit) - build failure

jahorton · 2024-05-16T01:36:56Z

resources/build/build-utils-ci.inc.sh

 #
-# builder_publish_to_pack and builder_publish_to_npm are similar:
-#  * builder_publish_to_npm publishes to the public registry
-#  * builder_publish_to_pack creates a local tarball which can be used to test
+# If --npm-publish is set:
+#  * then builder_publish_npm publishes to the public registry
+#  * else builder_publish_npm creates a local tarball which can be used to test
 #
 # Usage:
 # ```bash
-#   builder_publish_to_npm
+#   builder_publish_npm
 # ```
 #
-function builder_publish_to_npm() {
-  _builder_publish_npm_package publish
-}
-
-function builder_publish_to_pack() {
-  _builder_publish_npm_package pack
+function builder_publish_npm() {
+  if builder_has_option --npm-publish; then
+    # Require --dry-run if local or test to avoid accidentally clobbering npm packages
+    if [[ $VERSION_ENVIRONMENT =~ local|test ]] && ! builder_has_option --dry-run; then
+      builder_die "publish --npm-publish must use --dry-run flag for local or test builds"
+    fi
+    _builder_publish_npm_package publish
+  else
+    _builder_publish_npm_package pack
+  fi


I see that this gets referenced by a lot of the child scripts, especially in the earlier files. Figured I'd highlight this as reference, as the merge of builder_publish_to_npm and builder_publish_to_pack is significant.

Also, I see that this conditions upon a non-implicit builder option - --dry-run; that's an interesting decision, but I can understand it. (non-implicit: --dry-run is not automatically available for all builder scripts, unlike --debug and --verbose.)

I hear you on this. tl;dir: I don't have a great answer. It's a bit repetitious for now but we can easily revisit in the future if we need to.

More details...

I could upgrade --dry-run to a global option but I'm hesitant because it won't work on most scripts, so that's confusing (and I don't think we want to start supporting --dry-run everywhere either -- busywork with little benefit). --debug and --verbose work nearly everywhere.

One option I considered is pulling this out to be an exported DRY_RUN variable. That could avoid us defining it on every script that does publishing.

FWIW, I do plan to using the option more later on for any other actions that send files to other systems (e.g. when we deploy to downloads, app store, play store, etc ad infinitum).

jahorton

There's a lot here, so here's a partial review: I've looked over all the common/ + common/windows file changes. Hopefully the feedback's useful, given my comparatively limited knowledge of Windows & Developer build structure.

jahorton · 2024-05-16T01:41:06Z

resources/build/win/environment.inc.sh

+COMMON_OUTLIB="$KEYMAN_ROOT/common/windows/lib"
+
+if builder_is_debug_build || [[ $VERSION_ENVIRONMENT == local ]] || [[ ! -z ${TEAMCITY_PR_NUMBER+x} ]]; then
+  # We do a fast build for debug builds, local builds, test PR builds but not for master/beta/stable release builds


What is the difference between a "fast" build and one that isn't? I'm lacking context, since I'm not usually working with Windows or Developer source.

Fast build in this context is currently only used when building installers -- reducing compression level which is really slow:

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

Lines 11 to 17 in 3ecc958

if [[ $GO_FAST == 1 ]]; then

# for debug builds, we turn off compression because it is so hideously slow

# for test builds, we also turn off compression

WIXLIGHTCOMPRESSION=-dcl:none

else

WIXLIGHTCOMPRESSION=-dcl:high

fi

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

Lines 5 to 9 in 3ecc958

if [[ $GO_FAST == 1 ]]; then

"$WZZIPPATH" a -mx1 "$@"

else

"$WZZIPPATH" a -mx9 "$@"

fi

jahorton · 2024-05-16T01:48:12Z

common/windows/delphi/ext/build.sh

+  "$DEVTOOLS" -ri
+}
+
+builder_run_action reset do_reset


reset? First time I've seen that as an action name in one of our scripts.

Lacking context here - what's the intended difference between reset and clean?

Reset just removes all the Keyman packages and paths from the Delphi IDE. It's kinda ugly, and it is legacy that we never really use. I copied it across but we could probably remove it altogether.

common/windows/delphi/ext/cef4delphi/build.sh

common/windows/delphi/ext/dcpcrypt/build.sh

common/windows/delphi/ext/sentry/build.sh

common/windows/delphi/tools/Makefile

common/windows/delphi/tools/certificates/build.sh

Relates to #11317. All projects now have a build.sh. Still need to work on inst/, remove old Makefiles, and update CI.

Relates to #11317.

…ld scripts also, remove old Makefiles

This finishes the changes for all build infrastructure for Keyman Developer to use build.sh.

Now builds from a clean repo: developer/src/build.sh configure build test publish * Splits kmbrowserhost into kmdbrowserhost for Developer; this means that Developer Browser Host now inherits the Developer settings rather than the Keyman for Windows settings, and simplifies distribution and management. The only difference between the two is in the startup code so this seems like a good split. * Cleanup of various build scripts and dependencies.

Use a new flag `--npm-publish` in conjunction with `publish` action, so that the default will always be to `npm pack` if the new flag is not specified. This flag is also guarded in the actual npm publish code to ensure that it can only run in the appropriate CI alpha/beta/stable environment, and not in local or test. This then also removes the separate `pack` action. Also removes legacy boilerplate from a number of build scripts.

Also some more script cleanup and ensure common/core packages are also published.

A round of cleanup and consistency for Developer build scripts.

Additional shared script resources for Developer and Windows, refactor of a code signing function, refactor of clean step in build.sh.

dinakaranr · 2024-05-20T10:35:04Z

Test Results

TEST_DEVELOPER (FAILED): I tested this issue with the attached "Keymandeveloper-18.0.39-alpha-test-11421" build on Windows 11 and 10 OS environment: Here is my observation.
I have followed the acceptance scenario from the help file. I executed the below scenarios
TEST_DEBUGGER_STARTS: passed
TEST_DEBUGGER_SINGLE_STEP: passed
TEST_DEBUGGER_PAUSE: passed
TEST_DEBUGGER_CHANGES: failed
TEST_DEBUGGER_PLATFORM: failed
and got an error "Test_debugger_pause and Test_Debugger_platform"

Steps to reproduce:

Installed the "keymandeveloper-18.0.39.exe" file.
Double-click on the "Keyman Developer" icon. The developer tool appeared correctly.
Open the Keyman keyboard project (*.kpj) file.
Open any keyman keyboard (*.kmn) file. (e.g., Tamil 99 text)
Navigate to the Debugger mode by pressing the F5 or (On build tab/Start Debugging).
Navigate to the Layout tab. Click on an empty text box.
Type a letter. Press the CTRL+Z key.
Here, the error message box appears. I am not able to test the Keyman developer build. So, I failed this issue. Thanks.
Please refer to the screenshot and video files

https://github.com/keymanapp/keyman/assets/19683059/11bc7a35-e403-4df5-a3e5-87be360211e1
Thanks.

mcdurdin · 2024-05-21T02:56:35Z

Thank you @dinakaranr. The issue you experienced is being tracked at #11486. I think apart from that, the install went well, so we can go ahead with merge on this soon.

TEST_DEVELOPER PASSED for all other use cases.

keyman-server · 2024-05-23T08:08:08Z

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

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

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

mcdurdin added developer/ chore labels May 12, 2024

mcdurdin changed the base branch from master to feat/common/11408-11394-builder-child-and-dependency-tweaks May 12, 2024 04:45

github-actions bot added windows/ common/ common/models/ developer/compilers/ common/resources/ Build infrastructure core/ Keyman Core common/web/ common/models/types/ developer/ide/ windows/config/ windows/engine/ docs labels May 13, 2024

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 13, 2024

mcdurdin marked this pull request as ready for review May 13, 2024 08:21

mcdurdin requested review from rc-swag, ermshiperete, jahorton and darcywong00 as code owners May 13, 2024 08:21

Base automatically changed from feat/common/11408-11394-builder-child-and-dependency-tweaks to master May 13, 2024 08:24

jahorton mentioned this pull request May 15, 2024

chore(mac): rework of main build script #11444

Merged

jahorton reviewed May 16, 2024

View reviewed changes

mcdurdin added 17 commits May 20, 2024 06:33

chore(developer): tike/build.sh

8c14b20

chore(developer): transition remaining projects to build.sh

86c62ff

Relates to #11317. All projects now have a build.sh. Still need to work on inst/, remove old Makefiles, and update CI.

chore(developer): move windows common Makefiles to build scripts

2499ebf

Relates to #11317.

chore(developer): use builder.inc.sh instead of build-utils.sh in bui…

c8a1c6a

…ld scripts also, remove old Makefiles

chore(developer): cleanup test paths

2d249cb

chore(developer): refactor remaining build scripts

41df6d1

This finishes the changes for all build infrastructure for Keyman Developer to use build.sh.

chore(developer): remove old inst makefiles

378cd72

chore(common): add dependency

c136676

chore(common): adjust dependency order

b399637

chore(developer): SIGNTIME env var support

4e679dc

Also some more script cleanup and ensure common/core packages are also published.

chore(developer): cleanup on build scripts

8b6d6b0

A round of cleanup and consistency for Developer build scripts.

chore(developer): address review comments

b3da216

chore(developer): move to keymancore-2.dll

5c27377

chore(developer): checkout correct CEF version during release build

a062c87

chore(developer): minor cleanup on scripts

8b2070c

Additional shared script resources for Developer and Windows, refactor of a code signing function, refactor of clean step in build.sh.

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

mcdurdin changed the base branch from master to fix/common/builder-run-action-quotes May 19, 2024 23:35

Base automatically changed from fix/common/builder-run-action-quotes to master May 20, 2024 03:32

mcdurdin removed the DO-NOT-MERGE label May 20, 2024

keymanapp-test-bot bot added user-test-failed and removed user-test-required User tests have not been completed labels May 20, 2024

keymanapp-test-bot bot removed the user-test-failed label May 21, 2024

mcdurdin merged commit 1e66b6e into master May 23, 2024
23 of 25 checks passed

mcdurdin deleted the chore/developer/11317-build.sh branch May 23, 2024 07:22

mcdurdin mentioned this pull request May 24, 2024

bug(developer): npm publish is not working in 18.0.44-alpha #11538

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

mcdurdin commented May 11, 2024 •

edited

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

edited

jahorton May 16, 2024

mcdurdin May 16, 2024

jahorton left a comment

jahorton May 16, 2024

mcdurdin May 16, 2024

jahorton May 16, 2024

mcdurdin May 16, 2024

dinakaranr commented May 20, 2024

mcdurdin commented May 21, 2024

keyman-server commented May 23, 2024

	if [[ $GO_FAST == 1 ]]; then
	# for debug builds, we turn off compression because it is so hideously slow
	# for test builds, we also turn off compression
	WIXLIGHTCOMPRESSION=-dcl:none
	else
	WIXLIGHTCOMPRESSION=-dcl:high
	fi

	if [[ $GO_FAST == 1 ]]; then
	"$WZZIPPATH" a -mx1 "$@"
	else
	"$WZZIPPATH" a -mx9 "$@"
	fi

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

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

Conversation

mcdurdin commented May 11, 2024 • edited

User Testing

keymanapp-test-bot bot commented May 11, 2024 • edited

User Test Results

Test Artifacts

jahorton May 16, 2024

Choose a reason for hiding this comment

mcdurdin May 16, 2024

Choose a reason for hiding this comment

jahorton left a comment

Choose a reason for hiding this comment

jahorton May 16, 2024

Choose a reason for hiding this comment

mcdurdin May 16, 2024

Choose a reason for hiding this comment

jahorton May 16, 2024

Choose a reason for hiding this comment

mcdurdin May 16, 2024

Choose a reason for hiding this comment

dinakaranr commented May 20, 2024

Test Results

mcdurdin commented May 21, 2024

keyman-server commented May 23, 2024

mcdurdin commented May 11, 2024 •

edited

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

edited