It is currently not possible to apply --params after a leftover.
This patch proposes a way to allow it and to deal with the ambiguity
that it poses.
In the simple case, we have:
babeltrace2 my-traces --params=foo=2
All source components auto-discovered from the `my-traces` leftover will
receive the `foo=2` parameter.
If we have more than one leftover, but _no_ cross-leftover grouping,
then it is also intuitive:
babeltrace2 my-traces-1 --params=foo=2 my-traces-2 --params=bar=3
... all source components discovered from `my-traces-1` will receive
`foo=2` and all source components discovered from `my-traces-2` will
receive `bar=3`.
It becomes less obvious when components are given inputs coming from
multiple leftovers (because of the auto-discovery grouping feature):
which parameters do they receive? For example, if the following line:
babeltrace2 my-traces-1 --params=foo=2,bar=3 my-traces-2 --params=foo=4
leads to these components getting instantiated, with these inputs:
* Source component X with inputs `my-traces-1/x` and `my-traces-2/x`.
* Source component Y with input `my-traces-1/y`
In this case, each component receives the parameters of all leftovers
that contributed to its inputs, in the same order as they are provided
on the command line. The resulting `run` command line for the example
above could therefore look like:
... --component x:src.my.comp --params=foo=2,bar=3 --params=foo=4 \
--component y:src.my.comp --params=foo=2,bar=3
resulting in these parameters being passed to the components:
* Source component X receives parameters `foo=4,bar=3`
* Source component Y receives parameters `foo=2,bar=3`
Implementation details
----------------------
The auto discovery mechanism now returns, for each result, which
input from the passed `inputs` array contributed to that result.
This allows us, for the component that we create from a given result, to
get the parameters from the leftovers that have contributed to it.
Change-Id: Ic048e4e137c2e1f93b6da13a62629343500cb75a
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/1809
Tested-by: jenkins <jenkins@lttng.org>
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
/*
* Create `struct implicit_component_args` structures for each of the source
* components we identified. Add them to `component_args`.
+ *
+ * `leftover_params` is an array where each element is an array of strings
+ * containing all the arguments to `--params` that apply to the leftover at the
+ * same index. For example, if, for a leftover, the following `--params`
+ * options applied:
+ *
+ * --params=a=2 --params=b=3,c=4
+ *
+ * its entry in `leftover_params` would contain
+ *
+ * ["a=2", "b=3,c=4"]
*/
static
-void create_implicit_component_args_from_auto_discovered_sources(
- const struct auto_source_discovery *auto_disc, GPtrArray *component_args)
+int create_implicit_component_args_from_auto_discovered_sources(
+ const struct auto_source_discovery *auto_disc,
+ const bt_value *leftover_params,
+ GPtrArray *component_args)
{
gchar *cc_name = NULL;
struct implicit_component_args *comp = NULL;
for (i = 0; i < len; i++) {
struct auto_source_discovery_result *res =
g_ptr_array_index(auto_disc->results, i);
+ uint64_t orig_indices_i, orig_indices_count;
g_free(cc_name);
cc_name = g_strdup_printf("source.%s.%s", res->plugin_name, res->source_cc_name);
if (!cc_name) {
BT_CLI_LOGE_APPEND_CAUSE_OOM();
- goto end;
+ goto error;
}
comp = create_implicit_component_args(cc_name);
if (!comp) {
- goto end;
+ goto error;
+ }
+
+ /*
+ * Append parameters of all the leftovers that contributed to
+ * this component instance coming into existence.
+ */
+ orig_indices_count = bt_value_array_get_size(res->original_input_indices);
+ for (orig_indices_i = 0; orig_indices_i < orig_indices_count; orig_indices_i++) {
+ const bt_value *orig_idx_value =
+ bt_value_array_borrow_element_by_index(
+ res->original_input_indices, orig_indices_i);
+ uint64_t orig_idx = bt_value_integer_unsigned_get(orig_idx_value);
+ const bt_value *params_array =
+ bt_value_array_borrow_element_by_index_const(
+ leftover_params, orig_idx);
+ uint64_t params_i, params_count;
+
+ params_count = bt_value_array_get_size(params_array);
+ for (params_i = 0; params_i < params_count; params_i++) {
+ const bt_value *params_value =
+ bt_value_array_borrow_element_by_index_const(
+ params_array, params_i);
+ const char *params = bt_value_string_get(params_value);
+ bt_value_array_append_element_status append_status;
+
+ append_status = bt_value_array_append_string_element(
+ comp->extra_params, "--params");
+ if (append_status != BT_VALUE_ARRAY_APPEND_ELEMENT_STATUS_OK) {
+ BT_CLI_LOGE_APPEND_CAUSE("Failed to append array element.");
+ goto error;
+ }
+
+ append_status = bt_value_array_append_string_element(
+ comp->extra_params, params);
+ if (append_status != BT_VALUE_ARRAY_APPEND_ELEMENT_STATUS_OK) {
+ BT_CLI_LOGE_APPEND_CAUSE("Failed to append array element.");
+ goto error;
+ }
+ }
}
status = append_parameter_to_args(comp->extra_params, "inputs", res->inputs);
if (status != 0) {
- goto end;
+ goto error;
}
g_ptr_array_add(component_args, comp);
comp = NULL;
}
+ status = 0;
+ goto end;
+
+error:
+ status = -1;
+
end:
g_free(cc_name);
if (comp) {
destroy_implicit_component_args(comp);
}
+
+ return status;
}
/*
GList *filter_names = NULL;
GList *sink_names = NULL;
bt_value *leftovers = NULL;
+ bt_value *leftover_params = NULL;
struct implicit_component_args implicit_ctf_output_args = { 0 };
struct implicit_component_args implicit_lttng_live_args = { 0 };
struct implicit_component_args implicit_dummy_args = { 0 };
goto error;
}
+ leftover_params = bt_value_array_create();
+ if (!leftover_params) {
+ BT_CLI_LOGE_APPEND_CAUSE_OOM();
+ goto error;
+ }
+
if (auto_source_discovery_init(&auto_disc) != 0) {
goto error;
}
break;
}
case OPT_PARAMS:
- if (current_item_type != CONVERT_CURRENT_ITEM_TYPE_COMPONENT) {
- BT_CLI_LOGE_APPEND_CAUSE(
- "No current component (--component option) of which to set parameters:\n %s",
- arg);
- goto error;
- }
+ if (current_item_type == CONVERT_CURRENT_ITEM_TYPE_COMPONENT) {
+ /*
+ * The current item is a component (--component option),
+ * pass it directly to the run args.
+ */
+ if (bt_value_array_append_string_element(run_args,
+ "--params")) {
+ BT_CLI_LOGE_APPEND_CAUSE_OOM();
+ goto error;
+ }
- if (bt_value_array_append_string_element(run_args,
- "--params")) {
- BT_CLI_LOGE_APPEND_CAUSE_OOM();
- goto error;
- }
+ if (bt_value_array_append_string_element(run_args, arg)) {
+ BT_CLI_LOGE_APPEND_CAUSE_OOM();
+ goto error;
+ }
+ } else if (current_item_type == CONVERT_CURRENT_ITEM_TYPE_LEFTOVER) {
+ /* The current item is a leftover, record it in `leftover_params`. */
+ bt_value *array;
+ uint64_t idx = bt_value_array_get_size(leftover_params) - 1;
- if (bt_value_array_append_string_element(run_args, arg)) {
- BT_CLI_LOGE_APPEND_CAUSE_OOM();
+ array = bt_value_array_borrow_element_by_index(leftover_params, idx);
+ bt_value_array_append_string_element(array, arg);
+ } else {
+ BT_CLI_LOGE_APPEND_CAUSE(
+ "No current component (--component option) or non-option argument of which to set parameters:\n %s",
+ arg);
goto error;
}
break;
BT_CLI_LOGE_APPEND_CAUSE_OOM();
goto error;
}
+
+ append_status = bt_value_array_append_empty_array_element(leftover_params);
+ if (append_status != BT_VALUE_ARRAY_APPEND_ELEMENT_STATUS_OK) {
+ BT_CLI_LOGE_APPEND_CAUSE_OOM();
+ goto error;
+ }
} else {
abort();
}
goto error;
}
- create_implicit_component_args_from_auto_discovered_sources(
- &auto_disc, discovered_source_args);
+ status = create_implicit_component_args_from_auto_discovered_sources(
+ &auto_disc, leftover_params, discovered_source_args);
+ if (status != 0) {
+ goto error;
+ }
}
}
goto error;
}
+ res->original_input_indices = bt_value_array_create();
+ if (!res->original_input_indices) {
+ BT_CLI_LOGE_APPEND_CAUSE("Failed to allocate an array value.");
+ goto error;
+ }
+
goto end;
error:
auto_source_discovery_result_destroy(res);
return status;
}
+static
+const bt_value *borrow_array_value_last_element_const(const bt_value *array)
+{
+ uint64_t last_index = bt_value_array_get_size(array) - 1;
+
+ return bt_value_array_borrow_element_by_index_const(array, last_index);
+}
+
/*
* Assign `input` to source component class `source_cc_name` of plugin
* `plugin_name`, in the group with key `group`.
const char *plugin_name,
const char *source_cc_name,
const char *group,
- const char *input)
+ const char *input,
+ uint64_t original_input_index)
{
int status;
bt_value_array_append_element_status append_status;
guint len;
guint i;
struct auto_source_discovery_result *res = NULL;
+ bool append_index;
len = auto_disc->results->len;
i = len;
goto error;
}
+ /*
+ * Append `original_input_index` to `original_input_indices` if not
+ * there already. We process the `inputs` array in order, so if it is
+ * present, it has to be the last element.
+ */
+ if (bt_value_array_is_empty(res->original_input_indices)) {
+ append_index = true;
+ } else {
+ const bt_value *last_index_value;
+ uint64_t last_index;
+
+ last_index_value =
+ borrow_array_value_last_element_const(res->original_input_indices);
+ last_index = bt_value_integer_unsigned_get(last_index_value);
+
+ BT_ASSERT(last_index <= original_input_index);
+
+ append_index = (last_index != original_input_index);
+ }
+
+ if (append_index) {
+ append_status = bt_value_array_append_unsigned_integer_element(
+ res->original_input_indices, original_input_index);
+
+ if (append_status != BT_VALUE_ARRAY_APPEND_ELEMENT_STATUS_OK) {
+ BT_CLI_LOGE_APPEND_CAUSE("Failed to append an unsigned integer value.");
+ goto error;
+ }
+ }
status = 0;
goto end;
*/
static
int support_info_query_all_sources(const char *input,
- const char *input_type, size_t plugin_count,
+ const char *input_type,
+ uint64_t original_input_index,
+ size_t plugin_count,
const char *plugin_restrict,
const char *component_class_restrict,
enum bt_logging_level log_level,
BT_LOGI("Input %s is awarded to component class source.%s.%s with weight %f",
input, plugin_name, source_name, winner.weigth);
- status = auto_source_discovery_add(auto_disc, plugin_name, source_name, group, input);
+ status = auto_source_discovery_add(auto_disc, plugin_name,
+ source_name, group, input, original_input_index);
if (status != 0) {
goto error;
}
static
int auto_discover_source_for_input_as_string(const char *input,
+ uint64_t original_input_index,
size_t plugin_count, const char *plugin_restrict,
const char *component_class_restrict,
enum bt_logging_level log_level,
struct auto_source_discovery *auto_disc)
{
return support_info_query_all_sources(input, "string",
- plugin_count, plugin_restrict, component_class_restrict,
- log_level, auto_disc);
+ original_input_index, plugin_count, plugin_restrict,
+ component_class_restrict, log_level, auto_disc);
}
static
int auto_discover_source_for_input_as_dir_or_file_rec(GString *input,
+ uint64_t original_input_index,
size_t plugin_count, const char *plugin_restrict,
const char *component_class_restrict,
enum bt_logging_level log_level,
if (g_file_test(input->str, G_FILE_TEST_IS_REGULAR)) {
/* It's a file. */
status = support_info_query_all_sources(input->str,
- "file", plugin_count,
+ "file", original_input_index, plugin_count,
plugin_restrict, component_class_restrict, log_level, auto_disc);
} else if (g_file_test(input->str, G_FILE_TEST_IS_DIR)) {
GDir *dir;
/* It's a directory. */
status = support_info_query_all_sources(input->str,
- "directory", plugin_count,
+ "directory", original_input_index, plugin_count,
plugin_restrict, component_class_restrict, log_level,
auto_disc);
g_string_append(input, dirent);
status = auto_discover_source_for_input_as_dir_or_file_rec(
- input, plugin_count,
+ input, original_input_index, plugin_count,
plugin_restrict, component_class_restrict,
log_level, auto_disc);
static
int auto_discover_source_for_input_as_dir_or_file(const char *input,
+ uint64_t original_input_index,
size_t plugin_count, const char *plugin_restrict,
const char *component_class_restrict,
enum bt_logging_level log_level,
}
status = auto_discover_source_for_input_as_dir_or_file_rec(
- mutable_input, plugin_count, plugin_restrict,
+ mutable_input, original_input_index, plugin_count, plugin_restrict,
component_class_restrict, log_level, auto_disc);
g_string_free(mutable_input, TRUE);
input_value = bt_value_array_borrow_element_by_index_const(inputs, i_inputs);
input = bt_value_string_get(input_value);
- status = auto_discover_source_for_input_as_string(input,
+ status = auto_discover_source_for_input_as_string(input, i_inputs,
plugin_count, plugin_restrict, component_class_restrict,
log_level, auto_disc);
if (status < 0) {
}
status = auto_discover_source_for_input_as_dir_or_file(input,
- plugin_count, plugin_restrict, component_class_restrict,
- log_level, auto_disc);
+ i_inputs, plugin_count, plugin_restrict,
+ component_class_restrict, log_level, auto_disc);
if (status < 0) {
/* Fatal error. */
goto end;
/* Array of input strings. */
bt_value *inputs;
+
+ /*
+ * Array of integers: indices of the original inputs that contributed
+ * to this result.
+ */
+ bt_value *original_input_indices;
};
int auto_source_discovery_init(struct auto_source_discovery *auto_disc);
--- /dev/null
+#!/bin/bash
+#
+# Copyright (C) 2019 Simon Marchi <simon.marchi@efficios.com>
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License, version 2 only, as
+# published by the Free Software Foundation.
+#
+# 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., 51
+# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test how parameters are applied to sources auto-discovered by the convert
+# command.
+
+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
+SH_TAP=1 source "$UTILSSH"
+
+NUM_TESTS=4
+
+plan_tests $NUM_TESTS
+
+data_dir="${BT_TESTS_DATADIR}/cli/convert/auto-source-discovery-params"
+plugin_dir="${data_dir}"
+dir_a="${data_dir}/dir-a"
+dir_b="${data_dir}/dir-b"
+dir_ab="${data_dir}/dir-ab"
+
+expected_file=$(mktemp -t expected.XXXXXX)
+stderr_expected=/dev/null
+
+# Apply params to two components from one leftover.
+cat > "$expected_file" <<END
+TestSourceA: ('test-allo', 'madame')
+TestSourceB: ('test-allo', 'madame')
+END
+
+bt_diff_cli_sorted "$expected_file" "$stderr_expected" \
+ convert --plugin-path "${plugin_dir}" \
+ "${dir_ab}" --params 'test-allo="madame"'
+ok "$?" "apply params to two components from one leftover"
+
+# Apply params to two components from two distinct leftovers.
+cat > "$expected_file" <<END
+TestSourceA: ('test-allo', 'madame')
+TestSourceB: ('test-bonjour', 'monsieur')
+END
+
+bt_diff_cli_sorted "$expected_file" "$stderr_expected" \
+ convert --plugin-path "${plugin_dir}" \
+ "${dir_a}" --params 'test-allo="madame"' "${dir_b}" --params 'test-bonjour="monsieur"'
+ok "$?" "apply params to two leftovers"
+
+# Apply params to one component coming from one leftover and one component coming from two leftovers (1).
+cat > "$expected_file" <<END
+TestSourceA: ('test-allo', 'madame'), ('test-bonjour', 'monsieur')
+TestSourceB: ('test-bonjour', 'monsieur')
+END
+
+bt_diff_cli_sorted "$expected_file" "$stderr_expected" \
+ convert --plugin-path "${plugin_dir}" \
+ "${dir_a}" --params 'test-allo="madame"' "${dir_ab}" --params 'test-bonjour="monsieur"'
+ok "$?" "apply params to one component coming from one leftover and one component coming from two leftovers (1)"
+
+# Apply params to one component coming from one leftover and one component coming from two leftovers (2).
+cat > "$expected_file" <<END
+TestSourceA: ('test-bonjour', 'monsieur'), ('test-salut', 'les amis')
+TestSourceB: ('test-bonjour', 'madame'), ('test-salut', 'les amis')
+END
+
+bt_diff_cli_sorted "$expected_file" "$stderr_expected" \
+ convert --plugin-path "${plugin_dir}" \
+ "${dir_ab}" --params 'test-bonjour="madame",test-salut="les amis"' "${dir_a}" --params 'test-bonjour="monsieur"'
+ok "$?" "apply params to one component coming from one leftover and one component coming from two leftovers (2)"
+
+rm -f "$expected_file"
output_path=$(cygpath -m "$output_path")
fi
-plan_tests 75
+plan_tests 73
test_bt_convert_run_args 'path leftover' "$path_to_trace" "--component auto-disc-source-ctf-fs:source.ctf.fs --params 'inputs=[\"$path_to_trace\"]' --component pretty:sink.text.pretty --component muxer:filter.utils.muxer --connect auto-disc-source-ctf-fs:muxer --connect muxer:pretty"
test_bt_convert_run_args 'path leftovers' "$path_to_trace $path_to_trace2" "--component auto-disc-source-ctf-fs:source.ctf.fs --params 'inputs=[\"$path_to_trace\", \"${path_to_trace2}\"]' --component pretty:sink.text.pretty --component muxer:filter.utils.muxer --connect auto-disc-source-ctf-fs:muxer --connect muxer:pretty"
test_bt_convert_fails '-o ctf without --output' 'my-trace -o ctf'
test_bt_convert_fails '-o ctf + --output with implicit sink.text.pretty' "my-trace -o ctf --output $output_path --no-delta"
test_bt_convert_fails '--stream-intersection' "$path_to_trace --stream-intersection"
-test_bt_convert_fails '--params after leftover' "--component src.ctf.fs $path_to_trace --params=a=2" 'No current component (--component option) of which to set parameters'
test_bt_convert_fails '--log-level after leftover' "--component src.ctf.fs $path_to_trace --log-level=W" 'No current component (--component option) to assign a log level to'
rm -f "${tmp_stderr}"
--- /dev/null
+import bt2
+import os
+
+# This file defines source component classes that print the parameters they
+# receive in their __init__ which start with 'test-'.
+
+
+class TestIter(bt2._UserMessageIterator):
+ pass
+
+
+class Base:
+ @classmethod
+ def _print_test_params(cls, params):
+ items = sorted([str(x) for x in params.items() if x[0].startswith('test-')])
+ print('{}: {}'.format(cls.__name__, ', '.join(items)))
+
+
+@bt2.plugin_component_class
+class TestSourceA(Base, bt2._UserSourceComponent, message_iterator_class=TestIter):
+ def __init__(self, params):
+ self._print_test_params(params)
+
+ @staticmethod
+ def _user_query(priv_query_exec, obj, params):
+ # Match files starting with 'aaa'.
+
+ if obj == 'babeltrace.support-info':
+ if params['type'] != 'file':
+ return 0
+
+ name = os.path.basename(str(params['input']))
+
+ if name.startswith('aaa'):
+ return {'weight': 1, 'group': 'aaa'}
+ else:
+ return 0
+ else:
+ raise bt2.UnknownObject
+
+
+@bt2.plugin_component_class
+class TestSourceB(Base, bt2._UserSourceComponent, message_iterator_class=TestIter):
+ def __init__(self, params):
+ self._print_test_params(params)
+
+ @staticmethod
+ def _user_query(priv_query_exec, obj, params):
+ # Match files starting with 'bbb'.
+
+ if obj == 'babeltrace.support-info':
+ if params['type'] != 'file':
+ return 0
+
+ name = os.path.basename(str(params['input']))
+
+ if name.startswith('bbb'):
+ return {'weight': 1, 'group': 'bbb'}
+ else:
+ return 0
+ else:
+ raise bt2.UnknownObject
+
+
+bt2.register_plugin(module_name=__name__, name="test")
projects / babeltrace.git / commitdiff
