having to pull twice

having to pull twice

[Michael P. Soulier]

I'm finding this happening from time to time.

soulierm@espresso:~/work/mitel-msl-tug$ git pull
remote: Counting objects: 97, done.
remote: Compressing objects: 100% (54/54), done.
remote: Total 65 (delta 34), reused 0 (delta 0)
Unpacking objects: 100% (65/65), done.
From pa:git/mitel-msl-tug
   92437d6..9c784c8  master     -> origin/master
Updating 92437d6..9c784c8
error: Entry 'mitel-msl-tug.spec' not uptodate. Cannot merge.
soulierm@espresso:~/work/mitel-msl-tug$ git status
# On branch master
# Your branch is behind 'origin/master' by 4 commits, and can be fast-forwarded.
#
nothing to commit (working directory clean)
soulierm@espresso:~/work/mitel-msl-tug$ git pull
Updating 92437d6..9c784c8
Fast forward
 mitel-msl-tug.spec                                 |    7 +-
 .../etc/e-smith/pgsql/init/tugdb.sh/20auth         |    4 +-
 .../etc/e-smith/pgsql/init/tugdb.sh/50apps         |   37 ---
 .../etc/e-smith/pgsql/init/tugdb.sh/90loaddata     |    8 +-
 .../etc/e-smith/pgsql/init/tugdb.sh/95schema       |    2 +-
 root/etc/e-smith/templates/etc/tug/tug.ini/00setup |    2 +
 .../e-smith/templates/etc/tug/tug.ini/10teleworker |   37 +++-
 .../templates/etc/tug/tug.ini/11teleworker_fixed   |    1 -
 .../e-smith/web/django/teleworker/scrc/views.py    |   11 +-
 root/usr/lib/scrc/migrate.pl                       |  322 --------------------
 root/usr/lib/tug/migrate/30_add_scrc               |    2 +-
 root/usr/lib/tug/migrate/31_add_scrc_connections   |   22 ++-
 12 files changed, 77 insertions(+), 378 deletions(-)
 delete mode 100755 root/usr/lib/scrc/migrate.pl
soulierm@espresso:~/work/mitel-msl-tug$ git --version
git version 1.6.0

When I pulled the first time, I was told that one of the files was
 not up-to-date, but status did not note any modified files. The
second pull then worked as expected.

That directory is mounted via NFS, and I'm curious as to
whether some kind of time skew could be the problem. But
then, if it is, why did it go away after the first pull?

Thanks,
Mike
-- 
Michael P. Soulier <msoulier@digitaltorque.ca>
"Any intelligent fool can make things bigger and more complex... It takes a
touch of genius - and a lot of courage to move in the opposite direction."
--Albert Einstein

Re: having to pull twice

[Shawn O. Pearce]

"Michael P. Soulier" <msoulier@digitaltorque.ca> wrote:
> I'm finding this happening from time to time.
> 
> soulierm@espresso:~/work/mitel-msl-tug$ git pull
...
> error: Entry 'mitel-msl-tug.spec' not uptodate. Cannot merge.
> soulierm@espresso:~/work/mitel-msl-tug$ git status
> # On branch master
> # Your branch is behind 'origin/master' by 4 commits, and can be fast-forwarded.
> #
> nothing to commit (working directory clean)
...
> That directory is mounted via NFS, and I'm curious as to
> whether some kind of time skew could be the problem. But
> then, if it is, why did it go away after the first pull?

Time or dev/ino skew in the index file vs. what we read from stat.

Running git-status rematched the index file to the working directory,
and during that rematch it noticed the file wasn't actually modified.

So it prints "nothing to commit" and the next pull works fine,
now that the stat data in the index matches the working directory.

-- 
Shawn.

Re: having to pull twice

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1105 bytes --]

Shawn O. Pearce wrote:
> "Michael P. Soulier" <msoulier@digitaltorque.ca> wrote:
> > I'm finding this happening from time to time.
> > 
> > soulierm@espresso:~/work/mitel-msl-tug$ git pull
> ...
> > error: Entry 'mitel-msl-tug.spec' not uptodate. Cannot merge.
[fixed by 'git status's index refresh]
> 
> Time or dev/ino skew in the index file vs. what we read from stat.
> 
> Running git-status rematched the index file to the working directory,
> and during that rematch it noticed the file wasn't actually modified.

This keeps coming up every week or so... maybe git-merge should
attempt to refresh the index automatically?  Of course it's an
expensive operation, but if you really want to do the merge you have
to bite that bullet anyway.

Unfortunately I can't see an obvious place to do that---the above
message comes from unpack-trees.c, where changes would also affect
e.g. checkout.

On the other hand, as near as I can tell this is a regression in
builtin-merge.  Miklos, do you know if/how this can be fixed?

- Thomas

-- 
Thomas Rast
trast@student.ethz.ch



[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: having to pull twice

[Alex Riesen]

Thomas Rast, Thu, Sep 25, 2008 09:05:02 +0200:
> Shawn O. Pearce wrote:
> > "Michael P. Soulier" <msoulier@digitaltorque.ca> wrote:
> > > I'm finding this happening from time to time.
> > > 
> > > soulierm@espresso:~/work/mitel-msl-tug$ git pull
> > ...
> > > error: Entry 'mitel-msl-tug.spec' not uptodate. Cannot merge.
> [fixed by 'git status's index refresh]
> > 
> > Time or dev/ino skew in the index file vs. what we read from stat.
> > 
> > Running git-status rematched the index file to the working directory,
> > and during that rematch it noticed the file wasn't actually modified.
> 
> This keeps coming up every week or so... maybe git-merge should
> attempt to refresh the index automatically?  Of course it's an
> expensive operation, but if you really want to do the merge you have
> to bite that bullet anyway.

Maybe set "core.trustctime" for this repo to false?

Re: having to pull twice

[Michael P. Soulier]

[-- Attachment #1: Type: text/plain, Size: 387 bytes --]

On 25/09/08 Alex Riesen said:

> Maybe set "core.trustctime" for this repo to false?

I can certainly try that, and mention it if it happens again. 

Mike
-- 
Michael P. Soulier <msoulier@digitaltorque.ca>
"Any intelligent fool can make things bigger and more complex... It
takes a touch of genius - and a lot of courage to move in the opposite
direction." --Albert Einstein

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: having to pull twice

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

On Thu, Sep 25, 2008 at 09:05:02AM +0200, Thomas Rast <trast@student.ethz.ch> wrote:
> On the other hand, as near as I can tell this is a regression in
> builtin-merge.  Miklos, do you know if/how this can be fixed?

I think Junio already fixed this in 446247d (merge: fix numerus bugs
around "trivial merge" area, 2008-08-23), so 1.6.0.1 or 1.6.0.2 should
not have this bug.

Michael, could you please upgrade frm 1.6.0 and confirm your problem
goes away?

Thanks.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: having to pull twice

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 1422 bytes --]

Miklos Vajna wrote:
> On Thu, Sep 25, 2008 at 09:05:02AM +0200, Thomas Rast <trast@student.ethz.ch> wrote:
> > On the other hand, as near as I can tell this is a regression in
> > builtin-merge.  Miklos, do you know if/how this can be fixed?
> 
> I think Junio already fixed this in 446247d (merge: fix numerus bugs
> around "trivial merge" area, 2008-08-23), so 1.6.0.1 or 1.6.0.2 should
> not have this bug.
> 
> Michael, could you please upgrade frm 1.6.0 and confirm your problem
> goes away?

That won't help.  I decided this was a good opportunity to learn about
'git bisect run', and bisected it to 1c7b76b (Build in merge,
2008-07-07).  This was with bad = 47a765d (pulled from Shawn's repo
this week) and good = v1.5.6.

In case you want to check my logic, the commands are below.

- Thomas


* Setup:

  mkdir $temp_repo
  cd $temp_repo
  git init
  echo foo > foo
  git add foo
  git commit -m initial
  echo a > foo
  git commit -m a foo
  git checkout -b side HEAD^
  echo b > foo
  git commit -m b foo
  git checkout master

* Bisection script:

-- 8< --
#!/bin/sh

rm -rf $temp_repo/{bin,libexec}
make -j3 prefix=$temp_repo install

PATH=$temp_repo/bin:$PATH
cd $temp_repo
touch foo
git merge side
test -f .git/MERGE_HEAD
s=$?
git reset --hard
if test $s != 0; then
    echo did not work
    exit 1
else
    echo worked
    exit 0
fi
-- >8 --



[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: having to pull twice

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 959 bytes --]

On Sat, Sep 27, 2008 at 04:16:29PM +0200, Thomas Rast <trast@student.ethz.ch> wrote:
> * Setup:
> 
>   mkdir $temp_repo
>   cd $temp_repo
>   git init
>   echo foo > foo
>   git add foo
>   git commit -m initial
>   echo a > foo
>   git commit -m a foo
>   git checkout -b side HEAD^
>   echo b > foo
>   git commit -m b foo
>   git checkout master

Oh, the racy git problem. ;-)

$ git reset --hard; touch foo; git merge side
HEAD is now at 5bafc63 a
Auto-merging foo
CONFLICT (content): Merge conflict in foo
Automatic merge failed; fix conflicts and then commit the result.

$ git reset --hard; sleep 1; touch foo;git merge side
HEAD is now at 5bafc63 a
error: Entry 'foo' not uptodate. Cannot merge.
fatal: merging of trees 86de5e13286a8449a8a706a58e63be6781770b12 and 9ee610433fb8854e497d62c648a612b5deb090cf failed

Thanks for the reproducer, I'll write a proper testcase for this and try to
provide a fix for it as well.

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: having to pull twice

[Thomas Rast]

[-- Attachment #1: Type: text/plain, Size: 782 bytes --]

Miklos Vajna wrote:
> 
> Oh, the racy git problem. ;-)
> 
> $ git reset --hard; touch foo; git merge side

IIUC, this is not the same as "racy git".  The racy case is if you
manage to get a file changed immediately after its index entry was
updated, so that it will look unchanged.  In this case, it's simply
the same file with a new mtime, which means it looks changed
superficially but still has the same contents.

Granted, you could call my test "racy" because it relies on 'make
install' taking at least one second, which by Moore's law should
happen sometime around 2019 ;-)

> Thanks for the reproducer, I'll write a proper testcase for this and try to
> provide a fix for it as well.

Thanks.

- Thomas

-- 
Thomas Rast
trast@student.ethz.ch




[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: having to pull twice

[Miklos Vajna]

[-- Attachment #1: Type: text/plain, Size: 545 bytes --]

On Sun, Sep 28, 2008 at 05:26:20PM +0200, Thomas Rast <trast@student.ethz.ch> wrote:
> IIUC, this is not the same as "racy git".  The racy case is if you
> manage to get a file changed immediately after its index entry was
> updated, so that it will look unchanged.  In this case, it's simply
> the same file with a new mtime, which means it looks changed
> superficially but still has the same contents.

Right. I called it "racy" because it had something to do with
timestamps. I guess we have no proper term for this kind of problem yet.
;-)

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] builtin-merge: refresh the index before calling a strategy

[Miklos Vajna]

In case a file is touched but has no real changes then we just have to
update the index and should not error out.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

On Sun, Sep 28, 2008 at 05:11:35PM +0200, Miklos Vajna <vmiklos@frugalware.org> wrote:
> Thanks for the reproducer, I'll write a proper testcase for this and
> try to
> provide a fix for it as well.

Try this one.

 builtin-merge.c  |   10 ++++++++++
 t/t7600-merge.sh |    9 +++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index 4c9ed5d..6c9dfe6 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -548,6 +548,16 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 	int i = 0, ret;
 	struct commit_list *j;
 	struct strbuf buf;
+	int index_fd;
+	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+
+	index_fd = hold_locked_index(lock, 1);
+	refresh_cache(REFRESH_QUIET);
+	if (active_cache_changed &&
+			(write_cache(index_fd, active_cache, active_nr) ||
+			 commit_locked_index(lock)))
+		return error("Unable to write index.");
+	rollback_lock_file(lock);
 
 	if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree")) {
 		int clean;
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 98cfc53..61db3c3 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -520,4 +520,13 @@ test_expect_success 'merge --no-ff --no-commit && commit' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'refresh the index before merging' '
+	git reset --hard c1 &&
+	sleep 1 &&
+	touch file &&
+	git merge c3
+'
+
+test_debug 'gitk --all'
+
 test_done
-- 
1.6.0.2