[igt-dev] [PATCH i-g-t 0/7] Fixes from updating igt packaging in Fedora

[igt-dev] [PATCH i-g-t 0/7] Fixes from updating igt packaging in Fedora

[Lyude]

From: Lyude Paul <lyude@redhat.com>

For work reasons, I've been tasked with getting Fedora running portions
of IGT in our own CI infrastructure. In doing so I needed to get our RPM
packaging up to date, building and passing the meson unit tests on
Fedora 29 and Fedora Rawhide. This ended up exposing some rather
interesting bugs, and also prompted me to update the Dockerfile and
Gitlab CI settings that we use for testing builds. So, here's my fixes
and improvements!

As well, a couple of open questions I still have with this series:

* For the first commit, "lib/tests: Fix test failures with meson
  0.50.0", is this an appropriate fix or should we just modify the
  return codes IGT spits out on test timeouts? (seems we used to have an
  error code specifically for timeouts)
* For "lib: Stop using assert() for runtime checks", there's still a
  bunch of other places in igt that I spotted using assert() - should we
  also completely replace those with igt_internal_assert()?

Lyude Paul (7):
  lib/tests: Fix test failures with meson 0.50.0
  meson: Don't redefine gettid if the C library provides it
  lib: Stop using assert() for runtime checks
  lib/igt_core: Just use igt_can_fail() in __igt_run_subtest()
  Use pkgconfig() macros with dnf in Fedora Dockerfile
  Update Dockerfile.fedora to Fedora 29
  Gitlab CI: Do builds with -Db_ndebug=true

 .gitlab-ci.yml                       |  1 +
 Dockerfile.fedora                    | 50 +++++++++++------------
 lib/igt_aux.h                        |  3 ++
 lib/igt_core.c                       | 22 +++++-----
 lib/igt_core.h                       | 18 +++++++++
 lib/tests/igt_assert.c               |  2 +-
 lib/tests/igt_can_fail.c             | 10 ++---
 lib/tests/igt_can_fail_simple.c      |  2 +-
 lib/tests/igt_exit_handler.c         | 18 ++++-----
 lib/tests/igt_fork.c                 |  4 +-
 lib/tests/igt_invalid_subtest_name.c |  2 +-
 lib/tests/igt_no_exit.c              |  2 +-
 lib/tests/igt_segfault.c             |  2 +-
 lib/tests/igt_simulation.c           | 60 ++++++++++++++--------------
 lib/tests/igt_subtest_group.c        | 16 ++++----
 lib/tests/igt_tests_common.h         | 19 ++++-----
 lib/tests/meson.build                | 15 +++++++
 lib/tests/retcode_99_wrapper.sh      | 11 +++++
 meson.build                          |  3 ++
 19 files changed, 152 insertions(+), 108 deletions(-)
 create mode 100644 lib/tests/retcode_99_wrapper.sh

-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 1/7] lib/tests: Fix test failures with meson 0.50.0

[Lyude]

From: Lyude Paul <lyude@redhat.com>

Since meson 0.50.0, unit tests which return the GNU standard return code
99 will fail, regardless of whether or not should_fail:true is passed to
test(). Unfortunately, igt_alarm_handler() exits the application with
return code 99. However, since returning something other then 99 when a
test times out would also be a bug we can't really change the return
code igt_alarm_handler() returns.

Instead, we can fix this by simply running tests which are expected to
fail with return code 99 using a wrapper script that translates 99 to 1
and any other non-zero error code into 99. This essentially makes it so
that abnormal test failures are considered normal, and vice versa.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 lib/tests/meson.build           | 15 +++++++++++++++
 lib/tests/retcode_99_wrapper.sh | 11 +++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 lib/tests/retcode_99_wrapper.sh

diff --git a/lib/tests/meson.build b/lib/tests/meson.build
index 74efce39..108597cf 100644
--- a/lib/tests/meson.build
+++ b/lib/tests/meson.build
@@ -18,6 +18,9 @@ lib_tests = [
 lib_fail_tests = [
 	'igt_no_subtest',
 	'igt_simple_test_subtests',
+]
+
+lib_retcode_99_tests = [
 	'igt_timeout',
 ]
 
@@ -32,3 +35,15 @@ foreach lib_test : lib_fail_tests
 			dependencies : igt_deps)
 	test('lib: ' + lib_test, exec, should_fail : true)
 endforeach
+
+# Some tests are expected to fail with a retcode of 99. Since meson 0.50.0, unit
+# tests that return this error code are marked as failures regardless of the
+# value of should_fail. So, we run these tests in a wrapper to translate return
+# codes of 99 into 1, and non-zero error codes into 99 (so normal failures are
+# counted as abnormal failures)
+wrapper_99 = find_program('retcode_99_wrapper.sh')
+foreach lib_test : lib_retcode_99_tests
+	exec = executable(lib_test, lib_test + '.c', install : false,
+			dependencies : igt_deps)
+	test('lib: ' + lib_test, wrapper_99, args : [exec], should_fail : true)
+endforeach
diff --git a/lib/tests/retcode_99_wrapper.sh b/lib/tests/retcode_99_wrapper.sh
new file mode 100644
index 00000000..cfd8825d
--- /dev/null
+++ b/lib/tests/retcode_99_wrapper.sh
@@ -0,0 +1,11 @@
+#!/bin/sh
+
+"$@"
+last_ret="$?"
+if [ $last_ret -eq 99 ]; then
+	exit 1
+elif [ $last_ret -ne 0 ]; then
+	exit 99 # We expected to fail with 99, normal failures are abnormal
+else
+	exit $last_ret
+fi
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 2/7] meson: Don't redefine gettid if the C library provides it

[Lyude]

From: Lyude Paul <lyude@redhat.com>

glibc 2.30+ will actually include a definition for gettid() that makes
it so that users don't have to manually define a wrapper for it
themselves with syscall(). We don't currently check for this, and as a
result will end up redefining gettid() on the latest versions of glibc,
causing the build to fail:

FAILED: lib/76b5a35@@igt-igt_kmod_c@sta/igt_kmod.c.o
In file included from /usr/include/unistd.h:1170,
                 from ../../mnt/vol/lib/igt_core.h:43,
                 from ../../mnt/vol/lib/igt_kmod.c:28:
/usr/include/bits/unistd_ext.h:34:28: error: macro "gettid" passed 1 arguments, but takes just 0
   34 | extern __pid_t gettid (void) __THROW;
      |                            ^
In file included from ../../mnt/vol/lib/igt_kmod.c:27:
../../mnt/vol/lib/igt_aux.h:40: note: macro "gettid" defined here
   40 | #define gettid() syscall(__NR_gettid)
      |
[36/771] Compiling C object 'lib/76b5a35@@igt-igt_kms_c@sta/igt_kms.c.o'.
ninja: build stopped: subcommand failed.

So, fix this by by adding some meson checks to define HAVE_GETTID whenever the
host defines its own gettid(), and avoid redefining gettid() when HAVE_GETTID is
defined.

This fixes build intel-gpu-tools for me on Fedora Rawhide

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 lib/igt_aux.h | 3 +++
 meson.build   | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 55392790..39fefcbf 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -36,7 +36,10 @@
 #include <i915/gem_submission.h>
 
 /* signal interrupt helpers */
+#ifndef HAVE_GETTID
 #define gettid() syscall(__NR_gettid)
+#endif
+
 #define sigev_notify_thread_id _sigev_un._tid
 
 /* auxialiary igt helpers from igt_aux.c */
diff --git a/meson.build b/meson.build
index 557400a5..e73275dd 100644
--- a/meson.build
+++ b/meson.build
@@ -237,6 +237,9 @@ if cc.has_header('cpuid.h')
 	# FIXME: Do we need the example link test from configure.ac?
 	config.set('HAVE_CPUID_H', 1)
 endif
+if cc.has_header_symbol('unistd.h', 'gettid', args : '-D_GNU_SOURCE')
+	config.set('HAVE_GETTID', 1)
+endif
 
 if cc.has_member('struct sysinfo', 'totalram',
 		prefix : '#include <sys/sysinfo.h>')
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 3/7] lib: Stop using assert() for runtime checks

[Lyude]

From: Lyude Paul <lyude@redhat.com>

In the process of trying to get an up to date version of igt packaged
for Fedora I discovered that if igt was built with -Db_ndebug=true, a
significant portion of the test infrastructure unit tests would start
failing:

  1/265 lib: igt_assert                         OK       0.11 s
  2/265 lib: igt_can_fail                       OK       0.08 s
  3/265 lib: igt_can_fail_simple                OK       0.08 s
  4/265 lib: igt_exit_handler                   OK       0.05 s
  5/265 lib: igt_fork                           FAIL     0.05 s (killed by signal 9 SIGKILL)
  6/265 lib: igt_fork_helper                    OK       0.42 s
  7/265 lib: igt_hdmi_inject                    OK       0.05 s
  8/265 lib: igt_list_only                      OK       0.01 s
  9/265 lib: igt_invalid_subtest_name           OK       0.05 s
 10/265 lib: igt_no_exit                        OK       0.04 s
 11/265 lib: igt_segfault                       OK       0.38 s
 12/265 lib: igt_simulation                     OK       0.02 s
 13/265 lib: igt_stats                          OK       0.04 s
 14/265 lib: igt_subtest_group                  OK       0.03 s
 15/265 lib: igt_no_subtest                     SKIP     0.02 s
 16/265 lib: igt_simple_test_subtests           UNEXPECTEDPASS 0.02 s
 17/265 lib: igt_timeout                        EXPECTEDFAIL 1.02 s

Which appeared to stem from the fact that -Db_ndebug=true would strip
assert() calls. While on a first glance of lib/tests/igt_tests_common.h
one would assume that the only user of assert() was the test
infrastructure unit tests themselves, it turns out we've actually been
using this in multiple spots that seem to expect an unconditional
runtime check.

So in order to fix this, we introduce igt_internal_assert(). The purpose
of this function is to provide simple runtime error-checking for
conditions that warrant abort()ing IGT under any circumstances, and to
provide a internal_assert() replacement for our test infrastructure
tests to use. Then, remove all of the assert() usages in lib/tests/*
along with any of the assert() calls in lib/igt_core.c that affect test
behavior.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 lib/igt_core.c                       | 24 +++++------
 lib/igt_core.h                       | 18 +++++++++
 lib/tests/igt_assert.c               |  2 +-
 lib/tests/igt_can_fail.c             | 10 ++---
 lib/tests/igt_can_fail_simple.c      |  2 +-
 lib/tests/igt_exit_handler.c         | 18 ++++-----
 lib/tests/igt_fork.c                 |  4 +-
 lib/tests/igt_invalid_subtest_name.c |  2 +-
 lib/tests/igt_no_exit.c              |  2 +-
 lib/tests/igt_segfault.c             |  2 +-
 lib/tests/igt_simulation.c           | 60 ++++++++++++++--------------
 lib/tests/igt_subtest_group.c        | 16 ++++----
 lib/tests/igt_tests_common.h         | 19 ++++-----
 13 files changed, 97 insertions(+), 82 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index ae03e909..a637b94e 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -521,7 +521,7 @@ static void common_exit_handler(int sig)
 
 	/* When not killed by a signal check that igt_exit() has been properly
 	 * called. */
-	assert(sig != 0 || igt_exit_called);
+	igt_internal_assert(sig != 0 || igt_exit_called);
 }
 
 static void print_test_description(void)
@@ -611,7 +611,7 @@ static void common_init_config(void)
 
 	ret = g_key_file_get_integer(igt_key_file, "DUT", "SuspendResumeDelay",
 				     &error);
-	assert(!error || error->code != G_KEY_FILE_ERROR_INVALID_VALUE);
+	igt_internal_assert(!error || error->code != G_KEY_FILE_ERROR_INVALID_VALUE);
 
 	g_clear_error(&error);
 
@@ -903,9 +903,9 @@ bool __igt_run_subtest(const char *subtest_name)
 {
 	int i;
 
-	assert(!in_subtest);
-	assert(!in_fixture);
-	assert(test_with_subtests);
+	igt_internal_assert(!in_subtest);
+	igt_internal_assert(!in_fixture);
+	igt_internal_assert(test_with_subtests);
 
 	/* check the subtest name only contains a-z, A-Z, 0-9, '-' and '_' */
 	for (i = 0; subtest_name[i] != '\0'; i++)
@@ -978,7 +978,7 @@ bool igt_only_list_subtests(void)
 
 void __igt_subtest_group_save(int *save)
 {
-	assert(test_with_subtests);
+	igt_internal_assert(test_with_subtests);
 
 	*save = skip_subtests_henceforth;
 }
@@ -1032,7 +1032,7 @@ void igt_skip(const char *f, ...)
 	va_list args;
 	skipped_one = true;
 
-	assert(!test_child);
+	igt_internal_assert(!test_child);
 
 	if (!igt_only_list_subtests()) {
 		va_start(args, f);
@@ -1510,10 +1510,10 @@ void igt_exit(void)
 		exit(IGT_EXIT_SUCCESS);
 
 	/* Calling this without calling one of the above is a failure */
-	assert(!test_with_subtests ||
-	       skipped_one ||
-	       succeeded_one ||
-	       failed_one);
+	igt_internal_assert(!test_with_subtests ||
+			    skipped_one ||
+			    succeeded_one ||
+			    failed_one);
 
 	if (test_with_subtests && !failed_one) {
 		if (succeeded_one)
@@ -1531,7 +1531,7 @@ void igt_exit(void)
 		kill(test_children[c], SIGKILL);
 	assert(!num_test_children);
 
-	assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno == ECHILD);
+	igt_internal_assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno == ECHILD);
 
 	if (!test_with_subtests) {
 		struct timespec now;
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 47ffd9e7..eed39bbf 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -747,6 +747,24 @@ static inline void igt_ignore_warn(bool value)
 	else igt_debug("Test requirement passed: !(%s)\n", #expr); \
 } while (0)
 
+/**
+ * igt_internal_assert:
+ * @expr: condition to test
+ *
+ * If the given expression fails, print an error to stderr and abort()
+ * immediately. This is intended for use only by the igt helpers in order to
+ * check for incorrect usages of the helpers or other extraordinary conditions.
+ *
+ * This should never be used within a normal test. Doing so will alert
+ * velociraptors to your location.
+ */
+#define igt_internal_assert(expr) do { \
+	if (!(expr)) { \
+		igt_critical("Internal assertion failed: `%s`\n", #expr); \
+		abort(); \
+	} \
+} while (0)
+
 /* fork support code */
 bool __igt_fork(void);
 
diff --git a/lib/tests/igt_assert.c b/lib/tests/igt_assert.c
index 1caf5d88..a8ca1a31 100644
--- a/lib/tests/igt_assert.c
+++ b/lib/tests/igt_assert.c
@@ -62,7 +62,7 @@ static int do_fork(void)
 
 	switch (pid = fork()) {
 	case -1:
-		internal_assert(0);
+		igt_internal_assert(0);
 	case 0:
 		argc = ARRAY_SIZE(argv_run);
 		igt_simple_init(argc, argv_run);
diff --git a/lib/tests/igt_can_fail.c b/lib/tests/igt_can_fail.c
index 1e3d9558..ae546644 100644
--- a/lib/tests/igt_can_fail.c
+++ b/lib/tests/igt_can_fail.c
@@ -28,17 +28,17 @@
 
 igt_main
 {
-	internal_assert(igt_can_fail() == false);
+	igt_internal_assert(igt_can_fail() == false);
 
 	igt_fixture {
-		internal_assert(igt_can_fail());
+		igt_internal_assert(igt_can_fail());
 	}
 
-	internal_assert(igt_can_fail() == false);
+	igt_internal_assert(igt_can_fail() == false);
 
 	igt_subtest("subtest") {
-		internal_assert(igt_can_fail());
+		igt_internal_assert(igt_can_fail());
 	}
 
-	internal_assert(igt_can_fail() == false);
+	igt_internal_assert(igt_can_fail() == false);
 }
diff --git a/lib/tests/igt_can_fail_simple.c b/lib/tests/igt_can_fail_simple.c
index 8ff43d15..4f2e4a78 100644
--- a/lib/tests/igt_can_fail_simple.c
+++ b/lib/tests/igt_can_fail_simple.c
@@ -28,5 +28,5 @@
 
 igt_simple_main
 {
-	internal_assert(igt_can_fail());
+	igt_internal_assert(igt_can_fail());
 }
diff --git a/lib/tests/igt_exit_handler.c b/lib/tests/igt_exit_handler.c
index 892a7f14..220c9085 100644
--- a/lib/tests/igt_exit_handler.c
+++ b/lib/tests/igt_exit_handler.c
@@ -35,7 +35,7 @@ int pipes[2];
 
 static void exit_handler1(int sig)
 {
-	internal_assert(test == 1);
+	igt_internal_assert(test == 1);
 	test++;
 }
 
@@ -44,12 +44,12 @@ static void exit_handler2(int sig)
 	char tmp = 1;
 
 	/* ensure exit handlers are called in reverse */
-	internal_assert(test == 0);
+	igt_internal_assert(test == 0);
 	test++;
 
 	/* we need to get a side effect to the parent to make sure exit handlers
 	 * actually run. */
-	internal_assert(write(pipes[1], &tmp, 1) == 1);
+	igt_internal_assert(write(pipes[1], &tmp, 1) == 1);
 }
 
 enum test_type {
@@ -69,7 +69,7 @@ static int testfunc(enum test_type test_type)
 	int status;
 	char tmp = 0;
 
-	internal_assert(pipe2(pipes, O_NONBLOCK) == 0);
+	igt_internal_assert(pipe2(pipes, O_NONBLOCK) == 0);
 
 	pid = fork();
 
@@ -102,10 +102,10 @@ static int testfunc(enum test_type test_type)
 		igt_exit();
 	}
 
-	internal_assert(waitpid(pid, &status, 0) != -1);
+	igt_internal_assert(waitpid(pid, &status, 0) != -1);
 
-	internal_assert(read(pipes[0], &tmp, 1) == 1);
-	internal_assert(tmp == 1);
+	igt_internal_assert(read(pipes[0], &tmp, 1) == 1);
+	igt_internal_assert(tmp == 1);
 
 	return status;
 }
@@ -114,9 +114,9 @@ int main(int argc, char **argv)
 {
 	int status;
 
-	internal_assert(testfunc(SUC) == 0);
+	igt_internal_assert(testfunc(SUC) == 0);
 
-	internal_assert(testfunc(NORMAL) == 0);
+	igt_internal_assert(testfunc(NORMAL) == 0);
 
 	status = testfunc(FAIL);
 	internal_assert_wexited(status, 1);
diff --git a/lib/tests/igt_fork.c b/lib/tests/igt_fork.c
index 7e8b4f9b..b8e437e0 100644
--- a/lib/tests/igt_fork.c
+++ b/lib/tests/igt_fork.c
@@ -68,7 +68,7 @@ static void plain_fork_leak(void)
 
 	switch (pid = fork()) {
 	case -1:
-		internal_assert(0);
+		igt_internal_assert(0);
 	case 0:
 		sleep(1);
 	default:
@@ -92,7 +92,7 @@ static int do_fork(void (*test_to_run)(void))
 
 	switch (pid = fork()) {
 	case -1:
-		internal_assert(0);
+		igt_internal_assert(0);
 	case 0:
 		argc = ARRAY_SIZE(argv_run);
 		igt_simple_init(argc, argv_run);
diff --git a/lib/tests/igt_invalid_subtest_name.c b/lib/tests/igt_invalid_subtest_name.c
index 92e767ab..696687fc 100644
--- a/lib/tests/igt_invalid_subtest_name.c
+++ b/lib/tests/igt_invalid_subtest_name.c
@@ -66,7 +66,7 @@ static int do_fork(void (*test_to_run)(void))
 
 	switch (pid = fork()) {
 	case -1:
-		internal_assert(0);
+		igt_internal_assert(0);
 	case 0:
 		test_to_run();
 	default:
diff --git a/lib/tests/igt_no_exit.c b/lib/tests/igt_no_exit.c
index 82f00b52..3eeef5a4 100644
--- a/lib/tests/igt_no_exit.c
+++ b/lib/tests/igt_no_exit.c
@@ -62,7 +62,7 @@ static int do_fork(void (*test_to_run)(void))
 
 	switch (pid = fork()) {
 	case -1:
-		internal_assert(0);
+		igt_internal_assert(0);
 	case 0:
 		test_to_run();
 	default:
diff --git a/lib/tests/igt_segfault.c b/lib/tests/igt_segfault.c
index 0d872f67..b4c5bcbd 100644
--- a/lib/tests/igt_segfault.c
+++ b/lib/tests/igt_segfault.c
@@ -63,7 +63,7 @@ static int do_fork(void)
 
 	switch (pid = fork()) {
 	case -1:
-		internal_assert(0);
+		igt_internal_assert(0);
 	case 0:
 		argc = ARRAY_SIZE(argv_run);
 		if (simple) {
diff --git a/lib/tests/igt_simulation.c b/lib/tests/igt_simulation.c
index 3f3cd88f..899665d3 100644
--- a/lib/tests/igt_simulation.c
+++ b/lib/tests/igt_simulation.c
@@ -52,7 +52,7 @@ static int do_fork(void)
 
 	switch (pid = fork()) {
 	case -1:
-		internal_assert(0);
+		igt_internal_assert(0);
 	case 0:
 		if (simple) {
 			argc = 1;
@@ -90,7 +90,7 @@ static int do_fork(void)
 		       errno == EINTR)
 			;
 
-		internal_assert(WIFEXITED(status));
+		igt_internal_assert(WIFEXITED(status));
 
 		return status;
 	}
@@ -100,40 +100,40 @@ int main(int argc, char **argv)
 {
 	/* simple tests */
 	simple = true;
-	internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SKIP);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SKIP);
 
-	internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
 
 	/* subtests, list mode */
 	simple = false;
 	list_subtests = true;
 
 	in_fixture = false;
-	internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
 
-	internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
 
 	in_fixture = true;
-	internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
 
 
-	internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
 
 
 	in_fixture = false;
 	in_subtest = true;
-	internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
 
 
-	internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
 
 
 	/* subtest, run mode */
@@ -141,30 +141,30 @@ int main(int argc, char **argv)
 	list_subtests = false;
 
 	in_fixture = false;
-	internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SKIP);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SKIP);
 
-	internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
 
 
 	in_fixture = true;
-	internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SKIP);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SKIP);
 
 
-	internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
 
 
 	in_fixture = false;
 	in_subtest = true;
-	internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SKIP);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "1", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SKIP);
 
 
-	internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
-	internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
+	igt_internal_assert(setenv("INTEL_SIMULATION", "0", 1) == 0);
+	igt_internal_assert(WEXITSTATUS(do_fork()) == IGT_EXIT_SUCCESS);
 
 
 	return 0;
diff --git a/lib/tests/igt_subtest_group.c b/lib/tests/igt_subtest_group.c
index 7783d021..d41ac5fb 100644
--- a/lib/tests/igt_subtest_group.c
+++ b/lib/tests/igt_subtest_group.c
@@ -42,7 +42,7 @@ igt_main
 			}
 
 			igt_subtest("not-run") {
-				internal_assert(0);
+				igt_internal_assert(0);
 			}
 
 			igt_subtest_group {
@@ -50,35 +50,35 @@ igt_main
 				 * restore to "run testcases" when an outer
 				 * group is already in SKIP state. */
 				igt_subtest("still-not-run") {
-					internal_assert(0);
+					igt_internal_assert(0);
 				}
 			}
 		}
 
 		igt_subtest("run") {
 			t1 = true;
-			internal_assert(1);
+			igt_internal_assert(1);
 		}
 	}
 
 	igt_subtest_group {
 		igt_fixture {
-			internal_assert(t2 == 0);
+			igt_internal_assert(t2 == 0);
 			t2 = 1;
 		}
 
 		igt_subtest("run-again") {
-			internal_assert(t2 == 1);
+			igt_internal_assert(t2 == 1);
 			t2 = 2;
 		}
 
 		igt_fixture {
-			internal_assert(t2 == 2);
+			igt_internal_assert(t2 == 2);
 			t2 = 3;
 
 		}
 	}
 
-	internal_assert(t1);
-	internal_assert(t2 == 3);
+	igt_internal_assert(t1);
+	igt_internal_assert(t2 == 3);
 }
diff --git a/lib/tests/igt_tests_common.h b/lib/tests/igt_tests_common.h
index e66ee37c..c44bfc3c 100644
--- a/lib/tests/igt_tests_common.h
+++ b/lib/tests/igt_tests_common.h
@@ -25,25 +25,22 @@
 #ifndef IGT_LIB_TESTS_COMMON_H
 #define IGT_LIB_TESTS_COMMON_H
 
-#include <assert.h>
+#include "igt_core.h"
 
 /*
- * We need to hide assert from the cocci igt test refactor spatch.
- *
- * IMPORTANT: Test infrastructure tests are the only valid places where using
- * assert is allowed.
+ * IMPORTANT: We used to define a wrapper around assert() here, but since this
+ * breaks all of our tests if debug assertions are turned off this turned out to
+ * be a really bad idea. Don't add it back!
  */
-#define internal_assert assert
-
 static inline void internal_assert_wexited(int wstatus, int exitcode)
 {
-	internal_assert(WIFEXITED(wstatus) &&
-			WEXITSTATUS(wstatus) == exitcode);
+	igt_internal_assert(WIFEXITED(wstatus) &&
+			    WEXITSTATUS(wstatus) == exitcode);
 }
 
 static inline void internal_assert_wsignaled(int wstatus, int signal)
 {
-	internal_assert(WIFSIGNALED(wstatus) &&
-			WTERMSIG(wstatus) == signal);
+	igt_internal_assert(WIFSIGNALED(wstatus) &&
+			    WTERMSIG(wstatus) == signal);
 }
 #endif
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 4/7] lib/igt_core: Just use igt_can_fail() in __igt_run_subtest()

[Lyude]

From: Lyude Paul <lyude@redhat.com>

That's what it's there for.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 lib/igt_core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index a637b94e..503503ce 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -903,9 +903,7 @@ bool __igt_run_subtest(const char *subtest_name)
 {
 	int i;
 
-	igt_internal_assert(!in_subtest);
-	igt_internal_assert(!in_fixture);
-	igt_internal_assert(test_with_subtests);
+	igt_internal_assert(!igt_can_fail());
 
 	/* check the subtest name only contains a-z, A-Z, 0-9, '-' and '_' */
 	for (i = 0; subtest_name[i] != '\0'; i++)
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 5/7] Use pkgconfig() macros with dnf in Fedora Dockerfile

[Lyude]

From: Lyude Paul <lyude@redhat.com>

dnf supports installing packages by their pkgconfig names using the
pkgconfig() RPM macro. Since these more closely match the dependencies
that meson uses and don't have a chance of changing in the future like
Fedora package name do, let's use these with dnf instead.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 Dockerfile.fedora | 48 +++++++++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/Dockerfile.fedora b/Dockerfile.fedora
index 29520f7b..469e74b2 100644
--- a/Dockerfile.fedora
+++ b/Dockerfile.fedora
@@ -1,30 +1,28 @@
 FROM fedora:28
 
-RUN dnf install -y gcc \
-		flex \
-		meson \
-		bison  \
-		gtk-doc \
-		xdotool \
-		gsl-devel \
-		kmod-devel \
-		glib2-devel \
-		cairo-devel \
-		ninja-build \
-		procps-devel \
-		pixman-devel \
-		json-c-devel \
-		libdrm-devel \
-		libudev-devel \
-		xmlrpc-c-devel \
-		elfutils-devel \
-		libunwind-devel \
-		python-docutils \
-		libpciaccess-devel \
-		alsa-lib-devel \
-		valgrind-devel \
-		libXrandr-devel \
-		libXv-devel
+RUN dnf install -y \
+	gcc flex bison meson ninja-build xdotool \
+	'pkgconfig(libdrm)' \
+	'pkgconfig(pciaccess)' \
+	'pkgconfig(libkmod)' \
+	'pkgconfig(libprocps)' \
+	'pkgconfig(libunwind)' \
+	'pkgconfig(libdw)' \
+	'pkgconfig(pixman-1)' \
+	'pkgconfig(valgrind)' \
+	'pkgconfig(cairo)' \
+	'pkgconfig(libudev)' \
+	'pkgconfig(glib-2.0)' \
+	'pkgconfig(gsl)' \
+	'pkgconfig(alsa)' \
+	'pkgconfig(xmlrpc)' \
+	'pkgconfig(xmlrpc_util)' \
+	'pkgconfig(xmlrpc_client)' \
+	'pkgconfig(json-c)' \
+	'pkgconfig(gtk-doc)' \
+	'pkgconfig(xv)' \
+	'pkgconfig(xrandr)' \
+	python3-docutils
 
 # We need peg to build overlay
 RUN dnf install -y make
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 6/7] Update Dockerfile.fedora to Fedora 29

[Lyude]

From: Lyude Paul <lyude@redhat.com>

Latest stable release of Fedora

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 Dockerfile.fedora | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Dockerfile.fedora b/Dockerfile.fedora
index 469e74b2..6cb3e2ce 100644
--- a/Dockerfile.fedora
+++ b/Dockerfile.fedora
@@ -1,4 +1,4 @@
-FROM fedora:28
+FROM fedora:29
 
 RUN dnf install -y \
 	gcc flex bison meson ninja-build xdotool \
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 7/7] Gitlab CI: Do builds with -Db_ndebug=true

[Lyude]

From: Lyude Paul <lyude@redhat.com>

To make sure that no one ever uses assert() in the wrong places again,
let's build with -Db_ndebug=true so that assert() gets compiled out.

Signed-off-by: Lyude Paul <lyude@redhat.com>
---
 .gitlab-ci.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index ae8cbb67..0cc134ae 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -10,6 +10,7 @@ variables:
     -Dbuild_tests=true
     -Dbuild_runner=true
     -Dwith_libunwind=true
+    -Db_ndebug=true
   LANG: "C.UTF-8"
 
 stages:
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for Fixes from updating igt packaging in Fedora

[Patchwork]

== Series Details ==

Series: Fixes from updating igt packaging in Fedora
URL   : https://patchwork.freedesktop.org/series/59609/
State : success

== Summary ==

CI Bug Log - changes from IGT_4953 -> IGTPW_2877
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59609/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2877 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#108094]

  * igt@amdgpu/amd_basic@query-info:
    - fi-bsw-kefka:       NOTRUN -> SKIP [fdo#109271] +50

  * igt@amdgpu/amd_basic@semaphore:
    - fi-bdw-5557u:       NOTRUN -> SKIP [fdo#109271] +38

  * igt@gem_exec_basic@basic-bsd2:
    - fi-kbl-7500u:       NOTRUN -> SKIP [fdo#109271] +9

  * igt@gem_exec_basic@gtt-bsd2:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] +52

  * igt@gem_exec_basic@readonly-bsd2:
    - fi-pnv-d510:        NOTRUN -> SKIP [fdo#109271] +71

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         NOTRUN -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@kms_busy@basic-flip-c:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-bsw-kefka:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-pnv-d510:        NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       NOTRUN -> DMESG-WARN [fdo#103841]

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-hsw-peppy:       NOTRUN -> SKIP [fdo#109271] +46
    - fi-kbl-8809g:       NOTRUN -> SKIP [fdo#109271] +54

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       NOTRUN -> DMESG-FAIL [fdo#102614] / [fdo#107814]

  * igt@kms_psr@primary_page_flip:
    - fi-apl-guc:         NOTRUN -> SKIP [fdo#109271] +48

  * igt@runner@aborted:
    - fi-kbl-7500u:       NOTRUN -> FAIL [fdo#103841]

  
#### Possible fixes ####

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-glk-dsi:         FAIL [fdo#103191] -> PASS

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103841]: https://bugs.freedesktop.org/show_bug.cgi?id=103841
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107814]: https://bugs.freedesktop.org/show_bug.cgi?id=107814
  [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720


Participating hosts (42 -> 42)
------------------------------

  Additional (8): fi-bdw-5557u fi-hsw-peppy fi-apl-guc fi-kbl-7500u fi-kbl-8809g fi-pnv-d510 fi-bsw-kefka fi-byt-clapper 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-skl-guc fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus fi-kbl-r 


Build changes
-------------

    * IGT: IGT_4953 -> IGTPW_2877

  CI_DRM_5939: 757f5370dc4baed0475b6e28efd67ecc267e8745 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2877: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2877/
  IGT_4953: e03d0030391689cfd0fbca293d44d83dd7d9e356 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2877/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for Fixes from updating igt packaging in Fedora

[Patchwork]

== Series Details ==

Series: Fixes from updating igt packaging in Fedora
URL   : https://patchwork.freedesktop.org/series/59609/
State : success

== Summary ==

CI Bug Log - changes from IGT_4953_full -> IGTPW_2877_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59609/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2877_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@preemptive-hang-bsd2:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] +24

  * igt@gem_pread@stolen-uncached:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +20

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566] +6

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@i915_selftest@live_workarounds:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#108954]

  * igt@kms_atomic_transition@5x-modeset-transitions-fencing:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@extended-modeset-hang-oldfb-render-e:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +6

  * igt@kms_busy@extended-pageflip-hang-newfb-render-c:
    - shard-hsw:          PASS -> INCOMPLETE [fdo#103540]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-d:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_content_protection@atomic-dpms:
    - shard-kbl:          NOTRUN -> FAIL [fdo#110321] / [fdo#110336]

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-kbl:          PASS -> FAIL [fdo#103232]
    - shard-glk:          NOTRUN -> FAIL [fdo#103232]
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_frontbuffer_tracking@fbc-tilingchange:
    - shard-apl:          PASS -> FAIL [fdo#103167]
    - shard-glk:          PASS -> FAIL [fdo#103167]
    - shard-kbl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +13

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +4

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-indfb-pgflip-blt:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +20

  * igt@kms_lease@page_flip_implicit_plane:
    - shard-apl:          NOTRUN -> FAIL [fdo#110281]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max:
    - shard-glk:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_scaling@pipe-b-scaler-with-pixel-format:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         PASS -> SKIP [fdo#109642]

  * igt@kms_psr@psr2_primary_mmap_gtt:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +2

  * igt@kms_psr@sprite_plane_move:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@kms_universal_plane@universal-plane-gen9-features-pipe-f:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@perf_pmu@busy-accuracy-2-rcs0:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +39

  * igt@prime_nv_pcopy@test3_5:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +25

  
#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-mmap-cpu:
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-tilingchange:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +6

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-wc:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +10

  * igt@kms_psr@cursor_mmap_gtt:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS +2

  * igt@kms_psr@no_drrs:
    - shard-iclb:         FAIL [fdo#108341] -> PASS

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +4

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         FAIL [fdo#100047] -> PASS

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS +3

  
  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110281]: https://bugs.freedesktop.org/show_bug.cgi?id=110281
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 6)
------------------------------

  Missing    (1): shard-skl 


Build changes
-------------

    * IGT: IGT_4953 -> IGTPW_2877

  CI_DRM_5939: 757f5370dc4baed0475b6e28efd67ecc267e8745 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2877: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2877/
  IGT_4953: e03d0030391689cfd0fbca293d44d83dd7d9e356 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2877/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/7] lib/tests: Fix test failures with meson 0.50.0

[Petri Latvala]

On Tue, Apr 16, 2019 at 04:10:55PM -0400, Lyude wrote:
> From: Lyude Paul <lyude@redhat.com>
> 
> Since meson 0.50.0, unit tests which return the GNU standard return code
> 99 will fail, regardless of whether or not should_fail:true is passed to
> test().


... oh bugger.

But I agree with the reasoning for their change.



> Unfortunately, igt_alarm_handler() exits the application with
> return code 99. However, since returning something other then 99 when a
> test times out would also be a bug we can't really change the return
> code igt_alarm_handler() returns.

99 is IGT_EXIT_FAILURE, and we can change that btw. igt_runner doesn't
care for the exit code value as such except for tests without
subtests, and conflicts can only occur when invoking igt_results from
a different version of IGT. Even then unknown exit codes result in
"fail" so we're good. Other parts of IGT don't hardcode the value
(knocking on wood) and we don't support mix-and-match of components
anywhere else.

I'd say go for changing IGT_EXIT_FAILURE to 98 instead of this patch.


> Instead, we can fix this by simply running tests which are expected to
> fail with return code 99 using a wrapper script that translates 99 to 1
> and any other non-zero error code into 99. This essentially makes it so
> that abnormal test failures are considered normal, and vice versa.

(As a side note, all should_fail tests would have to be wrapped with
this, otherwise you just get the same problem with new lib/tests/stuff
that exits with igt_fail()/igt_assert())



-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/7] meson: Don't redefine gettid if the C library provides it

[Petri Latvala]

On Tue, Apr 16, 2019 at 04:10:56PM -0400, Lyude wrote:
> From: Lyude Paul <lyude@redhat.com>
> 
> glibc 2.30+ will actually include a definition for gettid() that makes
> it so that users don't have to manually define a wrapper for it
> themselves with syscall(). We don't currently check for this, and as a
> result will end up redefining gettid() on the latest versions of glibc,
> causing the build to fail:
> 
> FAILED: lib/76b5a35@@igt-igt_kmod_c@sta/igt_kmod.c.o
> In file included from /usr/include/unistd.h:1170,
>                  from ../../mnt/vol/lib/igt_core.h:43,
>                  from ../../mnt/vol/lib/igt_kmod.c:28:
> /usr/include/bits/unistd_ext.h:34:28: error: macro "gettid" passed 1 arguments, but takes just 0
>    34 | extern __pid_t gettid (void) __THROW;
>       |                            ^
> In file included from ../../mnt/vol/lib/igt_kmod.c:27:
> ../../mnt/vol/lib/igt_aux.h:40: note: macro "gettid" defined here
>    40 | #define gettid() syscall(__NR_gettid)
>       |
> [36/771] Compiling C object 'lib/76b5a35@@igt-igt_kms_c@sta/igt_kms.c.o'.
> ninja: build stopped: subcommand failed.
> 
> So, fix this by by adding some meson checks to define HAVE_GETTID whenever the
> host defines its own gettid(), and avoid redefining gettid() when HAVE_GETTID is
> defined.
> 
> This fixes build intel-gpu-tools for me on Fedora Rawhide

Unless that's the package name on Fedora (and then, why), igt-gpu-tools :)


> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
>  lib/igt_aux.h | 3 +++
>  meson.build   | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 55392790..39fefcbf 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -36,7 +36,10 @@
>  #include <i915/gem_submission.h>
>  
>  /* signal interrupt helpers */
> +#ifndef HAVE_GETTID
>  #define gettid() syscall(__NR_gettid)
> +#endif
> +
>  #define sigev_notify_thread_id _sigev_un._tid
>  
>  /* auxialiary igt helpers from igt_aux.c */
> diff --git a/meson.build b/meson.build
> index 557400a5..e73275dd 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -237,6 +237,9 @@ if cc.has_header('cpuid.h')
>  	# FIXME: Do we need the example link test from configure.ac?
>  	config.set('HAVE_CPUID_H', 1)
>  endif
> +if cc.has_header_symbol('unistd.h', 'gettid', args : '-D_GNU_SOURCE')
> +	config.set('HAVE_GETTID', 1)
> +endif


All users of gettid() have #include unistd.h? Check.

Are there any other direct calls to syscall(__NR_gettid)? Or
syscall(SYS_gettid)? (is that the same?)

benchmarks/gem_syslatency.c:#define gettid() syscall(__NR_gettid)
lib/igt_core.c:	      	       pid_t tid = syscall(SYS_gettid);
tests/core_auth.c:	       return syscall(SYS_gettid) == tid;
tests/drm_import_export.c:     igt_debug("start %ld\n", syscall(SYS_gettid));
tests/i915/gem_close_race.c:#define gettid() syscall(__NR_gettid)



-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/7] lib: Stop using assert() for runtime checks

[Petri Latvala]

On Tue, Apr 16, 2019 at 04:10:57PM -0400, Lyude wrote:
> From: Lyude Paul <lyude@redhat.com>
> 
> In the process of trying to get an up to date version of igt packaged
> for Fedora I discovered that if igt was built with -Db_ndebug=true, a
> significant portion of the test infrastructure unit tests would start
> failing:
> 
>   1/265 lib: igt_assert                         OK       0.11 s
>   2/265 lib: igt_can_fail                       OK       0.08 s
>   3/265 lib: igt_can_fail_simple                OK       0.08 s
>   4/265 lib: igt_exit_handler                   OK       0.05 s
>   5/265 lib: igt_fork                           FAIL     0.05 s (killed by signal 9 SIGKILL)
>   6/265 lib: igt_fork_helper                    OK       0.42 s
>   7/265 lib: igt_hdmi_inject                    OK       0.05 s
>   8/265 lib: igt_list_only                      OK       0.01 s
>   9/265 lib: igt_invalid_subtest_name           OK       0.05 s
>  10/265 lib: igt_no_exit                        OK       0.04 s
>  11/265 lib: igt_segfault                       OK       0.38 s
>  12/265 lib: igt_simulation                     OK       0.02 s
>  13/265 lib: igt_stats                          OK       0.04 s
>  14/265 lib: igt_subtest_group                  OK       0.03 s
>  15/265 lib: igt_no_subtest                     SKIP     0.02 s
>  16/265 lib: igt_simple_test_subtests           UNEXPECTEDPASS 0.02 s
>  17/265 lib: igt_timeout                        EXPECTEDFAIL 1.02 s
> 
> Which appeared to stem from the fact that -Db_ndebug=true would strip
> assert() calls. While on a first glance of lib/tests/igt_tests_common.h
> one would assume that the only user of assert() was the test
> infrastructure unit tests themselves, it turns out we've actually been
> using this in multiple spots that seem to expect an unconditional
> runtime check.


Umm, yes. You've uncovered a bug, but not what you think. With meson,
we don't include lib/check-ndebug.h anymore, only with autotools so
you missed the important part: Do not attempt to build IGT with
b_ndebug=true.


> So in order to fix this, we introduce igt_internal_assert(). The purpose
> of this function is to provide simple runtime error-checking for
> conditions that warrant abort()ing IGT under any circumstances, and to
> provide a internal_assert() replacement for our test infrastructure
> tests to use. Then, remove all of the assert() usages in lib/tests/*
> along with any of the assert() calls in lib/igt_core.c that affect test
> behavior.

In a nutshell: You wanted to build without asserts, so you removed all
the asserts from the code, replacing them with the same code? Why?

However, it's the thought that matters, and this is slightly going off
on a tangent. Those uses of assert in lib/ are for places where

1) something is fatally wrong and we need to drop everything and stop
   executing

2) cannot use igt_assert for it. That's for places where we can say
   "you tried testing your kernel but it has a bug". The lib/ asserts
   are for "IGT has a bug", or in a couple of cases, "your IGT setup
   has a bug".

Chris has been tossing around mentions of having a clear and usable
igt_is_broken_not_your_kernel__assert() and this might be a good spot
to introduce such a thing. Plain assert() is fine to achieve that
outcome, but igt.cocci (albeit a bit dusty) nukes those from tests/. A
'CRASH' test result is a fine outcome when it's triggered and conveys
the message across, we just need a clear looking igt API for it.




> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
>  lib/igt_core.c                       | 24 +++++------
>  lib/igt_core.h                       | 18 +++++++++
>  lib/tests/igt_assert.c               |  2 +-
>  lib/tests/igt_can_fail.c             | 10 ++---
>  lib/tests/igt_can_fail_simple.c      |  2 +-
>  lib/tests/igt_exit_handler.c         | 18 ++++-----
>  lib/tests/igt_fork.c                 |  4 +-
>  lib/tests/igt_invalid_subtest_name.c |  2 +-
>  lib/tests/igt_no_exit.c              |  2 +-
>  lib/tests/igt_segfault.c             |  2 +-
>  lib/tests/igt_simulation.c           | 60 ++++++++++++++--------------
>  lib/tests/igt_subtest_group.c        | 16 ++++----
>  lib/tests/igt_tests_common.h         | 19 ++++-----
>  13 files changed, 97 insertions(+), 82 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index ae03e909..a637b94e 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -521,7 +521,7 @@ static void common_exit_handler(int sig)
>  
>  	/* When not killed by a signal check that igt_exit() has been properly
>  	 * called. */
> -	assert(sig != 0 || igt_exit_called);
> +	igt_internal_assert(sig != 0 || igt_exit_called);
>  }
>  
>  static void print_test_description(void)
> @@ -611,7 +611,7 @@ static void common_init_config(void)
>  
>  	ret = g_key_file_get_integer(igt_key_file, "DUT", "SuspendResumeDelay",
>  				     &error);
> -	assert(!error || error->code != G_KEY_FILE_ERROR_INVALID_VALUE);
> +	igt_internal_assert(!error || error->code != G_KEY_FILE_ERROR_INVALID_VALUE);
>  
>  	g_clear_error(&error);
>  
> @@ -903,9 +903,9 @@ bool __igt_run_subtest(const char *subtest_name)
>  {
>  	int i;
>  
> -	assert(!in_subtest);
> -	assert(!in_fixture);
> -	assert(test_with_subtests);
> +	igt_internal_assert(!in_subtest);
> +	igt_internal_assert(!in_fixture);
> +	igt_internal_assert(test_with_subtests);
>  
>  	/* check the subtest name only contains a-z, A-Z, 0-9, '-' and '_' */
>  	for (i = 0; subtest_name[i] != '\0'; i++)
> @@ -978,7 +978,7 @@ bool igt_only_list_subtests(void)
>  
>  void __igt_subtest_group_save(int *save)
>  {
> -	assert(test_with_subtests);
> +	igt_internal_assert(test_with_subtests);
>  
>  	*save = skip_subtests_henceforth;
>  }
> @@ -1032,7 +1032,7 @@ void igt_skip(const char *f, ...)
>  	va_list args;
>  	skipped_one = true;
>  
> -	assert(!test_child);
> +	igt_internal_assert(!test_child);
>  
>  	if (!igt_only_list_subtests()) {
>  		va_start(args, f);
> @@ -1510,10 +1510,10 @@ void igt_exit(void)
>  		exit(IGT_EXIT_SUCCESS);
>  
>  	/* Calling this without calling one of the above is a failure */
> -	assert(!test_with_subtests ||
> -	       skipped_one ||
> -	       succeeded_one ||
> -	       failed_one);
> +	igt_internal_assert(!test_with_subtests ||
> +			    skipped_one ||
> +			    succeeded_one ||
> +			    failed_one);
>  
>  	if (test_with_subtests && !failed_one) {
>  		if (succeeded_one)
> @@ -1531,7 +1531,7 @@ void igt_exit(void)
>  		kill(test_children[c], SIGKILL);
>  	assert(!num_test_children);
>  
> -	assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno == ECHILD);
> +	igt_internal_assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno == ECHILD);
>  
>  	if (!test_with_subtests) {
>  		struct timespec now;
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index 47ffd9e7..eed39bbf 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -747,6 +747,24 @@ static inline void igt_ignore_warn(bool value)
>  	else igt_debug("Test requirement passed: !(%s)\n", #expr); \
>  } while (0)
>  
> +/**
> + * igt_internal_assert:
> + * @expr: condition to test
> + *
> + * If the given expression fails, print an error to stderr and abort()
> + * immediately. This is intended for use only by the igt helpers in order to
> + * check for incorrect usages of the helpers or other extraordinary conditions.
> + *
> + * This should never be used within a normal test. Doing so will alert
> + * velociraptors to your location.
> + */


Aside: I like this documentation block.



-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/7] lib/igt_core: Just use igt_can_fail() in __igt_run_subtest()

[Petri Latvala]

On Tue, Apr 16, 2019 at 04:10:58PM -0400, Lyude wrote:
> From: Lyude Paul <lyude@redhat.com>
> 
> That's what it's there for.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
>  lib/igt_core.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index a637b94e..503503ce 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -903,9 +903,7 @@ bool __igt_run_subtest(const char *subtest_name)
>  {
>  	int i;
>  
> -	igt_internal_assert(!in_subtest);
> -	igt_internal_assert(!in_fixture);
> -	igt_internal_assert(test_with_subtests);
> +	igt_internal_assert(!igt_can_fail());


For computers it's the same but for humans the meaning is a bit lost
with this change. Meh, whoever wants to implement nested subtests has
to change that part anyway...

Possible rebasing to the changes for the assert patch required, but

Reviewed-by: Petri Latvala <petri.latvala@intel.com>

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 5/7] Use pkgconfig() macros with dnf in Fedora Dockerfile

[Petri Latvala]

On Tue, Apr 16, 2019 at 04:10:59PM -0400, Lyude wrote:
> From: Lyude Paul <lyude@redhat.com>
> 
> dnf supports installing packages by their pkgconfig names using the
> pkgconfig() RPM macro. Since these more closely match the dependencies
> that meson uses and don't have a chance of changing in the future like
> Fedora package name do, let's use these with dnf instead.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>

Reviewed-by: Petri Latvala <petri.latvala@intel.com>

Do you have a gitlab fork with this patch where gitlab CI can prove
there aren't any typos?


> ---
>  Dockerfile.fedora | 48 +++++++++++++++++++++++------------------------
>  1 file changed, 23 insertions(+), 25 deletions(-)
> 
> diff --git a/Dockerfile.fedora b/Dockerfile.fedora
> index 29520f7b..469e74b2 100644
> --- a/Dockerfile.fedora
> +++ b/Dockerfile.fedora
> @@ -1,30 +1,28 @@
>  FROM fedora:28
>  
> -RUN dnf install -y gcc \
> -		flex \
> -		meson \
> -		bison  \
> -		gtk-doc \
> -		xdotool \
> -		gsl-devel \
> -		kmod-devel \
> -		glib2-devel \
> -		cairo-devel \
> -		ninja-build \
> -		procps-devel \
> -		pixman-devel \
> -		json-c-devel \
> -		libdrm-devel \
> -		libudev-devel \
> -		xmlrpc-c-devel \
> -		elfutils-devel \
> -		libunwind-devel \
> -		python-docutils \
> -		libpciaccess-devel \
> -		alsa-lib-devel \
> -		valgrind-devel \
> -		libXrandr-devel \
> -		libXv-devel
> +RUN dnf install -y \
> +	gcc flex bison meson ninja-build xdotool \
> +	'pkgconfig(libdrm)' \
> +	'pkgconfig(pciaccess)' \
> +	'pkgconfig(libkmod)' \
> +	'pkgconfig(libprocps)' \
> +	'pkgconfig(libunwind)' \
> +	'pkgconfig(libdw)' \
> +	'pkgconfig(pixman-1)' \
> +	'pkgconfig(valgrind)' \
> +	'pkgconfig(cairo)' \
> +	'pkgconfig(libudev)' \
> +	'pkgconfig(glib-2.0)' \
> +	'pkgconfig(gsl)' \
> +	'pkgconfig(alsa)' \
> +	'pkgconfig(xmlrpc)' \
> +	'pkgconfig(xmlrpc_util)' \
> +	'pkgconfig(xmlrpc_client)' \
> +	'pkgconfig(json-c)' \
> +	'pkgconfig(gtk-doc)' \
> +	'pkgconfig(xv)' \
> +	'pkgconfig(xrandr)' \
> +	python3-docutils
>  
>  # We need peg to build overlay
>  RUN dnf install -y make
> -- 
> 2.20.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/7] lib: Stop using assert() for runtime checks

[Lyude Paul]

On Wed, 2019-04-17 at 11:52 +0300, Petri Latvala wrote:
> On Tue, Apr 16, 2019 at 04:10:57PM -0400, Lyude wrote:
> > From: Lyude Paul <lyude@redhat.com>
> > 
> > In the process of trying to get an up to date version of igt packaged
> > for Fedora I discovered that if igt was built with -Db_ndebug=true, a
> > significant portion of the test infrastructure unit tests would start
> > failing:
> > 
> >   1/265 lib: igt_assert                         OK       0.11 s
> >   2/265 lib: igt_can_fail                       OK       0.08 s
> >   3/265 lib: igt_can_fail_simple                OK       0.08 s
> >   4/265 lib: igt_exit_handler                   OK       0.05 s
> >   5/265 lib: igt_fork                           FAIL     0.05 s (killed by
> > signal 9 SIGKILL)
> >   6/265 lib: igt_fork_helper                    OK       0.42 s
> >   7/265 lib: igt_hdmi_inject                    OK       0.05 s
> >   8/265 lib: igt_list_only                      OK       0.01 s
> >   9/265 lib: igt_invalid_subtest_name           OK       0.05 s
> >  10/265 lib: igt_no_exit                        OK       0.04 s
> >  11/265 lib: igt_segfault                       OK       0.38 s
> >  12/265 lib: igt_simulation                     OK       0.02 s
> >  13/265 lib: igt_stats                          OK       0.04 s
> >  14/265 lib: igt_subtest_group                  OK       0.03 s
> >  15/265 lib: igt_no_subtest                     SKIP     0.02 s
> >  16/265 lib: igt_simple_test_subtests           UNEXPECTEDPASS 0.02 s
> >  17/265 lib: igt_timeout                        EXPECTEDFAIL 1.02 s
> > 
> > Which appeared to stem from the fact that -Db_ndebug=true would strip
> > assert() calls. While on a first glance of lib/tests/igt_tests_common.h
> > one would assume that the only user of assert() was the test
> > infrastructure unit tests themselves, it turns out we've actually been
> > using this in multiple spots that seem to expect an unconditional
> > runtime check.
> 
> Umm, yes. You've uncovered a bug, but not what you think. With meson,
> we don't include lib/check-ndebug.h anymore, only with autotools so
> you missed the important part: Do not attempt to build IGT with
> b_ndebug=true.

If igt really isn't supposed to be built with -Db_ndebug=true we really should
add explicit checks for this in our meson.build file, e.g.
 
if get_option("b_ndebug") == true
    error("Building with -Db_ndebug=true is not supported")
endif

Especially since the default in meson upstream is going to be changing to having
-Db_ndebug=true on release builds pretty soon. I'll include a patch for this in
the v3 respin.

> 
> 
> > So in order to fix this, we introduce igt_internal_assert(). The purpose
> > of this function is to provide simple runtime error-checking for
> > conditions that warrant abort()ing IGT under any circumstances, and to
> > provide a internal_assert() replacement for our test infrastructure
> > tests to use. Then, remove all of the assert() usages in lib/tests/*
> > along with any of the assert() calls in lib/igt_core.c that affect test
> > behavior.
> 
> In a nutshell: You wanted to build without asserts, so you removed all
> the asserts from the code, replacing them with the same code? Why?
> 
> However, it's the thought that matters, and this is slightly going off
> on a tangent. Those uses of assert in lib/ are for places where
> 
> 1) something is fatally wrong and we need to drop everything and stop
>    executing
> 
> 2) cannot use igt_assert for it. That's for places where we can say
>    "you tried testing your kernel but it has a bug". The lib/ asserts
>    are for "IGT has a bug", or in a couple of cases, "your IGT setup
>    has a bug".
> 
> Chris has been tossing around mentions of having a clear and usable
> igt_is_broken_not_your_kernel__assert() and this might be a good spot
> to introduce such a thing. Plain assert() is fine to achieve that
> outcome, but igt.cocci (albeit a bit dusty) nukes those from tests/. A
> 'CRASH' test result is a fine outcome when it's triggered and conveys
> the message across, we just need a clear looking igt API for it.
> 
> 
> 
> 
> > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > ---
> >  lib/igt_core.c                       | 24 +++++------
> >  lib/igt_core.h                       | 18 +++++++++
> >  lib/tests/igt_assert.c               |  2 +-
> >  lib/tests/igt_can_fail.c             | 10 ++---
> >  lib/tests/igt_can_fail_simple.c      |  2 +-
> >  lib/tests/igt_exit_handler.c         | 18 ++++-----
> >  lib/tests/igt_fork.c                 |  4 +-
> >  lib/tests/igt_invalid_subtest_name.c |  2 +-
> >  lib/tests/igt_no_exit.c              |  2 +-
> >  lib/tests/igt_segfault.c             |  2 +-
> >  lib/tests/igt_simulation.c           | 60 ++++++++++++++--------------
> >  lib/tests/igt_subtest_group.c        | 16 ++++----
> >  lib/tests/igt_tests_common.h         | 19 ++++-----
> >  13 files changed, 97 insertions(+), 82 deletions(-)
> > 
> > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > index ae03e909..a637b94e 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -521,7 +521,7 @@ static void common_exit_handler(int sig)
> >  
> >  	/* When not killed by a signal check that igt_exit() has been properly
> >  	 * called. */
> > -	assert(sig != 0 || igt_exit_called);
> > +	igt_internal_assert(sig != 0 || igt_exit_called);
> >  }
> >  
> >  static void print_test_description(void)
> > @@ -611,7 +611,7 @@ static void common_init_config(void)
> >  
> >  	ret = g_key_file_get_integer(igt_key_file, "DUT", "SuspendResumeDelay",
> >  				     &error);
> > -	assert(!error || error->code != G_KEY_FILE_ERROR_INVALID_VALUE);
> > +	igt_internal_assert(!error || error->code !=
> > G_KEY_FILE_ERROR_INVALID_VALUE);
> >  
> >  	g_clear_error(&error);
> >  
> > @@ -903,9 +903,9 @@ bool __igt_run_subtest(const char *subtest_name)
> >  {
> >  	int i;
> >  
> > -	assert(!in_subtest);
> > -	assert(!in_fixture);
> > -	assert(test_with_subtests);
> > +	igt_internal_assert(!in_subtest);
> > +	igt_internal_assert(!in_fixture);
> > +	igt_internal_assert(test_with_subtests);
> >  
> >  	/* check the subtest name only contains a-z, A-Z, 0-9, '-' and '_' */
> >  	for (i = 0; subtest_name[i] != '\0'; i++)
> > @@ -978,7 +978,7 @@ bool igt_only_list_subtests(void)
> >  
> >  void __igt_subtest_group_save(int *save)
> >  {
> > -	assert(test_with_subtests);
> > +	igt_internal_assert(test_with_subtests);
> >  
> >  	*save = skip_subtests_henceforth;
> >  }
> > @@ -1032,7 +1032,7 @@ void igt_skip(const char *f, ...)
> >  	va_list args;
> >  	skipped_one = true;
> >  
> > -	assert(!test_child);
> > +	igt_internal_assert(!test_child);
> >  
> >  	if (!igt_only_list_subtests()) {
> >  		va_start(args, f);
> > @@ -1510,10 +1510,10 @@ void igt_exit(void)
> >  		exit(IGT_EXIT_SUCCESS);
> >  
> >  	/* Calling this without calling one of the above is a failure */
> > -	assert(!test_with_subtests ||
> > -	       skipped_one ||
> > -	       succeeded_one ||
> > -	       failed_one);
> > +	igt_internal_assert(!test_with_subtests ||
> > +			    skipped_one ||
> > +			    succeeded_one ||
> > +			    failed_one);
> >  
> >  	if (test_with_subtests && !failed_one) {
> >  		if (succeeded_one)
> > @@ -1531,7 +1531,7 @@ void igt_exit(void)
> >  		kill(test_children[c], SIGKILL);
> >  	assert(!num_test_children);
> >  
> > -	assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno == ECHILD);
> > +	igt_internal_assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno ==
> > ECHILD);
> >  
> >  	if (!test_with_subtests) {
> >  		struct timespec now;
> > diff --git a/lib/igt_core.h b/lib/igt_core.h
> > index 47ffd9e7..eed39bbf 100644
> > --- a/lib/igt_core.h
> > +++ b/lib/igt_core.h
> > @@ -747,6 +747,24 @@ static inline void igt_ignore_warn(bool value)
> >  	else igt_debug("Test requirement passed: !(%s)\n", #expr); \
> >  } while (0)
> >  
> > +/**
> > + * igt_internal_assert:
> > + * @expr: condition to test
> > + *
> > + * If the given expression fails, print an error to stderr and abort()
> > + * immediately. This is intended for use only by the igt helpers in order
> > to
> > + * check for incorrect usages of the helpers or other extraordinary
> > conditions.
> > + *
> > + * This should never be used within a normal test. Doing so will alert
> > + * velociraptors to your location.
> > + */
> 
> Aside: I like this documentation block.
> 
> 
> 

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/7] lib: Stop using assert() for runtime checks

[Petri Latvala]

On Tue, Apr 23, 2019 at 08:36:43PM -0400, Lyude Paul wrote:
> On Wed, 2019-04-17 at 11:52 +0300, Petri Latvala wrote:
> > On Tue, Apr 16, 2019 at 04:10:57PM -0400, Lyude wrote:
> > > From: Lyude Paul <lyude@redhat.com>
> > > 
> > > In the process of trying to get an up to date version of igt packaged
> > > for Fedora I discovered that if igt was built with -Db_ndebug=true, a
> > > significant portion of the test infrastructure unit tests would start
> > > failing:
> > > 
> > >   1/265 lib: igt_assert                         OK       0.11 s
> > >   2/265 lib: igt_can_fail                       OK       0.08 s
> > >   3/265 lib: igt_can_fail_simple                OK       0.08 s
> > >   4/265 lib: igt_exit_handler                   OK       0.05 s
> > >   5/265 lib: igt_fork                           FAIL     0.05 s (killed by
> > > signal 9 SIGKILL)
> > >   6/265 lib: igt_fork_helper                    OK       0.42 s
> > >   7/265 lib: igt_hdmi_inject                    OK       0.05 s
> > >   8/265 lib: igt_list_only                      OK       0.01 s
> > >   9/265 lib: igt_invalid_subtest_name           OK       0.05 s
> > >  10/265 lib: igt_no_exit                        OK       0.04 s
> > >  11/265 lib: igt_segfault                       OK       0.38 s
> > >  12/265 lib: igt_simulation                     OK       0.02 s
> > >  13/265 lib: igt_stats                          OK       0.04 s
> > >  14/265 lib: igt_subtest_group                  OK       0.03 s
> > >  15/265 lib: igt_no_subtest                     SKIP     0.02 s
> > >  16/265 lib: igt_simple_test_subtests           UNEXPECTEDPASS 0.02 s
> > >  17/265 lib: igt_timeout                        EXPECTEDFAIL 1.02 s
> > > 
> > > Which appeared to stem from the fact that -Db_ndebug=true would strip
> > > assert() calls. While on a first glance of lib/tests/igt_tests_common.h
> > > one would assume that the only user of assert() was the test
> > > infrastructure unit tests themselves, it turns out we've actually been
> > > using this in multiple spots that seem to expect an unconditional
> > > runtime check.
> > 
> > Umm, yes. You've uncovered a bug, but not what you think. With meson,
> > we don't include lib/check-ndebug.h anymore, only with autotools so
> > you missed the important part: Do not attempt to build IGT with
> > b_ndebug=true.
> 
> If igt really isn't supposed to be built with -Db_ndebug=true we really should
> add explicit checks for this in our meson.build file, e.g.
>  
> if get_option("b_ndebug") == true
>     error("Building with -Db_ndebug=true is not supported")
> endif
> 
> Especially since the default in meson upstream is going to be changing to having
> -Db_ndebug=true on release builds pretty soon. I'll include a patch for this in
> the v3 respin.

Yep, an explicit check sounds good. And overriding the default in the
project() call in toplevel meson.build. And having lib/check-ndebug.h
used in meson builds is a TODO item as well, in case people have a
manual -DNDEBUG in their cflags.


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 5/7] Use pkgconfig() macros with dnf in Fedora Dockerfile

[Lyude Paul]

On Wed, 2019-04-17 at 12:09 +0300, Petri Latvala wrote:
> On Tue, Apr 16, 2019 at 04:10:59PM -0400, Lyude wrote:
> > From: Lyude Paul <lyude@redhat.com>
> > 
> > dnf supports installing packages by their pkgconfig names using the
> > pkgconfig() RPM macro. Since these more closely match the dependencies
> > that meson uses and don't have a chance of changing in the future like
> > Fedora package name do, let's use these with dnf instead.
> > 
> > Signed-off-by: Lyude Paul <lyude@redhat.com>
> 
> Reviewed-by: Petri Latvala <petri.latvala@intel.com>
> 
> Do you have a gitlab fork with this patch where gitlab CI can prove
> there aren't any typos?
> 

Yes: https://gitlab.freedesktop.org/lyudess/igt-gpu-tools/commits/wip/fedora-docker-update
> 
> > ---
> >  Dockerfile.fedora | 48 +++++++++++++++++++++++------------------------
> >  1 file changed, 23 insertions(+), 25 deletions(-)
> > 
> > diff --git a/Dockerfile.fedora b/Dockerfile.fedora
> > index 29520f7b..469e74b2 100644
> > --- a/Dockerfile.fedora
> > +++ b/Dockerfile.fedora
> > @@ -1,30 +1,28 @@
> >  FROM fedora:28
> >  
> > -RUN dnf install -y gcc \
> > -		flex \
> > -		meson \
> > -		bison  \
> > -		gtk-doc \
> > -		xdotool \
> > -		gsl-devel \
> > -		kmod-devel \
> > -		glib2-devel \
> > -		cairo-devel \
> > -		ninja-build \
> > -		procps-devel \
> > -		pixman-devel \
> > -		json-c-devel \
> > -		libdrm-devel \
> > -		libudev-devel \
> > -		xmlrpc-c-devel \
> > -		elfutils-devel \
> > -		libunwind-devel \
> > -		python-docutils \
> > -		libpciaccess-devel \
> > -		alsa-lib-devel \
> > -		valgrind-devel \
> > -		libXrandr-devel \
> > -		libXv-devel
> > +RUN dnf install -y \
> > +	gcc flex bison meson ninja-build xdotool \
> > +	'pkgconfig(libdrm)' \
> > +	'pkgconfig(pciaccess)' \
> > +	'pkgconfig(libkmod)' \
> > +	'pkgconfig(libprocps)' \
> > +	'pkgconfig(libunwind)' \
> > +	'pkgconfig(libdw)' \
> > +	'pkgconfig(pixman-1)' \
> > +	'pkgconfig(valgrind)' \
> > +	'pkgconfig(cairo)' \
> > +	'pkgconfig(libudev)' \
> > +	'pkgconfig(glib-2.0)' \
> > +	'pkgconfig(gsl)' \
> > +	'pkgconfig(alsa)' \
> > +	'pkgconfig(xmlrpc)' \
> > +	'pkgconfig(xmlrpc_util)' \
> > +	'pkgconfig(xmlrpc_client)' \
> > +	'pkgconfig(json-c)' \
> > +	'pkgconfig(gtk-doc)' \
> > +	'pkgconfig(xv)' \
> > +	'pkgconfig(xrandr)' \
> > +	python3-docutils
> >  
> >  # We need peg to build overlay
> >  RUN dnf install -y make
> > -- 
> > 2.20.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
-- 
Cheers,
	Lyude Paul

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev