From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9592AC531DF for ; Wed, 14 Aug 2024 20:48:50 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D123110E0A5; Wed, 14 Aug 2024 20:48:49 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="n4kVXARV"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4CC8510E02C for ; Wed, 14 Aug 2024 20:48:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1723668528; x=1755204528; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=VRkckIK/ZOITVNJ2YVqRkw6oewPA7T8ViVr3cz3Ds5Y=; b=n4kVXARV7jlHn44ZlBDHc9DROP/3juFmN4JVuFSyzDhjjsWCcaeyxeAx G6tj/wP5DDASDfdqfZ441uaN3cBsZ05lD1AVhfpcBBtned1ih3PTKKU6P RoiASdpXvgvoYxSOPiN7WXI+UtO0u/00pf4tZymcS2qYjotASoOPyTmkt iTHLyxeEB5AlW53zgVqnX3ifcsbZj6Fvxvgz3eCP9vBRsxwcMfIKtKSca zpl7EpP+0micG8bms9EegqDX7WP4vvCWzFI5GrNCTMZC+f4feNsVtq8J6 fReAfWFH94TQ//aQOJAb27rRh4XJdxqN9UribLGn67X+t9eYgfOpBZq/R A==; X-CSE-ConnectionGUID: skQ0TjhlRLC6QaE1WTmU6w== X-CSE-MsgGUID: envcC2IMSRuFL8bUyTjm1Q== X-IronPort-AV: E=McAfee;i="6700,10204,11164"; a="21723269" X-IronPort-AV: E=Sophos;i="6.10,147,1719903600"; d="scan'208";a="21723269" Received: from fmviesa008.fm.intel.com ([10.60.135.148]) by orvoesa112.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Aug 2024 13:48:47 -0700 X-CSE-ConnectionGUID: NjrgEEx/Q86yvxIrxfIMBA== X-CSE-MsgGUID: cYJRkWcPRtWC5+JbGazH4w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.10,147,1719903600"; d="scan'208";a="59108169" Received: from ldmartin-desk2.corp.intel.com (HELO gjsousa-mobl2.intel.com) ([10.125.110.119]) by fmviesa008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Aug 2024 13:48:45 -0700 From: Gustavo Sousa To: igt-dev@lists.freedesktop.org Cc: Lucas De Marchi , Gustavo Sousa Subject: [PATCH i-g-t v4 6/6] runner: Allow multiple --hook options Date: Wed, 14 Aug 2024 17:48:01 -0300 Message-ID: <20240814204822.95283-7-gustavo.sousa@intel.com> X-Mailer: git-send-email 2.46.0 In-Reply-To: <20240814204822.95283-1-gustavo.sousa@intel.com> References: <20240814204822.95283-1-gustavo.sousa@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: igt-dev@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development mailing list for IGT GPU Tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" Test binaries now allow passing multiple --hook options, it just makes sense that igt_runner follows suit, so let's do it. Note that this requires having another file in the results directory for storing the hook strings, as metadata.txt does not support multivalued items. Since we are using a different file to store hook strings, take this opportunity to also allow multiline hook strings (which was not possible with metadata.txt). Reviewed-by: Lucas De Marchi Signed-off-by: Gustavo Sousa --- runner/executor.c | 4 +- runner/runner_tests.c | 19 ++++- runner/settings.c | 164 +++++++++++++++++++++++++++++++++++++++--- runner/settings.h | 3 +- 4 files changed, 173 insertions(+), 17 deletions(-) diff --git a/runner/executor.c b/runner/executor.c index a23cd58c0781..ac73e1ddec07 100644 --- a/runner/executor.c +++ b/runner/executor.c @@ -1564,10 +1564,10 @@ execute_test_process(int outfd, int errfd, int socketfd, } } - if (settings->hook_str) { + for (size_t i = 0; i < igt_vec_length(&settings->hook_strs); i++) { arg = strdup("--hook"); igt_vec_push(&arg_vec, &arg); - arg = strdup(settings->hook_str); + arg = strdup(*((char **)igt_vec_elem(&settings->hook_strs, i))); igt_vec_push(&arg_vec, &arg); } diff --git a/runner/runner_tests.c b/runner/runner_tests.c index 7470cc24052f..b806d45adbd6 100644 --- a/runner/runner_tests.c +++ b/runner/runner_tests.c @@ -202,7 +202,14 @@ static void assert_settings_equal(struct settings *one, struct settings *two) igt_assert_eq(one->piglit_style_dmesg, two->piglit_style_dmesg); igt_assert_eq(one->dmesg_warn_level, two->dmesg_warn_level); igt_assert_eq(one->prune_mode, two->prune_mode); - igt_assert_eqstr(one->hook_str, two->hook_str); + + igt_assert_eq(igt_vec_length(&one->hook_strs), igt_vec_length(&two->hook_strs)); + for (size_t i = 0; i < igt_vec_length(&one->hook_strs); i++) { + char **hook_str_one = igt_vec_elem(&one->hook_strs, i); + char **hook_str_two = igt_vec_elem(&two->hook_strs, i); + + igt_assert_eqstr(*hook_str_one, *hook_str_two); + } } static void assert_job_list_equal(struct job_list *one, struct job_list *two) @@ -294,6 +301,7 @@ igt_main igt_assert_eq(settings->include_regexes.size, 0); igt_assert_eq(settings->exclude_regexes.size, 0); igt_assert(igt_list_empty(&settings->env_vars)); + igt_assert(!igt_vec_length(&settings->hook_strs)); igt_assert(!settings->sync); igt_assert_eq(settings->log_level, LOG_LEVEL_NORMAL); igt_assert(!settings->overwrite); @@ -303,7 +311,6 @@ igt_main igt_assert_eq(settings->overall_timeout, 0); igt_assert(!settings->use_watchdog); igt_assert_eq(settings->prune_mode, 0); - igt_assert(!settings->hook_str); igt_assert(strstr(settings->test_root, "test-root-dir") != NULL); igt_assert(strstr(settings->results_path, "path-to-results") != NULL); @@ -467,6 +474,7 @@ igt_main "--coverage-per-test", "--collect-script", "/usr/bin/true", "--hook", "echo hello", + "--hook", "echo world", "--prune-mode=keep-subtests", "test-root-dir", "path-to-results", @@ -506,6 +514,10 @@ igt_main igt_assert_eqstr(env_var->key, "ENVS_WITH_JUST_KEYS"); igt_assert_eqstr(env_var->value, "SHOULD_WORK"); + igt_assert_eq(igt_vec_length(&settings->hook_strs), 2); + igt_assert_eqstr(*((char **)igt_vec_elem(&settings->hook_strs, 0)), "echo hello"); + igt_assert_eqstr(*((char **)igt_vec_elem(&settings->hook_strs, 1)), "echo world"); + igt_assert(settings->sync); igt_assert_eq(settings->log_level, LOG_LEVEL_VERBOSE); igt_assert(settings->overwrite); @@ -514,7 +526,6 @@ igt_main igt_assert_eq(settings->per_test_timeout, 72); igt_assert_eq(settings->overall_timeout, 360); igt_assert(settings->use_watchdog); - igt_assert_eqstr(settings->hook_str, "echo hello"); igt_assert_eq(settings->prune_mode, PRUNE_KEEP_SUBTESTS); igt_assert(strstr(settings->test_root, "test-root-dir") != NULL); igt_assert(strstr(settings->results_path, "path-to-results") != NULL); @@ -966,6 +977,8 @@ igt_main "--piglit-style-dmesg", "--prune-mode=keep-all", "--hook", "echo hello", + "--hook", "echo hello\necho newline", + "--hook", "echo hello\necho newline\\still the second line", testdatadir, dirname, }; diff --git a/runner/settings.c b/runner/settings.c index 6fd742cc826d..94b3b9fe641f 100644 --- a/runner/settings.c +++ b/runner/settings.c @@ -1,4 +1,5 @@ #include "igt_hook.h" +#include "igt_vec.h" #include "settings.h" #include "version.h" @@ -84,6 +85,7 @@ static struct { static const char settings_filename[] = "metadata.txt"; static const char env_filename[] = "environment.txt"; +static const char hooks_filename[] = "hooks.txt"; static bool set_log_level(struct settings* settings, const char *level) { @@ -518,6 +520,13 @@ static void free_env_vars(struct igt_list_head *env_vars) { } } +static void free_hook_strs(struct igt_vec *hook_strs) +{ + for (size_t i = 0; i < igt_vec_length(hook_strs); i++) + free(*((char **)igt_vec_elem(hook_strs, i))); + igt_vec_fini(hook_strs); +} + static bool file_exists_at(int dirfd, const char *filename) { return faccessat(dirfd, filename, F_OK, 0) == 0; @@ -620,6 +629,7 @@ void init_settings(struct settings *settings) { memset(settings, 0, sizeof(*settings)); IGT_INIT_LIST_HEAD(&settings->env_vars); + igt_vec_init(&settings->hook_strs, sizeof(char *)); } void clear_settings(struct settings *settings) @@ -632,6 +642,7 @@ void clear_settings(struct settings *settings) free_regexes(&settings->include_regexes); free_regexes(&settings->exclude_regexes); free_env_vars(&settings->env_vars); + free_hook_strs(&settings->hook_strs); init_settings(settings); } @@ -641,6 +652,7 @@ bool parse_options(int argc, char **argv, { int c; char *env_test_root; + char *hook_str; static struct option long_options[] = { {"version", no_argument, NULL, OPT_VERSION}, @@ -751,15 +763,8 @@ bool parse_options(int argc, char **argv, settings->code_coverage_script = bin_path(optarg); break; case OPT_HOOK: - /* FIXME: In order to allow line breaks, we should - * change the format of settings serialization. Maybe - * use JSON instead of our own format? */ - if (strchr(optarg, '\n')) { - fprintf(stderr, "Newlines in --hook are currently unsupported.\n"); - goto error; - } - /* FIXME: Allow as many options as allowed by test binaries. */ - settings->hook_str = optarg; + hook_str = strdup(optarg); + igt_vec_push(&settings->hook_strs, &hook_str); break; case OPT_HELP_HOOK: igt_hook_print_help(stdout, "--hook"); @@ -993,6 +998,49 @@ static bool serialize_environment(struct settings *settings, int dirfd) return true; } +static bool serialize_hook_strs(struct settings *settings, int dirfd) +{ + FILE *f; + + if (file_exists_at(dirfd, hooks_filename) && !settings->overwrite) { + usage(stderr, "%s already exists, not overwriting", hooks_filename); + return false; + } + + if ((f = fopenat_create(dirfd, hooks_filename, settings->overwrite)) == NULL) + return false; + + for (size_t i = 0; i < igt_vec_length(&settings->hook_strs); i++) { + const char *s = *((char **)igt_vec_elem(&settings->hook_strs, i)); + size_t len; + + while (*s) { + len = strcspn(s, "\\\n"); + + if (len > 0) + fwrite(s, len, 1, f); + + s += len; + if (!*s) + break; + + fputc('\\', f); + fputc(*s, f); + s++; + } + fputc('\n', f); + fputc('\n', f); + } + + if (settings->sync) { + fflush(f); + fsync(fileno(f)); + } + + fclose(f); + return true; +} + bool serialize_settings(struct settings *settings) { #define SERIALIZE_LINE(f, s, name, format) fprintf(f, "%s : " format "\n", #name, s->name) @@ -1062,7 +1110,6 @@ bool serialize_settings(struct settings *settings) SERIALIZE_LINE(f, settings, enable_code_coverage, "%d"); SERIALIZE_LINE(f, settings, cov_results_per_test, "%d"); SERIALIZE_LINE(f, settings, code_coverage_script, "%s"); - SERIALIZE_LINE(f, settings, hook_str, "%s"); if (settings->sync) { fflush(f); @@ -1078,6 +1125,13 @@ bool serialize_settings(struct settings *settings) } } + if (igt_vec_length(&settings->hook_strs)) { + if (!serialize_hook_strs(settings, dirfd)) { + close(dirfd); + return false; + } + } + if (settings->sync) fsync(dirfd); @@ -1126,7 +1180,6 @@ bool read_settings_from_file(struct settings *settings, FILE *f) PARSE_LINE(settings, name, val, enable_code_coverage, numval); PARSE_LINE(settings, name, val, cov_results_per_test, numval); PARSE_LINE(settings, name, val, code_coverage_script, val ? strdup(val) : NULL); - PARSE_LINE(settings, name, val, hook_str, val ? strdup(val) : NULL); printf("Warning: Unknown field in settings file: %s = %s\n", name, val); @@ -1196,6 +1249,82 @@ static bool read_env_vars_from_file(struct igt_list_head *env_vars, FILE *f) return true; } +static bool read_hook_strs_from_file(struct igt_vec *hook_strs, FILE *f) +{ + char *line = NULL; + ssize_t line_length; + size_t line_size = 0; + char *buf; + size_t buf_len = 0; + size_t buf_capacity = 128; + + buf = malloc(buf_capacity); + + while ((line_length = getline(&line, &line_size, f) != -1)) { + char *s = line; + + if (buf_len == 0) { + while (isspace(*s)) { + line_length--; + s++; + } + + if (*s == '\0' || *s == '#') + continue; + } + + if (line_length + 1 > buf_capacity - buf_len) { + while (line_length + 1 > buf_capacity - buf_len) + buf_capacity *= 2; + + buf = realloc(buf, buf_capacity); + } + + while (true) { + if (*s == '\0' || *s == '\n') { + char *buf_copy; + + if (!buf_len) + break; + + /* Reached the end of a hook string. */ + buf[buf_len] = '\0'; + buf_copy = strdup(buf); + igt_vec_push(hook_strs, &buf_copy); + buf_len = 0; + break; + } + + if (*s == '\\') { + s++; + + if (*s == '\0') + /* Weird case of backslash being the + * last character of the file. */ + s--; + } + + buf[buf_len++] = *s++; + + if (*s == '\0' || *s == '\n') + break; + } + } + + if (buf_len) { + char *buf_copy; + + buf[buf_len] = '\0'; + buf_copy = strdup(buf); + igt_vec_push(hook_strs, &buf_copy); + } + + free(buf); + free(line); + + return true; +} + bool read_settings_from_dir(struct settings *settings, int dirfd) { FILE *f; @@ -1226,5 +1355,18 @@ bool read_settings_from_dir(struct settings *settings, int dirfd) fclose(f); } + /* hooks file may not exist if no --hook was passed */ + if (file_exists_at(dirfd, hooks_filename)) { + if ((f = fopenat_read(dirfd, hooks_filename)) == NULL) + return false; + + if (!read_hook_strs_from_file(&settings->hook_strs, f)) { + fclose(f); + return false; + } + + fclose(f); + } + return true; } diff --git a/runner/settings.h b/runner/settings.h index d3afb56de039..8335f0b8c813 100644 --- a/runner/settings.h +++ b/runner/settings.h @@ -8,6 +8,7 @@ #include #include "igt_list.h" +#include "igt_vec.h" enum { LOG_LEVEL_NORMAL = 0, @@ -55,6 +56,7 @@ struct settings { struct regex_list include_regexes; struct regex_list exclude_regexes; struct igt_list_head env_vars; + struct igt_vec hook_strs; bool sync; int log_level; bool overwrite; @@ -72,7 +74,6 @@ struct settings { char *code_coverage_script; bool enable_code_coverage; bool cov_results_per_test; - char *hook_str; }; /** -- 2.46.0