From f4bd99f935d12b7528c8ff3ca46d08b212f9fd48 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 6 Nov 2024 10:59:29 +0700 Subject: [PATCH 1/8] fix(developer): reconnect --full-test in kmcmplib build and enable for CI The --full-test parameter in kmcmplib build.sh has not been working. This PR reconnects the parameter and also enables it by default for CI test builds (not release builds). Fixes: #12623 --- developer/src/kmcmplib/build.bat | 6 ++++- developer/src/kmcmplib/build.sh | 4 ++-- developer/src/kmcmplib/commands.inc.sh | 21 ++++++++++++++--- resources/build/build-utils-ci.inc.sh | 32 ++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 6 deletions(-) diff --git a/developer/src/kmcmplib/build.bat b/developer/src/kmcmplib/build.bat index 7b5dcf110d8..fd6aa7308d4 100644 --- a/developer/src/kmcmplib/build.bat +++ b/developer/src/kmcmplib/build.bat @@ -80,7 +80,11 @@ shift if "!COMMAND!" == "configure" ( echo === Configuring Keyman KMX Compiler for Windows !ARCH! !BUILDTYPE! === if exist build\!ARCH!\!BUILDTYPE! rd /s/q build\!ARCH!\!BUILDTYPE! - meson setup build\!ARCH!\!BUILDTYPE! !STATIC_LIBRARY! --buildtype !BUILDTYPE! --werror %1 %2 %3 %4 %5 %6 %7 %8 %9 || exit !errorlevel! + if "%1"=="--full-test" ( + meson setup build\!ARCH!\!BUILDTYPE! !STATIC_LIBRARY! --buildtype !BUILDTYPE! --werror -D full_test=true %2 %3 %4 %5 %6 %7 %8 %9 || exit !errorlevel! + ) else ( + meson setup build\!ARCH!\!BUILDTYPE! !STATIC_LIBRARY! --buildtype !BUILDTYPE! --werror %1 %2 %3 %4 %5 %6 %7 %8 %9 || exit !errorlevel! + ) shift ) diff --git a/developer/src/kmcmplib/build.sh b/developer/src/kmcmplib/build.sh index bba19d35558..f0a6686ba18 100755 --- a/developer/src/kmcmplib/build.sh +++ b/developer/src/kmcmplib/build.sh @@ -78,7 +78,7 @@ do_action() { done } -if builder_has_option --full-test; then +if should_do_full_test; then locate_keyboards_repo fi @@ -101,7 +101,7 @@ if builder_start_action configure; then # We have to checkout the keyboards repo in a 'configure' action because # otherwise meson will not get the right list of keyboard source files, # even though we only use it in the 'test' action - if builder_has_option --full-test; then + if should_do_full_test; then checkout_keyboards fi diff --git a/developer/src/kmcmplib/commands.inc.sh b/developer/src/kmcmplib/commands.inc.sh index d3647c41dd5..280343853c3 100644 --- a/developer/src/kmcmplib/commands.inc.sh +++ b/developer/src/kmcmplib/commands.inc.sh @@ -33,12 +33,19 @@ do_configure() { STANDARD_MESON_ARGS="--cross-file wasm.defs.build --cross-file wasm.build --default-library static" fi + local FULL_TEST= + local FULL_TEST_WIN= + if should_do_full_test; then + FULL_TEST="-D full_test=true" + FULL_TEST_WIN=--full-test + fi + if [[ $target =~ ^(x86|x64)$ ]]; then - cmd //C build.bat $target $BUILDER_CONFIGURATION configure "${builder_extra_params[@]}" + cmd //C build.bat $target $BUILDER_CONFIGURATION configure $FULL_TEST_WIN "${builder_extra_params[@]}" else pushd "$THIS_SCRIPT_PATH" > /dev/null # Additional arguments are used by Linux build, e.g. -Dprefix=${INSTALLDIR} - meson setup "$MESON_PATH" --werror --buildtype $BUILDER_CONFIGURATION $STANDARD_MESON_ARGS "${builder_extra_params[@]}" + meson setup "$MESON_PATH" --werror --buildtype $BUILDER_CONFIGURATION $STANDARD_MESON_ARGS $FULL_TEST "${builder_extra_params[@]}" popd > /dev/null fi builder_finish_action success configure:$target @@ -78,7 +85,7 @@ do_test() { # Works on a local clone of keyboards repository, to avoid clobbering # user's existing keyboards repo, if present - if builder_has_option --full-test; then + if should_do_full_test; then checkout_keyboards fi @@ -125,4 +132,12 @@ build_meson_cross_file_for_wasm() { local R=$(echo $EMSCRIPTEN_BASE | sed 's_/_\\/_g') fi sed -e "s/\$EMSCRIPTEN_BASE/$R/g" wasm.build.$BUILDER_OS.in > wasm.build +} + +should_do_full_test() { + if builder_has_option --full-test || builder_is_ci_test_build; then + return 0 + fi + + return 1 } \ No newline at end of file diff --git a/resources/build/build-utils-ci.inc.sh b/resources/build/build-utils-ci.inc.sh index fd9b25e52aa..a67f285254b 100644 --- a/resources/build/build-utils-ci.inc.sh +++ b/resources/build/build-utils-ci.inc.sh @@ -7,6 +7,38 @@ . "$KEYMAN_ROOT/resources/build/jq.inc.sh" +# +# Returns 0 if current build is running in CI, as a pull request test, or as a +# mainline branch test, or as a release build +# +builder_is_ci_build() { + if builder_is_ci_release_build || builder_is_ci_test_build; then + return 0 + fi + return 1 +} + +# +# Returns 0 if current build is running as a release build in CI +# +builder_is_ci_release_build() { + if [[ "$VERSION_ENVIRONMENT" =~ ^alpha|beta|stable$ ]]; then + return 0 + fi + return 1 +} + +# +# Returns 0 if current build is running in CI, as a pull request test, or as a +# mainline branch test +# +builder_is_ci_test_build() { + if [[ "$VERSION_ENVIRONMENT" == test ]]; then + return 0 + fi + return 1 +} + # # Returns 0 if current build is in CI and triggered from a pull request. If it # returns 0, then a call is made to GitHub to get pull request details, and the From d88f16e1533a0bcd87ab0ec515cee2749013d9da Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 6 Nov 2024 11:09:14 +0700 Subject: [PATCH 2/8] chore(developer): include build-utils-ci.inc.sh in kmcmplib --- developer/src/kmcmplib/build.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/developer/src/kmcmplib/build.sh b/developer/src/kmcmplib/build.sh index f0a6686ba18..aaf71846763 100755 --- a/developer/src/kmcmplib/build.sh +++ b/developer/src/kmcmplib/build.sh @@ -7,6 +7,7 @@ THIS_SCRIPT="$(readlink -f "${BASH_SOURCE[0]}")" ## END STANDARD BUILD SCRIPT INCLUDE . "$KEYMAN_ROOT/resources/shellHelperFunctions.sh" +. "$KEYMAN_ROOT/resources/build/build-utils-ci.inc.sh" . "$THIS_SCRIPT_PATH/checkout-keyboards.inc.sh" . "$THIS_SCRIPT_PATH/commands.inc.sh" From 3c680e3334db28fbb33d6661d01460a1287acd98 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 6 Nov 2024 12:57:55 +0700 Subject: [PATCH 3/8] fix(developer): normalize path separators in unit tests in kmcmplib While kmc did this already in its callbacks, the unit test callbacks did not, which meant that some keyboard tests failed on Linux and macOS. Relates-to: #12623 --- developer/src/kmcmplib/tests/util_callbacks.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/developer/src/kmcmplib/tests/util_callbacks.cpp b/developer/src/kmcmplib/tests/util_callbacks.cpp index d512463a0b4..47d22b0bb27 100644 --- a/developer/src/kmcmplib/tests/util_callbacks.cpp +++ b/developer/src/kmcmplib/tests/util_callbacks.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include "util_filesystem.h" #include "../src/compfile.h" @@ -29,6 +30,14 @@ void msgproc(const KMCMP_COMPILER_RESULT_MESSAGE &message, void* context) { printf(")\n"); } +void normalizeSlashes(std::string& s) { +#ifdef _WIN32 + std::replace(s.begin(), s.end(), '/', '\\'); +#else + std::replace(s.begin(), s.end(), '\\', '/'); +#endif +} + const std::vector loadfileProc(const std::string& filename, const std::string& baseFilename) { std::string resolvedFilename = filename; if(baseFilename.length() && IsRelativePath(filename.c_str())) { @@ -40,6 +49,8 @@ const std::vector loadfileProc(const std::string& filename, const std:: } } + normalizeSlashes(resolvedFilename); + std::vector buf; FILE* fp = Open_File(resolvedFilename.c_str(), "rb"); From 411cd06df72b1205bd96b6f73889e276e41fe06f Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 6 Nov 2024 13:34:28 +0700 Subject: [PATCH 4/8] fix(developer): use C.UTF-8 locale to for consistent cross-platform whitespace management On mac arch build, kmcmplib is failing to trim U+2002, unlike all other platforms. Using UTF-8 locale should treat the ISO 30112 POSIX space characters, which includes U+2002, as whitespace. --- developer/src/kmcmplib/src/CompilerInterfaces.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/developer/src/kmcmplib/src/CompilerInterfaces.cpp b/developer/src/kmcmplib/src/CompilerInterfaces.cpp index ba368028322..566e7a4b6c5 100644 --- a/developer/src/kmcmplib/src/CompilerInterfaces.cpp +++ b/developer/src/kmcmplib/src/CompilerInterfaces.cpp @@ -14,6 +14,8 @@ EXTERN bool kmcmp_CompileKeyboard( KMCMP_COMPILER_RESULT& result ) { + std::setlocale(LC_ALL, "C.UTF-8"); + FILE_KEYBOARD fk; fk.extra = new KMCMP_COMPILER_RESULT_EXTRA; fk.extra->kmnFilename = pszInfile; From 4e2f83ad509e5d63efd00e6127394bb5db7c4bb2 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 6 Nov 2024 13:39:12 +0700 Subject: [PATCH 5/8] chore(developer): skip anii and sil_kmhmu in full test anii.kmn and sil_kmhmu.kmn both have mismatching case in filename references for icons. For now, we will disable tests for these two keyboards, so that tests pass on Linux (which has case-sensitive filesystem). Note: the filename case was already addressed in the keyboards repo, so when we realign to a more recent commit for the test fixtures, we should be able to include these tests again. Relates-to: #12623 --- developer/src/kmcmplib/tests/get-test-source.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/developer/src/kmcmplib/tests/get-test-source.sh b/developer/src/kmcmplib/tests/get-test-source.sh index beebce3aca3..aa7d81d492e 100755 --- a/developer/src/kmcmplib/tests/get-test-source.sh +++ b/developer/src/kmcmplib/tests/get-test-source.sh @@ -9,5 +9,6 @@ set -eu find "$1" -name '*.kmn' | \ grep -E '(release|experimental)/([a-z0-9_]+)/([a-z0-9_]+)/source/\3\.kmn$' | \ - grep -v masaram_gondi + grep -vE 'masaram_gondi|anii|sil_kmhmu' # #12623: exclude masaram_gondi due to #11806 +# #12630: exclude anii, sil_kmhmu as ico references have mismatching case \ No newline at end of file From 3a359b580a9f69433eed9bd136f030d0b15c8db5 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 6 Nov 2024 13:43:08 +0700 Subject: [PATCH 6/8] chore(developer): include clocale for CompilerInterfaces, for setlocale call --- developer/src/kmcmplib/src/CompilerInterfaces.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/developer/src/kmcmplib/src/CompilerInterfaces.cpp b/developer/src/kmcmplib/src/CompilerInterfaces.cpp index 566e7a4b6c5..71d72ac216e 100644 --- a/developer/src/kmcmplib/src/CompilerInterfaces.cpp +++ b/developer/src/kmcmplib/src/CompilerInterfaces.cpp @@ -1,4 +1,5 @@ #include "pch.h" +#include #include #include #include "kmcmplib.h" From 0df0287cb3a66117e731f4c2ede47701c296db2f Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 6 Nov 2024 13:56:41 +0700 Subject: [PATCH 7/8] chore(developer): remove setlocale again, add exclusions for fv_statimcets, fv_nuucaanul --- developer/src/kmcmplib/src/CompilerInterfaces.cpp | 4 ---- developer/src/kmcmplib/tests/get-test-source.sh | 6 ++++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/developer/src/kmcmplib/src/CompilerInterfaces.cpp b/developer/src/kmcmplib/src/CompilerInterfaces.cpp index 71d72ac216e..0e424c93b00 100644 --- a/developer/src/kmcmplib/src/CompilerInterfaces.cpp +++ b/developer/src/kmcmplib/src/CompilerInterfaces.cpp @@ -1,5 +1,4 @@ #include "pch.h" -#include #include #include #include "kmcmplib.h" @@ -14,9 +13,6 @@ EXTERN bool kmcmp_CompileKeyboard( const void* procContext, KMCMP_COMPILER_RESULT& result ) { - - std::setlocale(LC_ALL, "C.UTF-8"); - FILE_KEYBOARD fk; fk.extra = new KMCMP_COMPILER_RESULT_EXTRA; fk.extra->kmnFilename = pszInfile; diff --git a/developer/src/kmcmplib/tests/get-test-source.sh b/developer/src/kmcmplib/tests/get-test-source.sh index aa7d81d492e..a343bb7ff49 100755 --- a/developer/src/kmcmplib/tests/get-test-source.sh +++ b/developer/src/kmcmplib/tests/get-test-source.sh @@ -9,6 +9,8 @@ set -eu find "$1" -name '*.kmn' | \ grep -E '(release|experimental)/([a-z0-9_]+)/([a-z0-9_]+)/source/\3\.kmn$' | \ - grep -vE 'masaram_gondi|anii|sil_kmhmu' + grep -vE 'masaram_gondi|anii|sil_kmhmu|fv_statimcets|fv_nuucaanul' # #12623: exclude masaram_gondi due to #11806 -# #12630: exclude anii, sil_kmhmu as ico references have mismatching case \ No newline at end of file +# #12630: exclude anii, sil_kmhmu as ico references have mismatching case +# #12630: exclude fv_statimcets, fv_nuucaanul as these include U+2002 which is not +# treated as whitespace on mac arch \ No newline at end of file From 1c39a0e38fc9ece96e4b37cceed68c562072c9c7 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Thu, 7 Nov 2024 06:03:04 +0700 Subject: [PATCH 8/8] chore: fix PR number in comment --- developer/src/kmcmplib/tests/get-test-source.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/developer/src/kmcmplib/tests/get-test-source.sh b/developer/src/kmcmplib/tests/get-test-source.sh index a343bb7ff49..a195fc271dc 100755 --- a/developer/src/kmcmplib/tests/get-test-source.sh +++ b/developer/src/kmcmplib/tests/get-test-source.sh @@ -11,6 +11,6 @@ find "$1" -name '*.kmn' | \ grep -E '(release|experimental)/([a-z0-9_]+)/([a-z0-9_]+)/source/\3\.kmn$' | \ grep -vE 'masaram_gondi|anii|sil_kmhmu|fv_statimcets|fv_nuucaanul' # #12623: exclude masaram_gondi due to #11806 -# #12630: exclude anii, sil_kmhmu as ico references have mismatching case -# #12630: exclude fv_statimcets, fv_nuucaanul as these include U+2002 which is not +# #12631: exclude anii, sil_kmhmu as ico references have mismatching case +# #12631: exclude fv_statimcets, fv_nuucaanul as these include U+2002 which is not # treated as whitespace on mac arch \ No newline at end of file