-
Notifications
You must be signed in to change notification settings - Fork 709
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. |
1977e2e
to
8c23e43
Compare
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. |
Co-authored-by: Frank Morgner <[email protected]> Co-authored-by: Jussi Pakkanen <[email protected]>
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.
How do you use or find gengetopt on Windows? There are binaries available on sourceforge, but I would prefer avoiding that (old) package and rather build gengetopt from source. Also, gengetopt needs to be configured with --include-getopt
on Windows
Just use a package from MSYS2. Use this branch if you want to try building on Windows. It currently compiles all components except the minidriver. Also OpenPACE fails to build as a subproject (it does not honor DESTDIR on Windows for some reason). |
Why don't you want to use your own version from By the way, what do you think about gnulib implementation? |
Right, I forgot about that. I think meson (this PR's branch) doesn't notice that getopt is missing for including this file. I'll try meson.dev.
I don't have strong feelings, I just noticed the build failure on my machine. |
I followed your recommendation and set it up with gengetopt from msys2 and your meson.dev branch. However, I needed to modify some code regarding the missing getopt on Windows (getopt.diff)diff --git a/src/tools/Makefile.am b/src/tools/Makefile.am
index 1202420fb..3ddd691b6 100644
--- a/src/tools/Makefile.am
+++ b/src/tools/Makefile.am
@@ -143,8 +143,8 @@ endif
.PHONY: cmdline
cmdline:
@for f in *.ggo.in; do $(do_subst) < "$$f" > "$${f%.in}"; done
- @for f in *.ggo; do $(GENGETOPT) --file-name="$${f%.ggo}-cmdline" --output-dir=$(builddir) < "$$f"; done
- $(AM_V_GEN)$(GENGETOPT) --file-name=opensc-asn1-cmdline --output-dir=$(builddir) --unamed-opts < opensc-asn1.ggo
+ @for f in *.ggo; do $(GENGETOPT) --include-getopt --file-name="$${f%.ggo}-cmdline" --output-dir=$(builddir) < "$$f"; done
+ $(AM_V_GEN)$(GENGETOPT) --include-getopt --file-name=opensc-asn1-cmdline --output-dir=$(builddir) --unamed-opts < opensc-asn1.ggo
if WIN32
LIBS += -lshlwapi
diff --git a/src/tools/eidenv.c b/src/tools/eidenv.c
index 62ddd2164..8b607a8a0 100644
--- a/src/tools/eidenv.c
+++ b/src/tools/eidenv.c
@@ -29,7 +29,12 @@
#include <stdlib.h>
#include <string.h>
+#ifdef HAVE_GETOPT
#include <getopt.h>
+#else
+#include "common/compat_getopt.h"
+#endif
+
#include "libopensc/opensc.h"
#include "libopensc/asn1.h"
#include "libopensc/cards.h"
diff --git a/src/tools/meson.build b/src/tools/meson.build
index a5cd5f92d..792837b22 100644
--- a/src/tools/meson.build
+++ b/src/tools/meson.build
@@ -88,6 +88,7 @@ executable('dtrust-tool',
egk_tool_target = custom_target(
command:[proggengetopt,
'--input=@INPUT@',
+ '--include-getopt',
'--set-version=' + meson.project_version(),
'--output-dir=@OUTDIR@',
'--file-name=egk-tool-cmdline'
@@ -134,6 +135,7 @@ executable('eidenv',
goid_tool_target = custom_target(
command:[proggengetopt,
'--input=@INPUT@',
+ '--include-getopt',
'--set-version=' + meson.project_version(),
'--output-dir=@OUTDIR@',
'--file-name=goid-tool-cmdline'
@@ -197,6 +199,7 @@ executable('openpgp-tool',
opensc_asn1_target = custom_target(
command:[proggengetopt,
'--input=@INPUT@',
+ '--include-getopt',
'--set-version=' + meson.project_version(),
'--output-dir=@OUTDIR@',
'--file-name=opensc-asn1-cmdline'
@@ -257,6 +260,7 @@ executable('opensc-tool',
pkcs11_register_target = custom_target(
command:[proggengetopt,
'--input=@INPUT@',
+ '--include-getopt',
'--set-version=' + meson.project_version(),
'--output-dir=@OUTDIR@',
'--file-name=pkcs11-register-cmdline'
@@ -447,6 +451,7 @@ if conf.get('ENABLE_OPENSSL')
npa_tool_target = custom_target(
command:[proggengetopt,
'--input=@INPUT@',
+ '--include-getopt',
'--set-version=' + meson.project_version(),
'--output-dir=@OUTDIR@',
'--file-name=npa-tool-cmdline'
@@ -487,6 +492,7 @@ if conf.get('ENABLE_NOTIFY')
opensc_notify_targets += custom_target(
command:[proggengetopt,
'--input=@INPUT@',
+ '--include-getopt',
'--set-version=' + meson.project_version(),
'--output-dir=@OUTDIR@',
'--file-name=opensc-notify-cmdline'
diff --git a/src/tools/openpgp-tool.c b/src/tools/openpgp-tool.c
index dfd72eab9..5ca004027 100644
--- a/src/tools/openpgp-tool.c
+++ b/src/tools/openpgp-tool.c
@@ -37,7 +37,11 @@
#include <string.h>
#include <ctype.h>
+#ifdef HAVE_GETOPT_H
#include <getopt.h>
+#else
+#include "common/compat_getopt.h"
+#endif
#include "libopensc/opensc.h"
#include "libopensc/asn1.h"
#include "libopensc/cards.h"
diff --git a/src/tools/opensc-explorer.c b/src/tools/opensc-explorer.c
index 108e4f876..3c6ae8eb5 100644
--- a/src/tools/opensc-explorer.c
+++ b/src/tools/opensc-explorer.c
@@ -50,7 +50,11 @@
#include "libopensc/internal.h"
#include "libopensc/iso7816.h"
#include "common/compat_strlcpy.h"
+#ifdef HAVE_GETOPT_H
#include <getopt.h>
+#else
+#include "common/compat_getopt.h"
+#endif
#include "util.h"
#define DIM(v) (sizeof(v)/sizeof((v)[0]))
diff --git a/src/tools/util.h b/src/tools/util.h
index 07c33f26b..7e62300c4 100644
--- a/src/tools/util.h
+++ b/src/tools/util.h
@@ -16,7 +16,11 @@
#endif
#include <sys/stat.h>
+#ifdef HAVE_GETOPT_H
#include <getopt.h>
+#else
+#include "common/compat_getopt.h"
+#endif
#include "libopensc/opensc.h"
#ifdef __cplusplus
|
Please show the error message. Also, |
This would be the first instance of the problem, that is solved with the patch above: PS> meson compile -C builddir
Activating VS 17.7.4
INFO: automatically activated MSVC compiler environment
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: "C:\Program Files\Meson\ninja.EXE" -C C:/Users/Frank/OpenSC/OpenSC/builddir
ninja: Entering directory `C:/Users/Frank/OpenSC/OpenSC/builddir'
[110/231] Compiling C object src/libopensc/opensc-11.dll.p/log.c.obj
../src/libopensc/log.c(221): warning C4312: "Typumwandlung": Konvertierung von "int" in größeren Typ "HANDLE"
../src/libopensc/log.c(259): warning C4312: "Typumwandlung": Konvertierung von "int" in größeren Typ "HANDLE"
[153/231] Compiling C object src/tools/libutil.a.p/util.c.obj
FAILED: src/tools/libutil.a.p/util.c.obj
"cl" "-Isrc/tools\libutil.a.p" "-Isrc/tools" "-I..\src\tools" "-I." "-I.." "-Isrc" "-I..\src" "/MDd" "/nologo" "/showIncludes" "/utf-8" "/W2" "/Od" "/Zi" "-DHAVE_CONFIG_H" "/Fdsrc/tools\libutil.a.p\util.c.pdb" /Fosrc/tools/libutil.a.p/util.c.obj "/c" ../src/tools/util.c
C:\Users\Frank\OpenSC\OpenSC\src\tools\util.h(19): fatal error C1083: Datei (Include) kann nicht geöffnet werden: "getopt.h": No such file or directory
[162/231] Compiling C object src/libopensc/opensc-11.dll.p/reader-pcsc.c.obj
ninja: build stopped: subcommand failed. |
I suspect that you configured the project in MSYS2 environment, didn't you? If so, Meson correctly detects that What I did is just adding |
That's exactly what I did as well. The build failure that I received, makes perfectly sense since on Windows (and in my msys2 installation) there is no implementation of getopt. And since OpenSC's variant requires the inclusion of a different file ( |
Then I don't understand why it does not work for you. Could you post the full log from
Your current build system works fine without this patch. See here. |
... because of this line: Line 23 in 19b0e98
|
I don't see anything suspicious in the journal and can't reproduce on MSVC 2022. Please let me know if you get some clue.
Meson also have such a line. |
I updated the "meson.dev" branch. It now compiles the minidriver also. EDIT: the minidriver uses some internal symbols of libopensc, so compiles with -Dlibopensc=static only. |
Please test the updated "meson.dev". Does it work for you now? |
Fixes #3110, #2577.