repo.Index.Stage() won't notice a change if it occurs within the same second #688

nulltoken · 2014-06-03T21:35:01Z

The test below actually fails

[Fact]
public void CanSuccessfullyStageAModifiedFileOfTheSameSizeWithinTheSameSecond()
{
    string repoPath = InitNewRepository();

    using (var repo = new Repository(repoPath))
    {
        for (int i = 0; i < 10; i++)
        {
            Touch(repo.Info.WorkingDirectory, "test.txt",
                       Guid.NewGuid().ToString());

            repo.Index.Stage("test.txt");

            Assert.DoesNotThrow(() => repo.Commit(
                        "Commit", Constants.Signature, Constants.Signature));
        } 
    }
}

See this SO question for some background information.

nulltoken · 2014-04-23T13:52:09Z

/cc @arrbee @ethomson

carlosmn · 2014-04-23T13:53:23Z

I don't recall exactly what Index.Stage() in the lower layers, but this is widespread in the library. If the mtime and size match, we often won't load the new contents of the file.

nulltoken · 2014-04-23T13:56:43Z

It looks like @arrbee added some foundation code to support this in libgit2/libgit2@e35e268

arrbee · 2014-04-23T22:15:31Z

Yes - it is a problem. If a file appears to have no changes to time and size, then we try not to actually reread it. Despite this, we get libgit2/libgit2#1625 where we do excessive OID calculation and go too slow. It seems like libgit2 is right now being both too forgiving about allowing unchanged stat data to hide a change and too rigid about calculating OIDs for some files that are not changed. I had promised to work on this a while ago - I'll bump it back up the priority list...

nulltoken · 2014-06-03T19:20:25Z

Related #747

nulltoken · 2014-11-19T08:30:51Z

Related SO question "Index.RetrieveStatus On files that don't change length return unaltered"

nulltoken · 2015-01-21T10:52:42Z

Yet another related SO question "Repository.Commit() is throwing EmptyCommitException even though there are changes in the working directory"

nulltoken · 2015-06-17T09:23:07Z

@carlosmn Rebased on top of latest libgit2. Still failing 😿

carlosmn · 2015-06-17T09:38:32Z

The error I see on AppVeyor was due to the null references on the Filter tests, one of the builds did pass fine. I've restarted.

ethomson · 2015-06-17T09:40:57Z

Looks like an unrelated failure.

nulltoken · 2015-06-17T10:01:55Z

Indeed, but randomly fails on my local box.

Applying the following patch

diff --git a/LibGit2Sharp.Tests/StageFixture.cs b/LibGit2Sharp.Tests/StageFixture.cs
index 9a6c624..85633dc 100644
--- a/LibGit2Sharp.Tests/StageFixture.cs
+++ b/LibGit2Sharp.Tests/StageFixture.cs
@@ -382,11 +382,18 @@ public void CanSuccessfullyStageTheContentOfAModifiedFileOfTheSameSizeWithinTheS
             {
                 for (int i = 0; i < 10; i++)
                 {
+                    Console.WriteLine("Iteration " + i);
+
+                    var content = Guid.NewGuid().ToString();
+
                     Touch(repo.Info.WorkingDirectory, "test.txt",
-                               Guid.NewGuid().ToString());
+                               content);
+                    Console.WriteLine("Content = " + content);

                     repo.Stage("test.txt");

+                    Console.WriteLine("Index entry Id = " + repo.Index["test.txt"].Id.Sha);
+
                     Assert.DoesNotThrow(() => repo.Commit(
                                 "Commit", Constants.Signature, Constants.Signature));
                 }

Produces the following output

------ Test started: Assembly: LibGit2Sharp.Tests.dll ------

Test 'LibGit2Sharp.Tests.StageFixture.CanSuccessfullyStageTheContentOfAModifiedFileOfTheSameSizeWithinTheSameSecond' failed:
    Assert.DoesNotThrow() failure
Expected: (No exception)
Actual:   LibGit2Sharp.EmptyCommitException: No changes; nothing to commit.
    Repository.cs(1071,0): at LibGit2Sharp.Repository.Commit(String message, Signature author, Signature committer, CommitOptions options)
    RepositoryExtensions.cs(616,0): at LibGit2Sharp.RepositoryExtensions.Commit(IRepository repository, String message, Signature author, Signature committer)
    StageFixture.cs(397,0): at LibGit2Sharp.Tests.StageFixture.<>c__DisplayClass28.<CanSuccessfullyStageTheContentOfAModifiedFileOfTheSameSizeWithinTheSameSecond>b__26()
    at Xunit.Record.Exception(ThrowsDelegate code)

Output from LibGit2Sharp.Tests.StageFixture.CanSuccessfullyStageTheContentOfAModifiedFileOfTheSameSizeWithinTheSameSecond:
  ProcessInvocation86.exe Information: 0 : Using default test path value
  ProcessInvocation86.exe Information: 0 : Test working directory set to 'C:\Users\Em\AppData\Local\Temp\LibGit2Sharp-TestRepos'
  Iteration 0
  Content = 2baf6bd9-3a2d-46c0-8f87-b818ca6b06c8
  Index entry Id = 4eba44411bf83ca543cae495abf28901dd75c40a
  Iteration 1
  Content = bd1901cf-86fc-4cdd-b6a2-dc5068b9326e
  Index entry Id = d504d7cf49ca3f989a0505fa20f364d8f5b94ed9
  Iteration 2
  Content = 2abd9538-274b-4273-8abe-36e3ac445a96
  Index entry Id = fb139ef35f0025a116f0fff071405f0f83f07879
  Iteration 3
  Content = cf8130ee-ff55-4682-bccf-26d121a614fc
  Index entry Id = d1dd201310f1404b957dc3413e632a86501fc239
  Iteration 4
  Content = 508655ff-1f2a-4d91-ab58-d4a0783e0169
  Index entry Id = d1dd201310f1404b957dc3413e632a86501fc239

0 passed, 1 failed, 0 skipped, took 1,31 seconds (xUnit.net 1.9.2 build 1705).

nulltoken · 2015-06-17T10:08:31Z

Looks like an unrelated failure.

wrt AppVeyor. I restarted it. It now shows

LibGit2Sharp.Tests.StageFixture.CanSuccessfullyStageTheContentOfAModifiedFileOfTheSameSizeWithinTheSameSecond [FAIL]
   Assert.DoesNotThrow() failure
   Expected: (No exception)
   Actual:   LibGit2Sharp.EmptyCommitException: No changes; nothing to commit.
   Stack Trace:
      c:\projects\libgit2sharp\LibGit2Sharp\Repository.cs(1071,0): at LibGit2Sharp.Repository.Commit(String message, Signature author, Signature committer, CommitOptions options)
      c:\projects\libgit2sharp\LibGit2Sharp.Tests\StageFixture.cs(390,0): at LibGit2Sharp.Tests.StageFixture.<>c__DisplayClass28.<CanSuccessfullyStageTheContentOfAModifiedFileOfTheSameSizeWithinTheSameSecond>b__26()
      at Xunit.Record.Exception(ThrowsDelegate code)

carlosmn · 2015-06-17T11:16:23Z

I've been adding my own debug prints (including collecting the sizes into the managed struct) and it looks like we do behave exactly as we should wrt zeroing out files with the same timestamp as the index, which is the bit which we just merged a fix for.

When it fails to update the entry (and thus have this error happen) is when an iteration happens across the second barrier, and the rewritten test.txt is e.g. at second 24 and the index was last written at second 23 (which was the prvious iteration). During this iteration , the timestamps won't match and we'll write out the object/file's size (in this case 36) so we won't force a re-read the next time.

And it is in the iteration after the one I've described above that we fail to detect the size of the file. This is odd, as I was sure this did in fact work. The timestamps are different, so we have to re-read the file to check. I suspect a timestamp going backwards on us, which IIRC FAT can do, though I'm not sure if NTFS is capable of such a feat.

carlosmn · 2015-06-17T12:41:27Z

So, a two-year-old TODO to actually implement the bit I was sure we already had Is what bit us. If you run this on top of libgit2/libgit2#3226 you should be unable to make it fail.

carlosmn · 2015-06-26T12:12:32Z

Now for reals, putting this on top of libgit2's master really should make it work.

repo.Index.Stage() won't notice a change if it occurs within the same second

nulltoken mentioned this pull request Jun 3, 2014

Test making two successive commits to same file. #747

Closed

nulltoken added Bug labels Jun 3, 2014

kitbellew mentioned this pull request Jun 4, 2014

Release LibGit2Sharp v0.18.1 #751

Merged

nulltoken force-pushed the ntk/subsecond_staging branch from a6ddc5c to 472eb2c Compare August 28, 2014 08:06

nulltoken force-pushed the vNext branch from 49987df to 0e5bfbc Compare October 8, 2014 22:05

nulltoken mentioned this pull request Nov 13, 2014

Checkout implementation does not correctly detect modified files in workdir libgit2/libgit2#2703

Closed

nulltoken mentioned this pull request Jan 18, 2015

NextVersion.txt and GitVersionConfig.yaml support for dynamic repositories GitTools/GitVersion#352

Closed

nulltoken force-pushed the ntk/subsecond_staging branch 2 times, most recently from 4875855 to 5386591 Compare January 18, 2015 13:21

ahmelsayed mentioned this pull request Jan 20, 2015

adding LibGit2SharpRepository to replace GitExeRepository projectkudu/kudu#1440

Closed

nulltoken force-pushed the ntk/subsecond_staging branch from 5386591 to 230fed9 Compare June 17, 2015 09:15

nulltoken force-pushed the ntk/subsecond_staging branch 2 times, most recently from 49778e4 to 794a8bd Compare June 30, 2015 22:10

Ensure Stage() detects minor changes in a tight loop

db02403

nulltoken force-pushed the ntk/subsecond_staging branch from 794a8bd to db02403 Compare July 4, 2015 17:37

nulltoken added a commit that referenced this pull request Jul 4, 2015

Merge pull request #688 from libgit2/ntk/subsecond_staging

302c162

repo.Index.Stage() won't notice a change if it occurs within the same second

nulltoken merged commit 302c162 into vNext Jul 4, 2015

nulltoken deleted the ntk/subsecond_staging branch July 4, 2015 17:45

nulltoken added this to the v0.22 milestone Jul 4, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

repo.Index.Stage() won't notice a change if it occurs within the same second #688

repo.Index.Stage() won't notice a change if it occurs within the same second #688

Uh oh!

nulltoken commented Jun 3, 2014

Uh oh!

nulltoken commented Apr 23, 2014

Uh oh!

carlosmn commented Apr 23, 2014

Uh oh!

nulltoken commented Apr 23, 2014

Uh oh!

arrbee commented Apr 23, 2014

Uh oh!

nulltoken commented Jun 3, 2014

Uh oh!

nulltoken commented Nov 19, 2014

Uh oh!

nulltoken commented Jan 21, 2015

Uh oh!

nulltoken commented Jun 17, 2015

Uh oh!

carlosmn commented Jun 17, 2015

Uh oh!

ethomson commented Jun 17, 2015

Uh oh!

nulltoken commented Jun 17, 2015

Uh oh!

nulltoken commented Jun 17, 2015

Uh oh!

carlosmn commented Jun 17, 2015

Uh oh!

carlosmn commented Jun 17, 2015

Uh oh!

carlosmn commented Jun 26, 2015

Uh oh!

Uh oh!

repo.Index.Stage() won't notice a change if it occurs within the same second #688

repo.Index.Stage() won't notice a change if it occurs within the same second #688

Uh oh!

Conversation

nulltoken commented Jun 3, 2014

Uh oh!

nulltoken commented Apr 23, 2014

Uh oh!

carlosmn commented Apr 23, 2014

Uh oh!

nulltoken commented Apr 23, 2014

Uh oh!

arrbee commented Apr 23, 2014

Uh oh!

nulltoken commented Jun 3, 2014

Uh oh!

nulltoken commented Nov 19, 2014

Uh oh!

nulltoken commented Jan 21, 2015

Uh oh!

nulltoken commented Jun 17, 2015

Uh oh!

carlosmn commented Jun 17, 2015

Uh oh!

ethomson commented Jun 17, 2015

Uh oh!

nulltoken commented Jun 17, 2015

Uh oh!

nulltoken commented Jun 17, 2015

Uh oh!

carlosmn commented Jun 17, 2015

Uh oh!

carlosmn commented Jun 17, 2015

Uh oh!

carlosmn commented Jun 26, 2015

Uh oh!

Uh oh!