chore(linux): Build with meson instead of autotools 🏗️ by ermshiperete · Pull Request #8111 · keymanapp/keyman

ermshiperete · 2023-01-27T17:45:51Z

This change introduces meson for building ibus-keyman instead of autotools.

Closes #5981.

@keymanapp-test-bot skip

keymanapp-test-bot · 2023-01-27T17:45:54Z

User Test Results

Test specification and instructions

User tests are not required

This change introduces meson for building ibus-keyman instead of autotools. Closes #5981.

mcdurdin · 2023-01-31T05:42:28Z

The changes to core are going to be a little bit fun to merge with epic-ldml!

mcdurdin

This is looking good, but I think we need to look at how we are using meson throughout the entire repo and avoid referencing files from other projects (i.e. core from linux in this case).

core/src/meson.build

mcdurdin · 2023-01-31T05:45:32Z

core/src/meson.build

-  'km_kbp_processevent_api.cpp',
-  'jsonpp.cpp',
-  'mock/mock_processor.cpp',
+kmxfiles = files(


Suggested change

kmxfiles = files(

kmx_files = files(

and so on?

mcdurdin · 2023-01-31T05:47:20Z

linux/ibus-keyman/meson.build

+# define some variables. We wouldn't need this if we'd have a meson.build file
+# in the Keyman root directory...
+coredir = '@0@/../../core'.format(meson.current_source_dir())


The problem with putting a meson.build in the root is it impacts all paths, doesn't it?

That said, I'm not all that excited about linux builds listing all the files in core separately -- it undoes a lot of the good of using meson in the first place. I'd prefer to see us building core as a static library and importing it that way.

Good point. Refactored to create a static library and get rid of the direct references of core file.

For Core, we need to generate both static library and DLL on Windows -- static library used by Keyman Engine, DLL used by Developer.

linux/ibus-keyman/meson.build

mcdurdin · 2023-01-31T05:51:51Z

linux/ibus-keyman/src/meson.build

+utilfiles = files(
+  'keymanutil.c',
+  'keymanutil.h',
+  'kmpdetails.c',
+  'kmpdetails.h',
+)
+
+engine_sources = files(
+  'main.c',
+  'engine.c',
+  'engine.h',
+  'keycodes.h',
+  'keyman-service.c',
+  'keyman-service.h',
+)


Suggested change

utilfiles = files(

'keymanutil.c',

'keymanutil.h',

'kmpdetails.c',

'kmpdetails.h',

)

engine_sources = files(

'main.c',

'engine.c',

'engine.h',

'keycodes.h',

'keyman-service.c',

'keyman-service.h',

)

util_files = files(

'keymanutil.c',

'keymanutil.h',

'kmpdetails.c',

'kmpdetails.h',

)

engine_files = files(

'main.c',

'engine.c',

'engine.h',

'keycodes.h',

'keyman-service.c',

'keyman-service.h',

)

We should use either _files or _sources consistently.

mcdurdin · 2023-01-31T05:53:48Z

linux/ibus-keyman/tests/meson.build

@@ -0,0 +1,152 @@
+sources = [


Suggested change

sources = [

test_files = [

mcdurdin · 2023-01-31T05:54:35Z

linux/ibus-keyman/tests/scripts/find-tests.sh

+while IFS= read -r -d '' file; do
+  testname=$(basename "$file" .kmx)
+  printf "$(basename "$file")\t${testname#k_}\n"
+done < <(find . -name \*.kmx -print0 | sort -z)


Is this meant to be < <?

Yes, the first one is the redirection of input, the second one takes the output of the command.

emscripten doesn't support static libraries, so we explicitly build a static library unless we're building for wasm.

mcdurdin · 2023-01-31T22:09:27Z

Latest Developer build failed with:

[23:18:47][Step 1/4] 	copy C:\BuildAgent\work\7ac43416c45637e9\keyman\core\build\x86\Release\src\kmnkbp0-0.dll C:\BuildAgent\work\7ac43416c45637e9\keyman\developer\bin
[23:18:47][Step 1/4] The system cannot find the file specified.

mcdurdin

This looks much closer, good work. We just need to avoid changes to Core impacting other platforms

core/src/meson.build

linux/ibus-keyman/meson.build

The static library is currently only needed for the ibus-keyman tests, so we only build it on Linux. It can't build with emscripten, and on Windows it'd need some additional work.

mcdurdin

What is our minimum meson version requirement for Linux now? I am seeing conflicting data. We should make it consistent across all meson.build project specs and in our docs.

If we can go 0.49 or higher, then we can use / instead of join_paths() which would be superfantastic.

Apart from that, requesting lots of little tidy-up changes, mostly!

core/src/meson.build

mcdurdin · 2023-02-03T00:50:50Z

linux/ibus-keyman/meson.build

+    '@0@/../../core/build/arch/debug/src'.format(meson.current_source_dir()),
+    '@0@/../../core/build/arch/release/src'.format(meson.current_source_dir())


It might be better to conditionalize these on whether we are doing a debug build?

And then use core_dir:

core_dir / 'build/arch/debug/src' core_dir / 'build/arch/release/src'

linux/ibus-keyman/meson.build

mcdurdin · 2023-02-03T01:16:31Z

linux/ibus-keyman/tests/meson.build

+    env: [
+      'G_TEST_SRCDIR=@0@'.format(meson.current_source_dir()),
+      'G_TEST_BUILDDIR=@0@'.format(meson.current_build_dir()),
+    ],


Suggested change

env: [

'G_TEST_SRCDIR=@0@'.format(meson.current_source_dir()),

'G_TEST_BUILDDIR=@0@'.format(meson.current_build_dir()),

],

env: test_env,

mcdurdin · 2023-02-03T01:16:42Z

linux/ibus-keyman/tests/meson.build

+    env: [
+      'G_TEST_SRCDIR=@0@'.format(meson.current_source_dir()),
+      'G_TEST_BUILDDIR=@0@'.format(meson.current_build_dir()),
+    ],


Suggested change

env: [

'G_TEST_SRCDIR=@0@'.format(meson.current_source_dir()),

'G_TEST_BUILDDIR=@0@'.format(meson.current_build_dir()),

],

env: test_env,

mcdurdin · 2023-02-03T01:16:53Z

linux/ibus-keyman/tests/meson.build

+    env: [
+      'G_TEST_SRCDIR=@0@'.format(meson.current_source_dir()),
+      'G_TEST_BUILDDIR=@0@'.format(meson.current_build_dir()),
+    ],


Suggested change

env: [

'G_TEST_SRCDIR=@0@'.format(meson.current_source_dir()),

'G_TEST_BUILDDIR=@0@'.format(meson.current_build_dir()),

],

env: test_env,

mcdurdin · 2023-02-03T01:17:04Z

linux/ibus-keyman/tests/meson.build

+    env: [
+      'G_TEST_SRCDIR=@0@'.format(meson.current_source_dir()),
+      'G_TEST_BUILDDIR=@0@'.format(meson.current_build_dir()),
+    ],


Suggested change

env: [

'G_TEST_SRCDIR=@0@'.format(meson.current_source_dir()),

'G_TEST_BUILDDIR=@0@'.format(meson.current_build_dir()),

],

env: test_env,

linux/ibus-keyman/meson.build

- make use of new meson features - some refactoring

mcdurdin

LGTM

keyman-server · 2023-02-06T19:09:18Z

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

ermshiperete added this to the A17S5 milestone Jan 27, 2023

ermshiperete requested review from darcywong00, jahorton, mcdurdin and rc-swag as code owners January 27, 2023 17:45

github-actions bot added core/ Keyman Core docs linux/ linux/engine/ chore labels Jan 27, 2023

ermshiperete marked this pull request as draft January 27, 2023 21:19

ermshiperete force-pushed the chore/linux/5981_ibus-keyman_meson branch 2 times, most recently from ea0ac29 to 84c26fb Compare January 30, 2023 15:07

chore(linux): Build with meson instead of autotools

eb076c0

This change introduces meson for building ibus-keyman instead of autotools. Closes #5981.

ermshiperete force-pushed the chore/linux/5981_ibus-keyman_meson branch from 84c26fb to eb076c0 Compare January 30, 2023 15:36

ermshiperete marked this pull request as ready for review January 30, 2023 16:07

mcdurdin requested changes Jan 31, 2023

View reviewed changes

ermshiperete force-pushed the chore/linux/5981_ibus-keyman_meson branch 2 times, most recently from 874aa2b to 00b5f34 Compare January 31, 2023 16:29

chore(linux): Address code review comments

c176ebc

ermshiperete force-pushed the chore/linux/5981_ibus-keyman_meson branch from 00b5f34 to c176ebc Compare January 31, 2023 16:38

ermshiperete requested a review from mcdurdin January 31, 2023 17:43

chore(linux): Fix wasm build

d39ba17

emscripten doesn't support static libraries, so we explicitly build a static library unless we're building for wasm.

ermshiperete force-pushed the chore/linux/5981_ibus-keyman_meson branch from d3c5798 to d39ba17 Compare January 31, 2023 19:59

mcdurdin requested changes Jan 31, 2023

View reviewed changes

core/src/meson.build Outdated Show resolved Hide resolved

linux/ibus-keyman/meson.build Show resolved Hide resolved

chore(linux): Build static library only on Linux

1c602b8

The static library is currently only needed for the ibus-keyman tests, so we only build it on Linux. It can't build with emscripten, and on Windows it'd need some additional work.

ermshiperete requested a review from mcdurdin February 1, 2023 07:38

chore(linux): Fix a few bugs in meson scripts

0367df8

mcdurdin requested changes Feb 3, 2023

View reviewed changes

chore(linux): Address code review comments

e937e13

- make use of new meson features - some refactoring

ermshiperete requested a review from mcdurdin February 3, 2023 09:12

mcdurdin modified the milestones: A17S5, A17S6 Feb 4, 2023

mcdurdin approved these changes Feb 6, 2023

View reviewed changes

ermshiperete merged commit 1ef7db5 into master Feb 6, 2023

ermshiperete deleted the chore/linux/5981_ibus-keyman_meson branch February 6, 2023 16:47

srl295 mentioned this pull request Feb 10, 2023

chore: ldml merge from master 🙀 #8211

Merged

		'@0@/../../core/build/arch/debug/src'.format(meson.current_source_dir()),
		'@0@/../../core/build/arch/release/src'.format(meson.current_source_dir())

Uh oh!

Conversation

ermshiperete commented Jan 27, 2023

Uh oh!

keymanapp-test-bot bot commented Jan 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User Test Results

Uh oh!

mcdurdin commented Jan 31, 2023

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcdurdin commented Jan 31, 2023

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

keyman-server commented Feb 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

keymanapp-test-bot bot commented Jan 27, 2023 •

edited

Loading