Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial support for meson build system #216

Draft
wants to merge 27 commits into
base: main
Choose a base branch
from
Draft

Initial support for meson build system #216

wants to merge 27 commits into from

Conversation

xbjfk
Copy link

@xbjfk xbjfk commented Apr 29, 2022

TODO:

  • Tests
  • ABI checking
  • Update .spec file
  • GitHub CI
  • Update android.mk
  • Test meson cross compiling

Can anyone explain how the abi checking, spec file and tests work roughly so I can implement them?

@frozencemetery
Copy link
Member

Can anyone explain how the abi checking, spec file and tests work roughly so I can implement them?

ABI checking: This is all using the libabigail tooling. Some docs are here if that's helpful.

SPEC file: this is downstream packaging input. We can take care of it on merge, but I think it's a pretty easy change - there are macros for autotools, and macros for meson. Some more information is in the Fedora packaging guidelines.

Tests: There are a bunch of programs in tests/ that get built and run, typically generating a "result" file. Passing typically constitutes the "result" file matching the "goal" file. I don't know that this clearly maps on to how meson views testing - calling out to a shell script that does the running + comparisons after the programs have been built is probably fine.

src/meson.build Outdated Show resolved Hide resolved
@eli-schwartz
Copy link

Passing typically constitutes the "result" file matching the "goal" file. I don't know that this clearly maps on to how meson views testing - calling out to a shell script that does the running + comparisons after the programs have been built is probably fine.

Meson has no feature to compare such logic. I guess you could submit a feature request to add such a feature, though I'm not sure what it would look like 🤔 so it might be better as a custom test script wrapper.

Signed-off-by: Reagan Bohan <[email protected]>
@xbjfk xbjfk force-pushed the main branch 4 times, most recently from c009082 to c160db4 Compare May 27, 2022 11:37
Signed-off-by: Reagan Bohan <[email protected]>
xbjfk added 6 commits May 27, 2022 23:43
Signed-off-by: Reagan Bohan <[email protected]>
Signed-off-by: Reagan Bohan <[email protected]>
Signed-off-by: Reagan Bohan <[email protected]>
@enunes
Copy link
Contributor

enunes commented Oct 7, 2022

I think it would be nice to have this merged before a next efivar release.
This addresses #130 which is a bit of a pain for cross-compilation environments. The biggest pain is the makeguids tool which needs to be built/executed for the host architecture and currently it also shares util.o with the target libraries, so it needs some workarounds for a successful cross-compiled build. This rework to meson seems to address this.

One thing I noticed while trying this is that it attempts to run the build and run the tests by default, which again hits issues in cross-compilation environments. The tests generation (during build) attempt to run efisecdb which is a target binary. I don't know how to easily address this, one easy workaround to get this going could be to have an option to just disable the tests generation (e.g. I can get a successful build if I comment-out subdir('tests') for the cross builds).

About the Android.mk TODO, would it be possible at all to just keep the Android.mk for now, so this MR could get back on track? I have seen projects who transitioned to meson but ended up keeping Android.mk for a while until it was sorted out.

Thanks

@eli-schwartz
Copy link

I wonder why efisecdb is run at build time instead of by the test harness? The test harness for a passing test only runs cmp/diff.

@xbjfk
Copy link
Author

xbjfk commented Oct 9, 2022

One thing I noticed while trying this is that it attempts to run the build and run the tests by default, which again hits issues in cross-compilation environments. The tests generation (during build) attempt to run efisecdb which is a target binary. I don't know how to easily address this, one easy workaround to get this going could be to have an option to just disable the tests generation (e.g. I can get a successful build if I comment-out subdir('tests') for the cross builds).

Setting exe_wrapper (qemu-user) when cross compiling allows tests to run, but for people who don't have an exe_wrapper, I will disable tests.

About the Android.mk TODO, would it be possible at all to just keep the Android.mk for now, so this MR could get back on track? I have seen projects who transitioned to meson but ended up keeping Android.mk for a while until it was sorted out.

This is my intention, however I had to change the paths of the efivar-guids.h so I would like to change and then test the Android.mk to make sure 100% it works. I will experiment with this soon.

@enunes
Copy link
Contributor

enunes commented Oct 12, 2022

Thanks for going back into this. It now skips trying to build the tests and running the target binaries.
For the toolchain I tried though it still had one last failure which is it did not find pthreads for the threads-test binary.

I solved it locally with:

diff --git a/src/meson.build b/src/meson.build
index aaaf6b8..90cda43 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -52,6 +52,7 @@ else
   dl = c.find_library('dl', required : false)
 endif

+thread_dep = dependency('threads')

 lib_targets = {}
 xml_targets = []
@@ -90,7 +91,7 @@ efisecdb_sources = ['efisecdb.c', 'secdb-dump.c', 'util.c']
 efisecdb = executable('efisecdb', efisecdb_sources, gen_guids, install : true, link_with : [lib_targets['efivar'], lib_targets['efisec']], include_directories : incdir)

 thread_test_sources = ['thread-test.c']
-thread_test = executable('thread-test', thread_test_sources, gen_guids[1], link_with : lib_targets['efivar'], include_directories : incdir)
+thread_test = executable('thread-test', thread_test_sources, gen_guids[1], dependencies : thread_dep, link_with : lib_targets['efivar'], include_directories : incdir)

 pkg = import('pkgconfig')

With that, the initial meson support seems good to me. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants