CMake scripts cleanups #680

pcercuei · 2021-05-05T15:41:02Z

A set of patches to cleanup the CMake scripts.

This change is a bit more invasive than usual, as it changes the behaviour of the script. For that reason, it is possible that the autobuilders will fail, as their scripts may have to be enabled.

Instead of auto-enabling functions according to the environment (available libraries etc), the options are now never auto-selected, and the environment is probed according to the enabled options.

The goal of this is to make the builds reproductible: now, using the same configuration on two systems will result in the same features being enabled in the library.

The scripts have also been updated to use the 3.x CMake features.

The code already checked for the WITH_AIO macro, and almost all configurable options in libiio and IIOD use the "WITH" prefix. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

rgetz · 2021-05-06T02:38:16Z

The scripts have also been updated to use the 3.x CMake features.

yeah, the still shipping Zynq Images use old cmake - this would break too many boards in the field.

The new Kuiper images that we are moving to use 3.13; I don't think that's the default image yet.

root@analog:~# cmake --version
cmake version 3.13.4

plus it causes all CI builds to fail. Centos like this:

CMake Error at cmake/Install.cmake:177 (include):
  include could not find load file:

    LinuxPackaging.cmake
Call Stack (most recent call first):
  CMakeLists.txt:425 (include)

nunojsa · 2021-05-06T07:09:28Z

yeah, the still shipping Zynq Images use old cmake - this would break too many boards in the field.

FWIW, cmake 3.20.2 is already complaining about the used version...

CMake Deprecation Warning at tests/CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 2.8.12 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.

nunojsa

Apart from the cmake version bump question, looks a very good cleanup to me

nunojsa · 2021-05-06T07:27:48Z

iio-config.h.cmakein

@@ -17,9 +17,9 @@
 #cmakedefine01 WITH_NETWORK_EVENTFD
 #cmakedefine01 WITH_IIOD_USBD
 #cmakedefine01 WITH_LOCAL_CONFIG
+#cmakedefine01 WITH_AIO


Just a comment which has noting to do with this... We should definitely consider about replacing aio for io_uring and liburing for libiio2 😄

Yes, I will definitely have a look at io_uring. I want to see if it can help us with zero-copy.

It has both ring buffers shared between kernel and userspace so that's something already in the right way... Another neat thing is that we can potential reduce a lot the number of syscalls.

But I assume the IIO subsystem needs to support it (does it?).

Don't think so. At least for normal reads/writes on sysfs stuff. Now for ioctl (not sure if that stuff was merged; and yes, it seems io_uring is also allowing to re-implement ioctl), the last time I checked I think IIO would have to support it...

https://lwn.net/Articles/844875/ - not idea the outcome of the RFC

nunojsa · 2021-05-06T07:31:29Z

iiod/CMakeLists.txt

@@ -24,7 +24,7 @@ find_library(LIBAIO_LIBRARIES aio)
 find_path(LIBAIO_INCLUDE_DIR libaio.h)

 if (LIBAIO_LIBRARIES AND LIBAIO_INCLUDE_DIR)
-	option(ENABLE_AIO "Build IIOD with async. I/O support" ON)
+	option(WITH_AIO "Build IIOD with async. I/O support" ON)


Does this mean that we only provide the option if the libraries are installed in the system? If so, I'm not sure if it would not be better to always provide the option and look for the libraries if the option is enabled. Then make it explicit that dependencies are needed (cmake failure)... Otherwise, users might not even be aware that this feature is possible...

nunojsa · 2021-05-06T07:33:43Z

CMakeLists.txt

+		message(SEND_ERROR "Unable to find libusb-1.0 dependency.\n"
+			"If you want to disable the USB backend, set WITH_USB_BACKEND=OFF.")
+	endif()
+


Hmm, you can disregard my previous comment 👍

pcercuei · 2021-05-06T08:22:53Z

The scripts have also been updated to use the 3.x CMake features.

yeah, the still shipping Zynq Images use old cmake - this would break too many boards in the field.

Isn't there a "backports" feature like on Ubuntu/Debian?

pcercuei · 2021-05-06T09:04:18Z

@rgetz Now would maybe be a good time to branch libiio2. Then we can up the CMake requirements in the libiio2 branch.

rgetz · 2021-05-06T13:52:09Z

@rgetz Now would maybe be a good time to branch libiio2. Then we can up the CMake requirements in the libiio2 branch.

that makes total sense to me.

rgetz · 2021-05-06T19:41:12Z

but the CI failures still should be fixed, or you won't have much test coverage going forward...

pcercuei · 2021-05-07T08:12:01Z

@rgetz yes. Don't worry ;)

Old OSes (CentOS 7, Ubuntu 16.04) require s6_addr32 to be defined for the IN6_IS_ADDR_LINKLOCAL() macro to work. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

This function is not used in libiio, but is used by IIOD. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

We do not want to compile with GNU-specific extensions, as we want the code to be as portable as possible. Thanksfully we don't actually need _GNU_SOURCE to be defined, removing it only triggers an error in network-unix.c about an undefined structure, which we can fix by including the right header. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

Instead of making build options available according to whether or not a library is found, only search for the library if the option has been enabled. This change may temporarily break the auto-builders (which will need to be updated), but it makes the build *reproductible*, and that's very important. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

Disable serial backend, enable ZSTD support. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

Disable serial and USB backends. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

Enable ZSTD support, except on Ubuntu 16.04 where it doesn't build, and explicitely set the build options as it is required now. Note that the serial backend is disabled on Ubuntu 16.04 as its version of libserialport is too old, and USB support in IIOD is disabled in CentOS 7 as its kernel headers are too old. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

pcercuei · 2021-05-07T11:28:29Z

Alright, it should be ready for merging now.

Note that USB support in IIOD is disabled in CentOS 7, and UART support and ZSTD compression are disabled on Ubuntu 16.04, as the corresponding dependencies are too old on these OSes.

With the changes introduced in [1], libiio now looks for system dependencies depending on enabled options. Hence, as the serial backend is enabled by default, libserialport has to be set as DEPENDS. This patch adds the SERIAL_BACKEND to the list of PACKAGECONFIG for libiio. [1]: analogdevicesinc/libiio#680 Signed-off-by: Nuno Sá <nuno.sa@analog.com>

This fix is still related with the changes introduced by [1]. If 'HAVE_DNS_SD' is ON, we need to have the correspondent avahi dependencies set... However, for microblaze builds, zeroconf is not part of the petalinux 'DISTRO_FEATURES'. Hence, we disable 'DHAVE_DNS_SD' if zeroconf is not present. [1]: analogdevicesinc/libiio#680 Signed-off-by: Nuno Sá <nuno.sa@analog.com>

iiod: Use WITH_AIO instead of ENABLE_AIO

826e451

The code already checked for the WITH_AIO macro, and almost all configurable options in libiio and IIOD use the "WITH" prefix. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

nunojsa approved these changes May 6, 2021

View reviewed changes

pcercuei force-pushed the clean-cmake branch 6 times, most recently from d20077e to b6d4eea Compare May 6, 2021 11:54

pcercuei force-pushed the clean-cmake branch 6 times, most recently from 9707c35 to 73ec11b Compare May 7, 2021 10:16

pcercuei added 6 commits May 7, 2021 11:50

network: Fix IN6_IS_ADDR_LINKLOCAL() macro on old OSes

70c4e0e

Old OSes (CentOS 7, Ubuntu 16.04) require s6_addr32 to be defined for the IN6_IS_ADDR_LINKLOCAL() macro to work. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

CMake: Move check for pthread_setname_np to IIOD

adf33f2

This function is not used in libiio, but is used by IIOD. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

CI: Update build script for Mac OSX

9c87375

Disable serial backend, enable ZSTD support. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

CI: Update build script for Windows

ff96fc8

Disable serial and USB backends. Signed-off-by: Paul Cercueil <paul@crapouillou.net>

pcercuei force-pushed the clean-cmake branch from 73ec11b to 2f73fdb Compare May 7, 2021 10:52

pcercuei force-pushed the clean-cmake branch from 2f73fdb to c31cc2f Compare May 7, 2021 11:08

pcercuei merged commit c32fab4 into master May 11, 2021

pcercuei deleted the clean-cmake branch May 11, 2021 12:03

This was referenced May 17, 2021

[master][meta-oe] libiio: add serial_backend support plus minor tweak openembedded/meta-openembedded#359

Closed

meta-adi-xilinx: fix libiio configuration analogdevicesinc/meta-adi#98

Merged

nunojsa mentioned this pull request May 26, 2021

meta-adi-xilinx: libiio: fix build for microblaze analogdevicesinc/meta-adi#99

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CMake scripts cleanups #680

CMake scripts cleanups #680

pcercuei commented May 5, 2021

rgetz commented May 6, 2021

nunojsa commented May 6, 2021

nunojsa left a comment

nunojsa May 6, 2021 •

edited

Loading

pcercuei May 6, 2021

nunojsa May 6, 2021

pcercuei May 6, 2021

nunojsa May 6, 2021

nunojsa May 6, 2021

nunojsa May 6, 2021

nunojsa May 6, 2021

pcercuei commented May 6, 2021

pcercuei commented May 6, 2021

rgetz commented May 6, 2021

rgetz commented May 6, 2021

pcercuei commented May 7, 2021

pcercuei commented May 7, 2021

CMake scripts cleanups #680

CMake scripts cleanups #680

Conversation

pcercuei commented May 5, 2021

rgetz commented May 6, 2021

nunojsa commented May 6, 2021

nunojsa left a comment

Choose a reason for hiding this comment

nunojsa May 6, 2021 • edited Loading

Choose a reason for hiding this comment

pcercuei May 6, 2021

Choose a reason for hiding this comment

nunojsa May 6, 2021

Choose a reason for hiding this comment

pcercuei May 6, 2021

Choose a reason for hiding this comment

nunojsa May 6, 2021

Choose a reason for hiding this comment

nunojsa May 6, 2021

Choose a reason for hiding this comment

nunojsa May 6, 2021

Choose a reason for hiding this comment

nunojsa May 6, 2021

Choose a reason for hiding this comment

pcercuei commented May 6, 2021

pcercuei commented May 6, 2021

rgetz commented May 6, 2021

rgetz commented May 6, 2021

pcercuei commented May 7, 2021

pcercuei commented May 7, 2021

nunojsa May 6, 2021 •

edited

Loading