diff mbox

[i-g-t] lib: add a environment variable to control output

Message ID 1447089433-10749-1-git-send-email-thomas.wood@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thomas Wood Nov. 9, 2015, 5:17 p.m. UTC 
Disable output of terminal control characters and progress meters when
IGT_PLAIN_OUTPUT is set in the environment.

Cc: Derek Morton <derek.j.morton@intel.com>
Signed-off-by: Thomas Wood <thomas.wood@intel.com>
---
 lib/igt_aux.c  |  2 +-
 lib/igt_core.c | 23 ++++++++++++++---------
 lib/igt_core.h |  2 ++
 3 files changed, 17 insertions(+), 10 deletions(-)

Comments

Chris Wilson Nov. 9, 2015, 8:58 p.m. UTC | #1 
On Mon, Nov 09, 2015 at 05:17:13PM +0000, Thomas Wood wrote:
> Disable output of terminal control characters and progress meters when
> IGT_PLAIN_OUTPUT is set in the environment.
> 
> Cc: Derek Morton <derek.j.morton@intel.com>
> Signed-off-by: Thomas Wood <thomas.wood@intel.com>
> ---
>  lib/igt_aux.c  |  2 +-
>  lib/igt_core.c | 23 ++++++++++++++---------
>  lib/igt_core.h |  2 ++
>  3 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index f3c76ae..4d08d68 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -231,7 +231,7 @@ static void igt_interactive_info(const char *format, ...)
>  {
>  	va_list args;
>  
> -	if (!isatty(STDERR_FILENO))
> +	if (!isatty(STDERR_FILENO) || __igt_plain_output)

Hmm, would it not be a bug if we reach this point and we haven't
initialised __igt_plain_output from common_init()? 
-Chris
Thomas Wood Nov. 10, 2015, 10:43 a.m. UTC | #2 
On 9 November 2015 at 20:58, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, Nov 09, 2015 at 05:17:13PM +0000, Thomas Wood wrote:
>> Disable output of terminal control characters and progress meters when
>> IGT_PLAIN_OUTPUT is set in the environment.
>>
>> Cc: Derek Morton <derek.j.morton@intel.com>
>> Signed-off-by: Thomas Wood <thomas.wood@intel.com>
>> ---
>>  lib/igt_aux.c  |  2 +-
>>  lib/igt_core.c | 23 ++++++++++++++---------
>>  lib/igt_core.h |  2 ++
>>  3 files changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
>> index f3c76ae..4d08d68 100644
>> --- a/lib/igt_aux.c
>> +++ b/lib/igt_aux.c
>> @@ -231,7 +231,7 @@ static void igt_interactive_info(const char *format, ...)
>>  {
>>       va_list args;
>>
>> -     if (!isatty(STDERR_FILENO))
>> +     if (!isatty(STDERR_FILENO) || __igt_plain_output)
>
> Hmm, would it not be a bug if we reach this point and we haven't
> initialised __igt_plain_output from common_init()?

Yes, except that common_init is called from within the igt_main or
_init functions, so calling igt_interactive_info before one of these
functions would be incorrect anyway.


> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Derek Morton Nov. 11, 2015, 10:09 a.m. UTC | #3 
Hi Thomas,
I have ran with your patches on android. The IGT_PLAIN_OUTPUT environment variable works fine for me. My only comment would be that it should be documented somewhere.

I will remove the formatting change from my gem_exec_nop patch as with this I don't think it is needed.

//Derek 

-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Thomas Wood

Sent: Tuesday, November 10, 2015 10:43 AM
To: Chris Wilson; Wood, Thomas; Intel Graphics Development
Subject: Re: [Intel-gfx] [PATCH i-g-t] lib: add a environment variable to control output

On 9 November 2015 at 20:58, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Mon, Nov 09, 2015 at 05:17:13PM +0000, Thomas Wood wrote:

>> Disable output of terminal control characters and progress meters 

>> when IGT_PLAIN_OUTPUT is set in the environment.

>>

>> Cc: Derek Morton <derek.j.morton@intel.com>

>> Signed-off-by: Thomas Wood <thomas.wood@intel.com>

>> ---

>>  lib/igt_aux.c  |  2 +-

>>  lib/igt_core.c | 23 ++++++++++++++---------  lib/igt_core.h |  2 ++

>>  3 files changed, 17 insertions(+), 10 deletions(-)

>>

>> diff --git a/lib/igt_aux.c b/lib/igt_aux.c index f3c76ae..4d08d68 

>> 100644

>> --- a/lib/igt_aux.c

>> +++ b/lib/igt_aux.c

>> @@ -231,7 +231,7 @@ static void igt_interactive_info(const char 

>> *format, ...)  {

>>       va_list args;

>>

>> -     if (!isatty(STDERR_FILENO))

>> +     if (!isatty(STDERR_FILENO) || __igt_plain_output)

>

> Hmm, would it not be a bug if we reach this point and we haven't 

> initialised __igt_plain_output from common_init()?


Yes, except that common_init is called from within the igt_main or _init functions, so calling igt_interactive_info before one of these functions would be incorrect anyway.


> -Chris

>

> --

> Chris Wilson, Intel Open Source Technology Centre 

> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org

> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index f3c76ae..4d08d68 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -231,7 +231,7 @@  static void igt_interactive_info(const char *format, ...)
 {
 	va_list args;
 
-	if (!isatty(STDERR_FILENO))
+	if (!isatty(STDERR_FILENO) || __igt_plain_output)
 		return;
 
 	if (igt_log_level > IGT_LOG_INFO)
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 7123455..ea9a68b 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -227,6 +227,8 @@  static enum {
 	CONT = 0, SKIP, FAIL
 } skip_subtests_henceforth = CONT;
 
+bool __igt_plain_output = false;
+
 /* fork support state */
 pid_t *test_children;
 int num_test_children;
@@ -523,11 +525,15 @@  static int common_init(int *argc, char **argv,
 	int extra_opt_count;
 	int all_opt_count;
 	int ret = 0;
-	char *env = getenv("IGT_LOG_LEVEL");
+	const char *env;
+
+	if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT"))
+		__igt_plain_output = true;
 
-	if (isatty(STDOUT_FILENO))
+	if (!__igt_plain_output)
 		setlocale(LC_ALL, "");
 
+	env = getenv("IGT_LOG_LEVEL");
 	if (env) {
 		if (strcmp(env, "debug") == 0)
 			igt_log_level = IGT_LOG_DEBUG;
@@ -779,12 +785,10 @@  bool __igt_run_subtest(const char *subtest_name)
 	}
 
 	if (skip_subtests_henceforth) {
-		bool istty = isatty(STDOUT_FILENO);
-
 		printf("%sSubtest %s: %s%s\n",
-		       (istty) ? "\x1b[1m" : "", subtest_name,
+		       (!__igt_plain_output) ? "\x1b[1m" : "", subtest_name,
 		       skip_subtests_henceforth == SKIP ?
-		       "SKIP" : "FAIL", (istty) ? "\x1b[0m" : "");
+		       "SKIP" : "FAIL", (!__igt_plain_output) ? "\x1b[0m" : "");
 		return false;
 	}
 
@@ -828,14 +832,15 @@  static void exit_subtest(const char *result)
 {
 	struct timespec now;
 	double elapsed;
-	bool istty = isatty(STDOUT_FILENO);
 
 	gettime(&now);
 	elapsed = now.tv_sec - subtest_time.tv_sec;
 	elapsed += (now.tv_nsec - subtest_time.tv_nsec) * 1e-9;
 
-	printf("%sSubtest %s: %s (%.3fs)%s\n", (istty) ? "\x1b[1m" : "",
-	       in_subtest, result, elapsed, (istty) ? "\x1b[0m" : "");
+	printf("%sSubtest %s: %s (%.3fs)%s\n",
+	       (!__igt_plain_output) ? "\x1b[1m" : "",
+	       in_subtest, result, elapsed,
+	       (!__igt_plain_output) ? "\x1b[0m" : "");
 	fflush(stdout);
 
 	in_subtest = NULL;
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 5ae0965..a244fc3 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -45,6 +45,8 @@ 
 
 
 extern const char* __igt_test_description __attribute__((weak));
+extern bool __igt_plain_output;
+
 
 /**
  * IGT_TEST_DESCRIPTION: