Hi James,
Thanks for prodding me. I've been procrastinating for far too long.
* James McCoy <james...@debian.org>, 2014-10-09, 22:09:
I suggest that sadt skips non executable tests with a warning and
documents this behaviour (as it does with missing dependencies), or
displays a warning when modifying the permissions. It took me some
time to find who was interfering with my version control system.
Thanks, that's a pretty strong argument that chmod'ing tests is not a
good idea, at least not by default.
Do you intend to make some changes for this soon?
I've attached a patch for skipping non-executable tests instead of
chmod+x'ing them. But now I'm a bit hesitant that this is the right
thing to do...
I think it would be more user-friendly if sadt did this:
* If sadt have to make copy of the source tree anyway (i.e., one of the
tests declares the rw-build-tree restriction), then fix permissions in
the copy.
* Otherwise, try chmod+x'ing the test file in the source tree, and if
that fails, skip the test.
* Restore the original permissions afterwards.
Does it sound sane?
--
Jakub Wilk
diff --git a/scripts/sadt b/scripts/sadt
--- a/scripts/sadt
+++ b/scripts/sadt
@@ -26,7 +26,6 @@
import queue as queuemod
import re
import shutil
-import stat
import subprocess as ipc
import sys
import tempfile
@@ -34,15 +33,6 @@
import debian.deb822 as deb822
-def chmod_x(path):
- '''
- chmod a+X <path>
- '''
- old_mode = stat.S_IMODE(os.stat(path).st_mode)
- new_mode = old_mode | ((old_mode & 0o444) >> 2)
- if old_mode != new_mode:
- os.chmod(path, new_mode)
-
def annotate_output(child):
queue = queuemod.Queue()
def reader(fd, tag):
@@ -246,6 +236,10 @@
def run(self, test, progress, ignored_restrictions=(), rw_build_tree=None, built_source_tree=None):
progress.start(test)
+ path = os.path.join(self.tests_directory, test)
+ if not os.access(path, os.X_OK):
+ progress.skip('{path} is not executable'.format(path=path))
+ return
ignored_restrictions = set(ignored_restrictions)
if rw_build_tree:
ignored_restrictions.add('rw-build-tree')
@@ -269,7 +263,6 @@
def _run(self, test, progress, allow_stderr=False):
path = os.path.join(self.tests_directory, test)
- chmod_x(path)
tmpdir1 = tempfile.mkdtemp(prefix='sadt.')
tmpdir2 = tempfile.mkdtemp(prefix='sadt.')
environ = dict(os.environ)
diff --git a/scripts/sadt.pod b/scripts/sadt.pod
--- a/scripts/sadt.pod
+++ b/scripts/sadt.pod
@@ -39,6 +39,9 @@
B<sadt> doesn't implement any virtualisation arrangements, therefore it skips
tests that declare the B<breaks-testbed> restriction.
+B<sadt> doesn't change the test files permissions; it skips tests that don't
+have the executable bits set.
+
=head1 OPTIONS
=over 4
