Fix: output non-LTTng CTF trace with same relative path as input
authorSimon Marchi <simon.marchi@efficios.com>
Mon, 9 Sep 2019 22:07:08 +0000 (18:07 -0400)
committerFrancis Deslauriers <francis.deslauriers@efficios.com>
Thu, 19 Sep 2019 13:53:57 +0000 (09:53 -0400)
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 <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/2026
Reviewed-by: Francis Deslauriers <francis.deslauriers@efficios.com>
Tested-by: jenkins <jenkins@lttng.org>
src/cli/babeltrace2-cfg-cli-args.c
src/plugins/ctf/fs-src/fs.c
src/plugins/ctf/fs-src/fs.h
src/plugins/ctf/fs-src/query.c
tests/Makefile.am
tests/cli/test_output_path_ctf_non_lttng_trace [new file with mode: 0755]

index e7ee709a91f2b1504f7e0133870aff6905843121..0f33f86a824340fed12e8d9857003afabae967aa 100644 (file)
@@ -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;
index 9fa1792e565d7a19d2e995f46240853d03f28c9b..454b4aeea88eb13613b4afdd3e3ffbe9d54f050b 100644 (file)
@@ -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;
        }
 
index e40f4ce78adfd6c138a2a4fe09b3d815163657fa..10a17ba8e08a6552135c719f1ea26ba9e1a47929 100644 (file)
@@ -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);
 
index 82e9c90eeb5bf8cdac53e0ab43510ad6e21a2a7a..048bc48acd54bd8c2538bc24c5e490cbec4108fa 100644 (file)
@@ -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;
        }
 
index 17f6a952a1d544195a677335cfe291933a4f190a..24c89fbeb9388c6bea9d48e257d1e389f1053d65 100644 (file)
@@ -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 (executable)
index 0000000..a18644c
--- /dev/null
@@ -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}"
This page took 0.032237 seconds and 4 git commands to generate.