From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:33
Hi,
On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
There was a problem with racy conditions, which this revision fixes.
The problem was that fstat was using the builtin implementation, which for
for some reason is off by some amount of seconds. (This is probably due to
some leap-year issue in one of the implementations. However, Microsoft tells
us to use 116444736000000000 in http://support.microsoft.com/kb/167296, so
I'll stick with that.)
Also, since both stat and lstat proved to be rather slow, having our own
version of fstat is probably also wise. At least we now control all the
stat'ing, so we _know_ they are compatible.
Also note that this revision makes git_lstat call itself after modifying
the filename, instead of the builtin stat, for the same reasons.
At least some of these informations should go into the commit message,
too.
With the our own implementations of lstat & fstat, the following test cases
are now fixed:
t4116-apply-reverte.sh
ok 3: apply in reverse
t4200-rerere.sh
ok 17: young records still live
However, the following test cases seems to fail now:
t6024-recursive-merge.sh
FAIL 1: setup tests
FAIL 3: result contains a conflict
FAIL 4: virtual trees were processed
FAIL 5: refuse to merge binaries
See attached test case logs.
Are some of these test cases unstable, so the result will fluctuate on
Windows?
I saw some funny stuff on Windows, like test cases succeeding when run
interactively, but failing when run from "make test".
BTW it would have been way easier to apply your patch, had you followed
SubmittingPatches...
To make it easier on others, I just uploaded it into the "teststat" branch
on 4msysgit.git (subject to removal in a few days).
First comment: it seems git_fstat() is not declared properly, so there are
quite a few compiler warnings.
Running the tests now.
Ciao,
Dscho
From: Marius Storm-Olsen <hidden> Date: 2016-06-15 22:43:33
Johannes Schindelin said the following on 03.09.2007 15:33:
On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
quoted
There was a problem with racy conditions, which this revision fixes.
The problem was that fstat was using the builtin implementation, which for
for some reason is off by some amount of seconds. (This is probably due to
some leap-year issue in one of the implementations. However, Microsoft tells
us to use 116444736000000000 in http://support.microsoft.com/kb/167296, so
I'll stick with that.)
Also, since both stat and lstat proved to be rather slow, having our own
version of fstat is probably also wise. At least we now control all the
stat'ing, so we _know_ they are compatible.
Also note that this revision makes git_lstat call itself after modifying
the filename, instead of the builtin stat, for the same reasons.
At least some of these informations should go into the commit message,
too.
Sure
quoted
With the our own implementations of lstat & fstat, the following test cases
are now fixed:
t4116-apply-reverte.sh
ok 3: apply in reverse
t4200-rerere.sh
ok 17: young records still live
However, the following test cases seems to fail now:
t6024-recursive-merge.sh
FAIL 1: setup tests
FAIL 3: result contains a conflict
FAIL 4: virtual trees were processed
FAIL 5: refuse to merge binaries
See attached test case logs.
Are some of these test cases unstable, so the result will fluctuate on
Windows?
I saw some funny stuff on Windows, like test cases succeeding when run
interactively, but failing when run from "make test".
Ok, I ran 'make test', so maybe that's it? I'll rerun them later.
BTW it would have been way easier to apply your patch, had you followed
SubmittingPatches...
Heh, I actually tried, using the Thunderbird way. Of course the
attachments are non-conforming :-) What was the problem? Whitespace
issues, Windows EOL, attachments, or all of the above? :-)
To make it easier on others, I just uploaded it into the "teststat" branch
on 4msysgit.git (subject to removal in a few days).
Cool, thanks
First comment: it seems git_fstat() is not declared properly, so there are
quite a few compiler warnings.
/me slaps self. Right, sorry 'bout that. I'll amend the declaration in
git-compat-util.h.
From: Johannes Sixt <hidden> Date: 2016-06-15 22:43:33
Johannes Schindelin schrieb:
I saw some funny stuff on Windows, like test cases succeeding when run
interactively, but failing when run from "make test".
That's very likely the issue that we work around by inserting "sleep 1"
at strategic points, which is a timing (race condition) issue and does
not depend on interactive vs. "make test".
-- Hannes
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:33
Hi,
On Mon, 3 Sep 2007, Johannes Sixt wrote:
Johannes Schindelin schrieb:
quoted
I saw some funny stuff on Windows, like test cases succeeding when run
interactively, but failing when run from "make test".
That's very likely the issue that we work around by inserting "sleep 1" at
strategic points, which is a timing (race condition) issue and does not
depend on interactive vs. "make test".
Makes sense to me now. Especially around t5510 -- t5701 I see those (and
I do not run "make test" often, since it takes _ages_.
Thanks,
Dscho
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:33
Hi,
On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
Johannes Schindelin said the following on 03.09.2007 15:33:
quoted
BTW it would have been way easier to apply your patch, had you
followed SubmittingPatches...
Heh, I actually tried, using the Thunderbird way. Of course the
attachments are non-conforming :-) What was the problem? Whitespace
issues, Windows EOL, attachments, or all of the above? :-)
From: Marius Storm-Olsen <hidden> Date: 2016-06-15 22:43:33
Johannes Schindelin wrote:
On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
quoted
Johannes Schindelin said the following on 03.09.2007 15:33:
quoted
BTW it would have been way easier to apply your patch, had you
followed SubmittingPatches...
Heh, I actually tried, using the Thunderbird way. Of course the
attachments are non-conforming :-) What was the problem? Whitespace
issues, Windows EOL, attachments, or all of the above? :-)
git am said that the patch was empty.
Hmm, must be the attachments then. I'll use the 4msysgit.git repo from
now on. I assume it'll be ok if I +push to the teststat branch?
--
.marius
From: Marius Storm-Olsen <hidden> Date: 2016-06-15 22:43:33
Johannes Schindelin wrote:
To make it easier on others, I just uploaded it into the "teststat"
branch on 4msysgit.git (subject to removal in a few days).
Ok, I've updated the patch in the 4msysgit.git repo, 'teststat' branch.
RFC, and please test.
The patch also incorporates some of Hannes local changes, with some
modifications. Hannes, does it look ok for you? You can add a tag to the
commit message if you'd like, and just +push it.
On Hannes' request (and to which I fully agree), I've gone back to the
old implementation of filetime_to_time_t(), since it was a bit 'nasty'.
(If we want to target CE in the future, it will quite possibly break)
http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=f4f3fbddf6e0f16f66f94cedf66614e0e3643496
First comment: it seems git_fstat() is not declared properly, so
there are quite a few compiler warnings.
This is also fixed, of course.
Hope this is the final 'cut' :-)
Later!
--
.marius
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:33
Hi,
On Mon, 3 Sep 2007, Marius Storm-Olsen wrote:
Johannes Schindelin wrote:
quoted
To make it easier on others, I just uploaded it into the "teststat"
branch on 4msysgit.git (subject to removal in a few days).
Ok, I've updated the patch in the 4msysgit.git repo, 'teststat' branch.
RFC, and please test.
I'm too tired for more, but the first test which fails (consistently) is
t4200 here:
* FAIL 17: young records still live
test -f .git/rr-cache/08f6c39f296af7e0dd1b3b7d8bba18d0365f605f/preimage
&& test -f .git/rr-cache/4000000000000000000000000000000000000000/preimage
Ciao,
Dscho
From: Johannes Sixt <hidden> Date: 2016-06-15 22:43:33
Marius Storm-Olsen schrieb:
Johannes Schindelin wrote:
quoted
To make it easier on others, I just uploaded it into the "teststat"
branch on 4msysgit.git (subject to removal in a few days).
Ok, I've updated the patch in the 4msysgit.git repo, 'teststat' branch.
RFC, and please test.
Thanks a lot! I've pushed it out in mingw.git's master.
The reason that t4200-rerere.sh fails is that we now store UTC in st_mtime.
However, for the garbage-collection we compare this entry to a local time
stamp. Therefore, I've pushed out a fixup patch at the top of mingw.git's
devel branch that converts mtime to local time
(http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=1b62ecb31068af06c2fa7664f06c6c36316aac2c).
Would you kindly conduct the performance test with this patch? I'm afraid
that this makes us substantially slower.
-- Hannes
From: David Kastrup <hidden> Date: 2016-06-15 22:43:33
Johannes Sixt [off-list ref] writes:
Marius Storm-Olsen schrieb:
quoted
Johannes Schindelin wrote:
quoted
To make it easier on others, I just uploaded it into the "teststat"
branch on 4msysgit.git (subject to removal in a few days).
Ok, I've updated the patch in the 4msysgit.git repo, 'teststat' branch.
RFC, and please test.
Thanks a lot! I've pushed it out in mingw.git's master.
The reason that t4200-rerere.sh fails is that we now store UTC in
st_mtime. However, for the garbage-collection we compare this entry
to a local time stamp. Therefore, I've pushed out a fixup patch at
the top of mingw.git's devel branch that converts mtime to local
time
(http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=1b62ecb31068af06c2fa7664f06c6c36316aac2c). Would
you kindly conduct the performance test with this patch? I'm afraid
that this makes us substantially slower.
Wouldn't it make more sense to convert local time to mtime? That's
one conversion per second at most rather than one conversion per file.
--
David Kastrup
From: Marius Storm-Olsen <hidden> Date: 2016-06-15 22:43:33
Johannes Sixt said the following on 04.09.2007 09:41:
Marius Storm-Olsen schrieb:
quoted
Johannes Schindelin wrote:
quoted
To make it easier on others, I just uploaded it into the
"teststat" branch on 4msysgit.git (subject to removal in a few
days).
Ok, I've updated the patch in the 4msysgit.git repo, 'teststat'
branch. RFC, and please test.
Thanks a lot! I've pushed it out in mingw.git's master.
Ops, already in master branch?
Ok, I found out that the custom fstat function was incomplete, so I
completed it. However, since you've already pushed it to your main
branch, I've added it as a separate commit to the 4msysgit.git
'teststat' branch. It might also explain some of the testfailures we
were seeing, but I haven't finished the test run yet. (So, consider
the patch something to play with, and don't commit it to your 'master'
branch yet! ;-)
http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=f15974add93bdfa92775c77c00e7c65aefd42127
The reason that t4200-rerere.sh fails is that we now store UTC in
st_mtime. However, for the garbage-collection we compare this entry
to a local time stamp. Therefore, I've pushed out a fixup patch at
the top of mingw.git's devel branch that converts mtime to local
time
(http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=1b62ecb31068af06c2fa7664f06c6c36316aac2c).
Would you kindly conduct the performance test with this patch? I'm
afraid that this makes us substantially slower.
Ok, I can give it a performance test, but I tend to agree with David
Kastrup there. It would be better if we rather fix the places where we
check with the local timestamp instead; depending of course on how
many places we actually do this.
We'll see how much the timezone conversion in the custom stat
functions actually hurt us performance wise.
--
.marius
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:33
Hi,
On Tue, 4 Sep 2007, Johannes Sixt wrote:
Marius Storm-Olsen schrieb:
quoted
Johannes Schindelin wrote:
quoted
To make it easier on others, I just uploaded it into the "teststat"
branch on 4msysgit.git (subject to removal in a few days).
Ok, I've updated the patch in the 4msysgit.git repo, 'teststat' branch.
RFC, and please test.
Thanks a lot! I've pushed it out in mingw.git's master.
The reason that t4200-rerere.sh fails is that we now store UTC in st_mtime.
However, for the garbage-collection we compare this entry to a local time
stamp.
Thanks for the explanation.
Therefore, I've pushed out a fixup patch at the top of mingw.git's devel
branch that converts mtime to local time
On Linux, we compare to UTC to begin with, right? We should do that here,
too... So if time(NULL) does not return UTC on MinGW, we have to wrap
that function, too.
Ciao,
Dscho
Looks good, although you should now handle INVALID_HANDLE_VALUE at the
beginning of git_fstat() like this:
HANDLE fh = (HANDLE)_get_osfhandle(fd);
if (fh == INVALID_HANDLE_VALUE)
return -1; /* errno has been set */
if (GetFileInformationByHandle(...
Ok, I can give it a performance test, but I tend to agree with David
Kastrup there. It would be better if we rather fix the places where we
check with the local timestamp instead; depending of course on how many
places we actually do this.
We'll see how much the timezone conversion in the custom stat functions
actually hurt us performance wise.
I'd make the decision on the grounds of a perfomance test. If it turns out
that the penalty is bearable, we should keep this stuff private to the MinGW
build. Otherwise, we would need MinGW specific code at the call sites
(unless we can hide the opposite conversion in some other wrapper function).
... time passes ...
Ok, I just tested FileTimeToLocalFileTime() in a tight loop, and I can run
it 100,000,000 times per second. So I'm confident that there won't be any
noticable degradation with my proposed change.
-- Hannes
Looks good, although you should now handle INVALID_HANDLE_VALUE at the
beginning of git_fstat() like this:
HANDLE fh = (HANDLE)_get_osfhandle(fd);
if (fh == INVALID_HANDLE_VALUE)
return -1; /* errno has been set */
if (GetFileInformationByHandle(...
Actually, that's already handled.
GetFileType will report FILE_TYPE_UNKNOWN (0), and GetLastError() will
return a result != NO_ERROR (<-- so you can follow the codepath, but
it actually returns ERROR_INVALID_HANDLE (6))
So, it will fall all the way through the function, and end up
returning -1, with errno = EBADF.
(I use the
case FILE_TYPE_UNKNOWN:
if (GetLastError() != NO_ERROR)
break;
construct, since the documentation says that an FILE_TYPE_UNKNOWN is
still a _valid_ handle iff GetLastError() returns NO_ERROR. So, then
we pass it on to the normal fstat function to let that figure out what
we're dealing with)
quoted
Ok, I can give it a performance test, but I tend to agree with David
Kastrup there. It would be better if we rather fix the places where we
check with the local timestamp instead; depending of course on how many
places we actually do this.
We'll see how much the timezone conversion in the custom stat functions
actually hurt us performance wise.
I'd make the decision on the grounds of a perfomance test. If it turns out
that the penalty is bearable, we should keep this stuff private to the MinGW
build. Otherwise, we would need MinGW specific code at the call sites
(unless we can hide the opposite conversion in some other wrapper function).
... time passes ...
Ok, I just tested FileTimeToLocalFileTime() in a tight loop, and I can run
it 100,000,000 times per second. So I'm confident that there won't be any
noticable degradation with my proposed change.
Ok. I haven't done the performance test with Git yet, but we'll see.
If it's not noticeable, I'll add it to all the timestamps we have.
--
.marius
Looks good, although you should now handle INVALID_HANDLE_VALUE at the
beginning of git_fstat() like this:
Actually, that's already handled.
It's not a big deal: It's an unlikely code path, actually an indication of a
coding error, so you can leave your version.
quoted
Ok, I just tested FileTimeToLocalFileTime() in a tight loop, and I can
run it 100,000,000 times per second. So I'm confident that there won't
be any noticable degradation with my proposed change.
Ok. I haven't done the performance test with Git yet, but we'll see. If
it's not noticeable, I'll add it to all the timestamps we have.
From: Johannes Sixt <hidden> Date: 2016-06-15 22:43:33
Johannes Schindelin schrieb:
On Tue, 4 Sep 2007, Johannes Sixt wrote:
quoted
Therefore, I've pushed out a fixup patch at the top of mingw.git's devel
branch that converts mtime to local time
On Linux, we compare to UTC to begin with, right? We should do that here,
too... So if time(NULL) does not return UTC on MinGW, we have to wrap
that function, too.
According to MSDN, time(NULL) returns "the number of seconds elapsed since
[epoch] according to the system clock". Please don't ask me what "the system
clock" is.
Reading the implementation of time(), it starts with GetLocalTime(),
determines whether daylight saving is in effect, and continues with another
round of timezone adjustment - mind you: _not_ a timezone reversal (!!).
Doesn't this look extremely bogus?
It seems we really need a wrapper for time().
-- Hannes
From: Marius Storm-Olsen <hidden> Date: 2016-06-15 22:43:33
Johannes Sixt said the following on 04.09.2007 13:36:
Johannes Schindelin schrieb:
quoted
On Tue, 4 Sep 2007, Johannes Sixt wrote:
quoted
Therefore, I've pushed out a fixup patch at the top of
mingw.git's devel branch that converts mtime to local time
On Linux, we compare to UTC to begin with, right? We should do
that here, too... So if time(NULL) does not return UTC on MinGW,
we have to wrap that function, too.
According to MSDN, time(NULL) returns "the number of seconds
elapsed since [epoch] according to the system clock". Please don't
ask me what "the system clock" is.
Reading the implementation of time(), it starts with
GetLocalTime(), determines whether daylight saving is in effect,
and continues with another round of timezone adjustment - mind you:
_not_ a timezone reversal (!!). Doesn't this look extremely bogus?
It seems we really need a wrapper for time().
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:33
Hi,
On Tue, 4 Sep 2007, Johannes Sixt wrote:
Johannes Schindelin schrieb:
quoted
On Tue, 4 Sep 2007, Johannes Sixt wrote:
quoted
Therefore, I've pushed out a fixup patch at the top of mingw.git's devel
branch that converts mtime to local time
On Linux, we compare to UTC to begin with, right? We should do that here,
too... So if time(NULL) does not return UTC on MinGW, we have to wrap that
function, too.
According to MSDN, time(NULL) returns "the number of seconds elapsed since
[epoch] according to the system clock". Please don't ask me what "the system
clock" is.
I think I know. From my QEmu adventures I know that DOS/Windows expects
the system clock to be set to local time, in contrast to _all_ other
operating systems.
Reading the implementation of time(), it starts with GetLocalTime(),
determines whether daylight saving is in effect, and continues with
another round of timezone adjustment - mind you: _not_ a timezone
reversal (!!). Doesn't this look extremely bogus?
It seems we really need a wrapper for time().
I absolutely concur. Something like this (most of it is blatantly copied
from Marius' patch)?
-- snip --
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:33
Hi,
On Tue, 4 Sep 2007, Johannes Schindelin wrote:
On Tue, 4 Sep 2007, Johannes Sixt wrote:
quoted
Johannes Schindelin schrieb:
quoted
On Tue, 4 Sep 2007, Johannes Sixt wrote:
quoted
Therefore, I've pushed out a fixup patch at the top of mingw.git's
devel branch that converts mtime to local time
On Linux, we compare to UTC to begin with, right? We should do that
here, too... So if time(NULL) does not return UTC on MinGW, we have
to wrap that function, too.
According to MSDN, time(NULL) returns "the number of seconds elapsed
since [epoch] according to the system clock". Please don't ask me what
"the system clock" is.
I think I know. From my QEmu adventures I know that DOS/Windows expects
the system clock to be set to local time, in contrast to _all_ other
operating systems.
Now I am utterly confused. MSDN says
FILETIME
Contains a 64-bit value representing the number of 100-nanosecond
intervals since January 1, 1601 (UTC).
Hmm.
Ciao,
Dscho
From: Johannes Sixt <hidden> Date: 2016-06-15 22:43:33
Johannes Schindelin schrieb:
On Tue, 4 Sep 2007, Johannes Sixt wrote:
quoted
Reading the implementation of time(), it starts with GetLocalTime(),
determines whether daylight saving is in effect, and continues with
another round of timezone adjustment - mind you: _not_ a timezone
reversal (!!). Doesn't this look extremely bogus?
It seems we really need a wrapper for time().
I absolutely concur. Something like this (most of it is blatantly copied
from Marius' patch)?
Well, I don't think it'll make a difference. The tiny test program below
prints twice the same number. My analysis of the time() implementation is
obviously flawed.
-- Hannes
#include <windows.h>
#include <stdio.h>
#include <time.h>
int main()
{
time_t t = time(NULL);
FILETIME ft;
GetSystemTimeAsFileTime(&ft);
long long winTime = ((long long)ft.dwHighDateTime << 32)
+ ft.dwLowDateTime;
winTime -= 116444736000000000LL;
winTime /= 10000000;
printf("%d %d\n", t, (int) winTime);
return 0;
}
Neat, with the custom stat() changes cherry-picked on top of
4msysgit.git 'devel' branch, I only have one failing testcase
t6024-recursive-merge.sh
when running
$ NO_SYMLINKS=1 make -k
The rest are passing with flying colors!
--
.marius
Neat, with the custom stat() changes cherry-picked on top of
4msysgit.git 'devel' branch, I only have one failing testcase
t6024-recursive-merge.sh
when running
$ NO_SYMLINKS=1 make -k
The rest are passing with flying colors!
And that one will eventually pass if only you try it often enough. See
71ee4210c in mingw.git.
-- Hannes
Neat, with the custom stat() changes cherry-picked on top of 4msysgit.git
'devel' branch, I only have one failing testcase
t6024-recursive-merge.sh
when running
$ NO_SYMLINKS=1 make -k
The rest are passing with flying colors!
Really? It is failing again? IIRC it was Linus' patch (which I
cherry-picked into 59f8c189a5) that fixed it for me.
Ciao,
Dscho
Neat, with the custom stat() changes cherry-picked on top of 4msysgit.git
'devel' branch, I only have one failing testcase
t6024-recursive-merge.sh
when running
$ NO_SYMLINKS=1 make -k
The rest are passing with flying colors!
Bad news. I do not know if it was the newest version I tried, but I could
no longer fetch... said something about some bad file.
Ciao,
Dscho
Neat, with the custom stat() changes cherry-picked on top of 4msysgit.git
'devel' branch, I only have one failing testcase
t6024-recursive-merge.sh
when running
$ NO_SYMLINKS=1 make -k
The rest are passing with flying colors!
And that one will eventually pass if only you try it often enough. See
71ee4210c in mingw.git.
Do you have Linus' patch applied? The one where the config is read at the
start of write-tree?
The problem only occurred since we have core.crlf = input unilaterally
now.
Ciao,
Dscho
From: Marius Storm-Olsen <hidden> Date: 2016-06-15 22:43:33
Johannes Schindelin wrote:
On Tue, 4 Sep 2007, Marius Storm-Olsen wrote:
quoted
Neat, with the custom stat() changes cherry-picked on top of
4msysgit.git 'devel' branch, I only have one failing testcase
t6024-recursive-merge.sh when running $ NO_SYMLINKS=1 make -k
The rest are passing with flying colors!
Bad news. I do not know if it was the newest version I tried, but I
could no longer fetch... said something about some bad file.
Then you're missing this patch:
http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=f15974add93bdfa92775c77c00e7c65aefd42127
I guess the quickest way is to manually apply this patch and recompile.
(or add the '#undef fstat', and have git_fstat just 'return fstat(fd, buf)')
The problem is that without this patch fstat(0, buf) would fail with bad
filedescriptor instead of returning the st_mode = S_IFIFO.
--
.marius
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:33
Hi,
On Tue, 4 Sep 2007, Marius Storm-Olsen wrote:
Johannes Schindelin wrote:
quoted
On Tue, 4 Sep 2007, Marius Storm-Olsen wrote:
quoted
Neat, with the custom stat() changes cherry-picked on top of
4msysgit.git 'devel' branch, I only have one failing testcase
t6024-recursive-merge.sh when running $ NO_SYMLINKS=1 make -k
The rest are passing with flying colors!
Bad news. I do not know if it was the newest version I tried, but I
could no longer fetch... said something about some bad file.
Then you're missing this patch:
http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=f15974add93bdfa92775c77c00e7c65aefd42127
I guess the quickest way is to manually apply this patch and recompile.
(or add the '#undef fstat', and have git_fstat just 'return fstat(fd, buf)')
The problem is that without this patch fstat(0, buf) would fail with bad
filedescriptor instead of returning the st_mode = S_IFIFO.
I guessed as much, but could not fetch the patch, since fetch was broken
;-)
For some utterly strange reason, "git fetch" accessed "git-fetch" in the
cwd, not in /bin/... Funny.
Ciao,
Dscho
Neat, with the custom stat() changes cherry-picked on top of 4msysgit.git
'devel' branch, I only have one failing testcase
t6024-recursive-merge.sh
when running
$ NO_SYMLINKS=1 make -k
The rest are passing with flying colors!
And that one will eventually pass if only you try it often enough. See
71ee4210c in mingw.git.
Do you have Linus' patch applied? The one where the config is read at the
start of write-tree?
The problem only occurred since we have core.crlf = input unilaterally
now.
Yes, I have that patch, but I don't have core.crlf set.
There are a lot of tests that do a series of commits in a row, with file
changes such that the file size does _not_ change, aka "racy git" problem.
This problem is obviously not 100% fixed on MSys: We do not correctly detect
that there are files that might be modified ("racily clean" files). Some
tests are prone to trigger the bug, others not.
-- Hannes
From: David Kastrup <hidden> Date: 2016-06-15 22:43:33
Johannes Schindelin [off-list ref] writes:
On Tue, 4 Sep 2007, Marius Storm-Olsen wrote:
quoted
The problem is that without this patch fstat(0, buf) would fail
with bad filedescriptor instead of returning the st_mode = S_IFIFO.
I guessed as much, but could not fetch the patch, since fetch was broken
;-)
For some utterly strange reason, "git fetch" accessed "git-fetch" in
the cwd, not in /bin/... Funny.
Regardless of what you write in your PATH variable, Windows will
_always_ search for binaries first in your current work directory. It
is not really funny.
--
David Kastrup
From: Marius Storm-Olsen <hidden> Date: 2016-06-15 22:43:33
Johannes Schindelin wrote:
On Tue, 4 Sep 2007, Marius Storm-Olsen wrote:
quoted
Johannes Schindelin wrote:
quoted
Bad news. I do not know if it was the newest version I tried,
but I could no longer fetch... said something about some bad
file.
Then you're missing this patch:
http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=f15974add93bdfa92775c77c00e7c65aefd42127
I guess the quickest way is to manually apply this patch and
recompile. (or add the '#undef fstat', and have git_fstat just
'return fstat(fd, buf)')
The problem is that without this patch fstat(0, buf) would fail
with bad filedescriptor instead of returning the st_mode = S_IFIFO.
I guessed as much, but could not fetch the patch, since fetch was
broken ;-)
On Tue, Sep 04, 2007 at 01:57:38PM +0100, Johannes Schindelin wrote:
Hi,
On Tue, 4 Sep 2007, Johannes Schindelin wrote:
quoted
On Tue, 4 Sep 2007, Johannes Sixt wrote:
quoted
Johannes Schindelin schrieb:
quoted
On Tue, 4 Sep 2007, Johannes Sixt wrote:
quoted
Therefore, I've pushed out a fixup patch at the top of mingw.git's
devel branch that converts mtime to local time
On Linux, we compare to UTC to begin with, right? We should do that
here, too... So if time(NULL) does not return UTC on MinGW, we have
to wrap that function, too.
According to MSDN, time(NULL) returns "the number of seconds elapsed
since [epoch] according to the system clock". Please don't ask me what
"the system clock" is.
I think I know. From my QEmu adventures I know that DOS/Windows expects
the system clock to be set to local time, in contrast to _all_ other
operating systems.
Now I am utterly confused. MSDN says
FILETIME
Contains a 64-bit value representing the number of 100-nanosecond
intervals since January 1, 1601 (UTC).
Hmm.
[Warning: war stories ahead...]
If you really, really want to know more:
http://search.cpan.org/~shay/Win32-UTCFileTime-1.45/lib/Win32/UTCFileTime.pm
--
Rutger Nijlunsing ---------------------------------- eludias ed dse.nl
never attribute to a conspiracy which can be explained by incompetence
----------------------------------------------------------------------
From: Reece Dunn <hidden> Date: 2016-06-15 22:43:33
On 04/09/07, Rutger Nijlunsing [off-list ref] wrote:
On Tue, Sep 04, 2007 at 01:57:38PM +0100, Johannes Schindelin wrote:
quoted
Hi,
On Tue, 4 Sep 2007, Johannes Schindelin wrote:
quoted
On Tue, 4 Sep 2007, Johannes Sixt wrote:
quoted
Johannes Schindelin schrieb:
quoted
On Tue, 4 Sep 2007, Johannes Sixt wrote:
quoted
Therefore, I've pushed out a fixup patch at the top of mingw.git's
devel branch that converts mtime to local time
On Linux, we compare to UTC to begin with, right? We should do that
here, too... So if time(NULL) does not return UTC on MinGW, we have
to wrap that function, too.
According to MSDN, time(NULL) returns "the number of seconds elapsed
since [epoch] according to the system clock". Please don't ask me what
"the system clock" is.
I think I know. From my QEmu adventures I know that DOS/Windows expects
the system clock to be set to local time, in contrast to _all_ other
operating systems.
Now I am utterly confused. MSDN says
FILETIME
Contains a 64-bit value representing the number of 100-nanosecond
intervals since January 1, 1601 (UTC).
Hmm.
Hmm, this may explain something that I have been observing on
Windows+cygwin. When I run `git diff`, I sometimes get it reporting
that all (from what I can tell) files have changed, like a `find .
-type f -exec touch {} \;` command was run.
I was going to report this on a new thread, but this looks like a more
relevant place to do so.
My Windows machine is currently in Daylight Savings Time mode, and
from my observations, I have only seen this repeat first thing the
next day. I am not sure why, but every 24hrs, it looks as if the file
time reported by and checked from git is different to that reported by
stat.
I have not had time yet to play around with the mingw port and this
new stat implementation to see if it addresses this issue.
- Reece
From: Marius Storm-Olsen <hidden> Date: 2016-06-15 22:43:33
Rutger Nijlunsing said the following on 04.09.2007 23:02:
On Tue, Sep 04, 2007 at 01:57:38PM +0100, Johannes Schindelin wrote:
quoted
Now I am utterly confused. MSDN says
FILETIME
Contains a 64-bit value representing the number of 100-nanosecond
intervals since January 1, 1601 (UTC).
Hmm.
Thanks, seems like it's the right decision then to ensure that we use
UTC throughout Git on Windows
Hannes & Dscho, looks like we should revert
http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=683775c00d9fb95bcbe4632f95b67a96b902fa59
then, and rather apply Dscho's patch for a custom time()
implementation. Dscho, was the custom implementation of time() enough
to fix the issues for you?
--
.marius
From: Johannes Schindelin <hidden> Date: 2016-06-15 22:43:33
Hi,
On Wed, 5 Sep 2007, Marius Storm-Olsen wrote:
Rutger Nijlunsing said the following on 04.09.2007 23:02:
quoted
On Tue, Sep 04, 2007 at 01:57:38PM +0100, Johannes Schindelin wrote:
quoted
Now I am utterly confused. MSDN says
FILETIME
Contains a 64-bit value representing the number of 100-nanosecond
intervals since January 1, 1601 (UTC).
Hmm.
Thanks, seems like it's the right decision then to ensure that we use UTC
throughout Git on Windows
Hannes & Dscho, looks like we should revert
http://repo.or.cz/w/git/mingw/4msysgit.git?a=commitdiff;h=683775c00d9fb95bcbe4632f95b67a96b902fa59
then, and rather apply Dscho's patch for a custom time() implementation.
Dscho, was the custom implementation of time() enough to fix the issues
for you?
Umm. I am helplessly overloaded with non-Git work, and just made sure
that it compiles ;-) Besides, I did not really understand what I was
doing; just copying your code to convert from FILETIME to time_t.
Sorry,
Dscho
From: Johannes Sixt <hidden> Date: 2016-06-15 22:43:33
Johannes Sixt schrieb:
Marius Storm-Olsen schrieb:
quoted
Johannes Schindelin wrote:
quoted
To make it easier on others, I just uploaded it into the "teststat"
branch on 4msysgit.git (subject to removal in a few days).
Ok, I've updated the patch in the 4msysgit.git repo, 'teststat' branch.
RFC, and please test.
Thanks a lot! I've pushed it out in mingw.git's master.
The reason that t4200-rerere.sh fails is that we now store UTC in
st_mtime. However, for the garbage-collection we compare this entry to a
local time stamp.
This analysis is incorrect, I think. The reason we fail seems to be that
t4200 uses test-chmtime, which uses utime(). Likely, we need a wrapper for
that one?
-- Hannes
From: Marius Storm-Olsen <hidden> Date: 2016-06-15 22:43:33
Johannes Sixt wrote:
Johannes Sixt schrieb:
quoted
Marius Storm-Olsen schrieb:
quoted
Johannes Schindelin wrote:
quoted
To make it easier on others, I just uploaded it into the
"teststat" branch on 4msysgit.git (subject to removal in a few
days).
Ok, I've updated the patch in the 4msysgit.git repo, 'teststat'
branch. RFC, and please test.
Thanks a lot! I've pushed it out in mingw.git's master.
The reason that t4200-rerere.sh fails is that we now store UTC in
st_mtime. However, for the garbage-collection we compare this entry
to a local time stamp.
This analysis is incorrect, I think. The reason we fail seems to be
that t4200 uses test-chmtime, which uses utime(). Likely, we need a
wrapper for that one?
Ok, could you make a quick patch to add a git_utime() (and probably
git_time()) and see if the tests pass without the UTC to Local time patch?
--
.marius