Re: Submodule/contents conflict

From: Jeff King <peff@peff.net>
Date: Apr 28, 2017, 08:30 UTC
Message-ID: <20170428083015.m2vowojjcdgfkcak@sigill.intra.peff.net>
In-Reply-To: <B12D69ADE0C84BBBB9A35C50F1C74A01@PhilipOakley>

On Thu, Apr 27, 2017 at 11:52:24PM +0100, Philip Oakley wrote:

Show 8 quoted lines

> > But note that none of that should ever affect _what_ gets checked out at
> > a file or content level. It may only affect the timestamps on the
> > resulting files. And I think those timestamps are not something Git
> > makes any promises about.
> 
> It's not actually clear to users what Git promises in cases like this which
> confuses user expectations - the make file issue does appear to come up
> quite often.

Yeah, I wouldn't mind a documentation update there. I have no idea where to put it that people would find it, though.

Show 18 quoted lines

> > So if Git can elide a write and keep a timestamp up to date, that is a
> > great optimization and we should do that. But from an outside viewer's
> > perspective, we guarantee nothing. We might choose to rewrite a
> > stat-dirty file (updating its timestamp) rather than read its contents
> > to see if it might have the same content that we're about to write. And
> > the file may look stat dirty only because of the racy-git file/index
> > timestamp problem, even if it wasn't actually modified.
> 
> I'm not sure how in this case we would get the stat-dirty state? We should
> be able to determine that the file has existed, as originally checked out,
> for some while (i.e. past the racy FS time) and is unmodified, so that as
> long as the original checkout OID and the required new OID are the same we
> should be able to avoid the file overwrite (or merge).
> 
> It would require that the now time be used as a stand-in for the new OID
> file's stat time (given the object store doesn't store date stamps for
> files) to check for the racy-git situation. These negative information
> scenarios can be tricky.

I don't think the "now" time during the read is relevant for racy git. The problem is the timestamp of the index versus the timestamp of the file itself. So updating the index in the same second the file was touched (like a test script often does):

  echo foo >bar && git add bar

will have a racily-dirty entry, and a subsequent index refresh will re-read the file. You can verify this with strace:

  $ echo foo >bar && git add bar
  $ strace git update-index --refresh 2>&1 | grep '"bar"'
  lstat("bar", {st_mode=S_IFREG|0644, st_size=4, ...}) = 0
  open("bar", O_RDONLY|O_CLOEXEC)         = 4

  [now update the index so it has a later timestamp]
  $ sleep 1
  $ echo foo >unrelated && git add unrelated
  $ strace  git update-index --refresh 2>&1 | grep '"bar"'
  lstat("bar", {st_mode=S_IFREG|0644, st_size=4, ...}) = 0

Interestingly, I would have thought that the first update-index call would "de-racify" the entry by rewriting the index. But we don't actually write, presumably because we eventually realize that there are no entries to update. But it might actually be worth doing the write, because it avoids further file-content reads later on (and most workflows tend to do a lot of reads; every git-status is going to rehash the file until the next index update).

-Peff