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

cmake: build fails due to semicolons in CMAKE_C_FLAGS set by custom pcscd.cmake & openssl.cmake #474

Open
wfrisch opened this issue Feb 15, 2024 · 1 comment · May be fixed by #475
Open

Comments

@wfrisch
Copy link

wfrisch commented Feb 15, 2024

yubico-piv-tool fails to build on a current openSUSE Tumbleweed (cmake version 3.28.1).

Symptom

Input:

git checkout yubico-piv-tool-2.5.1 
mkdir build/ && cd build/
cmake -DVERBOSE_CMAKE=1 ..
make VERBOSE=1

Output:

cd /home/wfrisch/src/yubico-piv-tool.git/build/lib && /usr/bin/cc -DOPENSSL_API_COMPAT=0x10000000L -DUSE_CERT_COMPRESS=\"1\" -I/home/wfrisch/src/yubico-piv-tool.git/lib -I/home/wfrisch/src/yubico-piv-tool.git/common -Wall -Werror -Wno-missing-braces -Wformat -Wformat-security -Wshadow -Wpointer-arith -Wmissing-prototypes -Wbad-function-cast -fstack-protector-all -Wno-pointer-sign -Wno-unused-result -DOPENSSL_LOAD_CONF;-I/usr/include -I/usr/include/PCSC -pthread -g -g2 -fno-omit-frame-pointer -std=gnu11 -fPIC -DSTATIC  -MD -MT lib/CMakeFiles/ykpiv.dir/ykpiv.c.o -MF CMakeFiles/ykpiv.dir/ykpiv.c.o.d -o CMakeFiles/ykpiv.dir/ykpiv.c.o -c /home/wfrisch/src/yubico-piv-tool.git/lib/ykpiv.c
cc: fatal error: no input files
compilation terminated.
/bin/sh: line 1: -I/usr/include: No such file or directory

This semicolon breaks the build: -DOPENSSL_LOAD_CONF;-I/usr/include.

Cause

As far as I can see, the root cause lies in the supplied custom modules pcscd.cmake and openssl.cmake. Both are using FindPkgConfig to retrieve the required CFLAGS and LDFLAGS. However FindPkgConfig returns lists, which are stored as semicolon-separated strings in CMake.

All but _FOUND may be a ;-list if the associated variable returned from pkg-config has multiple values.

Example:

include(FindPkgConfig)
pkg_check_modules(PCSC REQUIRED libpcsclite)
message("PCSC_LDFLAGS: ${PCSC_LDFLAGS}")

Output:

PCSC_LDFLAGS: -L/usr/lib64;-lpcsclite

I assume the developer of these custom modules was using a system where PCSC_CFLAGS and friends only contain one value and thus no semicolons.

Fix

The proper way to convert lists to strings in modern CMake is list(JOIN ...).

New in version 3.12.
Returns a string joining all list's elements using the glue string.

I will submit a PR shortly to address this bug in cmake/pcscd.cmake and cmake/openssl.cmake.

wfrisch added a commit to wfrisch/yubico-piv-tool that referenced this issue Feb 15, 2024
Both `openssl.cmake` and `pcscd.cmake` use FindPkgConfig to retrieve the
required CFLAGS and LDFLAGS. However FindPkgConfig returns lists [1],
which are stored as semicolon-separated strings in CMake.  This breaks
the build when there's more than one flag in any of those variables.

Fixes Yubico#474
wfrisch added a commit to wfrisch/yubico-piv-tool that referenced this issue Feb 15, 2024
Both `openssl.cmake` and `pcscd.cmake` use FindPkgConfig to retrieve the
required CFLAGS and LDFLAGS. However FindPkgConfig returns lists, which
are stored as semicolon-separated strings in CMake.  This breaks the
build when there's more than one flag in any of those variables.

Fixes Yubico#474
@wfrisch wfrisch linked a pull request Feb 15, 2024 that will close this issue
@wfrisch wfrisch changed the title cmake: build fails due to semicolons in CMAKE_C_FLAGS set by custom pcsc.cmake & openssl.cmake cmake: build fails due to semicolons in CMAKE_C_FLAGS set by custom pcscd.cmake & openssl.cmake Feb 15, 2024
@aveenismail
Copy link
Member

Thank you for bringing this issue to our attention and providing the PR to fix.

However, we still aim to have yubico-piv-tool build on older Linux platforms that do not support cmake version newer than 3.5 so this PR will be breaking building on those platforms. Now that we know about this issue though, we'll be looking into finding a way to fix it without having to use a newer cmake version.

Thank you again for the PR, it is highly appreciate it.

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

Successfully merging a pull request may close this issue.

2 participants