git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Alex Riesen <raa.lkml@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] Fix t6031 on filesystems without working exec bit
Date: Wed, 21 May 2008 10:14:43 -0700	[thread overview]
Message-ID: <7v8wy360l8.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <20080519060015.GA3179@steel.home> (Alex Riesen's message of "Mon, 19 May 2008 08:00:15 +0200")

Alex Riesen <raa.lkml@gmail.com> writes:

> Junio C Hamano, Mon, May 19, 2008 06:51:16 +0200:
> ...
>> I have to wonder if this is enough on a filesystem with usable executable
>> bit.  Has this been tested on both kinds of filesystems?
>
> Yes and yes.

I am very much regretting for having extracted the above answer from you.
After actually running the test myself on a machine with executable bit, I
have to conclude that I now have to review any patch from you with much
more suspicion than I have done before.  Unless the test I ran on my
machine is lying to me, that is...

>> You aren't setting +x on work tree file anymore, but only flipping the bit
>> inside the index before committing.  Because of this change, after "b1"
>> commit, work tree has a local modification relative to the commit (namely,
>> reversion of chmod +x is in the work tree), which is different from the
>> original test sequence.  Doesn't this local modification interact with
>> switching to a1 branch and what merge-recursive does?
>
> I think no (I depend on this in my workflows on both systems).

Look at what the "switch to a1 branch after committing to b1" does again.
You modify "file1" and commit to "b1" and you now have a local change in
file1 (because you update only the index and not the work tree) that is
effectively a revert of what you committed just now, and a1 and b1 differs
at that path.  You get "Entry 'file1' not uptodate. Cannot merge."  when
you try to switch to "a1".

    $ sh t6031-merge-recursive.sh -i -v
    * expecting success: 
            : >file1 &&
            git add file1 &&
            git commit -m initial &&
            git checkout -b a1 master &&
            : >dummy &&
            git add dummy &&
            git commit -m a &&
            git checkout -b b1 master &&
            git update-index --chmod=+x file1 &&
            git commit -m b1 &&
            git checkout a1 &&
            git merge-recursive master -- a1 b1 &&
            test -x file1

    Created initial commit bae7a40: initial
     0 files changed, 0 insertions(+), 0 deletions(-)
     create mode 100644 file1
    Switched to a new branch "a1"
    Created commit 1f64f65: a
     0 files changed, 0 insertions(+), 0 deletions(-)
     create mode 100644 dummy
    Switched to a new branch "b1"
    Created commit 98a1cf4: b1
     0 files changed, 0 insertions(+), 0 deletions(-)
     mode change 100644 => 100755 file1
    error: Entry 'file1' not uptodate. Cannot merge.
    * FAIL 1: mode change in one branch: keep changed version

If we squash the following on top of your patch, I think we can help
filesystems without executable bit without breaking filesystems with one.
Can you see if it works on your executable-bit-less setup?

-- >8 --
 t/t6031-merge-recursive.sh |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/t/t6031-merge-recursive.sh b/t/t6031-merge-recursive.sh
index f1c91c8..8073e0c 100755
--- a/t/t6031-merge-recursive.sh
+++ b/t/t6031-merge-recursive.sh
@@ -3,6 +3,9 @@
 test_description='merge-recursive: handle file mode'
 . ./test-lib.sh
 
+# Note that we follow "chmod +x F" with "update-index --chmod=+x F" to
+# help filesystems that do not have the executable bit.
+
 test_expect_success 'mode change in one branch: keep changed version' '
 	: >file1 &&
 	git add file1 &&
@@ -12,6 +15,7 @@ test_expect_success 'mode change in one branch: keep changed version' '
 	git add dummy &&
 	git commit -m a &&
 	git checkout -b b1 master &&
+	chmod +x file1 &&
 	git update-index --chmod=+x file1 &&
 	git commit -m b1 &&
 	git checkout a1 &&
@@ -24,6 +28,7 @@ test_expect_success 'mode change in both branches: expect conflict' '
 	git checkout -b a2 master &&
 	: >file2 &&
 	H=$(git hash-object file2) &&
+	chmod +x file2 &&
 	git update-index --add --chmod=+x file2 &&
 	git commit -m a2 &&
 	git checkout -b b2 master &&

  reply	other threads:[~2008-05-21 17:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-18 14:57 [PATCH] Fix t6031 on filesystems without working exec bit Alex Riesen
2008-05-19  4:51 ` Junio C Hamano
2008-05-19  6:00   ` Alex Riesen
2008-05-21 17:14     ` Junio C Hamano [this message]
2008-05-22  8:16       ` Alex Riesen
2008-05-22  9:04         ` Junio C Hamano
2008-05-22 13:12           ` Alex Riesen
2008-05-23 19:12             ` 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=7v8wy360l8.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=raa.lkml@gmail.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).