From db1cdaf0186ff036757a5501bb6cbd38d5f97954 Mon Sep 17 00:00:00 2001 From: Tomi Ollila Date: Thu, 13 Jun 2013 22:47:33 +0300 Subject: [PATCH] Re: [PATCH 1/1] test/basic: replaced find -perm +111 with portable alternative --- e1/d3bdc48c2d44cb82c1c24a94775dbb34fb43ba | 127 ++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 e1/d3bdc48c2d44cb82c1c24a94775dbb34fb43ba diff --git a/e1/d3bdc48c2d44cb82c1c24a94775dbb34fb43ba b/e1/d3bdc48c2d44cb82c1c24a94775dbb34fb43ba new file mode 100644 index 000000000..c48bfe108 --- /dev/null +++ b/e1/d3bdc48c2d44cb82c1c24a94775dbb34fb43ba @@ -0,0 +1,127 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 3ADDA431FC2 + for ; Thu, 13 Jun 2013 12:47:50 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 0 +X-Spam-Level: +X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none] + autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id 6L8GKctEND5q for ; + Thu, 13 Jun 2013 12:47:41 -0700 (PDT) +Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) + by olra.theworths.org (Postfix) with ESMTP id 98014431FBC + for ; Thu, 13 Jun 2013 12:47:41 -0700 (PDT) +Received: from guru.guru-group.fi (localhost [IPv6:::1]) + by guru.guru-group.fi (Postfix) with ESMTP id 596C51002BA; + Thu, 13 Jun 2013 22:47:33 +0300 (EEST) +From: Tomi Ollila +To: Austin Clements +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) +X-Face: HhBM'cA~ +MIME-Version: 1.0 +Content-Type: text/plain +Cc: notmuch@notmuchmail.org +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Thu, 13 Jun 2013 19:47:50 -0000 + +On Mon, Jun 10 2013, Austin Clements 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 -- 2.26.2