Re: [PATCH try 2] t1301-shared-repo.sh: don't let a default ACL interfere with the test

Johannes Sixt [off-list ref] writes:

quoted

quoted

But that would also paper over unanticipated bad interactions with strange
ACLs that people might set, wouldn't it? By not placing this into
test-lib.sh there is a higher chance that such an interaction is revealed,
and we can react on it (educate users or fix the code).

What do you exactly mean by "educate users or fix the code"?  For example,
by not putting this setfacl in test-lib.sh, t1301 revealed that with a
default ACL higher up, "git init --shared" would not work as expected.

Then what?

 - Do you mean, by "educate users", that we teach users not to play fun
   games with ACL in a git controled working tree?

Correct. In the case of a shared repository we can educate users not to
play with ACLs.

quoted

 - Do you mean, by "fix the code", that we teach adjust_shared_perm() to
   deal with ACL?

Correct in principle, but we need not go this route in the case of shared
repositories because we better educate users.

If that is the case what difference does your suggestion of not putting it
in test-lib.sh make?  We discourage users from playing ACL games, and we
protect ourselves from such by making sure the trash directory used for
running tests are not contaminated with ACL.  Wouldn't it make more sense
to do so for all the tests, so that future test writers do not have to
worry about it?

Junio C Hamano schrieb:

Johannes Sixt [off-list ref] writes:

quoted

quoted

 - Do you mean, by "educate users", that we teach users not to play fun
   games with ACL in a git controled working tree?

Correct. In the case of a shared repository we can educate users not to
play with ACLs.

quoted

 - Do you mean, by "fix the code", that we teach adjust_shared_perm() to
   deal with ACL?

Correct in principle, but we need not go this route in the case of shared
repositories because we better educate users.

If that is the case what difference does your suggestion of not putting it
in test-lib.sh make?  We discourage users from playing ACL games, and we
protect ourselves from such by making sure the trash directory used for
running tests are not contaminated with ACL.  Wouldn't it make more sense
to do so for all the tests, so that future test writers do not have to
worry about it?

We have to decide case by case. In the case of shared directories it makes
sense to suggest "do not play ACL games". In other cases, however, this
suggestion could not work out that well, and a workaround in the code is
the better solutions. But we do not know what those other cases are, and
the test suite may be a tool to uncover them.

Perhaps I'm worrying too much because a case that warrants a change in the
code would either have to happen frequently or be backed with very strong
arguments that ACLs are required in that particular way. (And in both
cases there would still have be conflicts in the way how git handles
permissions - we wouldn't care otherwise.) But no such case has turned up
so far.

-- Hannes