git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Kjetil Barvik <barvik@broadpark.no>,
	Git Mailing List <git@vger.kernel.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: [PATCH 2/2] Windows: Skip fstat/lstat optimization in write_entry()
Date: Mon, 20 Apr 2009 10:17:00 +0200	[thread overview]
Message-ID: <49EC2F7C.8070209@viscovery.net> (raw)

From: Johannes Sixt <j6t@kdbg.org>

Commit e4c72923 (write_entry(): use fstat() instead of lstat() when file
is open, 2009-02-09) introduced an optimization of write_entry().
Unfortunately, we cannot take advantage of this optimization on Windows
because there is no guarantee that the time stamps are updated before the
file is closed:

  "The only guarantee about a file timestamp is that the file time is
   correctly reflected when the handle that makes the change is closed."

(http://msdn.microsoft.com/en-us/library/ms724290(VS.85).aspx)

The failure of this optimization on Windows can be observed most easily by
running a 'git checkout' that has to update several large files. In this
case, 'git checkout' will report modified files, but infact only the
timestamps were incorrectly recorded in the index, as can be verified by a
subsequent 'git diff', which shows no change.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 My gut feeling was right: We cannot have this optimization on Windows.

 http://thread.gmane.org/gmane.comp.version-control.git/108351/focus=108357

 I've a repository where I can reproduce the error quite easily and this
 fixes it.

 -- Hannes (who forgot to add Dscho and git@vger on the first send attempt)

 Makefile          |    8 ++++++++
 entry.c           |    3 ++-
 git-compat-util.h |    6 ++++++
 3 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 076a732..a01d603 100644
--- a/Makefile
+++ b/Makefile
@@ -167,6 +167,10 @@ all::
 # Define NO_EXTERNAL_GREP if you don't want "git grep" to ever call
 # your external grep (e.g., if your system lacks grep, if its grep is
 # broken, or spawning external process is slower than built-in grep git has).
+#
+# Define UNRELIABLE_FSTAT if your system's fstat does not return the same
+# information on a not yet closed file that lstat would return for the same
+# file after it was closed.

 GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -833,6 +837,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
 	NO_NSEC = YesPlease
 	USE_WIN32_MMAP = YesPlease
+	UNRELIABLE_FSTAT = UnfortunatelyYes
 	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/regex -Icompat/fnmatch
 	COMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
@@ -1111,6 +1116,9 @@ endif
 ifdef NO_EXTERNAL_GREP
 	BASIC_CFLAGS += -DNO_EXTERNAL_GREP
 endif
+ifdef UNRELIABLE_FSTAT
+	BASIC_CFLAGS += -DUNRELIABLE_FSTAT
+endif

 ifeq ($(TCLTK_PATH),)
 NO_TCLTK=NoThanks
diff --git a/entry.c b/entry.c
index 5daacc2..915514a 100644
--- a/entry.c
+++ b/entry.c
@@ -147,7 +147,8 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout

 		wrote = write_in_full(fd, new, size);
 		/* use fstat() only when path == ce->name */
-		if (state->refresh_cache && !to_tempfile && !state->base_dir_len) {
+		if (fstat_is_reliable() &&
+		    state->refresh_cache && !to_tempfile && !state->base_dir_len) {
 			fstat(fd, &st);
 			fstat_done = 1;
 		}
diff --git a/git-compat-util.h b/git-compat-util.h
index d94c683..bf00f35 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -409,4 +409,10 @@ void git_qsort(void *base, size_t nmemb, size_t size,
 #endif
 #endif

+#ifdef UNRELIABLE_FSTAT
+#define fstat_is_reliable() 0
+#else
+#define fstat_is_reliable() 1
+#endif
+
 #endif

-- 
1.6.3.rc1.989.ga175e.dirty

             reply	other threads:[~2009-04-20  8:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-20  8:17 Johannes Sixt [this message]
2009-04-20 10:05 ` [PATCH 2/2] Windows: Skip fstat/lstat optimization in write_entry() Johannes Schindelin
2009-04-20 11:03 ` Dmitry Potapov
2009-04-20 12:34   ` Hannu Koivisto
2009-04-20 12:58   ` Alex Riesen
2009-04-20 13:33     ` Dmitry Potapov
2009-04-20 13:54       ` Alex Riesen
2009-04-20 14:19         ` Johannes Sixt
2009-04-20 14:25           ` Alex Riesen
2009-04-20 14:27             ` Alex Riesen
2009-04-20 21:17       ` Junio C Hamano
2009-04-20 22:17         ` Alex Riesen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49EC2F7C.8070209@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=barvik@broadpark.no \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).