From 005d49d64c989f20ba4fb40833064ac2d17514ef Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 9 Sep 2019 18:07:08 -0400 Subject: [PATCH] Fix: output non-LTTng CTF trace with same relative path as input Commit 73760435c5a4 ("cli: automatically detect sources for leftover arguments") changed the behavior of the CLI when converting non-LTTng CTF traces [1]. The intended behavior, prior to this commit, is to replicate in the output directory the directory structure from the input directory. So let's say we have CTF traces at `a/b/c1` and `a/b/c2` (metadata files inside those directories) and we do: babeltrace2 a -c sink.ctf.fs -p 'path="out"' We expect to have traces output in `out/b/c1` and `out/b/c2`. Before the introduction of automatic source discovery, the src.ctf.fs component would receive `a` as the input path, recurse to find the traces, and set the name of each trace to be the path of the trace relative to `a` (`b/c1` and `b/c2` in the example above). The sink.ctf.fs component would then use add the trace name to the user-provided output path to build the output path of each trace. Now, when used with automatic source discovery (and especially since they have lost the ability to recurse), src.ctf.fs components receive the direct path to each trace (one trace per component). So, in the example above, that is `a/b/c1` and `a/b/c2`. Since they don't have access to the top-level directory the user specified, they can no longer compute the trace name that the sink.ctf.fs component so desperately needs. Currently (since 48881202c2ef ("ctf: make src.ctf.fs not recurse")), the normalized path is passed as the trace name. So we end up with the absolute path of the input trace under the output directory. To restore the original behavior, this patch makes the CLI (which has access to the required information) compute the trace name the same way src.ctf.fs used to do, and pass it as the `trace-name` parameter to src.ctf.fs components. It only does so when the trace has a single input directory. [1] This behavior only affects non-LTTng CTF traces, because sink.ctf.fs first tries to build the output path using certain environment fields (which LTTng traces provide), and only falls back on the trace name if the required fields are not present. So for LTTng traces (or other traces that have the required env fields), the trace name does not matter. Change-Id: Iaba02fbae856ac5ddc11724613457bcc0b5b4019 Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/2026 Reviewed-by: Francis Deslauriers Tested-by: jenkins --- src/cli/babeltrace2-cfg-cli-args.c | 42 +++++++++++- src/plugins/ctf/fs-src/fs.c | 29 +++++++-- src/plugins/ctf/fs-src/fs.h | 7 +- src/plugins/ctf/fs-src/query.c | 9 +-- tests/Makefile.am | 6 +- .../cli/test_output_path_ctf_non_lttng_trace | 65 +++++++++++++++++++ 6 files changed, 144 insertions(+), 14 deletions(-) create mode 100755 tests/cli/test_output_path_ctf_non_lttng_trace diff --git a/src/cli/babeltrace2-cfg-cli-args.c b/src/cli/babeltrace2-cfg-cli-args.c index e7ee709a..0f33f86a 100644 --- a/src/cli/babeltrace2-cfg-cli-args.c +++ b/src/cli/babeltrace2-cfg-cli-args.c @@ -2981,6 +2981,9 @@ end: * Create `struct implicit_component_args` structures for each of the * source components we identified. Add them to `component_args`. * + * `non_opts` is an array of the non-option arguments passed on the command + * line. + * * `non_opt_params` is an array where each element is an array of * strings containing all the arguments to `--params` that apply to the * non-option argument at the same index. For example, if, for a @@ -2996,6 +2999,7 @@ end: static int create_implicit_component_args_from_auto_discovered_sources( const struct auto_source_discovery *auto_disc, + const bt_value *non_opts, const bt_value *non_opt_params, const bt_value *non_opt_loglevels, GPtrArray *component_args) @@ -3086,6 +3090,42 @@ int create_implicit_component_args_from_auto_discovered_sources( } } + /* + * If single input and a src.ctf.fs component, provide the + * relative path from the path passed on the command line to the + * found trace. + */ + if (bt_value_array_get_length(res->inputs) == 1 && + strcmp(res->plugin_name, "ctf") == 0 && + strcmp(res->source_cc_name, "fs") == 0) { + const bt_value *orig_idx_value = + bt_value_array_borrow_element_by_index( + res->original_input_indices, 0); + uint64_t orig_idx = bt_value_integer_unsigned_get(orig_idx_value); + const bt_value *non_opt_value = + bt_value_array_borrow_element_by_index_const( + non_opts, orig_idx); + const char *non_opt = bt_value_string_get(non_opt_value); + const bt_value *input_value = + bt_value_array_borrow_element_by_index_const( + res->inputs, 0); + const char *input = bt_value_string_get(input_value); + + BT_ASSERT(orig_indices_count == 1); + BT_ASSERT(g_str_has_prefix(input, non_opt)); + + input += strlen(non_opt); + + while (G_IS_DIR_SEPARATOR(*input)) { + input++; + } + + if (strlen(input) > 0) { + append_string_parameter_to_args(comp->extra_params, + "trace-name", input); + } + } + status = append_parameter_to_args(comp->extra_params, "inputs", res->inputs); if (status != 0) { goto error; @@ -4064,7 +4104,7 @@ struct bt_config *bt_config_convert_from_args(int argc, const char *argv[], } status = create_implicit_component_args_from_auto_discovered_sources( - &auto_disc, non_opt_params, non_opt_loglevels, + &auto_disc, non_opts, non_opt_params, non_opt_loglevels, discovered_source_args); if (status != 0) { goto error; diff --git a/src/plugins/ctf/fs-src/fs.c b/src/plugins/ctf/fs-src/fs.c index 9fa1792e..454b4aee 100644 --- a/src/plugins/ctf/fs-src/fs.c +++ b/src/plugins/ctf/fs-src/fs.c @@ -1097,6 +1097,7 @@ static int ctf_fs_component_create_ctf_fs_trace_one_path( struct ctf_fs_component *ctf_fs, const char *path_param, + const char *trace_name, GPtrArray *traces, bt_self_component *self_comp, bt_self_component_class *self_comp_class) @@ -1133,7 +1134,7 @@ int ctf_fs_component_create_ctf_fs_trace_one_path( } ctf_fs_trace = ctf_fs_trace_create(self_comp, norm_path->str, - norm_path->str, &ctf_fs->metadata_config, log_level); + trace_name, &ctf_fs->metadata_config, log_level); if (!ctf_fs_trace) { BT_COMP_LOGE_APPEND_CAUSE(self_comp, "Cannot create trace for `%s`.", norm_path->str); @@ -1971,6 +1972,7 @@ end: int ctf_fs_component_create_ctf_fs_trace( struct ctf_fs_component *ctf_fs, const bt_value *paths_value, + const bt_value *trace_name_value, bt_self_component *self_comp, bt_self_component_class *self_comp_class) { @@ -1978,6 +1980,7 @@ int ctf_fs_component_create_ctf_fs_trace( uint64_t i; bt_logging_level log_level = ctf_fs->log_level; GPtrArray *traces; + const char *trace_name; BT_ASSERT(bt_value_get_type(paths_value) == BT_VALUE_TYPE_ARRAY); BT_ASSERT(!bt_value_array_is_empty(paths_value)); @@ -1989,13 +1992,15 @@ int ctf_fs_component_create_ctf_fs_trace( goto error; } + trace_name = trace_name_value ? bt_value_string_get(trace_name_value) : NULL; + /* Start by creating a separate ctf_fs_trace object for each path. */ for (i = 0; i < bt_value_array_get_length(paths_value); i++) { const bt_value *path_value = bt_value_array_borrow_element_by_index_const(paths_value, i); const char *input = bt_value_string_get(path_value); ret = ctf_fs_component_create_ctf_fs_trace_one_path(ctf_fs, - input, traces, self_comp, self_comp_class); + input, trace_name, traces, self_comp, self_comp_class); if (ret) { goto end; } @@ -2229,7 +2234,9 @@ end: } bool read_src_fs_parameters(const bt_value *params, - const bt_value **inputs, struct ctf_fs_component *ctf_fs, + const bt_value **inputs, + const bt_value **trace_name, + struct ctf_fs_component *ctf_fs, bt_self_component *self_comp, bt_self_component_class *self_comp_class) { bool ret; @@ -2268,6 +2275,15 @@ bool read_src_fs_parameters(const bt_value *params, bt_value_integer_signed_get(value); } + /* trace-name parameter */ + *trace_name = bt_value_map_borrow_entry_value_const(params, "trace-name"); + if (*trace_name) { + if (!bt_value_is_string(*trace_name)) { + BT_COMP_OR_COMP_CLASS_LOGE_APPEND_CAUSE(self_comp, self_comp_class, + "trace-name must be a string"); + goto error; + } + } ret = true; goto end; @@ -2287,6 +2303,7 @@ struct ctf_fs_component *ctf_fs_create( { struct ctf_fs_component *ctf_fs = NULL; const bt_value *inputs_value; + const bt_value *trace_name_value; bt_self_component *self_comp = bt_self_component_source_as_self_component(self_comp_src); @@ -2296,15 +2313,15 @@ struct ctf_fs_component *ctf_fs_create( goto error; } - if (!read_src_fs_parameters(params, &inputs_value, ctf_fs, - self_comp, self_comp_class)) { + if (!read_src_fs_parameters(params, &inputs_value, &trace_name_value, + ctf_fs, self_comp, self_comp_class)) { goto error; } bt_self_component_set_data(self_comp, ctf_fs); if (ctf_fs_component_create_ctf_fs_trace(ctf_fs, inputs_value, - self_comp, self_comp_class)) { + trace_name_value, self_comp, self_comp_class)) { goto error; } diff --git a/src/plugins/ctf/fs-src/fs.h b/src/plugins/ctf/fs-src/fs.h index e40f4ce7..10a17ba8 100644 --- a/src/plugins/ctf/fs-src/fs.h +++ b/src/plugins/ctf/fs-src/fs.h @@ -252,6 +252,7 @@ BT_HIDDEN int ctf_fs_component_create_ctf_fs_trace( struct ctf_fs_component *ctf_fs, const bt_value *paths_value, + const bt_value *trace_name_value, bt_self_component *self_comp, bt_self_component_class *self_comp_class); @@ -266,6 +267,8 @@ void ctf_fs_destroy(struct ctf_fs_component *ctf_fs); * - The mandatory `paths` parameter is returned in `*paths`. * - The optional `clock-class-offset-s` and `clock-class-offset-ns`, if * present, are recorded in the `ctf_fs` structure. + * - The optional `trace-name` parameter is returned in `*trace_name` if + * present, else `*trace_name` is set to NULL. * * `self_comp` and `self_comp_class` are used for logging, only one of them * should be set. @@ -275,7 +278,9 @@ void ctf_fs_destroy(struct ctf_fs_component *ctf_fs); BT_HIDDEN bool read_src_fs_parameters(const bt_value *params, - const bt_value **paths, struct ctf_fs_component *ctf_fs, + const bt_value **paths, + const bt_value **trace_name, + struct ctf_fs_component *ctf_fs, bt_self_component *self_comp, bt_self_component_class *self_comp_class); diff --git a/src/plugins/ctf/fs-src/query.c b/src/plugins/ctf/fs-src/query.c index 82e9c90e..048bc48a 100644 --- a/src/plugins/ctf/fs-src/query.c +++ b/src/plugins/ctf/fs-src/query.c @@ -339,6 +339,7 @@ bt_component_class_query_method_status trace_infos_query( self_comp_class_src); bt_value *result = NULL; const bt_value *inputs_value = NULL; + const bt_value *trace_name_value; int ret = 0; bt_value *trace_info = NULL; bt_value_array_append_element_status append_status; @@ -357,14 +358,14 @@ bt_component_class_query_method_status trace_infos_query( goto error; } - if (!read_src_fs_parameters(params, &inputs_value, ctf_fs, NULL, - self_comp_class)) { + if (!read_src_fs_parameters(params, &inputs_value, &trace_name_value, + ctf_fs, NULL, self_comp_class)) { status = BT_COMPONENT_CLASS_QUERY_METHOD_STATUS_ERROR; goto error; } - if (ctf_fs_component_create_ctf_fs_trace(ctf_fs, inputs_value, NULL, - self_comp_class)) { + if (ctf_fs_component_create_ctf_fs_trace(ctf_fs, inputs_value, + trace_name_value, NULL, self_comp_class)) { goto error; } diff --git a/tests/Makefile.am b/tests/Makefile.am index 17f6a952..24c89fbe 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -37,6 +37,7 @@ dist_check_SCRIPTS = \ cli/convert/test_convert_args \ cli/test_intersection \ cli/test_output_ctf_metadata \ + cli/test_output_path_ctf_non_lttng_trace \ cli/test_packet_seq_num \ cli/test_trace_copy \ cli/test_trace_read \ @@ -56,10 +57,11 @@ TESTS_ARGPAR = \ TESTS_CLI = \ cli/convert/test_convert_args \ - cli/test_trace_read \ - cli/test_packet_seq_num \ cli/test_intersection \ + cli/test_output_path_ctf_non_lttng_trace \ + cli/test_packet_seq_num \ cli/test_trace_copy \ + cli/test_trace_read \ cli/test_trimmer TESTS_LIB = \ diff --git a/tests/cli/test_output_path_ctf_non_lttng_trace b/tests/cli/test_output_path_ctf_non_lttng_trace new file mode 100755 index 00000000..a18644c5 --- /dev/null +++ b/tests/cli/test_output_path_ctf_non_lttng_trace @@ -0,0 +1,65 @@ +#!/bin/bash + +# Copyright (C) EfficiOS Inc. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; only version 2 +# of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + +# This test verifies that generic (non-LTTng) CTF traces are output with the +# expected directory structure. +# +# Traces found when invoking +# +# babeltrace2 in -c sink.ctf.fs -p 'path="out"' +# +# are expected to use the same directory structure relative to `out` as the +# original traces had relative to `in`. + +SH_TAP=1 + +if [ "x${BT_TESTS_SRCDIR:-}" != "x" ]; then + UTILSSH="$BT_TESTS_SRCDIR/utils/utils.sh" +else + UTILSSH="$(dirname "$0")/../utils/utils.sh" +fi + +# shellcheck source=../utils/utils.sh +source "$UTILSSH" + +plan_tests 3 + +temp_input_dir=$(mktemp -d) +temp_output_dir=$(mktemp -d) + +mkdir -p "${temp_input_dir}/a/b/c" +cp -a "${BT_CTF_TRACES_PATH}/intersection/3eventsintersect" "${temp_input_dir}/a/b/c" + +mkdir -p "${temp_input_dir}/a/b/c" +cp -a "${BT_CTF_TRACES_PATH}/intersection/3eventsintersectreverse" "${temp_input_dir}/a/b/c" + +mkdir -p "${temp_input_dir}/d/e/f" +cp -a "${BT_CTF_TRACES_PATH}/intersection/nointersect" "${temp_input_dir}/d/e/f" + +"${BT_TESTS_BT2_BIN}" "${temp_input_dir}" -c sink.ctf.fs -p "path=\"${temp_output_dir}\"" + +test -f "${temp_output_dir}/a/b/c/3eventsintersect/metadata" +ok "$?" "3eventsintersect output trace exists" + +test -f "${temp_output_dir}/a/b/c/3eventsintersectreverse/metadata" +ok "$?" "3eventsintersectreverse output trace exists" + +test -f "${temp_output_dir}/d/e/f/nointersect/metadata" +ok "$?" "nointersect output trace exists" + +rm -rf "${temp_input_dir}" "${temp_output_dir}" -- 2.34.1