amdgpu plugin: Make possible to install in other location (during build)

[carnil]

Hi

I'm currently trying to package 3.17 for Debian and noticed some problems with the newly introduced amdgpu plugin: The PLUGINDIR is almost fixed to /var/lib/criu which is not a problem, but during build the makefile makes it hard to have the plugins first installed below DESTDIR:

Something like

diff --git a/plugins/amdgpu/Makefile b/plugins/amdgpu/Makefile
index 84b9f871473d..42152c68e1cf 100644
--- a/plugins/amdgpu/Makefile
+++ b/plugins/amdgpu/Makefile
@@ -50,16 +50,16 @@ clean: amdgpu_plugin_clean amdgpu_plugin_test_clean
 mrproper: clean
 
 install:
-       $(Q) mkdir -p $(PLUGINDIR)
+       $(Q) mkdir -p $(DESTDIR)$(PLUGINDIR)
 ifeq ($(CONFIG_AMDGPU),y)
        $(E) "  INSTALL " $(PLUGIN_NAME)
-       $(Q) install -m 644 $(PLUGIN_SOBJ) $(PLUGINDIR)
+       $(Q) install -m 644 $(PLUGIN_SOBJ) $(DESTDIR)$(PLUGINDIR)
 endif
 .PHONY: install
 
 uninstall:
 ifeq ($(CONFIG_AMDGPU),y)
        $(E) " UNINSTALL" $(PLUGIN_NAME)
-       $(Q) $(RM) $(PLUGINDIR)/$(PLUGIN_SOBJ)
+       $(Q) $(RM) $(DESTDIR)$(PLUGINDIR)/$(PLUGIN_SOBJ)
 endif
 .PHONY: uninstall

I did not submit a full patch for it, as I'm not yet sure this is how you would like to handle it and I have not yet done much contributions, sorry about that :-(

A secondary problem is the build will fail afterwards ith the above changes applied, have not further investigated yet, yere the excerpt of the log:

gcc amdgpu_plugin_topology.c tests/test_topology_remap.c -o test_topology_remap -DCOMPILE_TESTS -iquote../../../criu/i
nclude -iquote../../criu/include -iquote../../criu/arch/x86/include/ -iquote../../ -I .
gcc -g -Wall -Werror -D _GNU_SOURCE -shared -nostartfiles -fPIC -I ../../compel/include/uapi amdgpu_plugin.c amdgpu_pl
ugin_topology.c criu-amdgpu.pb-c.c -o amdgpu_plugin.so -iquote../../../criu/include -iquote../../criu/include -iquote.
./../criu/arch/x86/include/ -iquote../../ -lpthread -lrt -ldrm -ldrm_amdgpu -I/usr/include/libdrm
make[4]: Nothing to be done for 'all'.                                                                                
make[4]: Nothing to be done for 'all'.                                                                                
amdgpu_plugin_topology.c:15:10: fatal error: common/list.h: No such file or directory
   15 | #include "common/list.h"
      |          ^~~~~~~~~~~~~~~
compilation terminated.
make[4]: Nothing to be done for 'all'.
tests/test_topology_remap.c:203:10: fatal error: common/list.h: No such file or directory
  203 | #include "common/list.h"
      |          ^~~~~~~~~~~~~~~
compilation terminated.
make[3]: *** [Makefile:39: test_topology_remap] Error 1
make[3]: *** Waiting for unfinished jobs....
Note: Writing crit.1
Note: Writing compel.1
In file included from amdgpu_plugin.c:27:
kfd_ioctl.h:26:10: fatal error: drm/drm.h: No such file or directory
   26 | #include <drm/drm.h>
      |          ^~~~~~~~~~~
compilation terminated.
Note: Writing amdgpu_plugin.1 
make[4]: Nothing to be done for 'all'.
In file included from amdgpu_plugin_topology.c:18:
kfd_ioctl.h:26:10: fatal error: drm/drm.h: No such file or directory
   26 | #include <drm/drm.h>
      |          ^~~~~~~~~~~
compilation terminated.
  INSTALL  criu/criu
  INSTALL  scripts/criu-ns
make[4]: 'lib/c/built-in.o' is up to date.
make[3]: *** [Makefile:31: amdgpu_plugin.so] Error 1
make[2]: *** [Makefile:328: amdgpu_plugin] Error 2
make[2]: *** Waiting for unfinished jobs....
Note: Writing criu.8
  INSTALL  criu.8
  INSTALL  crit.1 criu-ns.1 compel.1 amdgpu_plugin.1
make[2]: Leaving directory '/build/criu-3.17'
dh_auto_install: error: make -j16 install DESTDIR=/build/criu-3.17/debian/criu AM_UPDATE_INFO_DIR=no "INSTALL=install 
--strip-program=true" DESTDIR=/build/criu-3.17/debian/criu PREFIX=/usr LIBEXECDIR=/usr/lib PYTHON=python3 returned exi
t code 2
make[1]: *** [debian/rules:12: override_dh_auto_install] Error 25
make[1]: Leaving directory '/build/criu-3.17'

[carnil]

Related: #1876

[rst0git]

fatal error: drm/drm.h: No such file or directory

@carnil In this comment Rajneesh mentioned that the <drm/drm.h> headers should be provided by the kernel src package (i.e., linux-headers): https://packages.debian.org/bullseye/all/linux-headers-5.10.0-13-common/filelist

[siris]

Hi there,
I am working on packaging criu as part of the Funtoo Linux Distribution and when trying to compile the amdgpu plugin the we are seeing a similar No such file or directory errors as @carnil in regards to the common/list.h header included in the amdgpu plugin:

make -C plugins/amdgpu all
make[1]: Nothing to be done for 'all'.
protoc-c --proto_path=. --c_out=. criu-amdgpu.proto
gcc amdgpu_plugin_topology.c tests/test_topology_remap.c -o test_topology_remap -DCOMPILE_TESTS -iquote../../../criu/include -iquote../../criu/include -iquote../../criu/arch/x86/include/ -iquote../../ -I .
make -r -R -f /var/tmp/portage/sys-process/criu-3.17/work/criu-3.17/scripts/nmk/scripts/main.mk makefile=Makefile obj=criu/arch/x86 all
gcc -g -Wall -Werror -D _GNU_SOURCE -shared -nostartfiles -fPIC -I ../../compel/include/uapi amdgpu_plugin.c amdgpu_plugin_topology.c criu-amdgpu.pb-c.c -o amdgpu_plugin.so -iquote../../../criu/include -iquote../../criu/include -iquote../../criu/arch/x86/include/ -iquote../../ -lpthread -lrt -ldrm -ldrm_amdgpu -I/usr/include/libdrm
amdgpu_plugin_topology.c:15:10: fatal error: common/list.h: No such file or directory
   15 | #include "common/list.h"
      |          ^~~~~~~~~~~~~~~
compilation terminated.
make[2]: Nothing to be done for 'all'.
make -r -R -f /var/tmp/portage/sys-process/criu-3.17/work/criu-3.17/scripts/nmk/scripts/main.mk makefile=Makefile.library obj=criu/pie all
tests/test_topology_remap.c:203:10: fatal error: common/list.h: No such file or directory
  203 | #include "common/list.h"
      |          ^~~~~~~~~~~~~~~
compilation terminated.
make[1]: *** [Makefile:39: test_topology_remap] Error 1

The second of these errors resulted after running this command in our Funtoo ebuild:

emake -j1 CC="$(tc-getCC)" ARCH="$(criu_arch)" V=1 WERROR=0 test

Reference to our package management code triggering this error from criu version 3.17 is here: sys-process/criu/templates/criu.tmpl

@rst0git any ideas on what provides the common/list.h header so we can properly include it as a dependency? We are using the upstream sys-kernel/linux-headers version 5.10 and this does not appear to be apart of it.

[adrianreber]

common/list.h is a criu include file. The common/list.h is a bug in the amdgpu plugin Makefile. If you rename your criu directory from criu-3.17 to criu it should work. One of the include paths while compiling amdgpu_plugin_topology.c is wrong.

@fxkamd Can you, or one of your team, provide a fix to make sure the include paths in plugins/amgpu are not relying on the name of the checkout. I think you might just have one ../ to much in your include file path directive.

[adrianreber]

@carnil @siris CRIU does a pkg-config libdrm to check if the amdgpu plugin should be built. On Fedora we do not include libdrm-devel during package build in the build environment and so we have not seen the error. A workaround for your build problems would be to remove the equivalent of libdrm-devel from your buildroot if possible.

[carnil]

@adrianreber Thanks, I will probably then first focus on updating criu to 3.17 (and not include libdrm-dev in the Build-Depends to avoid trying to build it).

[carnil]

@adrianreber: For now I have uploaded criu 3.17 to Debian without the amdgpu plugin: https://tracker.debian.org/news/1324914/accepted-criu-317-1-source-into-experimental/

[siris]

@adrianreber we also mitigated the common/list.h bug by disabling the ability to compile the amdgpu plugin for CRIU version 3.17 on Funtoo Linux.

In regards to leveraging pkg-config libdrm as a compilation switch for the amdgpu plugin, this doesn't work too well in source based Linux distributions because many of them, like Gentoo and Funtoo Linux, do not have the concept of *-devel packages like many binary base Linux distributions.

One idea, leveraging a build system like meson or cmake for CRIU may make the compilation of CRIU more flexible across different Linux distributions.

If you are interested in seeing the workaround (its hacky) we are using in Funtoo Linux check out the ebuild template source code for the sys-process/criu-3.17 package.

We will stay tuned to a new release of CRIU with the bug fix, then we can remove workaround from Funtoo's CRIU ebuild template.

[eli-schwartz]

In regards to leveraging pkg-config libdrm as a compilation switch for the amdgpu plugin, this doesn't work too well in source based Linux distributions because many of them, like Gentoo and Funtoo Linux, do not have the concept of *-devel packages like many binary base Linux distributions.

Meson or cmake won't really help here if the problem is https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Automagic_dependencies as that's fully possible to do in both.

It will make it easier, if the project chooses to be careful, to avoid such pitfalls. What is really needed is a build option to control whether the amdgpu plugin is built, which is e.g. passed to Meson's dependency('libdrm', required: get_option('amdgpu-plugin')).

The default could then be behavior-preserving: check automatically for it. But distros can set it to force-enabled or force-disabled.

...

It is also possible, albeit harder, to do configure-time options in a custom Makefile. Just check if a Makefile variable has been defined as make OPT_FOO=true or make OPT_FOO=false, and override the probe checks. But Makefiles are not a very ergonomic language to write this sort of logic in.

[rst0git]

@fxkamd Can you, or one of your team, provide a fix to make sure the include paths in plugins/amgpu are not relying on the name of the checkout. I think you might just have one ../ to much in your include file path directive.

I've opened a pull request with a fix: #1890

[siris]

In regards to leveraging pkg-config libdrm as a compilation switch for the amdgpu plugin, this doesn't work too well in source based Linux distributions because many of them, like Gentoo and Funtoo Linux, do not have the concept of *-devel packages like many binary base Linux distributions.

Meson or cmake won't really help here if the problem is https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Automagic_dependencies as that's fully possible to do in both.

It will make it easier, if the project chooses to be careful, to avoid such pitfalls. What is really needed is a build option to control whether the amdgpu plugin is built, which is e.g. passed to Meson's dependency('libdrm', required: get_option('amdgpu-plugin')).

The default could then be behavior-preserving: check automatically for it. But distros can set it to force-enabled or force-disabled.

...

It is also possible, albeit harder, to do configure-time options in a custom Makefile. Just check if a Makefile variable has been defined as make OPT_FOO=true or make OPT_FOO=false, and override the probe checks. But Makefiles are not a very ergonomic language to write this sort of logic in.

Note that Funtoo has forked quite a lot from Gentoo, but in both Gentoo and Funtoo there is direct support for Meson build options quite cleanly through something called a Portage eclass. For an example meson ebuild that is used to toggle certain compile time options see: repos/kit-fixups/browse/desktop-kit/curated/x11-wm/enlightenment/templates/enlightenment.tmpl

In Funtoo Linux we follow the direction of the the upstream software provider on mostly all packages included. So whatever works best for the CRIU project maintainers, we will work with and adapt to. I hope I did not come off as suggesting a change to CRUI build system, I simply wanted to offer some ideas to be helpful 😄

All the help here as been lovely, thank you @rst0git , @adrianreber , and @eli-schwartz

[fxkamd]

Thank you, @rst0git. I had asked @rajbhar to look into this on our end. You beat him to it. Your pull request looks good to me. I'll comment there as well.