Skip to contentSkip to conversation

libseccomp: Add new config option to build and install tests

PATCH Superseded Markos Chandras 3 participants 13 messages Sep 17, 2015 – Nov 10, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 libseccomp: Add new config option to build and install tests

Add new config option to allow building and installing the libseccomp testsuite on the target. It's useful for testing the kernels' seccomp behavior for the target architecture.

---
 package/libseccomp/Config.in     |  9 +++++++++
 package/libseccomp/libseccomp.mk | 19 +++++++++++++++++++
 2 files changed, 28 insertions(+)
package/libseccomp/Config.in +9 -0
index 4c34c5466592..760a6f321142 100644
@@ -15,6 +15,15 @@ config BR2_PACKAGE_LIBSECCOMP
 
 	  https://github.com/seccomp/libseccomp
 
+config BR2_PACKAGE_LIBSECCOMP_TESTS
+	bool "Install libseccomp tests"
+	depends on BR2_PACKAGE_BASH && BR2_PACKAGE_LIBSECCOMP
+	help
+	  Build and install the libseccomp testsuite.
+
+comment "libseccomp testsuite needs bash as running shell"
+	depends on !BR2_PACKAGE_BASH && BR2_PACKAGE_LIBSECCOMP
+
 comment "libseccomp needs a toolchain w/ headers >= 3.12"
 	depends on BR2_aarch64 || BR2_mips || BR2_mipsel || BR2_mips64 || \
 		BR2_mips64el || BR2_i386 || BR2_x86_64
package/libseccomp/libseccomp.mk +19 -0
index a188298006b1..6edf96f92bfa 100644
@@ -17,4 +17,23 @@ define LIBSECCOMP_FIXUP_M4_DIR
 endef
 LIBSECCOMP_POST_EXTRACT_HOOKS += LIBSECCOMP_FIXUP_M4_DIR
 
+ifeq ($(BR2_PACKAGE_LIBSECCOMP_TESTS),y)
+
+define LIBSECCOMP_TESTS_BUILD
+	$(MAKE) -C $(@D)/tests check-build
+endef
+
+define LIBSECCOMP_TESTS_INSTALL
+	mkdir -p $(TARGET_DIR)/usr/libseccomp/{tests,tools} && \
+	for x in tests tools; do \
+		find $(@D)/$$x -maxdepth 1 \( -name "*.tests" -o -perm -a=x \) \
+			-type f -exec cp {} $(TARGET_DIR)/usr/libseccomp/$$x/ \; ; \
+	done
+endef
+
+LIBSECCOMP_POST_BUILD_HOOKS += LIBSECCOMP_TESTS_BUILD
+LIBSECCOMP_POST_INSTALL_TARGET_HOOKS += LIBSECCOMP_TESTS_INSTALL
+
+endif
+
 $(eval $(autotools-package))
Signature · 1 line
2.5.2
#1 lore raw

PATCH 1/1 libseccomp: Add new config option to build and install tests

Dear Markos Chandras,

On 09/17/2015 02:53 PM, Markos Chandras wrote:
[snip]

package/libseccomp/libseccomp.mk define LIBSECCOMP_FIXUP_M4_DIR · line 23

Quoted 12 lines
package/libseccomp/libseccomp.mk +4 -0
index a188298006b1..6edf96f92bfa 100644
@@ -17,4 +17,23 @@ define LIBSECCOMP_FIXUP_M4_DIR
 endef
 LIBSECCOMP_POST_EXTRACT_HOOKS += LIBSECCOMP_FIXUP_M4_DIR
 
+ifeq ($(BR2_PACKAGE_LIBSECCOMP_TESTS),y)
+
+define LIBSECCOMP_TESTS_BUILD
+	$(MAKE) -C $(@D)/tests check-build

You should be adding $(TARGET_MAKE_ENV) in front of $(MAKE). Anyway, there is a better solution which will make this define unnecessary. See comments below.

package/libseccomp/libseccomp.mk define LIBSECCOMP_FIXUP_M4_DIR · line 24

Quoted 1 line
package/libseccomp/libseccomp.mk +1 -0
+endef

Instead of doing a define to build the tests, you can change the target and use the 'check-build' one, which will not only build the tests and tools, but also will trigger the 'all' target. So:

ifeq ($(BR2_PACKAGE_LIBSECCOMP_TESTS),y)
# The 'check-build' target builds the tests and tools, and also

# triggers the 'all' target.
LIBSECCOMP_MAKE_OPTS += check-build

package/libseccomp/libseccomp.mk define LIBSECCOMP_FIXUP_M4_DIR · line 33

Quoted 8 lines
package/libseccomp/libseccomp.mk +8 -0
+define LIBSECCOMP_TESTS_INSTALL
+	mkdir -p $(TARGET_DIR)/usr/libseccomp/{tests,tools} && \
+	for x in tests tools; do \
+		find $(@D)/$$x -maxdepth 1 \( -name "*.tests" -o -perm -a=x \) \
+			-type f -exec cp {} $(TARGET_DIR)/usr/libseccomp/$$x/ \; ; \
+	done
+endef
+

I would remove this empty line since you are adding to the hooks variable the define immediately above.

package/libseccomp/libseccomp.mk define LIBSECCOMP_FIXUP_M4_DIR · line 34

Quoted 1 line
package/libseccomp/libseccomp.mk +1 -0
+LIBSECCOMP_POST_BUILD_HOOKS += LIBSECCOMP_TESTS_BUILD

This post-build hook is not needed since the define has been replaced by the LIBSECCOMP_MAKE_OPTS line.

package/libseccomp/libseccomp.mk define LIBSECCOMP_FIXUP_M4_DIR · line 35

Quoted 2 lines
package/libseccomp/libseccomp.mk +2 -0
+LIBSECCOMP_POST_INSTALL_TARGET_HOOKS += LIBSECCOMP_TESTS_INSTALL
+

And I would remove this empty line as well.

Regards,

Vincent.

Quoted 4 lines
package/libseccomp/libseccomp.mk +2 -0
+endif
+
 $(eval $(autotools-package))
#2 lore raw

PATCH v2 1/1 libseccomp: Add new config option to build and install tests

Add new config option to allow building and installing the libseccomp testsuite on the target. It's useful for testing the kernels' seccomp behavior for the target architecture.

---
changes since v1:
- Drop compile hook for tests and use MAKE_OPTS instead

---
 package/libseccomp/Config.in     |  9 +++++++++
 package/libseccomp/libseccomp.mk | 16 ++++++++++++++++
 2 files changed, 25 insertions(+)
package/libseccomp/Config.in +9 -0
index 4c34c5466592..760a6f321142 100644
@@ -15,6 +15,15 @@ config BR2_PACKAGE_LIBSECCOMP
 
 	  https://github.com/seccomp/libseccomp
 
+config BR2_PACKAGE_LIBSECCOMP_TESTS
+	bool "Install libseccomp tests"
+	depends on BR2_PACKAGE_BASH && BR2_PACKAGE_LIBSECCOMP
+	help
+	  Build and install the libseccomp testsuite.
+
+comment "libseccomp testsuite needs bash as running shell"
+	depends on !BR2_PACKAGE_BASH && BR2_PACKAGE_LIBSECCOMP
+
 comment "libseccomp needs a toolchain w/ headers >= 3.12"
 	depends on BR2_aarch64 || BR2_mips || BR2_mipsel || BR2_mips64 || \
 		BR2_mips64el || BR2_i386 || BR2_x86_64
package/libseccomp/libseccomp.mk +16 -0
index a188298006b1..5ebd8c54ac1f 100644
@@ -17,4 +17,20 @@ define LIBSECCOMP_FIXUP_M4_DIR
 endef
 LIBSECCOMP_POST_EXTRACT_HOOKS += LIBSECCOMP_FIXUP_M4_DIR
 
+ifeq ($(BR2_PACKAGE_LIBSECCOMP_TESTS),y)
+
+LIBSECCOMP_MAKE_OPTS += check-build
+
+define LIBSECCOMP_TESTS_INSTALL
+	mkdir -p $(TARGET_DIR)/usr/libseccomp/{tests,tools} && \
+	for x in tests tools; do \
+		find $(@D)/$$x -maxdepth 1 \( -name "*.tests" -o -perm -a=x \) \
+			-type f -exec cp {} $(TARGET_DIR)/usr/libseccomp/$$x/ \; ; \
+	done
+endef
+
+LIBSECCOMP_POST_INSTALL_TARGET_HOOKS += LIBSECCOMP_TESTS_INSTALL
+
+endif
+
 $(eval $(autotools-package))
Signature · 1 line
2.5.2
#3 lore raw

PATCH v2 1/1 libseccomp: Add new config option to build and install tests

Dear Markos Chandras,

Quoted 5 lines · Markos Chandras

Add new config option to allow building and installing the libseccomp testsuite on the target. It's useful for testing the kernels' seccomp behavior for the target architecture.

Vicente Olivert Riera <[email protected]>

Build test for MIPS architecture:

$ file output/target/usr/libseccomp/tests/01-sim-allow
output/target/usr/libseccomp/tests/01-sim-allow: ELF 32-bit MSB
executable, MIPS, MIPS32 rel2 version 1 (SYSV), dynamically linked (uses
shared libs), for GNU/Linux 2.6.32, with unknown capability 0x41000000 =
0xf676e75, with unknown capability 0x10000 = 0x70403, stripped

Regards,

Vincent.

Quoted 54 lines

---
changes since v1:
- Drop compile hook for tests and use MAKE_OPTS instead

---
 package/libseccomp/Config.in     |  9 +++++++++
 package/libseccomp/libseccomp.mk | 16 ++++++++++++++++
 2 files changed, 25 insertions(+)
package/libseccomp/Config.in +9 -0
index 4c34c5466592..760a6f321142 100644
@@ -15,6 +15,15 @@ config BR2_PACKAGE_LIBSECCOMP
 
 	  https://github.com/seccomp/libseccomp
 
+config BR2_PACKAGE_LIBSECCOMP_TESTS
+	bool "Install libseccomp tests"
+	depends on BR2_PACKAGE_BASH && BR2_PACKAGE_LIBSECCOMP
+	help
+	  Build and install the libseccomp testsuite.
+
+comment "libseccomp testsuite needs bash as running shell"
+	depends on !BR2_PACKAGE_BASH && BR2_PACKAGE_LIBSECCOMP
+
 comment "libseccomp needs a toolchain w/ headers >= 3.12"
 	depends on BR2_aarch64 || BR2_mips || BR2_mipsel || BR2_mips64 || \
 		BR2_mips64el || BR2_i386 || BR2_x86_64
package/libseccomp/libseccomp.mk +16 -0
index a188298006b1..5ebd8c54ac1f 100644
@@ -17,4 +17,20 @@ define LIBSECCOMP_FIXUP_M4_DIR
 endef
 LIBSECCOMP_POST_EXTRACT_HOOKS += LIBSECCOMP_FIXUP_M4_DIR
 
+ifeq ($(BR2_PACKAGE_LIBSECCOMP_TESTS),y)
+
+LIBSECCOMP_MAKE_OPTS += check-build
+
+define LIBSECCOMP_TESTS_INSTALL
+	mkdir -p $(TARGET_DIR)/usr/libseccomp/{tests,tools} && \
+	for x in tests tools; do \
+		find $(@D)/$$x -maxdepth 1 \( -name "*.tests" -o -perm -a=x \) \
+			-type f -exec cp {} $(TARGET_DIR)/usr/libseccomp/$$x/ \; ; \
+	done
+endef
+
+LIBSECCOMP_POST_INSTALL_TARGET_HOOKS += LIBSECCOMP_TESTS_INSTALL
+
+endif
+
 $(eval $(autotools-package))
#5 lore raw

PATCH v2 1/1 libseccomp: Add new config option to build and install tests

In reply to Markos Chandras

Markos,

package/libseccomp/libseccomp.mk define LIBSECCOMP_FIXUP_M4_DIR · line 28

Quoted 9 lines · Markos Chandras
package/libseccomp/libseccomp.mk +9 -0
+ifeq ($(BR2_PACKAGE_LIBSECCOMP_TESTS),y)
+
+LIBSECCOMP_MAKE_OPTS += check-build
+
+define LIBSECCOMP_TESTS_INSTALL
+	mkdir -p $(TARGET_DIR)/usr/libseccomp/{tests,tools} && \
+	for x in tests tools; do \
+		find $(@D)/$$x -maxdepth 1 \( -name "*.tests" -o -perm -a=x \) \
+			-type f -exec cp {} $(TARGET_DIR)/usr/libseccomp/$$x/ \; ; \

This has the effect of installing one .py file for each test even if Python is not installed on the target. They are pretty small, so maybe we don't care, but it doesn't look really nice.

Also, this whole thing seems pretty crappy in libseccomp. Why don't they have simply a --enable-tests option, which will simply do the right thing?

Thanks,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#6 lore raw

PATCH v2 1/1 libseccomp: Add new config option to build and install tests

Hi Thomas,

Quoted 4 lines · Thomas Petazzoni

Markos,

On Fri, 18 Sep 2015 09:19:31 +0100, Markos Chandras wrote:

Quoted 13 lines

package/libseccomp/libseccomp.mk define LIBSECCOMP_FIXUP_M4_DIR · line 28

Quoted 9 lines
package/libseccomp/libseccomp.mk +9 -0
+ifeq ($(BR2_PACKAGE_LIBSECCOMP_TESTS),y)
+
+LIBSECCOMP_MAKE_OPTS += check-build
+
+define LIBSECCOMP_TESTS_INSTALL
+	mkdir -p $(TARGET_DIR)/usr/libseccomp/{tests,tools} && \
+	for x in tests tools; do \
+		find $(@D)/$$x -maxdepth 1 \( -name "*.tests" -o -perm -a=x \) \
+			-type f -exec cp {} $(TARGET_DIR)/usr/libseccomp/$$x/ \; ; \

This has the effect of installing one .py file for each test even if Python is not installed on the target. They are pretty small, so maybe we don't care, but it doesn't look really nice.

Hmm makes sense. I will rework it then.

Quoted 4 lines

Also, this whole thing seems pretty crappy in libseccomp. Why don't they have simply a --enable-tests option, which will simply do the right thing?

It's probably because this is yet another testsuite and like any other package testsuite you don't really need it installed. it's supposed to run as part of package verification during build and installation. but for cross-builds, it's useful to have it on the target.

Signature · 1 line
markos
#7 lore raw

PATCH v3 1/1 libseccomp: Add new config option to build and install tests

In reply to Thomas Petazzoni

Add new config option to allow building and installing the libseccomp testsuite on the target. It's useful for testing the kernels' seccomp behavior for the target architecture.

---
changes since v2:
- Move installation path for tests to /usr/share/libseccomp
- Do not install python files. We don't support python + libseccomp at the moment so there is no point in installing these tests yet.

changes since v1:
- Drop compile hook for tests and use MAKE_OPTS instead

---
 package/libseccomp/Config.in     |  9 +++++++++
 package/libseccomp/libseccomp.mk | 16 ++++++++++++++++
 2 files changed, 25 insertions(+)
package/libseccomp/Config.in +9 -0
index 4c34c5466592..760a6f321142 100644
@@ -15,6 +15,15 @@ config BR2_PACKAGE_LIBSECCOMP
 
 	  https://github.com/seccomp/libseccomp
 
+config BR2_PACKAGE_LIBSECCOMP_TESTS
+	bool "Install libseccomp tests"
+	depends on BR2_PACKAGE_BASH && BR2_PACKAGE_LIBSECCOMP
+	help
+	  Build and install the libseccomp testsuite.
+
+comment "libseccomp testsuite needs bash as running shell"
+	depends on !BR2_PACKAGE_BASH && BR2_PACKAGE_LIBSECCOMP
+
 comment "libseccomp needs a toolchain w/ headers >= 3.12"
 	depends on BR2_aarch64 || BR2_mips || BR2_mipsel || BR2_mips64 || \
 		BR2_mips64el || BR2_i386 || BR2_x86_64
package/libseccomp/libseccomp.mk +16 -0
index a188298006b1..e6e578b85561 100644
@@ -17,4 +17,20 @@ define LIBSECCOMP_FIXUP_M4_DIR
 endef
 LIBSECCOMP_POST_EXTRACT_HOOKS += LIBSECCOMP_FIXUP_M4_DIR
 
+ifeq ($(BR2_PACKAGE_LIBSECCOMP_TESTS),y)
+
+LIBSECCOMP_MAKE_OPTS += check-build
+
+define LIBSECCOMP_TESTS_INSTALL
+	mkdir -p $(TARGET_DIR)/usr/share/libseccomp/{tests,tools} && \
+	for x in tests tools; do \
+		find $(@D)/$$x -maxdepth 1 \( -name "*.tests" -o -perm -a=x \) -a ! -name "*.py" \
+			-type f -exec cp {} $(TARGET_DIR)/usr/share/libseccomp/$$x/ \; ; \
+	done
+endef
+
+LIBSECCOMP_POST_INSTALL_TARGET_HOOKS += LIBSECCOMP_TESTS_INSTALL
+
+endif
+
 $(eval $(autotools-package))
Signature · 1 line
2.5.2
#8 lore raw

PATCH v3 1/1 libseccomp: Add new config option to build and install tests

Dear Markos Chandras,

could you please mark your previous patch as superseded?

http://patchwork.ozlabs.org/patch/519170/

Quoted 5 lines · Markos Chandras

Add new config option to allow building and installing the libseccomp testsuite on the target. It's useful for testing the kernels' seccomp behavior for the target architecture.

Build test for MIPS architecture. All fine. Binary files built for the right architecture and no more .py files installed on target. Checked that now the tests and tools are installed in /usr/share/libseccomp/.

Regards,

Vincent.

Quoted 59 lines

---
changes since v2:
- Move installation path for tests to /usr/share/libseccomp
- Do not install python files. We don't support python + libseccomp at the moment so there is no point in installing these tests yet.

changes since v1:
- Drop compile hook for tests and use MAKE_OPTS instead

---
 package/libseccomp/Config.in     |  9 +++++++++
 package/libseccomp/libseccomp.mk | 16 ++++++++++++++++
 2 files changed, 25 insertions(+)
package/libseccomp/Config.in +9 -0
index 4c34c5466592..760a6f321142 100644
@@ -15,6 +15,15 @@ config BR2_PACKAGE_LIBSECCOMP
 
 	  https://github.com/seccomp/libseccomp
 
+config BR2_PACKAGE_LIBSECCOMP_TESTS
+	bool "Install libseccomp tests"
+	depends on BR2_PACKAGE_BASH && BR2_PACKAGE_LIBSECCOMP
+	help
+	  Build and install the libseccomp testsuite.
+
+comment "libseccomp testsuite needs bash as running shell"
+	depends on !BR2_PACKAGE_BASH && BR2_PACKAGE_LIBSECCOMP
+
 comment "libseccomp needs a toolchain w/ headers >= 3.12"
 	depends on BR2_aarch64 || BR2_mips || BR2_mipsel || BR2_mips64 || \
 		BR2_mips64el || BR2_i386 || BR2_x86_64
package/libseccomp/libseccomp.mk +16 -0
index a188298006b1..e6e578b85561 100644
@@ -17,4 +17,20 @@ define LIBSECCOMP_FIXUP_M4_DIR
 endef
 LIBSECCOMP_POST_EXTRACT_HOOKS += LIBSECCOMP_FIXUP_M4_DIR
 
+ifeq ($(BR2_PACKAGE_LIBSECCOMP_TESTS),y)
+
+LIBSECCOMP_MAKE_OPTS += check-build
+
+define LIBSECCOMP_TESTS_INSTALL
+	mkdir -p $(TARGET_DIR)/usr/share/libseccomp/{tests,tools} && \
+	for x in tests tools; do \
+		find $(@D)/$$x -maxdepth 1 \( -name "*.tests" -o -perm -a=x \) -a ! -name "*.py" \
+			-type f -exec cp {} $(TARGET_DIR)/usr/share/libseccomp/$$x/ \; ; \
+	done
+endef
+
+LIBSECCOMP_POST_INSTALL_TARGET_HOOKS += LIBSECCOMP_TESTS_INSTALL
+
+endif
+
 $(eval $(autotools-package))
#9 lore raw

PATCH v3 1/1 libseccomp: Add new config option to build and install tests

In reply to Markos Chandras
Quoted 11 lines · Markos Chandras

Add new config option to allow building and installing the libseccomp testsuite on the target. It's useful for testing the kernels' seccomp behavior for the target architecture.

---
changes since v2:
- Move installation path for tests to /usr/share/libseccomp
- Do not install python files. We don't support python + libseccomp at the moment so there is no point in installing these tests yet.

Quoted 3 lines

changes since v1:
- Drop compile hook for tests and use MAKE_OPTS instead
---

ping?

Signature · 1 line
markos
#10 lore raw

PATCH v3 1/1 libseccomp: Add new config option to build and install tests

Quoted 12 lines · Markos Chandras
Quoted 11 lines · Markos Chandras

Add new config option to allow building and installing the libseccomp testsuite on the target. It's useful for testing the kernels' seccomp behavior for the target architecture.

---
changes since v2:
- Move installation path for tests to /usr/share/libseccomp
- Do not install python files. We don't support python + libseccomp at the moment so there is no point in installing these tests yet.

Quoted 6 lines
Quoted 3 lines

changes since v1:
- Drop compile hook for tests and use MAKE_OPTS instead
---

ping?

ping?

Signature · 1 line
markos
#11 lore raw

PATCH v3 1/1 libseccomp: Add new config option to build and install tests

Markos,

Quoted 1 line · Markos Chandras

ping?

I think the reason I still don't apply this patch is because I'm not happy with Buildroot doing all this complicated logic to install the tests, and that it should be part of the libseccomp build system to provide a mechanism to do that easily.

So if you want to see this integrated, I would recommend to work with upstream libseccomp to implement something allowing the installation of tests, and then use it in Buildroot.

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#12 lore raw

PATCH v3 1/1 libseccomp: Add new config option to build and install tests

Quoted 6 lines · Thomas Petazzoni

Markos,

Quoted 1 line · Markos Chandras

ping?

Quoted 8 lines

I think the reason I still don't apply this patch is because I'm not happy with Buildroot doing all this complicated logic to install the tests, and that it should be part of the libseccomp build system to provide a mechanism to do that easily.

So if you want to see this integrated, I would recommend to work with upstream libseccomp to implement something allowing the installation of tests, and then use it in Buildroot.

Hi Thomas,

Ok fair enough

Signature · 1 line
markos