Re: [PATCH v4 2/5] perf config: Add '--system' and '--global' options to select which config file is used

From: Namhyung Kim
Date: Mon Jul 27 2015 - 04:08:39 EST


On Mon, Jul 27, 2015 at 12:58:28AM +0900, Taeung Song wrote:
> Which config file is used is decided in only perf_config().
> And a perf-confg command depend on perf_config()
> getting config file path. So add '--system' and '--global' options
> to select which config file to be used without perf_config().
> If file-options isn't used, default config file path is
> $HOME/.perfconfig.

I guess you borrowed the names from git config. But IMHO perf is
different since we don't have the 'local' (repository) concept. I
think we should use either system/user or global/local naming scheme.

Thanks,
Namhyung


>
> Signed-off-by: Taeung Song <treeze.taeung@xxxxxxxxx>
> ---
> tools/perf/Documentation/perf-config.txt | 14 +++++++++++++-
> tools/perf/builtin-config.c | 21 ++++++++++++++++++---
> tools/perf/util/cache.h | 2 ++
> tools/perf/util/config.c | 4 ++--
> 4 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
> index 1a339ea..f1e50b3 100644
> --- a/tools/perf/Documentation/perf-config.txt
> +++ b/tools/perf/Documentation/perf-config.txt
> @@ -8,7 +8,7 @@ perf-config - Get and set variables in a configuration file.
> SYNOPSIS
> --------
> [verse]
> -'perf config' -l | --list
> +'perf config' [<file-option>] -l | --list
>
> DESCRIPTION
> -----------
> @@ -21,6 +21,14 @@ OPTIONS
> --list::
> Show current config variables with key and value into each sections.
>
> +--global::
> + For writing and reading options: write to global
> + '$HOME/.perfconfig' file or read it.
> +
> +--system::
> + For writing and reading options: write to system-wide
> + '$(sysconfdir)/perfconfig' or read it.
> +
> CONFIGURATION FILE
> ------------------
>
> @@ -33,6 +41,10 @@ store a system-wide default configuration.
> The variables are divided into sections. In each section, the variables
> can are composed of a key and value.
>
> +When reading or writing, the values are read from the system and global
> +configuration files by default, and options '--system' and '--global'
> +can be used to tell the command to read from or write to only that location.
> +
> Syntax
> ~~~~~~
>
> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
> index f0541b8..27609bd 100644
> --- a/tools/perf/builtin-config.c
> +++ b/tools/perf/builtin-config.c
> @@ -14,6 +14,8 @@
> #include "util/debug.h"
>
> static int actions;
> +static bool use_system_config, use_global_config;
> +static const char *config_file_name;
>
> static const char * const config_usage[] = {
> "perf config [options]",
> @@ -23,6 +25,9 @@ static const char * const config_usage[] = {
> #define ACTION_LIST (1<<0)
>
> static const struct option config_options[] = {
> + OPT_GROUP("Config file location"),
> + OPT_BOOLEAN(0, "system", &use_system_config, "use system config file"),
> + OPT_BOOLEAN(0, "global", &use_global_config, "use global config file"),
> OPT_GROUP("Action"),
> OPT_BIT('l', "list", &actions,
> "show current config variables", ACTION_LIST),
> @@ -53,16 +58,26 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
> else
> has_option = false;
>
> + if (use_system_config && use_global_config) {
> + pr_err("Error: only one config file at a time\n");
> + usage_with_options(config_usage, config_options);
> + return -1;
> + } else if (use_global_config || (!use_system_config && !use_global_config))
> + config_file_name = mkpath("%s/.perfconfig", getenv("HOME"));
> + else if (use_system_config)
> + config_file_name = perf_etc_perfconfig();
> +
> switch (actions) {
> case ACTION_LIST:
> if (argc == 0)
> - ret = perf_config(show_config, NULL);
> + ret = perf_config_from_file(show_config, config_file_name, NULL);
> else
> goto out_err;
> goto out;
> default:
> - if (!has_option && argc == 0) {
> - ret = perf_config(show_config, NULL);
> + if ((!has_option || use_global_config || use_system_config)
> + && argc == 0) {
> + ret = perf_config_from_file(show_config, config_file_name, NULL);
> goto out;
> } else
> goto out_err;
> diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
> index c861373..bad3e4e 100644
> --- a/tools/perf/util/cache.h
> +++ b/tools/perf/util/cache.h
> @@ -22,11 +22,13 @@
> typedef int (*config_fn_t)(const char *, const char *, void *);
> extern int perf_default_config(const char *, const char *, void *);
> extern int perf_config(config_fn_t fn, void *);
> +extern int perf_config_from_file(config_fn_t fn, const char *filename, void *data);
> extern int perf_config_int(const char *, const char *);
> extern u64 perf_config_u64(const char *, const char *);
> extern int perf_config_bool(const char *, const char *);
> extern int config_error_nonbool(const char *);
> extern const char *perf_config_dirname(const char *, const char *);
> +extern const char *perf_etc_perfconfig(void);
>
> /* pager.c */
> extern void setup_pager(void);
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index e18f653..1a9862f 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -412,7 +412,7 @@ int perf_default_config(const char *var, const char *value,
> return 0;
> }
>
> -static int perf_config_from_file(config_fn_t fn, const char *filename, void *data)
> +int perf_config_from_file(config_fn_t fn, const char *filename, void *data)
> {
> int ret;
> FILE *f = fopen(filename, "r");
> @@ -430,7 +430,7 @@ static int perf_config_from_file(config_fn_t fn, const char *filename, void *dat
> return ret;
> }
>
> -static const char *perf_etc_perfconfig(void)
> +const char *perf_etc_perfconfig(void)
> {
> static const char *system_wide;
> if (!system_wide)
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/