threads / discuss / 46897

Git for Windows: mingw.c's strange usage of creation time as ctime?

Subject: Git for Windows: mingw.c's strange usage of creation time as ctime?

## tl;dr

3 messages between Oct 4, 2017 and Oct 4, 2017.

replies: 2people: 2as markdown or json

Marc Strapetz· Oct 3, 2017, 22:29 UTC · lore

compat/mingw.c assigns the Windows file creation time [1] to Git's ctime fields at line 591 and at line 705:

buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));

ftCreationTime > ftLastWriteTime is actually possible after copying a file, so it makes sense to include this timestamp somehow in the Index, but I think it would be better to use the maximum of ftLastWriteTime and ftCreationTime here which is less confusing and closer to Unix's ctime:

buf->st_ctime = max(buf->st_mtime,
                     filetime_to_time_t(&(fdata.ftCreationTime));

-Marc

[1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa365739(v=vs.85).aspx

Jonathan Nieder· Oct 4, 2017, 02:23 UTC · re: Marc Strapetz · lore

Re: Git for Windows: mingw.c's strange usage of creation time as ctime?

+git-for-windows Hi Marc,

Marc Strapetz wrote:

Show 13 quoted lines

> compat/mingw.c assigns the Windows file creation time [1] to Git's
> ctime fields at line 591 and at line 705:
>
> buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));
>
> ftCreationTime > ftLastWriteTime is actually possible after copying
> a file, so it makes sense to include this timestamp somehow in the
> Index, but I think it would be better to use the maximum of
> ftLastWriteTime and ftCreationTime here which is less confusing and
> closer to Unix's ctime:
>
> buf->st_ctime = max(buf->st_mtime,
>                     filetime_to_time_t(&(fdata.ftCreationTime));

Can you say more about the practical effect? Is this causing a bug in practice, is it a bug waiting to happen, or is it making the code difficult to understand without any ill effect expected at run time? (Any of those three is a valuable kind of feedback to offer --- I'm just trying to figure out which one you mean.)

By the way, do you have core.trustctime set to true or false?

Thanks, Jonathan

> -Marc
>
> [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa365739(v=vs.85).aspx

Marc Strapetz· Oct 4, 2017, 07:57 UTC · re: Jonathan Nieder · lore

Re: Git for Windows: mingw.c's strange usage of creation time as ctime?

Hello Jonathan,

On 04.10.2017 04:23, Jonathan Nieder wrote:

Show 22 quoted lines

> +git-for-windows
> Hi Marc,
> 
> Marc Strapetz wrote:
> 
>> compat/mingw.c assigns the Windows file creation time [1] to Git's
>> ctime fields at line 591 and at line 705:
>>
>> buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));
>>
>> ftCreationTime > ftLastWriteTime is actually possible after copying
>> a file, so it makes sense to include this timestamp somehow in the
>> Index, but I think it would be better to use the maximum of
>> ftLastWriteTime and ftCreationTime here which is less confusing and
>> closer to Unix's ctime:
>>
>> buf->st_ctime = max(buf->st_mtime,
>>                      filetime_to_time_t(&(fdata.ftCreationTime));
> 
> Can you say more about the practical effect?  Is this causing a bug in
> practice, is it a bug waiting to happen, or is it making the code
> difficult to understand without any ill effect expected at run time?

It's mainly about understanding: as a Windows guy it is quite tempting to read "ctime" as "creationTime" and getting this confirmed in code above. (I guess) based on this understanding I wrote wrong Java code 8 years ago for all platforms. My bad of course that I was not digging into Unix sources here.

In practice, .git/index timestamps will become more similar to Linux, getting rid of "strange" old ctime values.

Drawback will be that after such a code update every Index entry will appear as outdated and the initial "git status" will have to compare all file contents. If this is not acceptable, a comment for the two offending lines would make sense.

> By the way, do you have core.trustctime set to true or false?

It seems to be set to true by default on Windows and usually I'm not changing it.

-Marc