# [RFC] git-fsck-cache argument processing 18 messages from 2005-05-21 to 2005-05-21. Participants: Sean, Jeff Garzik, Junio C Hamano, Olivier Galibert, Linus Torvalds, Joel Becker. Thread: https://gitlist.dev/t/682 ## Sean, 2005-05-21 03:38 Subject: [RFC] git-fsck-cache argument processing Message-ID: <4870.10.10.10.24.1116646732.squirrel@linux1> URL: https://gitlist.dev/e/4870.10.10.10.24.1116646732.squirrel%40linux1 ``` Here is a first crack at using argp as suggested by Jeff Garzik to implement argument processing as requested by Junio and Linus. Each of the long arguments have been given a single character equivalent as well. This patch only converts fsck-cache to use argp in case anyone has objections to the basic format or style. The patch includes a version number inside of fsck-cache.c; this should really be in a separate include file so you can run any command with --version and get the same answer. With this change you have: $ git-fsck-cache -? Usage: git-fsck-cache [OPTION...] [HEAD-SHA1...] git-fsck-cache - repository consistency check -c, --cache Mark all objects referenced by cache as reachable -d, --delta-depth Show the maximum length of delta chains -r, --root Show root objects, ie. those without parents -t, --tags Show revision tags -u, --unreachable Show missing objects or deltas -?, --help Give this help list --usage Give a short usage message -V, --version Print program version And the following should work as expected: $ git-fsck-cache -crudt fsck-cache.c | 64 +++++++++++++++++++++++++++++++++++------------------------ 1 files changed, 39 insertions(+), 25 deletions(-) Sean fsck-cache.c: needs update Index: fsck-cache.c =================================================================== --- 58741c69570705801db4b785681790d636475695/fsck-cache.c (mode:100644) +++ uncommitted/fsck-cache.c (mode:100644) @@ -1,5 +1,7 @@ #include #include +#include +const char *argp_program_version = "git 1.0"; #include "cache.h" #include "commit.h" @@ -407,36 +409,48 @@ find_file_objects(git_dir, "refs"); } +#define O_UNREACH 'u' +#define O_TAGS 't' +#define O_ROOT 'r' +#define O_DELTA 'd' +#define O_CACHE 'c' + +static const char doc[] = "Perform repository consistency check"; + +static struct argp_option options[] = { + {"unreachable", O_UNREACH, 0, 0, "Show missing objects or deltas"}, + {"tags", O_TAGS, 0, 0, "Show revision tags"}, + {"root", O_ROOT, 0, 0, "Show root objects, ie. those without parents"}, + {"delta-depth", O_DELTA, 0, 0, "Show the maximum length of delta chains"}, + {"cache", O_CACHE, 0, 0, "Mark all objects referenced by cache as reachable"}, + { } +}; + +static error_t parse_opt (int key, char *arg, struct argp_state *state) +{ + switch (key) { + case O_UNREACH: show_unreachable = 1; break; + case O_TAGS: show_tags = 1; break; + case O_ROOT: show_root = 1; break; + case O_DELTA: show_max_delta_depth = 1; break; + case O_CACHE: keep_cache_objects = 1; break; + case ARGP_KEY_ARG: state->next = state->argc; break; + default: return ARGP_ERR_UNKNOWN; + } + return 0; +} + +static const struct argp argp = { options, parse_opt, "[HEAD-SHA1...]", doc }; + int main(int argc, char **argv) { int i, heads; char *sha1_dir; - for (i = 1; i < argc; i++) { - const char *arg = argv[i]; - - if (!strcmp(arg, "--unreachable")) { - show_unreachable = 1; - continue; - } - if (!strcmp(arg, "--tags")) { - show_tags = 1; - continue; - } - if (!strcmp(arg, "--root")) { - show_root = 1; - continue; - } - if (!strcmp(arg, "--delta-depth")) { - show_max_delta_depth = 1; - continue; - } - if (!strcmp(arg, "--cache")) { - keep_cache_objects = 1; - continue; - } - if (*arg == '-') - usage("git-fsck-cache [--tags] [[--unreachable] [--cache] *]"); + error_t rc = argp_parse(&argp, argc, argv, 0, NULL, NULL); + if (rc) { + fprintf(stderr, "argument failed: %s\n", strerror(rc)); + return 1; } sha1_dir = get_object_directory(); ``` ## Jeff Garzik, 2005-05-21 04:08 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <428EB444.7010200@pobox.com> URL: https://gitlist.dev/e/428EB444.7010200%40pobox.com In-Reply-To: <4870.10.10.10.24.1116646732.squirrel@linux1> ``` Sean wrote: > Here is a first crack at using argp as suggested by Jeff Garzik to > implement argument processing as requested by Junio and Linus. Each of > the long arguments have been given a single character equivalent as well. > > This patch only converts fsck-cache to use argp in case anyone has > objections to the basic format or style. The patch includes a version > number inside of fsck-cache.c; this should really be in a separate include > file so you can run any command with --version and get the same answer. Pretty good. You'll probably want some additional changes: 1) eliminate + case ARGP_KEY_ARG: state->next = state->argc; break; This will cause option processing to stop at the first unknown argument. 2) Pass-by-reference a variable to argp_parse(), which will store the index of the argument where processing stopped. This is the first hash/file/etc. non-option argument. (example code from posixutils) int parse_cmdline(struct cmdline_walker *cw) { error_t rc_argp; int idx = 0; rc_argp = argp_parse(cw->argp, cw->argc, cw->argv, 0, &idx, NULL); if (rc_argp) { fprintf(stderr, "argp_parse: %s\n", strerror(rc_argp)); return -rc_argp; } return idx; } 'idx' in this case is the first non-option argument, which can be passed directly to argv[]. From there, you perform standard iteration over the arguments provided on the command line, starting at argv[idx]. If you have a fixed number of arguments following the options, then your parse_opt function can easily parse those args as well: static error_t parse_opt (int key, char *arg, struct argp_state *state) { switch (key) { case '1': outmask |= OPT_FILE1; break; case '2': outmask |= OPT_FILE2; break; case '3': outmask |= OPT_DUP; break; case ARGP_KEY_ARG: switch(state->arg_num) { case 0: file1 = arg; break; /* 1st non-opt arg */ case 1: file2 = arg; break; /* 2nd non-opt arg */ default: argp_usage (state); break; /* too many args */ } break; case ARGP_KEY_END: if (state->arg_num < 2) /* not enough args */ argp_usage (state); break; default: return ARGP_ERR_UNKNOWN; } return 0; } ``` ## Sean, 2005-05-21 04:36 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <4966.10.10.10.24.1116650175.squirrel@linux1> URL: https://gitlist.dev/e/4966.10.10.10.24.1116650175.squirrel%40linux1 In-Reply-To: <428EB444.7010200@pobox.com> ``` On Sat, May 21, 2005 12:08 am, Jeff Garzik said: > Pretty good. You'll probably want some additional changes: > > 1) eliminate > + case ARGP_KEY_ARG: state->next = state->argc; break; > > This will cause option processing to stop at the first unknown argument. > > 2) Pass-by-reference a variable to argp_parse(), which will store the > index of the argument where processing stopped. This is the first > hash/file/etc. non-option argument. > Thanks Jeff, that's pretty cool. Here's an updated patch. With this updated patch, options following or even intermingled with the SHA1 list also are picked up, for example: $ git-fsck-cache 804c64ea864d0a8ee13f3de0b74158a3e9c3166d -crudt fsck-cache.c | 66 +++++++++++++++++++++++++++++++++++------------------------ 1 files changed, 40 insertions(+), 26 deletions(-) Sean fsck-cache.c: needs update Index: fsck-cache.c =================================================================== --- 58741c69570705801db4b785681790d636475695/fsck-cache.c (mode:100644) +++ uncommitted/fsck-cache.c (mode:100644) @@ -1,5 +1,7 @@ #include #include +#include +const char *argp_program_version = "git 1.0"; #include "cache.h" #include "commit.h" @@ -407,36 +409,48 @@ find_file_objects(git_dir, "refs"); } +#define O_UNREACH 'u' +#define O_TAGS 't' +#define O_ROOT 'r' +#define O_DELTA 'd' +#define O_CACHE 'c' + +static const char doc[] = "Perform repository consistency check"; + +static struct argp_option options[] = { + {"unreachable", O_UNREACH, 0, 0, "Show missing objects or deltas"}, + {"tags", O_TAGS, 0, 0, "Show revision tags"}, + {"root", O_ROOT, 0, 0, "Show root objects, ie. those without parents"}, + {"delta-depth", O_DELTA, 0, 0, "Show the maximum length of delta chains"}, + {"cache", O_CACHE, 0, 0, "Mark all objects referenced by cache as reachable"}, + { } +}; + +static error_t parse_opt (int key, char *arg, struct argp_state *state) +{ + switch (key) { + case O_UNREACH: show_unreachable = 1; break; + case O_TAGS: show_tags = 1; break; + case O_ROOT: show_root = 1; break; + case O_DELTA: show_max_delta_depth = 1; break; + case O_CACHE: keep_cache_objects = 1; break; + default: return ARGP_ERR_UNKNOWN; + } + return 0; +} + +static const struct argp argp = { options, parse_opt, "[HEAD-SHA1...]", doc }; + int main(int argc, char **argv) { int i, heads; char *sha1_dir; + int idx; - for (i = 1; i < argc; i++) { - const char *arg = argv[i]; - - if (!strcmp(arg, "--unreachable")) { - show_unreachable = 1; - continue; - } - if (!strcmp(arg, "--tags")) { - show_tags = 1; - continue; - } - if (!strcmp(arg, "--root")) { - show_root = 1; - continue; - } - if (!strcmp(arg, "--delta-depth")) { - show_max_delta_depth = 1; - continue; - } - if (!strcmp(arg, "--cache")) { - keep_cache_objects = 1; - continue; - } - if (*arg == '-') - usage("git-fsck-cache [--tags] [[--unreachable] [--cache] *]"); + error_t rc = argp_parse(&argp, argc, argv, 0, &idx, NULL); + if (rc) { + fprintf(stderr, "argument failed: %s\n", strerror(rc)); + return 1; } sha1_dir = get_object_directory(); @@ -450,7 +464,7 @@ expand_deltas(); heads = 0; - for (i = 1; i < argc; i++) { + for (i = idx; i < argc; i++) { const char *arg = argv[i]; if (*arg == '-') ``` ## Junio C Hamano, 2005-05-21 05:08 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <7voeb5np30.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7voeb5np30.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <4870.10.10.10.24.1116646732.squirrel@linux1> ``` The patch looks good. Before you proceed to convert the rest, could I ask you to first let us see the list of new set of options and semantics changes, if any ("checkout-cache -f -a" vs "checkout-cache -a -f" immediately comes to mind)? Presumably you would be doing the Documentation updates as well, so starting from the documentaiton updates before writing the actual code may be a good way for us to understand and ack on what is going to happen. ``` ## Jeff Garzik, 2005-05-21 05:09 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <428EC2A0.7030407@pobox.com> URL: https://gitlist.dev/e/428EC2A0.7030407%40pobox.com In-Reply-To: <4966.10.10.10.24.1116650175.squirrel@linux1> ``` Patch looks good to me... ``` ## Jeff Garzik, 2005-05-21 05:15 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <20050521051536.GA9387@havoc.gtf.org> URL: https://gitlist.dev/e/20050521051536.GA9387%40havoc.gtf.org In-Reply-To: <7voeb5np30.fsf@assigned-by-dhcp.cox.net> ``` On Fri, May 20, 2005 at 10:08:19PM -0700, Junio C Hamano wrote: > The patch looks good. Before you proceed to convert the rest, > could I ask you to first let us see the list of new set of > options and semantics changes, if any ("checkout-cache -f -a" vs > "checkout-cache -a -f" immediately comes to mind)? Typical implementation is agnostic on the ordering of options, but with a few lines of code in parse_opt() that need not always be the case. FWIW most users (including me!) would expect that order of options is -not- significant. Jeff ``` ## Junio C Hamano, 2005-05-21 05:59 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <7vekc1nmq4.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vekc1nmq4.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20050521051536.GA9387@havoc.gtf.org> ``` >>>>> "JG" == Jeff Garzik writes: JG> On Fri, May 20, 2005 at 10:08:19PM -0700, Junio C Hamano wrote: >> The patch looks good. Before you proceed to convert the rest, >> could I ask you to first let us see the list of new set of >> options and semantics changes, if any ("checkout-cache -f -a" vs >> "checkout-cache -a -f" immediately comes to mind)? JG> FWIW most users (including me!) would expect that order of JG> options is -not- significant. That set of users includes me. I was hoping that this round would not just change things to use argp, but at the same time attempt to "fix" the problems around argument parsing. JG> Typical implementation is agnostic on the ordering of JG> options, but with a few lines of code in parse_opt() that JG> need not always be the case. I think you are responding to my "semantic changes" question, but I did not mean to say that exactly emulating the current behaviour is the requirement, nor I meant to ask if doing so is impossible using argp(). I just wanted to see "the set of planned fixes" while we are at it. ``` ## Olivier Galibert, 2005-05-21 15:09 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <20050521150926.GA96606@dspnet.fr.eu.org> URL: https://gitlist.dev/e/20050521150926.GA96606%40dspnet.fr.eu.org In-Reply-To: <4870.10.10.10.24.1116646732.squirrel@linux1> ``` On Fri, May 20, 2005 at 11:38:52PM -0400, Sean wrote: > -?, --help Give this help list Could you make that '-h' please ? > -V, --version Print program version And that '-v'. -V traditionally means verbose, -v version. Yes, I know there are counter-examples, but statistically... OG. ``` ## Jeff Garzik, 2005-05-21 15:35 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <428F554E.7060406@pobox.com> URL: https://gitlist.dev/e/428F554E.7060406%40pobox.com In-Reply-To: <20050521150926.GA96606@dspnet.fr.eu.org> ``` Olivier Galibert wrote: > And that '-v'. -V traditionally means verbose, -v version. Yes, I > know there are counter-examples, but statistically... In my experience, lowercase 'v' means verbose :) Jeff ``` ## Sean, 2005-05-21 17:22 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <2015.10.10.10.24.1116696150.squirrel@linux1> URL: https://gitlist.dev/e/2015.10.10.10.24.1116696150.squirrel%40linux1 In-Reply-To: <20050521150926.GA96606@dspnet.fr.eu.org> ``` On Sat, May 21, 2005 11:09 am, Olivier Galibert said: > On Fri, May 20, 2005 at 11:38:52PM -0400, Sean wrote: >> -?, --help Give this help list > > Could you make that '-h' please ? > >> -V, --version Print program version > > And that '-v'. -V traditionally means verbose, -v version. Yes, I > know there are counter-examples, but statistically... > Hey Olivier, Both of these options are generated automatically by argp. I'm sure there is a way to override them, but i'd rather just leave them as given by argp. For the first case, if you try '-h' on the command line you get: $ git-fsck-cache -h git-fsck-cache: invalid option -- h Try `git-fsck-cache --help' or `git-fsck-cache --usage' for more information. So it leads to the proper help message. Sean ``` ## Olivier Galibert, 2005-05-21 18:49 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <20050521184909.GA1729@dspnet.fr.eu.org> URL: https://gitlist.dev/e/20050521184909.GA1729%40dspnet.fr.eu.org In-Reply-To: <2015.10.10.10.24.1116696150.squirrel@linux1> ``` On Sat, May 21, 2005 at 01:22:30PM -0400, Sean wrote: > Both of these options are generated automatically by argp. I see. Then I'll guess I'll have to put argp in the "crap" pile next to libtool and automake. > I'm sure there > is a way to override them, but i'd rather just leave them as given by > argp. For the first case, if you try '-h' on the command line you get: > > $ git-fsck-cache -h > git-fsck-cache: invalid option -- h > Try `git-fsck-cache --help' or `git-fsck-cache --usage' for more information. > > So it leads to the proper help message. How neat. Of course using the right option under (t)csh gives: galibert@m62:~ #201 >git-fsck-cache -? git-fsck-cache: No match. Importing this windowism is beyond stupid. OG. ``` ## Sean, 2005-05-21 19:00 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <2329.10.10.10.24.1116702037.squirrel@linux1> URL: https://gitlist.dev/e/2329.10.10.10.24.1116702037.squirrel%40linux1 In-Reply-To: <20050521184909.GA1729@dspnet.fr.eu.org> ``` On Sat, May 21, 2005 2:49 pm, Olivier Galibert said: > On Sat, May 21, 2005 at 01:22:30PM -0400, Sean wrote: >> Both of these options are generated automatically by argp. > > I see. Then I'll guess I'll have to put argp in the "crap" pile next > to libtool and automake. Okay now don't go saying things you can't take back . > How neat. Of course using the right option under (t)csh gives: > > galibert@m62:~ #201 >git-fsck-cache -? > git-fsck-cache: No match. > > Importing this windowism is beyond stupid. Hmmm, that kinda sucks, but the --help should still work fine. I suppose we could figure out how to use -h instead. Sean ``` ## Linus Torvalds, 2005-05-21 19:47 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.58.0505211245580.2206%40ppc970.osdl.org In-Reply-To: <4870.10.10.10.24.1116646732.squirrel@linux1> ``` On Fri, 20 May 2005, Sean wrote: > > -u, --unreachable Show missing objects or deltas That's the wrong description. fsck always shows missing objects, but "--unreachable" makes it also show objects that cannot be reached from any of the references (either passed in on the command line, or the implicit references we take if no explicit reference is given). So in many ways, "--unreachable" is about the _reverse_ of showing missing objects: it's about showing _extraneous_ objects that aren't needed by the ref that was passed in. Linus ``` ## Linus Torvalds, 2005-05-21 19:49 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.58.0505211248110.2206%40ppc970.osdl.org In-Reply-To: <4870.10.10.10.24.1116646732.squirrel@linux1> ``` Oh, and please fix your mailer. Your attachments show up for me as unreadable: [ Part 2, Application/OCTET-STREAM (Name: "fsck-cache-argp-v1.patch") 3.2KB. ] [ Cannot display this part. Press "V" then "S" to save in a file. ] ie I'd have to save them to a file to look at them, which is just silly. So either just add patches in-line (and make sure your mailer doesn't crap all over whitespace), or add them as text attachments so that people see the things in their mail readers. Linus ``` ## Sean, 2005-05-21 20:46 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <2475.10.10.10.24.1116708368.squirrel@linux1> URL: https://gitlist.dev/e/2475.10.10.10.24.1116708368.squirrel%40linux1 In-Reply-To: ``` On Sat, May 21, 2005 3:47 pm, Linus Torvalds said: > On Fri, 20 May 2005, Sean wrote: >> >> -u, --unreachable Show missing objects or deltas > > That's the wrong description. > > fsck always shows missing objects, but "--unreachable" makes it also show > objects that cannot be reached from any of the references (either passed > in on the command line, or the implicit references we take if no explicit > reference is given). > > So in many ways, "--unreachable" is about the _reverse_ of showing missing > objects: it's about showing _extraneous_ objects that aren't needed by the > ref that was passed in. Right, fixed up in attached patch which basically punts with a description of "Show unreachable objects", and leaves it to the documentation to do better. Sean ``` ## Sean, 2005-05-21 21:09 Subject: Re: [RFC] git-fsck-cache argument processing Message-ID: <2572.10.10.10.24.1116709742.squirrel@linux1> URL: https://gitlist.dev/e/2572.10.10.10.24.1116709742.squirrel%40linux1 In-Reply-To: <2475.10.10.10.24.1116708368.squirrel@linux1> ``` On Sat, May 21, 2005 4:46 pm, Sean said: > Right, fixed up in attached patch which basically punts with a > description of "Show unreachable objects", and leaves it to the > documentation to do better. Linus, Try 2 at that attachment thingy, hopefully this one will make your mailer happy. Sean Index: Documentation/git-fsck-cache.txt =================================================================== --- 85ec256656a7839ca7a2067792b828c9b63c8711/Documentation/git-fsck-cache.txt (mode:100644) +++ uncommitted/Documentation/git-fsck-cache.txt (mode:100644) @@ -9,7 +9,8 @@ SYNOPSIS -------- -'git-fsck-cache' [--tags] [--root] [[--unreachable] [--cache]