git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 16/33] t3210: test for spurious error messages for dangling packed refs

From
Junio C Hamano <gitster@pobox.com>
Date
Apr 17, 2013, 22:38 UTC
Message-ID
<7vbo9ceqb3.fsf@alter.siamese.dyndns.org>
In-Reply-To
<7v38upix82.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> Do we want to use approxidate_careful here to catch other junk?
>
> We can catch a misspelt argument or configuration value that way.
> That would be a good idea.
-- >8 --
Subject: date.c: add parse_expiry_date()

"git reflog --expire=all" tries to expire reflog entries up to the current second, because the approxidate() parser gives the current timestamp for anything it does not understand (and it does not know what time "all" means). When the user tells us to expire "all" (or set the expiration time to "now"), the user wants to remove all the reflog entries (no reflog entry should record future time).

Just set it to ULONG_MAX and to let everything that is older that timestamp expire.

While at it, allow "now" to be treated the same way for callers that parse expiry date timestamp with this function. Also use an error reporting version of approxidate() to report misspelled date. When the user says e.g. "--expire=mnoday" to delete entries two days or older on Wednesday, we wouldn't want the "unknown, default to now" logic to kick in.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * adoption to more users and tests are left as an exercise to the
   reader.
 builtin/reflog.c | 14 +++++++-------
 cache.h          |  1 +
 date.c           | 22 ++++++++++++++++++++++
 3 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index b3c9e27..44700f9 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -496,11 +496,9 @@ static int parse_expire_cfg_value(const char *var, const char *value, unsigned l
 {
 	if (!value)
 		return config_error_nonbool(var);
-	if (!strcmp(value, "never") || !strcmp(value, "false")) {
-		*expire = 0;
-		return 0;
-	}
-	*expire = approxidate(value);
+	if (parse_expiry_date(value, expire))
+		return error(_("%s' for '%s' is not a valid timestamp"),
+			     value, var);
 	return 0;
 }
 
@@ -613,11 +611,13 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 		if (!strcmp(arg, "--dry-run") || !strcmp(arg, "-n"))
 			cb.dry_run = 1;
 		else if (!prefixcmp(arg, "--expire=")) {
-			cb.expire_total = approxidate(arg + 9);
+			if (parse_expiry_date(arg + 9, &cb.expire_total))
+				die(_("'%s' is not a valid timestamp"), arg);
 			explicit_expiry |= EXPIRE_TOTAL;
 		}
 		else if (!prefixcmp(arg, "--expire-unreachable=")) {
-			cb.expire_unreachable = approxidate(arg + 21);
+			if (parse_expiry_date(arg + 21, &cb.expire_unreachable))
+				die(_("'%s' is not a valid timestamp"), arg);
 			explicit_expiry |= EXPIRE_UNREACH;
 		}
 		else if (!strcmp(arg, "--stale-fix"))
diff --git a/cache.h b/cache.h
index 3622e18..f43f6d9 100644
--- a/cache.h
+++ b/cache.h
@@ -878,6 +878,7 @@ void show_date_relative(unsigned long time, int tz, const struct timeval *now,
 			struct strbuf *timebuf);
 int parse_date(const char *date, char *buf, int bufsize);
 int parse_date_basic(const char *date, unsigned long *timestamp, int *offset);
+int parse_expiry_date(const char *date, unsigned long *timestamp);
 void datestamp(char *buf, int bufsize);
 #define approxidate(s) approxidate_careful((s), NULL)
 unsigned long approxidate_careful(const char *, int *);
diff --git a/date.c b/date.c
index 57331ed..876d679 100644
--- a/date.c
+++ b/date.c
@@ -705,6 +705,28 @@ int parse_date_basic(const char *date, unsigned long *timestamp, int *offset)
 	return 0; /* success */
 }
 
+int parse_expiry_date(const char *date, unsigned long *timestamp)
+{
+	int errors = 0;
+
+	if (!strcmp(date, "never") || !strcmp(date, "false"))
+		*timestamp = 0;
+	else if (!strcmp(date, "all") || !strcmp(date, "now"))
+		/*
+		 * We take over "now" here, which usually translates
+		 * to the current timestamp.  This is because the user
+		 * really means to expire everything she has done in
+		 * the past, and by definition reflogs are the record
+		 * of the past, and there is nothing from the future
+		 * to be kept.
+		 */
+		*timestamp = ULONG_MAX;
+	else
+		*timestamp = approxidate_careful(date, &errors);
+
+	return errors;
+}
+
 int parse_date(const char *date, char *result, int maxlen)
 {
 	unsigned long timestamp;
Previous: Junio C HamanoNext: Michael Haggerty
Message 38 of 64 in “Various cleanups around reference packing and peeling”
  1. 00/33 Various cleanups around reference packing and peelingMichael Haggerty, Apr 14, 2013
  2. 01/33 refs: document flags constants REF_*Michael Haggerty, Apr 14, 2013
  3. 02/33 refs: document the fields of struct ref_valueMichael Haggerty, Apr 14, 2013
  4. 03/33 refs: document do_for_each_ref() and do_one_ref()Michael Haggerty, Apr 14, 2013
  5. Junio C HamanoApr 15, 2013
  6. Michael HaggertyApr 16, 2013
  7. 04/33 refs: document how current_ref is usedMichael Haggerty, Apr 14, 2013
  8. 05/33 refs: define constant PEELED_LINE_LENGTHMichael Haggerty, Apr 14, 2013
  9. 06/33 do_for_each_ref_in_dirs(): remove dead codeMichael Haggerty, Apr 14, 2013
  10. 07/33 get_packed_ref(): return a ref_entryMichael Haggerty, Apr 14, 2013
  11. 08/33 peel_ref(): use function get_packed_ref()Michael Haggerty, Apr 14, 2013
  12. 09/33 repack_without_ref(): use function get_packed_ref()Michael Haggerty, Apr 14, 2013
  13. 10/33 refs: extract a function ref_resolves_to_object()Michael Haggerty, Apr 14, 2013
  14. Junio C HamanoApr 15, 2013
  15. Michael HaggertyApr 16, 2013
  16. Junio C HamanoApr 16, 2013
  17. 11/33 refs: extract function peel_object()Michael Haggerty, Apr 14, 2013
  18. 12/33 peel_object(): give more specific information in return valueMichael Haggerty, Apr 14, 2013
  19. Junio C HamanoApr 15, 2013
  20. 13/33 peel_ref(): fix return value for non-peelable, not-current referenceMichael Haggerty, Apr 14, 2013
  21. Junio C HamanoApr 15, 2013
  22. Michael HaggertyApr 16, 2013
  23. 14/33 refs: extract a function peel_entry()Michael Haggerty, Apr 14, 2013
  24. Junio C HamanoApr 15, 2013
  25. Michael HaggertyApr 16, 2013
  26. Junio C HamanoApr 16, 2013
  27. Michael HaggertyApr 16, 2013
  28. 15/33 refs: change the internal reference-iteration APIMichael Haggerty, Apr 14, 2013
  29. Junio C HamanoApr 15, 2013
  30. Michael HaggertyApr 16, 2013
  31. Junio C HamanoApr 16, 2013
  32. 16/33 t3210: test for spurious error messages for dangling packed refsMichael Haggerty, Apr 14, 2013
  33. Junio C HamanoApr 15, 2013
  34. Michael HaggertyApr 16, 2013
  35. Junio C HamanoApr 16, 2013
  36. Jeff KingApr 16, 2013
  37. Junio C HamanoApr 17, 2013
  38. Junio C HamanoApr 17, 2013
  39. 0/2 Add documentation for new expiry option valuesMichael Haggerty, Apr 18, 2013
  40. 1/2 git-gc.txt, git-reflog.txt: document new expiry optionsMichael Haggerty, Apr 18, 2013
  41. 2/2 api-parse-options.txt: document "no-" for non-boolean optionsMichael Haggerty, Apr 18, 2013
  42. prune: introduce OPT_EXPIRY_DATE() and use itJunio C Hamano, Apr 25, 2013
  43. Michael HaggertyApr 17, 2013
  44. 17/33 repack_without_ref(): silence errors for dangling packed refsMichael Haggerty, Apr 14, 2013
  45. Junio C HamanoApr 15, 2013
  46. Michael HaggertyApr 17, 2013
  47. 18/33 search_ref_dir(): return an index rather than a pointerMichael Haggerty, Apr 14, 2013
  48. 19/33 refs: change how packed refs are deletedMichael Haggerty, Apr 14, 2013
  49. 20/33 t3211: demonstrate loss of peeled refs if a packed ref is deletedMichael Haggerty, Apr 14, 2013
  50. 21/33 repack_without_ref(): write peeled refs in the rewritten fileMichael Haggerty, Apr 14, 2013
  51. Junio C HamanoApr 15, 2013
  52. 22/33 refs: extract a function write_packed_entry()Michael Haggerty, Apr 14, 2013
  53. 23/33 pack-refs: rename handle_one_ref() to pack_one_ref()Michael Haggerty, Apr 14, 2013
  54. 24/33 pack-refs: merge code from pack-refs.{c,h} into refs.{c,h}Michael Haggerty, Apr 14, 2013
  55. 25/33 pack_one_ref(): rename "path" parameter to "refname"Michael Haggerty, Apr 14, 2013
  56. 26/33 refs: use same lock_file object for both ref-packing functionsMichael Haggerty, Apr 14, 2013
  57. 27/33 pack_refs(): change to use do_for_each_entry()Michael Haggerty, Apr 14, 2013
  58. 28/33 refs: inline function do_not_prune()Michael Haggerty, Apr 14, 2013
  59. 29/33 pack_one_ref(): use function peel_entry()Michael Haggerty, Apr 14, 2013
  60. 30/33 pack_one_ref(): use write_packed_entry() to do the writingMichael Haggerty, Apr 14, 2013
  61. 31/33 pack_one_ref(): do some cheap tests before a more expensive oneMichael Haggerty, Apr 14, 2013
  62. 32/33 refs: change do_for_each_*() functions to take ref_cache argumentsMichael Haggerty, Apr 14, 2013
  63. Junio C HamanoApr 15, 2013
  64. 33/33 refs: handle the main ref_cache speciallyMichael Haggerty, Apr 14, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.