Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> Besides, I am pretty certain that there is a test in t9100 that *does*
> test the executable bit, properly requiring POSIXPERM.
>
> So I still would be in favor of using write_script: 1) our *intention* is
> to write a script, even if we do not currently execute it, and 2) if
> anybody is interested in supporting MSys1 (*cough* Hannes & Sebastian
> *cough*), they have a *much* easier time to fix it.
I do not think our intention is to write a script there, though.
You have commented out a different test, where a file called "file"
whose executable bit is flipped, and the result of the test depends
on the filesystem actually flipping the bit, using POSIXPERM. I do
not see anything more special in the way "exec.sh" is used in the
test compared to the use of that "file". They both are merely used
as a payload, with expected contents and expected setting of the
executable bit at each points in the test sequence.
By using POSIXPERM, js/mingw-tests topic cleanly identifies and
skips the tests that rely on the "chmod +x" acting as expected, but
by hiding the creation and setting of the executable bit behind
write_script for "exec.sh" (and for this file alone), the reader is
still forced to know the subtle linkage between write_script and
permission bits. On a system where executable bit is not needed to
execute a script (perhaps the file extension tells if a file is
executable on such a system), it is a plausible enhancement to
write_script to make it not to even attempt "chmod +x" on a system
without executable bit--after all, the helper _is_ about writing a
script. The executable bit is an implementation detail that nobody
cares about.
But the way "exec.sh" is used by that test is different. It merely
wants to have a file with that name with a fixed contents that has
executable bit set. As I already said, I didn't notice that during
the review and incorrectly suggested use of write_script, but that
was a mistake.
Ideally, it would have been better if the test was structured in
such a way that a set of pure-contents tests that do not involve the
executable bit and symbolic links are done first, with separate set
of tests that require POSIXPERM and/or SYMLINK. Then we wouldn't be
having this conversation. You would skip the whole thing in the
latter category. As the test that originally was written needs
POSIXPERM (but curiously not SYMLINK) even in the very initial
'setup' stage, a hack to make the set-up step behave differently
depending on POSIXPERM is unavoidable and tolerable, if we do not
want to rewrite the entire script (and I do not think neither of us
want to see that).
But that needs to be spelled out explicitly to allow people follow
what is going on more easily, e.g.
echo "#!/bin/sh" >exec.sh &&
{ test_have_prereq !POSIXPERM || chmod +x exec.sh } &&
The "file" test would not need such a construct as the whole thing
is skipped without POSIXPERM (and SYMLINK where it is aliased to
exec-2.sh).
While reviewing the change to this script, another thing I noticed
is this bit:
name='remove executable bit from a file'
test_expect_success POSIXPERM "$name" '
rm -f "$GIT_DIR"/index &&
git checkout -f -b mybranch5 ${remotes_git_svn} &&
chmod -x exec.sh &&
git update-index exec.sh &&
git commit -m "$name" &&
git svn set-tree --find-copies-harder --rmdir \
${remotes_git_svn}..mybranch5 &&
svn_cmd up "$SVN_TREE" &&
test ! -x "$SVN_TREE"/exec.sh'
This uses "chmod -x", but it does not need to. The only thing it
cares about is that the tree object that contains exec.sh record
that path with executable bit off, so "update-index --chmod" would
have allowed a !POSIXPERM system to record the same result.
There probably are more instance of similar "chmod" that you do not
have to skip with !POSIXPERM.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html