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

meson: add option to allow override default default_pkcs11_module #511

Merged

Conversation

embetrix
Copy link
Contributor

fix #510

@embetrix embetrix force-pushed the feature/allow-overwrite-default-pkcs11-module branch from 8dab31d to 5031b86 Compare January 22, 2025 12:41
Copy link
Member

@simo5 simo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some changes are needed

meson.build Outdated Show resolved Hide resolved
meson_options.txt Outdated Show resolved Hide resolved
@embetrix embetrix force-pushed the feature/allow-overwrite-default-pkcs11-module branch 2 times, most recently from 8b6bc83 to 57fc9db Compare January 22, 2025 15:10
@embetrix embetrix requested a review from simo5 January 22, 2025 15:12
meson.build Outdated Show resolved Hide resolved
@embetrix embetrix force-pushed the feature/allow-overwrite-default-pkcs11-module branch from 57fc9db to 20c7938 Compare January 22, 2025 15:14
@embetrix
Copy link
Contributor Author

I tested all combinations:

meson setup builddir 
meson setup builddir -Ddefault_pkcs11_module=""
meson setup builddir -Ddefault_pkcs11_module=/opt/path/mymod.so

Possible configs:

builddir/config.h:#define DEFAULT_PKCS11_MODULE "/usr/lib/x86_64-linux-gnu/p11-kit-proxy.so"
builddir/config.h:#define DEFAULT_PKCS11_MODULE "/usr/lib/x86_64-linux-gnu/p11-kit-proxy.so"
builddir/config.h:#define DEFAULT_PKCS11_MODULE "/opt/path/mymod.so"

simo5
simo5 previously approved these changes Jan 22, 2025
Copy link
Member

@simo5 simo5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@embetrix
Copy link
Contributor Author

embetrix commented Jan 22, 2025

@simo5 looks like libp11-kit-dev dependency is missing in the CI

@embetrix embetrix force-pushed the feature/allow-overwrite-default-pkcs11-module branch from 46c871d to 3b9c319 Compare January 22, 2025 15:42
@simo5
Copy link
Member

simo5 commented Jan 22, 2025

@simo5 looks like libp11-kit-dev dependency is missing in the CI

It may have been intentional, we do support building w/o p11-kit-dev ... in which case no default would be present so now that I think of it I guess we need to allow for no defaults

@embetrix embetrix force-pushed the feature/allow-overwrite-default-pkcs11-module branch from 3b9c319 to 94beabb Compare January 22, 2025 16:06
@simo5
Copy link
Member

simo5 commented Jan 22, 2025

Indeed we do support it not being defined:

#ifdef DEFAULT_PKCS11_MODULE

So I guess we need to allow an undefined defult, I guess the code needs to be:

if default_pkcs11_module != ''
  conf.set_quoted('DEFAULT_PKCS11_MODULE', default_pkcs11_module)
endif

And leave the CI with default undefined (ie revert the change to add p11-kit-dev)

@embetrix embetrix force-pushed the feature/allow-overwrite-default-pkcs11-module branch from 94beabb to ebab1e0 Compare January 22, 2025 16:15
@embetrix
Copy link
Contributor Author

@simo5: done

@simo5
Copy link
Member

simo5 commented Jan 22, 2025

Lately MacOS CI is flakier and times out, I tried to respin it and timed out again, so I am just going to ignore it for now.
Thanks for the speedy contribution.

@simo5 simo5 merged commit d7f5de2 into latchset:main Jan 22, 2025
40 of 41 checks passed
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.

Meson option to allow override of DEFAULT_PKCS11_MODULE path
2 participants