[pacman-dev] [PATCH] pactree: Add "--config" option
This allows for specifying an alternate configuration file path, similar to pacman's "--config" option. Given that there is currently no other way to tell pactree to read from another configuration file (except for patching or symlinking), this seems totally sensible - even if there are plans to refactor and/or replace the standalone configuration file parser. We do not define a short option for the sake of consistency with pacman's set of command line options. Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de> --- src/util/pactree.c | 18 +++++++++++++++--- 1 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/util/pactree.c b/src/util/pactree.c index 09fe101..5e98f79 100644 --- a/src/util/pactree.c +++ b/src/util/pactree.c @@ -76,6 +76,11 @@ static struct color_choices no_color = { "" }; +/* long operations */ +enum { + OP_CONFIG = 1000 +}; + /* globals */ alpm_handle_t *handle = NULL; alpm_list_t *walked = NULL; @@ -90,6 +95,7 @@ int reverse = 0; int unique = 0; int searchsyncs = 0; const char *dbpath = DBPATH; +const char *configfile = CONFFILE; #ifndef HAVE_STRNDUP /* A quick and dirty implementation derived from glibc */ @@ -154,7 +160,7 @@ static int register_syncs(void) { char line[LINE_MAX]; const alpm_siglevel_t level = ALPM_SIG_DATABASE | ALPM_SIG_DATABASE_OPTIONAL; - fp = fopen(CONFFILE, "r"); + fp = fopen(configfile, "r"); if(!fp) { return 1; } @@ -202,6 +208,8 @@ static int parse_options(int argc, char *argv[]) {"reverse", no_argument, 0, 'r'}, {"sync", no_argument, 0, 'S'}, {"unique", no_argument, 0, 'u'}, + + {"config", required_argument, 0, OP_CONFIG}, {0, 0, 0, 0} }; @@ -211,6 +219,9 @@ static int parse_options(int argc, char *argv[]) } switch(opt) { + case OP_CONFIG: + configfile = optarg; + break; case 'b': dbpath = optarg; break; @@ -263,11 +274,12 @@ static void usage(void) " -c, --color colorize output\n" " -d, --depth <#> limit the depth of recursion\n" " -g, --graph generate output for graphviz's dot\n" + " -h, --help display this help message\n" " -l, --linear enable linear output\n" " -r, --reverse show reverse dependencies\n" " -s, --sync search sync DBs instead of local\n" - " -u, --unique show dependencies with no duplicates (implies -l)\n\n" - " -h, --help display this help message\n"); + " -u, --unique show dependencies with no duplicates (implies -l)\n" + " --config <path> set an alternate configuration file\n"); } static void cleanup(void) -- 1.7.7
On Mon, Oct 10, 2011 at 02:38:59PM +0200, Lukas Fleischer wrote:
This allows for specifying an alternate configuration file path, similar to pacman's "--config" option.
Given that there is currently no other way to tell pactree to read from another configuration file (except for patching or symlinking), this seems totally sensible - even if there are plans to refactor and/or replace the standalone configuration file parser.
We do not define a short option for the sake of consistency with pacman's set of command line options.
Shouldn't we complain if the config file isn't found now that we're allowing a user specified location? As it stands, you can pass something like: pactree --config /not/a/real/path foo And pactree won't complain. If you actually add the -s flag to walk the sync DBs, pactree will just tell you 'foo not found', which isn't really very helpful. Maybe I'm just bitter after a weekend of fighting false negatives from another tool... dave
Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de> --- src/util/pactree.c | 18 +++++++++++++++--- 1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/src/util/pactree.c b/src/util/pactree.c index 09fe101..5e98f79 100644 --- a/src/util/pactree.c +++ b/src/util/pactree.c @@ -76,6 +76,11 @@ static struct color_choices no_color = { "" };
+/* long operations */ +enum { + OP_CONFIG = 1000 +}; + /* globals */ alpm_handle_t *handle = NULL; alpm_list_t *walked = NULL; @@ -90,6 +95,7 @@ int reverse = 0; int unique = 0; int searchsyncs = 0; const char *dbpath = DBPATH; +const char *configfile = CONFFILE;
#ifndef HAVE_STRNDUP /* A quick and dirty implementation derived from glibc */ @@ -154,7 +160,7 @@ static int register_syncs(void) { char line[LINE_MAX]; const alpm_siglevel_t level = ALPM_SIG_DATABASE | ALPM_SIG_DATABASE_OPTIONAL;
- fp = fopen(CONFFILE, "r"); + fp = fopen(configfile, "r"); if(!fp) { return 1; } @@ -202,6 +208,8 @@ static int parse_options(int argc, char *argv[]) {"reverse", no_argument, 0, 'r'}, {"sync", no_argument, 0, 'S'}, {"unique", no_argument, 0, 'u'}, + + {"config", required_argument, 0, OP_CONFIG}, {0, 0, 0, 0} };
@@ -211,6 +219,9 @@ static int parse_options(int argc, char *argv[]) }
switch(opt) { + case OP_CONFIG: + configfile = optarg; + break; case 'b': dbpath = optarg; break; @@ -263,11 +274,12 @@ static void usage(void) " -c, --color colorize output\n" " -d, --depth <#> limit the depth of recursion\n" " -g, --graph generate output for graphviz's dot\n" + " -h, --help display this help message\n" " -l, --linear enable linear output\n" " -r, --reverse show reverse dependencies\n" " -s, --sync search sync DBs instead of local\n" - " -u, --unique show dependencies with no duplicates (implies -l)\n\n" - " -h, --help display this help message\n"); + " -u, --unique show dependencies with no duplicates (implies -l)\n" + " --config <path> set an alternate configuration file\n"); }
static void cleanup(void) -- 1.7.7
On Mon, Oct 10, 2011 at 08:59:41AM -0400, Dave Reisner wrote:
On Mon, Oct 10, 2011 at 02:38:59PM +0200, Lukas Fleischer wrote:
This allows for specifying an alternate configuration file path, similar to pacman's "--config" option.
Given that there is currently no other way to tell pactree to read from another configuration file (except for patching or symlinking), this seems totally sensible - even if there are plans to refactor and/or replace the standalone configuration file parser.
We do not define a short option for the sake of consistency with pacman's set of command line options.
Shouldn't we complain if the config file isn't found now that we're allowing a user specified location? As it stands, you can pass something like:
pactree --config /not/a/real/path foo
And pactree won't complain. If you actually add the -s flag to walk the sync DBs, pactree will just tell you 'foo not found', which isn't really very helpful. Maybe I'm just bitter after a weekend of fighting false negatives from another tool...
Yeah, the error message should be more informative. Actually, what you currently get is $ ./pactree -lsr gtk error: failed to register sync DBs I noticed this when running `~/src/pacman/src/util/pactree -lrs gtk` while having my pacman source tree configured to use "/usr/local" as a prefix and using the official pacman package. I'll send another patch to address this. Thanks!
dave
Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de> --- src/util/pactree.c | 18 +++++++++++++++--- 1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/src/util/pactree.c b/src/util/pactree.c index 09fe101..5e98f79 100644 --- a/src/util/pactree.c +++ b/src/util/pactree.c @@ -76,6 +76,11 @@ static struct color_choices no_color = { "" };
+/* long operations */ +enum { + OP_CONFIG = 1000 +}; + /* globals */ alpm_handle_t *handle = NULL; alpm_list_t *walked = NULL; @@ -90,6 +95,7 @@ int reverse = 0; int unique = 0; int searchsyncs = 0; const char *dbpath = DBPATH; +const char *configfile = CONFFILE;
#ifndef HAVE_STRNDUP /* A quick and dirty implementation derived from glibc */ @@ -154,7 +160,7 @@ static int register_syncs(void) { char line[LINE_MAX]; const alpm_siglevel_t level = ALPM_SIG_DATABASE | ALPM_SIG_DATABASE_OPTIONAL;
- fp = fopen(CONFFILE, "r"); + fp = fopen(configfile, "r"); if(!fp) { return 1; } @@ -202,6 +208,8 @@ static int parse_options(int argc, char *argv[]) {"reverse", no_argument, 0, 'r'}, {"sync", no_argument, 0, 'S'}, {"unique", no_argument, 0, 'u'}, + + {"config", required_argument, 0, OP_CONFIG}, {0, 0, 0, 0} };
@@ -211,6 +219,9 @@ static int parse_options(int argc, char *argv[]) }
switch(opt) { + case OP_CONFIG: + configfile = optarg; + break; case 'b': dbpath = optarg; break; @@ -263,11 +274,12 @@ static void usage(void) " -c, --color colorize output\n" " -d, --depth <#> limit the depth of recursion\n" " -g, --graph generate output for graphviz's dot\n" + " -h, --help display this help message\n" " -l, --linear enable linear output\n" " -r, --reverse show reverse dependencies\n" " -s, --sync search sync DBs instead of local\n" - " -u, --unique show dependencies with no duplicates (implies -l)\n\n" - " -h, --help display this help message\n"); + " -u, --unique show dependencies with no duplicates (implies -l)\n" + " --config <path> set an alternate configuration file\n"); }
static void cleanup(void) -- 1.7.7
Our error message used to be very unclear when the configuration file could not be found: $ ./pactree -lsr gtk error: failed to register sync DBs Instead, display an accurate message and include the file name: $ ./pactree -lsr gtk error: config file /usr/local/etc/pacman.conf could not be read Also, move the error message inside register_syncs() to allow for differentiating between different errors that might require a handler in the future. Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de> --- src/util/pactree.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/pactree.c b/src/util/pactree.c index 5e98f79..997ba46 100644 --- a/src/util/pactree.c +++ b/src/util/pactree.c @@ -162,6 +162,7 @@ static int register_syncs(void) { fp = fopen(configfile, "r"); if(!fp) { + fprintf(stderr, "error: config file %s could not be read\n", configfile); return 1; } @@ -463,7 +464,6 @@ int main(int argc, char *argv[]) if(searchsyncs) { if(register_syncs() != 0) { - fprintf(stderr, "error: failed to register sync DBs\n"); ret = 1; goto finish; } -- 1.7.7
On Mon, Oct 10, 2011 at 03:22:52PM +0200, Lukas Fleischer wrote:
Our error message used to be very unclear when the configuration file could not be found:
$ ./pactree -lsr gtk error: failed to register sync DBs
Instead, display an accurate message and include the file name:
$ ./pactree -lsr gtk error: config file /usr/local/etc/pacman.conf could not be read
Also, move the error message inside register_syncs() to allow for differentiating between different errors that might require a handler in the future.
Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de>
I'm cool with this. Pushed both your patches to my working branch. Thanks, dave
participants (2)
-
Dave Reisner
-
Lukas Fleischer