From: Tomi Ollila <tomi.ollila@iki.fi>
To: Austin Clements <amdragon@MIT.EDU>
Subject: Re: [PATCH 1/1] test/basic: replaced find -perm +111 with portable
	alternative
In-Reply-To: <20130610155940.GE22196@mit.edu>
References: <1370641049-17390-1-git-send-email-tomi.ollila@iki.fi>
	<20130610155940.GE22196@mit.edu>
User-Agent: Notmuch/0.15.2+181~gfb53171 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-unknown-linux-gnu)
Date: Thu, 13 Jun 2013 22:47:33 +0300
Message-ID: <m2r4g56be2.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Cc: notmuch@notmuchmail.org
Precedence: list

On Mon, Jun 10 2013, Austin Clements <amdragon@MIT.EDU> wrote:

> LGTM.  Though, I wonder, why not *just* -perm -100?  That isn't quite
> a correct test of whether the user can execute it: e.g., if the file
> is owned by some other user and a group the current user isn't in,
> then -perm -1 is the correct test, though unless the file has some
> unusual permissions, -perm -100 is likely to pass anyway.  But the
> test you have (and the test that was there before) isn't quite correct
> either: if the file is owned by the current user and has some crazy
> permission like 0611, the user won't be able to execute it, even
> though someone else could.

While giving considerable amount of thought for such an insignificant
issue I came to realize this:

The purpose of the '-perm ...' part in that expression is not to check
whether the file is executable by the user but just to reduce the set
of files the whole expression returns without need to "blacklist" more
files that are already blacklisted with '! -name ...' subexpressions
("Makefile", ".gitignore" and so on).

With +111, /ppp and their portable alternative
( -perm -100 -or -perm -10 -or -perm 1 ) the implicit reduction this
part does is smaller than with -100.

The returned list is then compared with ${TESTS} and if there is no
exact match then this particular test fails.

Whatever this test result is, the execution of any file in ${TESTS}
will fail with "permission denied" if it is not executable by
the user running the tests.

I think that as we're doing this "shortcut" instead of full file
blacklisting, this should reduce the output less rather than
more and therefore use the version provided in this patch
instead of changing +111 to -100.

(In the future I'd like to see that we had some convention to name
the test scripts and either do comparison to that list or that
convention also dictates order and this test could be removed. There
are a few alternatives that we could think of...).

> It's too bad "-executable" is a GNU extension.

I'd have uses for that :D

Tomi

> Quoth Tomi Ollila on Jun 08 at 12:37 am:
>> The find option syntax `-perm +111` is deprecated gnu find feature.
>> The replacement `( -perm -100 -o -perm -10 -o -perm 1 )` should also
>> work outside of the GNU domain.
>> ---
>>  test/basic | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/test/basic b/test/basic
>> index 1b2a7d2..64eb7d7 100755
>> --- a/test/basic
>> +++ b/test/basic
>> @@ -53,7 +53,8 @@ test_expect_code 2 'failure to clean up causes the test to fail' '
>>  test_begin_subtest 'Ensure that all available tests will be run by notmuch-test'
>>  eval $(sed -n -e '/^TESTS="$/,/^"$/p' $TEST_DIRECTORY/notmuch-test)
>>  tests_in_suite=$(for i in $TESTS; do echo $i; done | sort)
>> -available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f -perm +111  \
>> +available=$(find "$TEST_DIRECTORY" -maxdepth 1 -type f \
>> +   '(' -perm -100 -o -perm -10 -o -perm -1 ')' \
>>      ! -name aggregate-results.sh    \
>>      ! -name arg-test                        \
>>      ! -name hex-xcode                       \
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch