* [PATCH 2/2] builtin-prune.c: fix object parsing and use parse_options() @ 2008-03-23 20:50 Michele Ballabio 2008-03-23 22:21 ` Johannes Schindelin 0 siblings, 1 reply; 22+ messages in thread From: Michele Ballabio @ 2008-03-23 20:50 UTC (permalink / raw) To: git; +Cc: Junio C Hamano git-prune used to remove loose objects whether they were specified in the command line or not. This patch makes git-prune behave as stated in the manpage: it does not prune any listed head nor reachable objects; the parsing machinery now uses parse_options(). Signed-off-by: Michele Ballabio <barra_cuda@katamail.com> --- builtin-prune.c | 50 ++++++++++++++++++++++++++++++-------------------- t/t5304-prune.sh | 2 +- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/builtin-prune.c b/builtin-prune.c index bb8ead9..7b3e15d 100644 --- a/builtin-prune.c +++ b/builtin-prune.c @@ -4,8 +4,12 @@ #include "revision.h" #include "builtin.h" #include "reachable.h" +#include "parse-options.h" -static const char prune_usage[] = "git-prune [-n]"; +static const char * const prune_usage[] = { + "git-prune [-n] [--expire <time>] [--] [<head>...]", + NULL +}; static int show_only; static unsigned long expire; @@ -121,32 +125,38 @@ static void remove_temporary_files(void) closedir(dir); } +static int parse_opt_expire(const struct option *opt, const char *arg, + int unset) +{ + expire = approxidate(arg); + return 0; +} + int cmd_prune(int argc, const char **argv, const char *prefix) { - int i; struct rev_info revs; - for (i = 1; i < argc; i++) { - const char *arg = argv[i]; - if (!strcmp(arg, "-n")) { - show_only = 1; - continue; - } - if (!strcmp(arg, "--expire")) { - if (++i < argc) { - expire = approxidate(argv[i]); - continue; - } - } - else if (!prefixcmp(arg, "--expire=")) { - expire = approxidate(arg + 9); - continue; - } - usage(prune_usage); - } + const struct option options[] = { + OPT_BOOLEAN('n', NULL, &show_only, + "do not remove, show only"), + OPT_CALLBACK(0, "expire", &expire, "time", + "expire objects older than <time>", + parse_opt_expire), + OPT_END() + }; + + argc = parse_options(argc, argv, options, prune_usage, + PARSE_OPT_STOP_AT_NON_OPTION | + PARSE_OPT_KEEP_DASHDASH); save_commit_buffer = 0; init_revisions(&revs, prefix); + + if (argc > 1) + argc = setup_revisions(argc, argv, &revs, NULL); + if (argc > 1) + die ("unrecognized argument: %s", argv[1]); + mark_reachable_objects(&revs, 1); prune_object_dir(get_object_directory()); diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh index d5f12f7..0638297 100644 --- a/t/t5304-prune.sh +++ b/t/t5304-prune.sh @@ -92,7 +92,7 @@ test_expect_success 'prune: prune unreachable heads' ' ' -test_expect_failure 'prune: do not prune heads listed as an argument' ' +test_expect_success 'prune: do not prune heads listed as an argument' ' : > file2 && git add file2 && -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] builtin-prune.c: fix object parsing and use parse_options() 2008-03-23 20:50 [PATCH 2/2] builtin-prune.c: fix object parsing and use parse_options() Michele Ballabio @ 2008-03-23 22:21 ` Johannes Schindelin 2008-03-24 12:31 ` Michele Ballabio 0 siblings, 1 reply; 22+ messages in thread From: Johannes Schindelin @ 2008-03-23 22:21 UTC (permalink / raw) To: Michele Ballabio; +Cc: git, Junio C Hamano Hi, On Sun, 23 Mar 2008, Michele Ballabio wrote: > -static const char prune_usage[] = "git-prune [-n]"; > +static const char * const prune_usage[] = { > + "git-prune [-n] [--expire <time>] [--] [<head>...]", > + NULL > +}; As you already use parse-options, should this not be rather static const char * const prune_usage[] = { "git-prune [options] [--] [<commit>...]", Hmm? > +static int parse_opt_expire(const struct option *opt, const char *arg, > + int unset) > +{ > + expire = approxidate(arg); > + return 0; > +} This would probably be a good candidate to live in parse-options.[ch], no? But yes, the patch is good! Ciao, Dscho ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] builtin-prune.c: fix object parsing and use parse_options() 2008-03-23 22:21 ` Johannes Schindelin @ 2008-03-24 12:31 ` Michele Ballabio 2008-03-24 13:13 ` Johannes Schindelin 0 siblings, 1 reply; 22+ messages in thread From: Michele Ballabio @ 2008-03-24 12:31 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Junio C Hamano On Sunday 23 March 2008, Johannes Schindelin wrote: > On Sun, 23 Mar 2008, Michele Ballabio wrote: > > > -static const char prune_usage[] = "git-prune [-n]"; > > +static const char * const prune_usage[] = { > > + "git-prune [-n] [--expire <time>] [--] [<head>...]", > > + NULL > > +}; > > As you already use parse-options, should this not be rather > > static const char * const prune_usage[] = { > "git-prune [options] [--] [<commit>...]", > > Hmm? Ok, but the usage string is quite short anyway... and other commands show a similar quite detailed usage string. Not that I care strongly about this, though. > > +static int parse_opt_expire(const struct option *opt, const char *arg, > > + int unset) > > +{ > > + expire = approxidate(arg); > > + return 0; > > +} > > This would probably be a good candidate to live in parse-options.[ch], no? Uhm, probably, yes. See the patch below. > But yes, the patch is good! Thank you. -- >8 -- parse-options.c: introduce callback function for approxidate() There are quite a few places that will need to call approxidate(), when they'll adopt the parse-options system, so this patch adds the function parse_opt_approxidate_cb(), to be used within OPT_CALLBACK, and converts the only user so far. Signed-off-by: Michele Ballabio <barra_cuda@katamail.com> --- builtin-prune.c | 9 +-------- parse-options.c | 7 +++++++ parse-options.h | 2 ++ 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/builtin-prune.c b/builtin-prune.c index 7b3e15d..a5d6fe5 100644 --- a/builtin-prune.c +++ b/builtin-prune.c @@ -125,13 +125,6 @@ static void remove_temporary_files(void) closedir(dir); } -static int parse_opt_expire(const struct option *opt, const char *arg, - int unset) -{ - expire = approxidate(arg); - return 0; -} - int cmd_prune(int argc, const char **argv, const char *prefix) { struct rev_info revs; @@ -141,7 +134,7 @@ int cmd_prune(int argc, const char **argv, const char *prefix) "do not remove, show only"), OPT_CALLBACK(0, "expire", &expire, "time", "expire objects older than <time>", - parse_opt_expire), + parse_opt_approxidate_cb), OPT_END() }; diff --git a/parse-options.c b/parse-options.c index 8e64316..6ec7fe8 100644 --- a/parse-options.c +++ b/parse-options.c @@ -409,3 +409,10 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) *(int *)(opt->value) = v; return 0; } + +int parse_opt_approxidate_cb(const struct option *opt, const char *arg, + int unset) +{ + *(unsigned int *)(opt->value) = approxidate(arg); + return 0; +} diff --git a/parse-options.h b/parse-options.h index 1af62b0..e6976ed 100644 --- a/parse-options.h +++ b/parse-options.h @@ -110,6 +110,8 @@ extern NORETURN void usage_with_options(const char * const *usagestr, /*----- some often used options -----*/ extern int parse_opt_abbrev_cb(const struct option *, const char *, int); +extern int parse_opt_approxidate_cb(const struct option *, const char *, + int); #define OPT__VERBOSE(var) OPT_BOOLEAN('v', "verbose", (var), "be verbose") #define OPT__QUIET(var) OPT_BOOLEAN('q', "quiet", (var), "be quiet") -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] builtin-prune.c: fix object parsing and use parse_options() 2008-03-24 12:31 ` Michele Ballabio @ 2008-03-24 13:13 ` Johannes Schindelin 2008-03-24 14:02 ` [PATCH 3/2] parse-options.c: introduce OPT_DATE Michele Ballabio 0 siblings, 1 reply; 22+ messages in thread From: Johannes Schindelin @ 2008-03-24 13:13 UTC (permalink / raw) To: Michele Ballabio; +Cc: git, Junio C Hamano Hi, On Mon, 24 Mar 2008, Michele Ballabio wrote: > diff --git a/parse-options.h b/parse-options.h > index 1af62b0..e6976ed 100644 > --- a/parse-options.h > +++ b/parse-options.h > @@ -110,6 +110,8 @@ extern NORETURN void usage_with_options(const char * const *usagestr, > > /*----- some often used options -----*/ > extern int parse_opt_abbrev_cb(const struct option *, const char *, int); > +extern int parse_opt_approxidate_cb(const struct option *, const char *, > + int); > > #define OPT__VERBOSE(var) OPT_BOOLEAN('v', "verbose", (var), "be verbose") > #define OPT__QUIET(var) OPT_BOOLEAN('q', "quiet", (var), "be quiet") And maybe a #define OPT_DATE(s, l, v, h) { OPTION_CALLBACK, (s), (l), (v), "time", (h), 0, parse_opt_approxidate_cb } Hmm? Ciao, Dscho ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/2] parse-options.c: introduce OPT_DATE 2008-03-24 13:13 ` Johannes Schindelin @ 2008-03-24 14:02 ` Michele Ballabio 2008-03-24 13:59 ` Johannes Schindelin ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Michele Ballabio @ 2008-03-24 14:02 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Junio C Hamano There are quite a few places that will need to call approxidate(), when they'll adopt the parse-options system, so this patch adds the function parse_opt_approxidate_cb(), used by OPT_DATE, and converts the only user so far. Signed-off-by: Michele Ballabio <barra_cuda@katamail.com> --- On Monday 24 March 2008, Johannes Schindelin wrote: > And maybe a > > #define OPT_DATE(s, l, v, h) { OPTION_CALLBACK, (s), (l), (v), "time", > (h), 0, parse_opt_approxidate_cb } Oh, right. Somehow I thought it was simpler not to do this. Thank you for your review and suggestions. This is on top of [PATCH 2/2] builtin-prune.c: fix object parsing and use parse_options() builtin-prune.c | 12 ++---------- parse-options.c | 7 +++++++ parse-options.h | 5 +++++ 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/builtin-prune.c b/builtin-prune.c index 7b3e15d..40581df 100644 --- a/builtin-prune.c +++ b/builtin-prune.c @@ -125,13 +125,6 @@ static void remove_temporary_files(void) closedir(dir); } -static int parse_opt_expire(const struct option *opt, const char *arg, - int unset) -{ - expire = approxidate(arg); - return 0; -} - int cmd_prune(int argc, const char **argv, const char *prefix) { struct rev_info revs; @@ -139,9 +132,8 @@ int cmd_prune(int argc, const char **argv, const char *prefix) const struct option options[] = { OPT_BOOLEAN('n', NULL, &show_only, "do not remove, show only"), - OPT_CALLBACK(0, "expire", &expire, "time", - "expire objects older than <time>", - parse_opt_expire), + OPT_DATE(0, "expire", &expire, + "expire objects older than <time>"), OPT_END() }; diff --git a/parse-options.c b/parse-options.c index 8e64316..6ec7fe8 100644 --- a/parse-options.c +++ b/parse-options.c @@ -409,3 +409,10 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) *(int *)(opt->value) = v; return 0; } + +int parse_opt_approxidate_cb(const struct option *opt, const char *arg, + int unset) +{ + *(unsigned int *)(opt->value) = approxidate(arg); + return 0; +} diff --git a/parse-options.h b/parse-options.h index 1af62b0..c98f89e 100644 --- a/parse-options.h +++ b/parse-options.h @@ -110,6 +110,8 @@ extern NORETURN void usage_with_options(const char * const *usagestr, /*----- some often used options -----*/ extern int parse_opt_abbrev_cb(const struct option *, const char *, int); +extern int parse_opt_approxidate_cb(const struct option *, const char *, + int); #define OPT__VERBOSE(var) OPT_BOOLEAN('v', "verbose", (var), "be verbose") #define OPT__QUIET(var) OPT_BOOLEAN('q', "quiet", (var), "be quiet") @@ -118,5 +120,8 @@ extern int parse_opt_abbrev_cb(const struct option *, const char *, int); { OPTION_CALLBACK, 0, "abbrev", (var), "n", \ "use <n> digits to display SHA-1s", \ PARSE_OPT_OPTARG, &parse_opt_abbrev_cb, 0 } +#define OPT_DATE(s, l, v, h) \ + { OPTION_CALLBACK, (s), (l), (v), "time",(h), 0, \ + parse_opt_approxidate_cb } #endif -- 1.5.4.3 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 3/2] parse-options.c: introduce OPT_DATE 2008-03-24 14:02 ` [PATCH 3/2] parse-options.c: introduce OPT_DATE Michele Ballabio @ 2008-03-24 13:59 ` Johannes Schindelin 2008-03-24 16:25 ` Michele Ballabio 2008-03-24 20:10 ` Junio C Hamano 2 siblings, 0 replies; 22+ messages in thread From: Johannes Schindelin @ 2008-03-24 13:59 UTC (permalink / raw) To: Michele Ballabio; +Cc: git, Junio C Hamano Hi, On Mon, 24 Mar 2008, Michele Ballabio wrote: > There are quite a few places that will need to call approxidate(), when > they'll adopt the parse-options system, so this patch adds the function > parse_opt_approxidate_cb(), used by OPT_DATE, and converts the only user > so far. Thanks! Ciao, Dscho ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/2] parse-options.c: introduce OPT_DATE 2008-03-24 14:02 ` [PATCH 3/2] parse-options.c: introduce OPT_DATE Michele Ballabio 2008-03-24 13:59 ` Johannes Schindelin @ 2008-03-24 16:25 ` Michele Ballabio 2008-03-24 20:03 ` Johannes Schindelin 2008-03-24 20:10 ` Junio C Hamano 2 siblings, 1 reply; 22+ messages in thread From: Michele Ballabio @ 2008-03-24 16:25 UTC (permalink / raw) To: git; +Cc: Johannes Schindelin, Junio C Hamano On Monday 24 March 2008, Michele Ballabio wrote: > + OPT_DATE(0, "expire", &expire, [...] > +#define OPT_DATE(s, l, v, h) \ Ooops. To be consistent, these should be OPT__DATE (with two underscores) instead (and in the commit message, too). ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/2] parse-options.c: introduce OPT_DATE 2008-03-24 16:25 ` Michele Ballabio @ 2008-03-24 20:03 ` Johannes Schindelin 0 siblings, 0 replies; 22+ messages in thread From: Johannes Schindelin @ 2008-03-24 20:03 UTC (permalink / raw) To: Michele Ballabio; +Cc: git, Junio C Hamano [-- Attachment #1: Type: TEXT/PLAIN, Size: 495 bytes --] Hi, On Mon, 24 Mar 2008, Michele Ballabio wrote: > On Monday 24 March 2008, Michele Ballabio wrote: > > + OPT_DATE(0, "expire", &expire, > > [...] > > > +#define OPT_DATE(s, l, v, h) \ > > Ooops. To be consistent, these should be OPT__DATE (with two > underscores) instead (and in the commit message, too). I thought OPT__BLA was reserved for --bla options? IOW OPT__DATE would not be usable to implement --expire, but only --date. I might be wrong, though. Ciao, Dscho ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/2] parse-options.c: introduce OPT_DATE 2008-03-24 14:02 ` [PATCH 3/2] parse-options.c: introduce OPT_DATE Michele Ballabio 2008-03-24 13:59 ` Johannes Schindelin 2008-03-24 16:25 ` Michele Ballabio @ 2008-03-24 20:10 ` Junio C Hamano 2008-03-24 21:18 ` Michele Ballabio 2 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2008-03-24 20:10 UTC (permalink / raw) To: Michele Ballabio; +Cc: Johannes Schindelin, git Michele Ballabio <barra_cuda@katamail.com> writes: > +int parse_opt_approxidate_cb(const struct option *opt, const char *arg, > + int unset) > +{ > + *(unsigned int *)(opt->value) = approxidate(arg); Doesn't approxidate return ulong, not uint? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/2] parse-options.c: introduce OPT_DATE 2008-03-24 20:10 ` Junio C Hamano @ 2008-03-24 21:18 ` Michele Ballabio 2008-03-25 6:58 ` Junio C Hamano ` (5 more replies) 0 siblings, 6 replies; 22+ messages in thread From: Michele Ballabio @ 2008-03-24 21:18 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin, git On Monday 24 March 2008, Junio C Hamano wrote: > Michele Ballabio <barra_cuda@katamail.com> writes: > > > +int parse_opt_approxidate_cb(const struct option *opt, const char *arg, > > + int unset) > > +{ > > + *(unsigned int *)(opt->value) = approxidate(arg); > > Doesn't approxidate return ulong, not uint? Yes, you're right. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/2] parse-options.c: introduce OPT_DATE 2008-03-24 21:18 ` Michele Ballabio @ 2008-03-25 6:58 ` Junio C Hamano 2008-03-25 6:59 ` [PATCH 1/5] " Junio C Hamano ` (4 subsequent siblings) 5 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2008-03-25 6:58 UTC (permalink / raw) To: Michele Ballabio; +Cc: Johannes Schindelin, git Michele Ballabio <barra_cuda@katamail.com> writes: > On Monday 24 March 2008, Junio C Hamano wrote: >> Michele Ballabio <barra_cuda@katamail.com> writes: >> >> > +int parse_opt_approxidate_cb(const struct option *opt, const char *arg, >> > + int unset) >> > +{ >> > + *(unsigned int *)(opt->value) = approxidate(arg); >> >> Doesn't approxidate return ulong, not uint? > > Yes, you're right. Now, it is getting somewhat tiring to keep track of "oops, that was wrong, and here is a fix-up on top". So here is my attempt to summarize by presenting them in an order that I think is sensible. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/5] parse-options.c: introduce OPT_DATE 2008-03-24 21:18 ` Michele Ballabio 2008-03-25 6:58 ` Junio C Hamano @ 2008-03-25 6:59 ` Junio C Hamano 2008-03-25 6:59 ` [PATCH 2/5] test_must_fail: 129 is a valid error code from usage() Junio C Hamano ` (3 subsequent siblings) 5 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2008-03-25 6:59 UTC (permalink / raw) To: Michele Ballabio; +Cc: Johannes Schindelin, git From: Michele Ballabio <barra_cuda@katamail.com> Date: Mon, 24 Mar 2008 15:02:21 +0100 There are quite a few places that will need to call approxidate(), when they'll adopt the parse-options system, so this patch adds the function parse_opt_approxidate_cb(), used by OPT_DATE. Signed-off-by: Michele Ballabio <barra_cuda@katamail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- parse-options.c | 7 +++++++ parse-options.h | 4 ++++ 2 files changed, 11 insertions(+), 0 deletions(-) diff --git a/parse-options.c b/parse-options.c index 8e64316..e87cafb 100644 --- a/parse-options.c +++ b/parse-options.c @@ -409,3 +409,10 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) *(int *)(opt->value) = v; return 0; } + +int parse_opt_approxidate_cb(const struct option *opt, const char *arg, + int unset) +{ + *(unsigned long *)(opt->value) = approxidate(arg); + return 0; +} diff --git a/parse-options.h b/parse-options.h index 1af62b0..4ee443d 100644 --- a/parse-options.h +++ b/parse-options.h @@ -94,6 +94,9 @@ struct option { #define OPT_SET_PTR(s, l, v, h, p) { OPTION_SET_PTR, (s), (l), (v), NULL, (h), 0, NULL, (p) } #define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), NULL, (h) } #define OPT_STRING(s, l, v, a, h) { OPTION_STRING, (s), (l), (v), (a), (h) } +#define OPT_DATE(s, l, v, h) \ + { OPTION_CALLBACK, (s), (l), (v), "time",(h), 0, \ + parse_opt_approxidate_cb } #define OPT_CALLBACK(s, l, v, a, h, f) \ { OPTION_CALLBACK, (s), (l), (v), (a), (h), 0, (f) } @@ -110,6 +113,7 @@ extern NORETURN void usage_with_options(const char * const *usagestr, /*----- some often used options -----*/ extern int parse_opt_abbrev_cb(const struct option *, const char *, int); +extern int parse_opt_approxidate_cb(const struct option *, const char *, int); #define OPT__VERBOSE(var) OPT_BOOLEAN('v', "verbose", (var), "be verbose") #define OPT__QUIET(var) OPT_BOOLEAN('q', "quiet", (var), "be quiet") -- 1.5.5.rc1.121.g1594 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/5] test_must_fail: 129 is a valid error code from usage() 2008-03-24 21:18 ` Michele Ballabio 2008-03-25 6:58 ` Junio C Hamano 2008-03-25 6:59 ` [PATCH 1/5] " Junio C Hamano @ 2008-03-25 6:59 ` Junio C Hamano 2008-03-25 10:01 ` Johannes Schindelin 2008-03-25 6:59 ` [PATCH 3/5] Add tests for git-prune Junio C Hamano ` (2 subsequent siblings) 5 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2008-03-25 6:59 UTC (permalink / raw) To: Michele Ballabio; +Cc: Johannes Schindelin, git From: Junio C Hamano <gitster@pobox.com> Date: Mon, 24 Mar 2008 23:07:08 -0700 When a git command is run under test_must_fail to make sure that the argument parser catches bogus command line, it exits with 129. We need to catch it as a valid "graceful error exit". Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/test-lib.sh | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 870b255..7c2a8ba 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -300,7 +300,7 @@ test_expect_code () { test_must_fail () { "$@" - test $? -gt 0 -a $? -le 128 + test $? -gt 0 -a $? -le 129 } # test_cmp is a helper function to compare actual and expected output. -- 1.5.5.rc1.121.g1594 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/5] test_must_fail: 129 is a valid error code from usage() 2008-03-25 6:59 ` [PATCH 2/5] test_must_fail: 129 is a valid error code from usage() Junio C Hamano @ 2008-03-25 10:01 ` Johannes Schindelin 2008-03-25 11:21 ` Johannes Sixt 0 siblings, 1 reply; 22+ messages in thread From: Johannes Schindelin @ 2008-03-25 10:01 UTC (permalink / raw) To: Junio C Hamano; +Cc: Michele Ballabio, git Hi, On Mon, 24 Mar 2008, Junio C Hamano wrote: > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 870b255..7c2a8ba 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -300,7 +300,7 @@ test_expect_code () { > > test_must_fail () { > "$@" > - test $? -gt 0 -a $? -le 128 > + test $? -gt 0 -a $? -le 129 IIRC exit status is a signed byte on Win32. Can somebody check? I'll be in a hurry in 1.5 hours and travelling for two days. Ciao, Dscho ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/5] test_must_fail: 129 is a valid error code from usage() 2008-03-25 10:01 ` Johannes Schindelin @ 2008-03-25 11:21 ` Johannes Sixt 2008-03-25 19:27 ` Johannes Schindelin 0 siblings, 1 reply; 22+ messages in thread From: Johannes Sixt @ 2008-03-25 11:21 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Junio C Hamano, Michele Ballabio, git Johannes Schindelin schrieb: > Hi, > > On Mon, 24 Mar 2008, Junio C Hamano wrote: > >> diff --git a/t/test-lib.sh b/t/test-lib.sh >> index 870b255..7c2a8ba 100644 >> --- a/t/test-lib.sh >> +++ b/t/test-lib.sh >> @@ -300,7 +300,7 @@ test_expect_code () { >> >> test_must_fail () { >> "$@" >> - test $? -gt 0 -a $? -le 128 >> + test $? -gt 0 -a $? -le 129 > > IIRC exit status is a signed byte on Win32. Can somebody check? Not at the shell level. This command: git branch foo bar baz exits with 129 both when invoked by bash ($?) and CMD (%ERRORLEVEL%). -- Hannes ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/5] test_must_fail: 129 is a valid error code from usage() 2008-03-25 11:21 ` Johannes Sixt @ 2008-03-25 19:27 ` Johannes Schindelin 0 siblings, 0 replies; 22+ messages in thread From: Johannes Schindelin @ 2008-03-25 19:27 UTC (permalink / raw) To: Johannes Sixt; +Cc: Junio C Hamano, Michele Ballabio, git Hi, On Tue, 25 Mar 2008, Johannes Sixt wrote: > This command: > > git branch foo bar baz > > exits with 129 both when invoked by bash ($?) and CMD (%ERRORLEVEL%). Thanks, Dscho ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 3/5] Add tests for git-prune 2008-03-24 21:18 ` Michele Ballabio ` (2 preceding siblings ...) 2008-03-25 6:59 ` [PATCH 2/5] test_must_fail: 129 is a valid error code from usage() Junio C Hamano @ 2008-03-25 6:59 ` Junio C Hamano 2008-03-25 6:59 ` [PATCH 4/5] builtin-prune.c: use parse_options() Junio C Hamano 2008-03-25 6:59 ` [PATCH 5/5] builtin-prune: protect objects listed on the command line Junio C Hamano 5 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2008-03-25 6:59 UTC (permalink / raw) To: Michele Ballabio; +Cc: Johannes Schindelin, git From: Michele Ballabio <barra_cuda@katamail.com> Date: Sun, 23 Mar 2008 22:34:34 +0100 It seems that git prune changed behaviour with respect to revisions added from command line, probably when it became a builtin. Currently, it prints a short usage and exits: instead, it should take those revisions into account and not prune them. So add a couple of test to point this out. We'll be fixing this by switching to parse_options(), so add tests to detect bogus command line parameters as well, to keep ourselves from introducing regressions. Signed-off-by: Michele Ballabio <barra_cuda@katamail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t5304-prune.sh | 34 ++++++++++++++++++++++++++++++++++ 1 files changed, 34 insertions(+), 0 deletions(-) diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh index 47090c4..3d81e1f 100644 --- a/t/t5304-prune.sh +++ b/t/t5304-prune.sh @@ -78,4 +78,38 @@ test_expect_success 'gc: start with ok gc.pruneExpire' ' ' +test_expect_success 'prune: prune nonsense parameters' ' + + test_must_fail git prune garbage && + test_must_fail git prune --- && + test_must_fail git prune --no-such-option + +' + +test_expect_success 'prune: prune unreachable heads' ' + + git config core.logAllRefUpdates false && + mv .git/logs .git/logs.old && + : > file2 && + git add file2 && + git commit -m temporary && + tmp_head=$(git rev-list -1 HEAD) && + git reset HEAD^ && + git prune && + test_must_fail git reset $tmp_head -- + +' + +test_expect_failure 'prune: do not prune heads listed as an argument' ' + + : > file2 && + git add file2 && + git commit -m temporary && + tmp_head=$(git rev-list -1 HEAD) && + git reset HEAD^ && + git prune -- $tmp_head && + git reset $tmp_head -- + +' + test_done -- 1.5.5.rc1.121.g1594 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/5] builtin-prune.c: use parse_options() 2008-03-24 21:18 ` Michele Ballabio ` (3 preceding siblings ...) 2008-03-25 6:59 ` [PATCH 3/5] Add tests for git-prune Junio C Hamano @ 2008-03-25 6:59 ` Junio C Hamano 2008-03-25 6:59 ` [PATCH 5/5] builtin-prune: protect objects listed on the command line Junio C Hamano 5 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2008-03-25 6:59 UTC (permalink / raw) To: Michele Ballabio; +Cc: Johannes Schindelin, git From: Michele Ballabio <barra_cuda@katamail.com> Date: Sun, 23 Mar 2008 21:50:29 +0100 Using the OPT_DATE() introduced earlier, this updates builtin-prune to use parse_options(). Signed-off-by: Michele Ballabio <barra_cuda@katamail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin-prune.c | 38 ++++++++++++++++---------------------- 1 files changed, 16 insertions(+), 22 deletions(-) diff --git a/builtin-prune.c b/builtin-prune.c index bb8ead9..71caac5 100644 --- a/builtin-prune.c +++ b/builtin-prune.c @@ -4,8 +4,12 @@ #include "revision.h" #include "builtin.h" #include "reachable.h" +#include "parse-options.h" -static const char prune_usage[] = "git-prune [-n]"; +static const char * const prune_usage[] = { + "git-prune [-n] [--expire <time>] [--] [<head>...]", + NULL +}; static int show_only; static unsigned long expire; @@ -123,32 +127,22 @@ static void remove_temporary_files(void) int cmd_prune(int argc, const char **argv, const char *prefix) { - int i; struct rev_info revs; - - for (i = 1; i < argc; i++) { - const char *arg = argv[i]; - if (!strcmp(arg, "-n")) { - show_only = 1; - continue; - } - if (!strcmp(arg, "--expire")) { - if (++i < argc) { - expire = approxidate(argv[i]); - continue; - } - } - else if (!prefixcmp(arg, "--expire=")) { - expire = approxidate(arg + 9); - continue; - } - usage(prune_usage); - } + const struct option options[] = { + OPT_BOOLEAN('n', NULL, &show_only, + "do not remove, show only"), + OPT_DATE(0, "expire", &expire, + "expire objects older than <time>"), + OPT_END() + }; save_commit_buffer = 0; init_revisions(&revs, prefix); - mark_reachable_objects(&revs, 1); + argc = parse_options(argc, argv, options, prune_usage, 0); + if (argc) + die ("unrecognized argument: %s", name); + mark_reachable_objects(&revs, 1); prune_object_dir(get_object_directory()); sync(); -- 1.5.5.rc1.121.g1594 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 5/5] builtin-prune: protect objects listed on the command line 2008-03-24 21:18 ` Michele Ballabio ` (4 preceding siblings ...) 2008-03-25 6:59 ` [PATCH 4/5] builtin-prune.c: use parse_options() Junio C Hamano @ 2008-03-25 6:59 ` Junio C Hamano 2008-03-27 16:32 ` Junio C Hamano 5 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2008-03-25 6:59 UTC (permalink / raw) To: Michele Ballabio; +Cc: Johannes Schindelin, git From: Junio C Hamano <gitster@pobox.com> Date: Mon, 24 Mar 2008 23:20:51 -0700 Finally, this resurrects the documented behaviour to protect other objects listed on the command line from getting pruned. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * This is done deliberately differently from what you did. Because we do not want to accept "we allow losing what's reachable from master" with "git prune master..next", setup_revisions() is not the right thing to use for this command. builtin-prune.c | 12 ++++++++++-- t/t5304-prune.sh | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/builtin-prune.c b/builtin-prune.c index 71caac5..ca50cca 100644 --- a/builtin-prune.c +++ b/builtin-prune.c @@ -140,8 +140,16 @@ int cmd_prune(int argc, const char **argv, const char *prefix) init_revisions(&revs, prefix); argc = parse_options(argc, argv, options, prune_usage, 0); - if (argc) - die ("unrecognized argument: %s", name); + while (argc--) { + struct object *object; + unsigned char sha1[20]; + const char *name = *argv++; + + if (!get_sha1(name, sha1) && (object = parse_object(sha1))) + add_pending_object(&revs, object, ""); + else + die ("unrecognized argument: %s", name); + } mark_reachable_objects(&revs, 1); prune_object_dir(get_object_directory()); diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh index 3d81e1f..9fd9d07 100644 --- a/t/t5304-prune.sh +++ b/t/t5304-prune.sh @@ -100,7 +100,7 @@ test_expect_success 'prune: prune unreachable heads' ' ' -test_expect_failure 'prune: do not prune heads listed as an argument' ' +test_expect_success 'prune: do not prune heads listed as an argument' ' : > file2 && git add file2 && -- 1.5.5.rc1.121.g1594 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] builtin-prune: protect objects listed on the command line 2008-03-25 6:59 ` [PATCH 5/5] builtin-prune: protect objects listed on the command line Junio C Hamano @ 2008-03-27 16:32 ` Junio C Hamano 2008-03-27 16:35 ` Johannes Schindelin 0 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2008-03-27 16:32 UTC (permalink / raw) To: Michele Ballabio; +Cc: Johannes Schindelin, git Junio C Hamano <junio@pobox.com> writes: > From: Junio C Hamano <gitster@pobox.com> > Date: Mon, 24 Mar 2008 23:20:51 -0700 > > Finally, this resurrects the documented behaviour to protect other > objects listed on the command line from getting pruned. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > * This is done deliberately differently from what you did. Because we do > not want to accept "we allow losing what's reachable from master" with > "git prune master..next", setup_revisions() is not the right thing to > use for this command. Ping? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] builtin-prune: protect objects listed on the command line 2008-03-27 16:32 ` Junio C Hamano @ 2008-03-27 16:35 ` Johannes Schindelin 2008-03-27 21:11 ` Michele Ballabio 0 siblings, 1 reply; 22+ messages in thread From: Johannes Schindelin @ 2008-03-27 16:35 UTC (permalink / raw) To: Junio C Hamano; +Cc: Michele Ballabio, git Hi, On Thu, 27 Mar 2008, Junio C Hamano wrote: > Junio C Hamano <junio@pobox.com> writes: > > > From: Junio C Hamano <gitster@pobox.com> > > Date: Mon, 24 Mar 2008 23:20:51 -0700 > > > > Finally, this resurrects the documented behaviour to protect other > > objects listed on the command line from getting pruned. > > > > Signed-off-by: Junio C Hamano <gitster@pobox.com> --- > > * This is done deliberately differently from what you did. Because > > we do not want to accept "we allow losing what's reachable from > > master" with "git prune master..next", setup_revisions() is not the > > right thing to use for this command. > > Ping? I did not see any problem with your implementation, but I thought Michele would look more deeply, as he obviously cares about the to-be-fixed behaviour. Ciao, Dscho ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 5/5] builtin-prune: protect objects listed on the command line 2008-03-27 16:35 ` Johannes Schindelin @ 2008-03-27 21:11 ` Michele Ballabio 0 siblings, 0 replies; 22+ messages in thread From: Michele Ballabio @ 2008-03-27 21:11 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Junio C Hamano, git On Thursday 27 March 2008, Johannes Schindelin wrote: > On Thu, 27 Mar 2008, Junio C Hamano wrote: > > > Junio C Hamano <junio@pobox.com> writes: > > > > > From: Junio C Hamano <gitster@pobox.com> > > > Date: Mon, 24 Mar 2008 23:20:51 -0700 > > > > > > Finally, this resurrects the documented behaviour to protect other > > > objects listed on the command line from getting pruned. > > > > > > Signed-off-by: Junio C Hamano <gitster@pobox.com> --- > > > * This is done deliberately differently from what you did. Because > > > we do not want to accept "we allow losing what's reachable from > > > master" with "git prune master..next", setup_revisions() is not the > > > right thing to use for this command. > > > > Ping? > > I did not see any problem with your implementation Me neither, but, as a nitpick, wouldn't something like if (!get_sha1(name, sha1)) { object = parse_object(sha1); if (!object) die("bad object %s", name); } else die("unrecognized argument: %s", name); be a bit more useful? ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2008-03-27 21:02 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-03-23 20:50 [PATCH 2/2] builtin-prune.c: fix object parsing and use parse_options() Michele Ballabio 2008-03-23 22:21 ` Johannes Schindelin 2008-03-24 12:31 ` Michele Ballabio 2008-03-24 13:13 ` Johannes Schindelin 2008-03-24 14:02 ` [PATCH 3/2] parse-options.c: introduce OPT_DATE Michele Ballabio 2008-03-24 13:59 ` Johannes Schindelin 2008-03-24 16:25 ` Michele Ballabio 2008-03-24 20:03 ` Johannes Schindelin 2008-03-24 20:10 ` Junio C Hamano 2008-03-24 21:18 ` Michele Ballabio 2008-03-25 6:58 ` Junio C Hamano 2008-03-25 6:59 ` [PATCH 1/5] " Junio C Hamano 2008-03-25 6:59 ` [PATCH 2/5] test_must_fail: 129 is a valid error code from usage() Junio C Hamano 2008-03-25 10:01 ` Johannes Schindelin 2008-03-25 11:21 ` Johannes Sixt 2008-03-25 19:27 ` Johannes Schindelin 2008-03-25 6:59 ` [PATCH 3/5] Add tests for git-prune Junio C Hamano 2008-03-25 6:59 ` [PATCH 4/5] builtin-prune.c: use parse_options() Junio C Hamano 2008-03-25 6:59 ` [PATCH 5/5] builtin-prune: protect objects listed on the command line Junio C Hamano 2008-03-27 16:32 ` Junio C Hamano 2008-03-27 16:35 ` Johannes Schindelin 2008-03-27 21:11 ` Michele Ballabio
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).