Re: [PATCH] add: add --chmod=+x / --chmod=-x options
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 25, 2016, 16:00 UTC
- Message-ID
- <xmqqshx6162k.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <xmqqh9dm37xk.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 22 quoted lines
>> @@ -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