Re: [PATCH 2/2] rerere: allow approxidate in gc.rerereResolved/gc.rerereUnresolved
Ramsay Jones writes: > On 19/08/17 21:30, Junio C Hamano wrote: > >> In the future, we may find other variables that only allow an >> integer that specifies "this many days" (or other unit of time) and >> allow them to also do the same, and at that point we probably would >> want to move the helper to a place that is not specific to the >> rerere machinery. Perhaps config.c would be such a good neutral >> place, as it will allow git_parse_signed() to go back to static to >> the file. > > You make git_parse_unsigned() external in this patch, in addition > to git_parse_signed(), but don't actually call it. Was that intended? Yes, it was done on purpose for symmetry and was done before I had that "perhaps this can be moved to config.c" vision. Perhaps I should follow up the topic with [PATCH 3/2] to really move it to config.c to clean it up. But not today. Thanks.
Re: [PATCH 2/2] rerere: allow approxidate in gc.rerereResolved/gc.rerereUnresolved
On 19/08/17 21:30, Junio C Hamano wrote:
> These two configuration variables are described in the documentation
> to take an expiry period expressed in the number of days:
>
> gc.rerereResolved::
> Records of conflicted merge you resolved earlier are
> kept for this many days when 'git rerere gc' is run.
> The default is 60 days.
>
> gc.rerereUnresolved::
> Records of conflicted merge you have not resolved are
> kept for this many days when 'git rerere gc' is run.
> The default is 15 days.
>
> There is no strong reason not to allow a more general "approxidate"
> expiry specification, e.g. "5.days.ago", or "never".
>
> Tweak the config_get_expiry() helper introduced in the previous step
> to use date.c::parse_expiry_date() to do so.
>
> In the future, we may find other variables that only allow an
> integer that specifies "this many days" (or other unit of time) and
> allow them to also do the same, and at that point we probably would
> want to move the helper to a place that is not specific to the
> rerere machinery. Perhaps config.c would be such a good neutral
> place, as it will allow git_parse_signed() to go back to static to
> the file.
You make git_parse_unsigned() external in this patch, in addition
to git_parse_signed(), but don't actually call it. Was that intended?
ATB,
Ramsay Jones
>
> But this will do for now.
>
> Signed-off-by: Junio C Hamano
> ---
> Documentation/config.txt | 2 ++
> config.c | 4 ++--
> config.h | 3 +++
> rerere.c | 14 --
> 4 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index d5c9c4cab6..ac95f5f954 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1553,11 +1553,13 @@ gc..reflogExpireUnreachable::
> gc.rerereResolved::
> Records of conflicted merge you resolved earlier are
> kept for this many days when 'git rerere gc' is run.
> + You can also use more human-readable "1.month.ago", etc.
> The default is 60 days. See linkgit:git-rerere[1].
>
> gc.rerereUnresolved::
> Records of conflicted merge you have not resolved are
> kept for this many days when 'git rerere gc' is run.
> + You can also use more human-readable "1.month.ago", etc.
> The default is 15 days. See linkgit:git-rerere[1].
>
> gitcvs.commitMsgAnnotation::
> diff --git a/config.c b/config.c
> index 231f9a750b..ac9071c5cf 100644
> --- a/config.c
> +++ b/config.c
> @@ -769,7 +769,7 @@ static int parse_unit_factor(const char *end, uintmax_t
> *val)
> return 0;
> }
>
> -static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
> +int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
> {
> if (value && *value) {
> char *end;
> @@ -799,7 +799,7 @@ static int git_parse_signed(const char *value, intmax_t
> *ret, intmax_t max)
> return 0;
> }
>
> -static int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t
> max)
> +int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max)
> {
> if (value && *value) {
> char *end;
> diff --git a/config.h b/config.h
> index 0352da117b..039a9295de 100644
> --- a/config.h
> +++ b/config.h
> @@ -215,4 +215,7 @@ struct key_value_info {
> extern NORETURN void git_die_config(const char *key, const char *err, ...)
> __attribute__((format(printf, 2, 3)));
> extern NORETURN void git_die_config_linenr(const char *key, const char
> *filename, int linenr);
>
> +int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max);
> +int git_parse_signed(const char *value, intmax_t *ret, intmax_t max);
> +
> #endif /* CONFIG_H */
> diff --git a/rerere.c b/rerere.c
> index f0b4bce881..8bbdfe8569 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -1178,11 +1178,21 @@ static void prune_one(struct rerere_id *id,
>
> static void config_get_expiry(const char *key, timestamp_t *cutoff,
> timestamp_t now)
> {
> - int days;
> + char *expiry_string;
> + intmax_t days;
> + timestamp_t when;
>
> - if (!git_config_get_int(key, &days)) {
> + if (git_config_get_string(key, &expiry_string))
> + return;
> +
> + if (git_parse_signed(expiry_string, &days,
> maximum_signed_value_of_type(int))) {
> const int scale = 86400;
> *cutoff = now - days * scale;
> + return;
> + }
> +
> + if (!parse_expiry_date(expiry_string, &when)) {
> + *cutoff = when;
> }
> }
>
>
[PATCH 2/2] rerere: allow approxidate in gc.rerereResolved/gc.rerereUnresolved
These two configuration variables are described in the documentation
to take an expiry period expressed in the number of days:
gc.rerereResolved::
Records of conflicted merge you resolved earlier are
kept for this many days when 'git rerere gc' is run.
The default is 60 days.
gc.rerereUnresolved::
Records of conflicted merge you have not resolved are
kept for this many days when 'git rerere gc' is run.
The default is 15 days.
There is no strong reason not to allow a more general "approxidate"
expiry specification, e.g. "5.days.ago", or "never".
Tweak the config_get_expiry() helper introduced in the previous step
to use date.c::parse_expiry_date() to do so.
In the future, we may find other variables that only allow an
integer that specifies "this many days" (or other unit of time) and
allow them to also do the same, and at that point we probably would
want to move the helper to a place that is not specific to the
rerere machinery. Perhaps config.c would be such a good neutral
place, as it will allow git_parse_signed() to go back to static to
the file.
But this will do for now.
Signed-off-by: Junio C Hamano
---
Documentation/config.txt | 2 ++
config.c | 4 ++--
config.h | 3 +++
rerere.c | 14 --
4 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d5c9c4cab6..ac95f5f954 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1553,11 +1553,13 @@ gc..reflogExpireUnreachable::
gc.rerereResolved::
Records of conflicted merge you resolved earlier are
kept for this many days when 'git rerere gc' is run.
+ You can also use more human-readable "1.month.ago", etc.
The default is 60 days. See linkgit:git-rerere[1].
gc.rerereUnresolved::
Records of conflicted merge you have not resolved are
kept for this many days when 'git rerere gc' is run.
+ You can also use more human-readable "1.month.ago", etc.
The default is 15 days. See linkgit:git-rerere[1].
gitcvs.commitMsgAnnotation::
diff --git a/config.c b/config.c
index 231f9a750b..ac9071c5cf 100644
--- a/config.c
+++ b/config.c
@@ -769,7 +769,7 @@ static int parse_unit_factor(const char *end, uintmax_t
*val)
return 0;
}
-static int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
+int git_parse_signed(const char *value, intmax_t *ret, intmax_t max)
{
if (value && *value) {
char *end;
@@ -799,7 +799,7 @@ static int git_parse_signed(const char *value, intmax_t
*ret, intmax_t max)
return 0;
}
-static int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max)
+int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max)
{
if (value && *value) {
char *end;
diff --git a/config.h b/config.h
index 0352da117b..039a9295de 100644
--- a/config.h
+++ b/config.h
@@ -215,4 +215,7 @@ struct key_value_info {
extern NORETURN void git_die_config(const char *key, const char *err, ...)
__attribute__((format(printf, 2, 3)));
extern NORETURN void git_die_config_linenr(const char *key, const char
*filename, int linenr);
+int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max);
+int git_parse_signed(const char *value, intmax_t *ret, intmax_t max);
+
#endif /* CONFIG_H */
diff --git a/rerere.c b/rerere.c
index f0b4bce881..8bbdfe8569 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1178,11 +1178,21 @@ static void prune_one(struct rerere_id *id,
static void config_get_expiry(const char *key, timestamp_t *cutoff,
timestamp_t now)
{
- int days;
+ char *expiry_string;
+ intmax_t days;
+ timestamp_t when;
- if (!git_config_get_int(key, &days)) {
+ if (git_config_get_string(key, &expiry_string))
+ return;
+
+ if (git_parse_signed(expiry_string, &days,
maximum_signed_value_of_type(int))) {
const int scale = 86400;
*cutoff = now - days * scale;
+ return;
+ }
+
+ if (!parse_expiry_date(expiry_string, &when)) {
+ *cutoff = when;
}
}
--
2.14.1-405-g52c75fc716
