Re: [PATCH 3/6] Introduce a new "printf format" for timestamps

[Junio C Hamano]

Junio C Hamano  writes:

> Johannes Schindelin  writes:
>
>> So let's introduce the pseudo format "PRItime" (currently simply being
>> "lu") so that it is easy later on to change the data type to time_t.
>
> The problem being solved is a good thing to solve, and 
>
>> -printf("author-time %lu\n", ci.author_time);
>> +printf("author-time %"PRItime"\n", ci.author_time);
> ...
> It would be better to introduce the timestamp_t we discussed earlier
> before (or at) this step, and typedef it to ulong first, and then in
> this step, change the above to
>
>   printf("author-time %"PRItime"\n", (timestamp_t)ci.author_time);
>
> to keep them in sync.

Nah, ignore me.  This was just me being silly.

I was somehow expecting (incorrecty) that we would pick one single
PRItime for everybody and end up doing an equivalent of

printf("%llu", (unsigned long long)(something_that_is_time_t))

But as long as the plan is to configure PRItime for the platform's
time_t (or whatever the final type for timestamp_t is), we do not
have to have any extra cast here.  The endgame will use the type
that is consistent with %PRItime for variables and structure fields,
and we do not want an extra cast.

Re: [PATCH 3/6] Introduce a new "printf format" for timestamps

[Junio C Hamano]

Johannes Schindelin  writes:

> So let's introduce the pseudo format "PRItime" (currently simply being
> "lu") so that it is easy later on to change the data type to time_t.

The problem being solved is a good thing to solve, and 

> - printf("author-time %lu\n", ci.author_time);
> + printf("author-time %"PRItime"\n", ci.author_time);

is one of the two ingredients to the solution for this line.  But
the final form would require casting ci.author_time to the type
expected by %PRItime format specifier.  With this change alone, you
could define PRItime to expect an winder type in the next step but
that would be a bad conversion.  IOW, changing only the format
without introducing an explicit cast appears to invite future
mistakes.

It would be better to introduce the timestamp_t we discussed earlier
before (or at) this step, and typedef it to ulong first, and then in
this step, change the above to

printf("author-time %"PRItime"\n", (timestamp_t)ci.author_time);

to keep them in sync.  And at a later step in the series, you can
update definition of PRItime and timestamp_t to make them wider at
the same time, and the changes in this patch like the above line
would not need to be touched again.

[PATCH 3/6] Introduce a new "printf format" for timestamps

[Johannes Schindelin]

Currently, Git's source code treats all timestamps as if they were
unsigned longs. Therefore, it is okay to write "%lu" when printing them.

There is a substantial problem with that, though: at least on Windows,
time_t is *larger* than unsigned long, and hence we will want to switch
to using time_t instead.

So let's introduce the pseudo format "PRItime" (currently simply being
"lu") so that it is easy later on to change the data type to time_t.

Signed-off-by: Johannes Schindelin 
---
 builtin/blame.c   |  6 +++---
 builtin/fsck.c|  2 +-
 builtin/log.c |  2 +-
 builtin/receive-pack.c|  4 ++--
 date.c| 26 +-
 fetch-pack.c  |  2 +-
 git-compat-util.h |  1 +
 t/helper/test-date.c  |  2 +-
 t/helper/test-parse-options.c |  2 +-
 upload-pack.c |  2 +-
 vcs-svn/fast_export.c |  4 ++--
 11 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index cffc6265408..c9486dd580b 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1736,11 +1736,11 @@ static int emit_one_suspect_detail(struct origin 
*suspect, int repeat)
get_commit_info(suspect->commit, , 1);
printf("author %s\n", ci.author.buf);
printf("author-mail %s\n", ci.author_mail.buf);
-   printf("author-time %lu\n", ci.author_time);
+   printf("author-time %"PRItime"\n", ci.author_time);
printf("author-tz %s\n", ci.author_tz.buf);
printf("committer %s\n", ci.committer.buf);
printf("committer-mail %s\n", ci.committer_mail.buf);
-   printf("committer-time %lu\n", ci.committer_time);
+   printf("committer-time %"PRItime"\n", ci.committer_time);
printf("committer-tz %s\n", ci.committer_tz.buf);
printf("summary %s\n", ci.summary.buf);
if (suspect->commit->object.flags & UNINTERESTING)
@@ -1853,7 +1853,7 @@ static const char *format_time(unsigned long time, const 
char *tz_str,
 
strbuf_reset(_buf);
if (show_raw_time) {
-   strbuf_addf(_buf, "%lu %s", time, tz_str);
+   strbuf_addf(_buf, "%"PRItime" %s", time, tz_str);
}
else {
const char *time_str;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 1a5caccd0f5..5413c76e7a6 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -407,7 +407,7 @@ static void fsck_handle_reflog_sha1(const char *refname, 
unsigned char *sha1,
if (timestamp && name_objects)
add_decoration(fsck_walk_options.object_names,
obj,
-   xstrfmt("%s@{%ld}", refname, 
timestamp));
+   xstrfmt("%s@{%"PRItime"}", refname, 
timestamp));
obj->used = 1;
mark_object_reachable(obj);
} else {
diff --git a/builtin/log.c b/builtin/log.c
index 55d20cc2d88..24612c2299a 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -903,7 +903,7 @@ static void get_patch_ids(struct rev_info *rev, struct 
patch_ids *ids)
 static void gen_message_id(struct rev_info *info, char *base)
 {
struct strbuf buf = STRBUF_INIT;
-   strbuf_addf(, "%s.%lu.git.%s", base,
+   strbuf_addf(, "%s.%"PRItime".git.%s", base,
(unsigned long) time(NULL),

git_committer_info(IDENT_NO_NAME|IDENT_NO_DATE|IDENT_STRICT));
info->message_id = strbuf_detach(, NULL);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 1dbb8a06922..4a878645847 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -456,12 +456,12 @@ static char *prepare_push_cert_nonce(const char *path, 
unsigned long stamp)
struct strbuf buf = STRBUF_INIT;
unsigned char sha1[20];
 
-   strbuf_addf(, "%s:%lu", path, stamp);
+   strbuf_addf(, "%s:%"PRItime, path, stamp);
hmac_sha1(sha1, buf.buf, buf.len, cert_nonce_seed, 
strlen(cert_nonce_seed));;
strbuf_release();
 
/* RFC 2104 5. HMAC-SHA1-80 */
-   strbuf_addf(, "%lu-%.*s", stamp, 20, sha1_to_hex(sha1));
+   strbuf_addf(, "%"PRItime"-%.*s", stamp, 20, sha1_to_hex(sha1));
return strbuf_detach(, NULL);
 }
 
diff --git a/date.c b/date.c
index a8848f6e141..97ab5fcc349 100644
--- a/date.c
+++ b/date.c
@@ -100,41 +100,41 @@ void show_date_relative(unsigned long time, int tz,
diff = now->tv_sec - time;
if (diff < 90) {
strbuf_addf(timebuf,
-Q_("%lu second ago", "%lu seconds ago", diff), diff);
+Q_("%"PRItime" second ago", "%"PRItime" seconds ago", 
diff), diff);
return;
}
/* Turn it into minutes */
diff = (diff + 30) / 60;
if (diff < 90) {
strbuf_addf(timebuf,
-