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
build: add support for meson #3128
base: master
Are you sure you want to change the base?
Conversation
@frankmorgner @Jakuje The libraries
Questions:
|
The OpenSC API is considered as an internal API (even though we still try to version it). The libssm-local.so is API for openSC to handle Secure Messaging. I think, in theory, it can be used as plug-able API for third party implementation of the SM talking to specific cards in "more secure" manner (read closed source). I think the reason to version them separately is the SM API is much more stable (there are only few functions) so I think the idea was not to break it with the changes in opensc API, but I do not know if there are any users of this now. Regarding the OpenSC API, as I mentioned previously, we consider it internal, we do not ship pkgconfig files and do not advise anyone using it outside of opensc tools. Generally, any application that wants to use OpenSC should go through the PKCS#11 API, which is standardized. |
@H5117 see https://www.sourceware.org/autobook/autobook/autobook_61.html for more information about the reasoning behind this library version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, this will really facilate the on-boarding process of new developers.
The following features are missing (did I miss any?), so that we would have to support autotools a little longer:
- unittests, p11tests (code coverage, valgrind, fuzzing even though a switch exists, cmocka)
- clang-tidy
- disable warning
-Wno-unknown-warning-option
if needed
One benefit over the autotools is that dependencies are listed explicitly for each executable.
Would it be possible to create some github workflow for visual studio an xcode on windows/mac and plain meson on linux?
The test scripts in |
The paths are defined as part of Line 11 in 1fb5655
So just creating an environment variable |
Meson automatically generates a target to check all sources in the project with clang-tidy (link). Is it OK for you or you need to check only specific files? EDIT: implemented. |
Could you explain why you use this option only for some tools and not for the whole project? |
By the way, should we install legacy symlinks for onepin-opensc-pkcs11.so in new build system? If someone decide to switch building to Meson, he can also switch to opensc-pkcs11.so. |
For some tools we are using code generation for CLI parsing. The generated files are causing this warning, so we need to disable this warning explicitly for those files. |
Lines 144 to 148 in 1fb5655
|
This is about PKCS#11 users rather than developers. We don't want to break existing configurations or scripts using the old location of the module. |
26f2067
to
4f9b154
Compare
Fixed. |
thank you. One thing we struggled setting up with the autotools is the pure static linking of the pkcs11 module. We had several tries of doing this, but did not manage to set this up correctly. |
I added options to control static linking. Works for me. Static linking
For external dependencies Meson has an option |
Thank you for this work so far. I think it is going in the right direction. I put there some inline comments. Can you also adjust the CI configuration and scripts to use the meson instead of autotools (at least for now to see how it behaves -- then we can figure out if we can adjust it somehow to run both). |
Yes, would be the preferred linking for pkcs11 modules (for both, internal and external dependencies). This would allow an application to include multiple pkcs11 modules with different types os versions of dependencies. For the opensc tools, however, shared linking woul be preferred for both. |
I added optional compiling of static external dependencies, so this is now possible. With commands: meson setup --prefix=/usr -Dcomponents=pkcs11 -Dp11kit=disabled -Dlibopensc=static --wrap-mode=forcefallback . build-static
cd build-static
meson configure -Dlibffi:default_library=static
meson configure -Dproxy-libintl:default_library=static I get this result:
Currently OpenPACE and readline are not compiled, but can be added later. |
Co-authored-by: Frank Morgner <frankmorgner@gmail.com> Co-authored-by: Jussi Pakkanen <jpakkane@gmail.com>
Implemented. Commands:
Result:
|
Thank you, this was a long standing issue with autotools that meson now solves. I really think we should do this by default for the pkcs#11 module (e.g. without additional configuration parameters), but maybe there are different opinions. I also really like that you already added a way of pulling and building the dependencies ("subprojects"). Is that done automatically when pkg-config cannot find it locally or does it need to be triggered during configuration? In any case, we should document the fact that we are pulling patches from mesonbuild.com in some cases. From the CI side, it would still be nice to get a Github workflow that builds with meson. (Maybe I find some time at the end of the week for that.) |
I think we should not. GNU/Linux distributions always compile OpenSC tools, so statically compiled PKCS#11 library would be just a waste of memory. The right place for statically compiled PKCS#11 module in my opinion — a build artefact on the releases page of the project.
Subprojects are compiled automatically, if they provide required dependencies (
My plan is to finish support for Windows first, to be sure that options or targets won't be changed. |
I see that |
Inno Setup still works and it is built in AppVeyor. However, we're only distributing the WiX installer for releases. I don't think ISS support is needed. |
It is used also for the mingw builds in github actions. They create exe installers, but I think we do not publish them as part of release though. Not sure if this pipeline is needed then. |
Fixes #3110.