Junio C Hamano <gits...@pobox.com> writes:
>> @@ -661,6 +663,10 @@ int add_to_index(struct index_state *istate, const char
>> *path, struct stat *st,
>>
>> if (trust_executable_bit && has_symlinks)
>> ce->ce_mode = create_ce_mode(st_mode);
>> + else if (force_executable)
>> + ce->ce_mode = create_ce_mode(0777);
>> + else if (force_notexecutable)
>> + ce->ce_mode = create_ce_mode(0666);
>
> This is an iffy design decision.
>
> Even when you are in core.filemode=true repository, if you
> explicitly said
>
> git add --chmod=+x READ.ME
>
> wouldn't you expect that the path would have executable bit in the
> index, whether it has it as executable in the filesystem? The above
> if/else cascade, because trust-executable-bit is tested first, will
> ignore force_* flags altogether, won't it? It also is strange that
> the decision to honor or ignore force_* flags is also tied to
> has_symlinks, which is a totally orthogonal concept.
Here is an additional patch to your tests. It repeats one of the
tests you added, but runs in a repository with core.filemode and
core.symlinks both enabled. The test fails to force executable bit
on platforms where it runs.
It passes with your patch if you drop core.symlinks, which is a good
demonstration why letting has_symlinks decide if force* is to be
honored is iffy.
t/t3700-add.sh | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index e551eaf..2afcb74 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -351,4 +351,15 @@ test_expect_success 'git add --chmod=-x stages an
executable file with -x' '
esac
'
+test_expect_success POSIXPERM,SYMLINKS 'git add --chmod=+x' '
+ git config core.filemode 1 &&
+ git config core.symlinks 1 &&
+ echo foo >foo2 &&
+ git add --chmod=+x foo2 &&
+ case "$(git ls-files --stage foo2)" in
+ 100755" "*foo2) echo pass;;
+ *) echo fail; git ls-files --stage foo2; (exit 1);;
+ esac
+'
+
test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
