public inbox for igt-dev@lists.freedesktop.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in
@ 2019-04-16 23:25 Lyude
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 1/8] lib/tests: Fix test failures with meson 0.50.0 Lyude
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Lyude @ 2019-04-16 23:25 UTC (permalink / raw)
  To: igt-dev

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()?

			    Changes since v1
* Add one last patch that came out of package review - soversioning,
  which both Debian and Fedora seem to require
  (see https://bugzilla.redhat.com/show_bug.cgi?id=1700559 )

Lyude Paul (8):
  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
  meson: Add .so versioning
  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/meson.build                      |  3 +-
 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 ++
 tools/meson.build                    |  3 +-
 21 files changed, 156 insertions(+), 110 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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [igt-dev] [PATCH i-g-t v2 1/8] lib/tests: Fix test failures with meson 0.50.0
  2019-04-16 23:25 [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in Lyude
@ 2019-04-16 23:25 ` Lyude
  2019-04-17 11:35   ` Daniel Vetter
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 2/8] meson: Don't redefine gettid if the C library provides it Lyude
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Lyude @ 2019-04-16 23:25 UTC (permalink / raw)
  To: igt-dev

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [igt-dev] [PATCH i-g-t v2 2/8] meson: Don't redefine gettid if the C library provides it
  2019-04-16 23:25 [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in Lyude
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 1/8] lib/tests: Fix test failures with meson 0.50.0 Lyude
@ 2019-04-16 23:25 ` Lyude
  2019-04-17 11:36   ` Daniel Vetter
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 3/8] lib: Stop using assert() for runtime checks Lyude
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Lyude @ 2019-04-16 23:25 UTC (permalink / raw)
  To: igt-dev

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [igt-dev] [PATCH i-g-t v2 3/8] lib: Stop using assert() for runtime checks
  2019-04-16 23:25 [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in Lyude
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 1/8] lib/tests: Fix test failures with meson 0.50.0 Lyude
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 2/8] meson: Don't redefine gettid if the C library provides it Lyude
@ 2019-04-16 23:25 ` Lyude
  2019-04-17 11:40   ` Daniel Vetter
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 4/8] meson: Add .so versioning Lyude
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Lyude @ 2019-04-16 23:25 UTC (permalink / raw)
  To: igt-dev

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [igt-dev] [PATCH i-g-t v2 4/8] meson: Add .so versioning
  2019-04-16 23:25 [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in Lyude
                   ` (2 preceding siblings ...)
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 3/8] lib: Stop using assert() for runtime checks Lyude
@ 2019-04-16 23:25 ` Lyude
  2019-04-17  9:16   ` Petri Latvala
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 5/8] lib/igt_core: Just use igt_can_fail() in __igt_run_subtest() Lyude
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Lyude @ 2019-04-16 23:25 UTC (permalink / raw)
  To: igt-dev

From: Lyude Paul <lyude@redhat.com>

While I'm pretty confident that no one cares to use libigt.so or
lib_aubdump.so anywhere outside of igt, many distributions including
Fedora and Debian strongly suggest that packages have some sort of so
versioning, even if it's just '0'. So, let's fulfill that minimum
requirement to make this easier to package.

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

diff --git a/lib/meson.build b/lib/meson.build
index a8462933..2aad9d9e 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -132,7 +132,8 @@ lib_igt_build = shared_library('igt',
     ['dummy.c'],
     link_whole: lib_intermediates,
     dependencies: lib_deps,
-    install : true
+    install : true,
+    soversion : '0',
 )
 
 lib_igt = declare_dependency(link_with : lib_igt_build,
diff --git a/tools/meson.build b/tools/meson.build
index 5d00f2e3..dfaed82a 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -93,7 +93,8 @@ install_subdir('registers', install_dir : datadir,
 shared_library('intel_aubdump', 'aubdump.c',
 	       dependencies : [ lib_igt_chipset, dlsym ],
 	       name_prefix : '',
-	       install : true)
+	       install : true,
+	       soversion : '0')
 
 executable('intel_gpu_top', 'intel_gpu_top.c',
 	   install : true,
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [igt-dev] [PATCH i-g-t v2 5/8] lib/igt_core: Just use igt_can_fail() in __igt_run_subtest()
  2019-04-16 23:25 [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in Lyude
                   ` (3 preceding siblings ...)
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 4/8] meson: Add .so versioning Lyude
@ 2019-04-16 23:25 ` Lyude
  2019-04-17 11:44   ` Daniel Vetter
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 6/8] Use pkgconfig() macros with dnf in Fedora Dockerfile Lyude
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Lyude @ 2019-04-16 23:25 UTC (permalink / raw)
  To: igt-dev

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [igt-dev] [PATCH i-g-t v2 6/8] Use pkgconfig() macros with dnf in Fedora Dockerfile
  2019-04-16 23:25 [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in Lyude
                   ` (4 preceding siblings ...)
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 5/8] lib/igt_core: Just use igt_can_fail() in __igt_run_subtest() Lyude
@ 2019-04-16 23:25 ` Lyude
  2019-04-17 11:45   ` Daniel Vetter
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 7/8] Update Dockerfile.fedora to Fedora 29 Lyude
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Lyude @ 2019-04-16 23:25 UTC (permalink / raw)
  To: igt-dev

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [igt-dev] [PATCH i-g-t v2 7/8] Update Dockerfile.fedora to Fedora 29
  2019-04-16 23:25 [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in Lyude
                   ` (5 preceding siblings ...)
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 6/8] Use pkgconfig() macros with dnf in Fedora Dockerfile Lyude
@ 2019-04-16 23:25 ` Lyude
  2019-04-17 11:45   ` Daniel Vetter
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 8/8] Gitlab CI: Do builds with -Db_ndebug=true Lyude
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Lyude @ 2019-04-16 23:25 UTC (permalink / raw)
  To: igt-dev

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [igt-dev] [PATCH i-g-t v2 8/8] Gitlab CI: Do builds with -Db_ndebug=true
  2019-04-16 23:25 [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in Lyude
                   ` (6 preceding siblings ...)
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 7/8] Update Dockerfile.fedora to Fedora 29 Lyude
@ 2019-04-16 23:25 ` Lyude
  2019-04-17 11:47   ` Daniel Vetter
  2019-04-17  0:16 ` [igt-dev] ✓ Fi.CI.BAT: success for Fixes from updating igt packaging in Patchwork
  2019-04-17  9:25 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  9 siblings, 1 reply; 21+ messages in thread
From: Lyude @ 2019-04-16 23:25 UTC (permalink / raw)
  To: igt-dev

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

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for Fixes from updating igt packaging in
  2019-04-16 23:25 [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in Lyude
                   ` (7 preceding siblings ...)
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 8/8] Gitlab CI: Do builds with -Db_ndebug=true Lyude
@ 2019-04-17  0:16 ` Patchwork
  2019-04-17  9:25 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  9 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-04-17  0:16 UTC (permalink / raw)
  To: Lyude; +Cc: igt-dev

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5942 -> IGTPW_2878
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_cpu_reloc@basic:
    - fi-icl-y:           PASS -> INCOMPLETE [fdo#110246]

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      PASS -> DMESG-FAIL [fdo#110235 ]

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

  * igt@runner@aborted:
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 
  [fdo#110246]: https://bugs.freedesktop.org/show_bug.cgi?id=110246


Participating hosts (48 -> 39)
------------------------------

  Missing    (9): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-skl-6260u fi-gdg-551 fi-cfl-8109u fi-pnv-d510 fi-byt-clapper fi-bdw-samus 


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

    * IGT: IGT_4953 -> IGTPW_2878

  CI_DRM_5942: 1d19629d12fdb73dd65d1df91cf69b89fd76cae6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2878: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2878/
  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_2878/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 4/8] meson: Add .so versioning
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 4/8] meson: Add .so versioning Lyude
@ 2019-04-17  9:16   ` Petri Latvala
  2019-04-17 11:43     ` Daniel Vetter
  0 siblings, 1 reply; 21+ messages in thread
From: Petri Latvala @ 2019-04-17  9:16 UTC (permalink / raw)
  To: Lyude; +Cc: igt-dev

On Tue, Apr 16, 2019 at 07:25:44PM -0400, Lyude wrote:
> From: Lyude Paul <lyude@redhat.com>
> 
> While I'm pretty confident that no one cares to use libigt.so or
> lib_aubdump.so anywhere outside of igt, many distributions including
> Fedora and Debian strongly suggest that packages have some sort of so
> versioning, even if it's just '0'. So, let's fulfill that minimum
> requirement to make this easier to package.
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> ---
>  lib/meson.build   | 3 ++-
>  tools/meson.build | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/meson.build b/lib/meson.build
> index a8462933..2aad9d9e 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -132,7 +132,8 @@ lib_igt_build = shared_library('igt',
>      ['dummy.c'],
>      link_whole: lib_intermediates,
>      dependencies: lib_deps,
> -    install : true
> +    install : true,
> +    soversion : '0',
>  )

This part:
Acked-by: Petri Latvala <petri.latvala@intel.com>



>  
>  lib_igt = declare_dependency(link_with : lib_igt_build,
> diff --git a/tools/meson.build b/tools/meson.build
> index 5d00f2e3..dfaed82a 100644
> --- a/tools/meson.build
> +++ b/tools/meson.build
> @@ -93,7 +93,8 @@ install_subdir('registers', install_dir : datadir,
>  shared_library('intel_aubdump', 'aubdump.c',
>  	       dependencies : [ lib_igt_chipset, dlsym ],
>  	       name_prefix : '',
> -	       install : true)
> +	       install : true,
> +	       soversion : '0')

This part: Is this needed? intel_aubdump.so is not a DSO as such, it's
only used with LD_LIBRARY_PRELOAD.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for Fixes from updating igt packaging in
  2019-04-16 23:25 [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in Lyude
                   ` (8 preceding siblings ...)
  2019-04-17  0:16 ` [igt-dev] ✓ Fi.CI.BAT: success for Fixes from updating igt packaging in Patchwork
@ 2019-04-17  9:25 ` Patchwork
  9 siblings, 0 replies; 21+ messages in thread
From: Patchwork @ 2019-04-17  9:25 UTC (permalink / raw)
  To: Lyude; +Cc: igt-dev

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5942_full -> IGTPW_2878_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_parallel@bsd1-fds:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +9

  * igt@gem_exec_store@cachelines-bsd2:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +17

  * igt@gem_mocs_settings@mocs-rc6-ctx-dirty-render:
    - shard-iclb:         NOTRUN -> SKIP [fdo#110206]

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#108686]

  * igt@gen3_render_tiledx_blits:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109289]

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          PASS -> SKIP [fdo#109271]

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566] +2

  * igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-f:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_chamelium@hdmi-crc-fast:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284] +1

  * igt@kms_cursor_crc@cursor-512x512-offscreen:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109279]

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-glk:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          PASS -> FAIL [fdo#104873]

  * igt@kms_flip@2x-flip-vs-dpms-off-vs-modeset-interruptible:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274]

  * igt@kms_force_connector_basic@force-edid:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109285]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-plflip-blt:
    - shard-apl:          PASS -> FAIL [fdo#103167]
    - shard-glk:          PASS -> FAIL [fdo#103167]
    - shard-kbl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-move:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +11

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-blt:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103167]

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

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-shrfb-msflip-blt:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] +8

  * igt@kms_invalid_dotclock:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109310]

  * igt@kms_lease@atomic_implicit_crtc:
    - shard-iclb:         NOTRUN -> FAIL [fdo#110279]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-e:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +2

  * igt@kms_pipe_crc_basic@read-crc-pipe-f:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271] +1

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

  * igt@kms_plane_scaling@pipe-a-scaler-with-rotation:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278] +1

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

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          PASS -> FAIL [fdo#109016] +1

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

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@v3d_get_bo_offset@get-bad-handle:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109315] +1

  
#### Possible fixes ####

  * igt@i915_pm_rpm@i2c:
    - shard-iclb:         DMESG-WARN [fdo#109982] -> PASS

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
    - shard-hsw:          FAIL [fdo#105767] -> PASS

  * 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 +4

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS +2

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

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

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

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [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#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109310]: https://bugs.freedesktop.org/show_bug.cgi?id=109310
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109982]: https://bugs.freedesktop.org/show_bug.cgi?id=109982
  [fdo#110206]: https://bugs.freedesktop.org/show_bug.cgi?id=110206
  [fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 6)
------------------------------

  Missing    (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005 


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

    * IGT: IGT_4953 -> IGTPW_2878
    * Piglit: piglit_4509 -> None

  CI_DRM_5942: 1d19629d12fdb73dd65d1df91cf69b89fd76cae6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2878: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2878/
  IGT_4953: e03d0030391689cfd0fbca293d44d83dd7d9e356 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 1/8] lib/tests: Fix test failures with meson 0.50.0
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 1/8] lib/tests: Fix test failures with meson 0.50.0 Lyude
@ 2019-04-17 11:35   ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2019-04-17 11:35 UTC (permalink / raw)
  To: Lyude; +Cc: igt-dev

On Tue, Apr 16, 2019 at 07:25:41PM -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(). 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',

The usual way I've solved this is with fork, and an igt_internal_assert
that positively checks for the right waitpid status. The fail tests are
kinda a cheap hack, and if they don't work it's imo better to do proper
testing instead of wrapping more glue around it.

There's tons of examples if you look at git log lib/tests/
-Daniel

>  ]
>  
> @@ -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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 2/8] meson: Don't redefine gettid if the C library provides it
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 2/8] meson: Don't redefine gettid if the C library provides it Lyude
@ 2019-04-17 11:36   ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2019-04-17 11:36 UTC (permalink / raw)
  To: Lyude; +Cc: igt-dev

On Tue, Apr 16, 2019 at 07:25:42PM -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
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>

Makes sense, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 3/8] lib: Stop using assert() for runtime checks
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 3/8] lib: Stop using assert() for runtime checks Lyude
@ 2019-04-17 11:40   ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2019-04-17 11:40 UTC (permalink / raw)
  To: Lyude; +Cc: igt-dev

On Tue, Apr 16, 2019 at 07:25:43PM -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.
> 
> 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(); \

I think this should _exit(), to avoid hilarity with our exit handlers
(which are abort handlers too).

> +	} \
> +} while (0)

Hm, kinda awkward if we put this into a header tests have to use. I'd like
to have an easy check that nothing in tests/ uses plain assert(). Maybe
add a new igt_internal.h. Up to you I'd say.

Aside from these nits lgtm, with them addressed somehow:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +
>  /* 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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 4/8] meson: Add .so versioning
  2019-04-17  9:16   ` Petri Latvala
@ 2019-04-17 11:43     ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2019-04-17 11:43 UTC (permalink / raw)
  To: Lyude, igt-dev

On Wed, Apr 17, 2019 at 12:16:59PM +0300, Petri Latvala wrote:
> On Tue, Apr 16, 2019 at 07:25:44PM -0400, Lyude wrote:
> > From: Lyude Paul <lyude@redhat.com>
> > 
> > While I'm pretty confident that no one cares to use libigt.so or
> > lib_aubdump.so anywhere outside of igt, many distributions including
> > Fedora and Debian strongly suggest that packages have some sort of so
> > versioning, even if it's just '0'. So, let's fulfill that minimum
> > requirement to make this easier to package.
> > 
> > Signed-off-by: Lyude Paul <lyude@redhat.com>
> > ---
> >  lib/meson.build   | 3 ++-
> >  tools/meson.build | 3 ++-
> >  2 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/lib/meson.build b/lib/meson.build
> > index a8462933..2aad9d9e 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -132,7 +132,8 @@ lib_igt_build = shared_library('igt',
> >      ['dummy.c'],
> >      link_whole: lib_intermediates,
> >      dependencies: lib_deps,
> > -    install : true
> > +    install : true,
> > +    soversion : '0',
> >  )
> 
> This part:
> Acked-by: Petri Latvala <petri.latvala@intel.com>

So we use .so becuase with meson it's a lot faster for relinking (it does
nothing as long as the symbol list hasn't changed). Which is really
awesome if you hack around on library internals. Plus the .so saves a bit
of space.

I think the correct fix here would be to build the igt library as .la
archive for release builds (or if distros wish to do that). Meson has some
knobs to handle that semi-automatically, but I've never looked into them
since I dont care. I think that would be the cleaner solution than lying
about the so number.

Slightly less lying would be 0.$igt_release, since we do break the so api
every release.
-Daniel
> 
> 
> 
> >  
> >  lib_igt = declare_dependency(link_with : lib_igt_build,
> > diff --git a/tools/meson.build b/tools/meson.build
> > index 5d00f2e3..dfaed82a 100644
> > --- a/tools/meson.build
> > +++ b/tools/meson.build
> > @@ -93,7 +93,8 @@ install_subdir('registers', install_dir : datadir,
> >  shared_library('intel_aubdump', 'aubdump.c',
> >  	       dependencies : [ lib_igt_chipset, dlsym ],
> >  	       name_prefix : '',
> > -	       install : true)
> > +	       install : true,
> > +	       soversion : '0')
> 
> This part: Is this needed? intel_aubdump.so is not a DSO as such, it's
> only used with LD_LIBRARY_PRELOAD.
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 5/8] lib/igt_core: Just use igt_can_fail() in __igt_run_subtest()
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 5/8] lib/igt_core: Just use igt_can_fail() in __igt_run_subtest() Lyude
@ 2019-04-17 11:44   ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2019-04-17 11:44 UTC (permalink / raw)
  To: Lyude; +Cc: igt-dev

On Tue, Apr 16, 2019 at 07:25:45PM -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());

Nice!

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>  
>  	/* 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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 6/8] Use pkgconfig() macros with dnf in Fedora Dockerfile
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 6/8] Use pkgconfig() macros with dnf in Fedora Dockerfile Lyude
@ 2019-04-17 11:45   ` Daniel Vetter
  2019-04-17 19:15     ` Lyude Paul
  0 siblings, 1 reply; 21+ messages in thread
From: Daniel Vetter @ 2019-04-17 11:45 UTC (permalink / raw)
  To: Lyude; +Cc: igt-dev

On Tue, Apr 16, 2019 at 07:25:46PM -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>

I believe the expert on this, blindly.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

btw did you push this to your personal gitlab clone to make sure it's all
still working?
-Daniel

> ---
>  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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 7/8] Update Dockerfile.fedora to Fedora 29
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 7/8] Update Dockerfile.fedora to Fedora 29 Lyude
@ 2019-04-17 11:45   ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2019-04-17 11:45 UTC (permalink / raw)
  To: Lyude; +Cc: igt-dev

On Tue, Apr 16, 2019 at 07:25:47PM -0400, Lyude wrote:
> From: Lyude Paul <lyude@redhat.com>
> 
> Latest stable release of Fedora
> 
> Signed-off-by: Lyude Paul <lyude@redhat.com>

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 8/8] Gitlab CI: Do builds with -Db_ndebug=true
  2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 8/8] Gitlab CI: Do builds with -Db_ndebug=true Lyude
@ 2019-04-17 11:47   ` Daniel Vetter
  0 siblings, 0 replies; 21+ messages in thread
From: Daniel Vetter @ 2019-04-17 11:47 UTC (permalink / raw)
  To: Lyude; +Cc: igt-dev

On Tue, Apr 16, 2019 at 07:25:48PM -0400, Lyude wrote:
> 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.

Hm, that doesn't catch if someone adds a bogus assert. 2nd build target
for a pure release build too much overkill? meson is quick generally, and
if distros start using this I think it'd make sense.
-Daniel

> 
> 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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [igt-dev] [PATCH i-g-t v2 6/8] Use pkgconfig() macros with dnf in Fedora Dockerfile
  2019-04-17 11:45   ` Daniel Vetter
@ 2019-04-17 19:15     ` Lyude Paul
  0 siblings, 0 replies; 21+ messages in thread
From: Lyude Paul @ 2019-04-17 19:15 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: igt-dev

On Wed, 2019-04-17 at 13:45 +0200, Daniel Vetter wrote:
> On Tue, Apr 16, 2019 at 07:25:46PM -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>
> 
> I believe the expert on this, blindly.
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> btw did you push this to your personal gitlab clone to make sure it's all
> still working?

yep-everything seemed to work fine on mine

> -Daniel
> 
> > ---
> >  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

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2019-04-17 19:15 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-16 23:25 [igt-dev] [PATCH i-g-t v2 0/8] Fixes from updating igt packaging in Lyude
2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 1/8] lib/tests: Fix test failures with meson 0.50.0 Lyude
2019-04-17 11:35   ` Daniel Vetter
2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 2/8] meson: Don't redefine gettid if the C library provides it Lyude
2019-04-17 11:36   ` Daniel Vetter
2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 3/8] lib: Stop using assert() for runtime checks Lyude
2019-04-17 11:40   ` Daniel Vetter
2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 4/8] meson: Add .so versioning Lyude
2019-04-17  9:16   ` Petri Latvala
2019-04-17 11:43     ` Daniel Vetter
2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 5/8] lib/igt_core: Just use igt_can_fail() in __igt_run_subtest() Lyude
2019-04-17 11:44   ` Daniel Vetter
2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 6/8] Use pkgconfig() macros with dnf in Fedora Dockerfile Lyude
2019-04-17 11:45   ` Daniel Vetter
2019-04-17 19:15     ` Lyude Paul
2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 7/8] Update Dockerfile.fedora to Fedora 29 Lyude
2019-04-17 11:45   ` Daniel Vetter
2019-04-16 23:25 ` [igt-dev] [PATCH i-g-t v2 8/8] Gitlab CI: Do builds with -Db_ndebug=true Lyude
2019-04-17 11:47   ` Daniel Vetter
2019-04-17  0:16 ` [igt-dev] ✓ Fi.CI.BAT: success for Fixes from updating igt packaging in Patchwork
2019-04-17  9:25 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox