search

keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
376 stars 102 forks source link

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

Closed jahorton closed 2 days ago

jahorton commented 2 weeks ago

This PR reworks /mac/build.sh to use the same patterns and formats as the primary build.sh scripts for all of our other platforms, thus making it builder-based.

User Testing

TEST_ABILITY_TO_TEST: Verify that the test-bot provides a valid link to the Keyman for macOS installer. Also do very light testing to ensure the app works as before.

There are no changes to the app itself, but since the build script's being overhauled, we want to make sure we didn't break things in subtle ways without realizing it.

keymanapp-test-bot[bot] commented 2 weeks ago

User Test Results

Test specification and instructions

Test Artifacts

mcdurdin commented 1 week ago

I have updated CI to use the new build script. The release build appears to have the wrong search path -- see debug in the paths referenced below -- for the engine tests, in a release build (never before run in that config I guess?):

https://build.palaso.org/buildConfiguration/Keyman_KeymanMac_PullRequests/464688?buildTab=log

[12:08:36] :     [Step 5/5] 3 warnings generated.
[12:08:36] :     [Step 5/5] ld: warning: search path '/Users/marc_durdin/buildAgent/work/99b311828f4ee7c/keyman/mac/KeymanEngine4Mac/../../core/build/mac-x86_64/debug/subprojects/icu/source/common' not found
[12:08:36] :     [Step 5/5] ld: warning: search path '/Users/marc_durdin/buildAgent/work/99b311828f4ee7c/keyman/mac/KeymanEngine4Mac/../../core/build/mac-x86_64/debug/subprojects/icu/source/i18n' not found
[12:08:36] :     [Step 5/5] ld: warning: search path '/Users/marc_durdin/buildAgent/work/99b311828f4ee7c/keyman/mac/KeymanEngine4Mac/../../core/build/mac/debug' not found
[12:08:36] :     [Step 5/5] ld: library 'sicuuc' not found

Note that the older CI would do a debug build to run the tests, and then do a release build. The 18.0 CI does a release build and runs tests on that build.

sgschantz commented 1 week ago

I ran the debug build locally, ran tests and installed and ran the app and everything looked good.

I did need to include --quick to install locally. If I did only ./build.sh --debug install:app then I got a notarization error.

Also, deleting a core library caused it to be rebuilt, which is good. I don't think that was happening with the old script. (maybe?)

The clean action should only clean mac products and not clean anything that the mac build is dependent upon in core since core is a separate target managed with its own build script -- is that correct? That might be a dumb question, but I don't think I understand the relationship, because core gets built if I delete it and run build:app for mac.

mcdurdin commented 1 week ago

The clean action should only clean mac products and not clean anything that the mac build is dependent upon in core since core is a separate target managed with its own build script -- is that correct?

Correct. Dependencies will be automatically configured and built if they are missing. No other actions are automatic (e.g. clean, test, etc).

See https://github.com/keymanapp/keyman/blob/b7c3d65702d179a829d0d23849836955e52c1c05/resources/build/builder.md#L186-L191 for more details.

mcdurdin commented 4 days ago

CI has been updated for test builds (and seems to be working), and for release builds (will be tested when this merges).

jahorton commented 4 days ago

One last remaining question: do we want to do something about this thread now, leave it as is and create a follow-up issue, or leave it as is permanently?

Perhaps it would be better to flip the [--quick] option and/or condition the install-type [local vs quicklocal] based on --debug? Talking with @sgschantz, he mentioned doing most local builds the equivalent of --debug and --quick.

Let's keep with the original design for those parameters for now and revisit once this is ready.

It appears we're pretty much there, so consider this the follow-up.

mcdurdin commented 4 days ago

One last remaining question: do we want to do something about this thread now, leave it as is and create a follow-up issue, or leave it as is permanently?

I think we leave it as is, and don't create a follow-up issue. I plan to do a future round of consolidation across all our build scripts and will probably revisit script-specific parameters and try and make them consistent (e.g. We have other parameters in Windows, Developer build scripts that could also benefit). But we don't need an issue for it because I would be doing a cross-script analysis anyway to start, so the issue would just add noise to our already very big queue.

dinakaranr commented 3 days ago

Test Results

  1. Installed the "Keyman 18.0.31-alpha-local.dmg" file and gave all permissions to the application.
  2. Open the OSK by clicking the keyboard icon, and then click "On-Screen Keyboard.".

Here, a blank screen appeared in the OSK. The Configuration button does not work on the "About" window.If "Always show on-screen keyboard" is enabled, then I am unable to install or uninstall the keyboard, and OSK is not allowed to do it. It seems to be a problem. Hence, I have failed as of now. Please refer to the screenshot. thanks

jahorton commented 3 days ago

Discussion with @sgschantz pointed out that I may just need to merge in more recent fixes to this PR. I'm not always on top of when I most recently merged stuff in... and swapping back to master on the dev machine I was using... yeah, it's probably that.

Your branch is behind 'origin/master' by 357 commits, [...]

🤦

@keymanapp-test-bot retest all

dinakaranr commented 3 days ago

Test Results

TEST_ABILITY_TO_TEST (PASSED): I tested this issue with the attached "Keyman 18.0.45-alpha-local" build on the macOS environment. Here is my observation.

  1. Installed the "Keyman 18.0.45-alpha-local.dmg" file and gave all permissions to the application.
  2. Open the OSK by clicking the keyboard icon, and then click "On-Screen Keyboard". Here, a blank screen does not appear in the OSK. The configuration button works on the "About" window. Thank you.
keyman-server commented 2 days ago

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

keyman-server commented 1 day ago

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

keyman-server commented 1 day ago

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

keyman-server commented 1 day ago

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