From 6a9ffea7e2f75c440e515a2ecb3d94f9bb89afdc Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Thu, 26 Jan 2017 10:08:28 +0100 Subject: [PATCH 01/12] add "oe test" subcommand I've tried to add small unit tests here and there, especially when low-level OS stuff is involved. This is an attempt at creating a place to put all these, allowing them to be run without weird and ad hoc command lines. The first test case I'm adding is a bit complex, but it's the simplest I could come up with to provoke the race in our makedirs helper. That race may show its ugly face when we do stuff in parallel processes and be very hard to debug ("what do you mean EEXIST, the code obviously tests for that before calling os.makedirs!"). Before fixing any race one needs a test case that reliably reproduces it. --- lib/oelite/cmd/__init__.py | 2 +- lib/oelite/cmd/test.py | 91 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 lib/oelite/cmd/test.py diff --git a/lib/oelite/cmd/__init__.py b/lib/oelite/cmd/__init__.py index 57090676..ae994c8d 100644 --- a/lib/oelite/cmd/__init__.py +++ b/lib/oelite/cmd/__init__.py @@ -1 +1 @@ -manifest_cmds = [ "bake", "setup", "show", "cherry", "autodoc", "add-layer" ] +manifest_cmds = [ "bake", "setup", "show", "cherry", "autodoc", "add-layer", "test" ] diff --git a/lib/oelite/cmd/test.py b/lib/oelite/cmd/test.py new file mode 100644 index 00000000..5c27557d --- /dev/null +++ b/lib/oelite/cmd/test.py @@ -0,0 +1,91 @@ +import errno +import os +import select +import shutil +import signal +import sys +import tempfile +import time +import unittest + +import oelite.util + +description = "Run tests of internal utility functions" +def add_parser_options(parser): + parser.add_option("-s", "--show", + action="store_true", default=False, + help="Show list of tests") + +class OEliteTest(unittest.TestCase): + def setUp(self): + self.wd = tempfile.mkdtemp() + os.chdir(self.wd) + + def tearDown(self): + os.chdir("/") + shutil.rmtree(self.wd) + +class MakedirsRaceTest(OEliteTest): + def child(self): + signal.alarm(2) # just in case of infinite recursion bugs + try: + # wait for go + select.select([self.r], [], [], 1) + oelite.util.makedirs(self.path) + # no exception? all right + res = "OK" + except OSError as e: + # errno.errorcode(errno.ENOENT) == "ENOENT" etc. + res = errno.errorcode.get(e.errno) or str(e.errno) + except Exception as e: + res = "??" + finally: + # Short pipe writes are guaranteed atomic + os.write(self.w, res+"\n") + os._exit(0) + + def setUp(self): + super(MakedirsRaceTest, self).setUp() + self.path = "x/" * 10 + self.r, self.w = os.pipe() + self.children = [] + for i in range(8): + pid = os.fork() + if pid == 0: + self.child() + self.children.append(pid) + + # @unittest.expectedFailure + def runTest(self): + """Test concurrent calls of oelite.util.makedirs""" + + os.write(self.w, "go go go\n") + time.sleep(0.01) + os.close(self.w) + with os.fdopen(self.r) as f: + v = [v.strip() for v in f] + d = {x: v.count(x) for x in v if x != "go go go"} + # On failure this won't give a very user-friendly error + # message, but it should contain information about the errors + # encountered. + self.assertEqual(d, {"OK": len(self.children)}) + self.assertTrue(os.path.isdir(self.path)) + + def tearDown(self): + for pid in self.children: + os.kill(pid, signal.SIGKILL) + os.waitpid(pid, 0) + super(MakedirsRaceTest, self).tearDown() + + +def run(options, args, config): + suite = unittest.TestSuite() + suite.addTest(MakedirsRaceTest()) + if options.show: + for t in suite: + print str(t), "--", t.shortDescription() + return 0 + runner = unittest.TextTestRunner(verbosity=3) + runner.run(suite) + + return 0 From 0ab11233a50263b4d628203badacc28a91e95824 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Wed, 25 Jan 2017 14:10:10 +0100 Subject: [PATCH 02/12] oelite/util.py: avoid races in makedirs The stdlib utility function os.makedirs raises EEXIST if the leaf directory already exists. Our makedirs helper tries to avoid that by checking for that condition upfront. However, when doing tasks in parallel, two different processes may call makedirs concurrently; both may initially see that the directory does not exist, one will successfully call os.makedirs(), while the other will get an EEXIST exception. Debugging that would be no fun, since the code clearly checks whether the path exists... This reimplements the helper to avoid such races. The previous implementation would also silently return success in the case where 'path' exists and is a regular file, which would in all likelyhood just lead to ENOTDIR later when the caller tries to create 'path + "/foo"'. It's better to fail early, and this is also consistent with 'mkdir -p' failing if the target exists but is not a directory. --- lib/oelite/cmd/test.py | 1 - lib/oelite/util.py | 28 ++++++++++++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/oelite/cmd/test.py b/lib/oelite/cmd/test.py index 5c27557d..460fad8c 100644 --- a/lib/oelite/cmd/test.py +++ b/lib/oelite/cmd/test.py @@ -55,7 +55,6 @@ def setUp(self): self.child() self.children.append(pid) - # @unittest.expectedFailure def runTest(self): """Test concurrent calls of oelite.util.makedirs""" diff --git a/lib/oelite/util.py b/lib/oelite/util.py index 016c0e32..3253b80a 100644 --- a/lib/oelite/util.py +++ b/lib/oelite/util.py @@ -4,6 +4,7 @@ import sys import subprocess import time +import errno from oelite.compat import dup_cloexec import oelite.signal @@ -208,10 +209,29 @@ def unique_list(seq): def makedirs(path, mode=0777): - if os.path.exists(path): - return - os.makedirs(path, mode) - return + # Create the directory 'path' and all necessary intermediate + # directories. The complexity stems from trying to ensure no + # exception is raised even if two processes concurrently try to do + # this operation. + while True: + try: + return os.mkdir(path, mode) + except OSError as e: + if e.errno == errno.EEXIST: + # Ensure that 'path' is a directory or a symlink to + # one. This will raise ENOTDIR here instead of later + # when the caller tries to create a file inside + # 'path'. + os.stat(path + "/.") + return None + elif e.errno == errno.ENOENT and path != "": + # makedirs("") must raise ENOENT, hence the second + # condition. We force u+rwx on all parent + # directories, anything else would be silly. + makedirs(os.path.dirname(path), mode | 0o700) + continue + else: + raise def touch(path, makedirs=False, truncate=False): From a881829dd4fca90d612e94ee446b046f44c1c88f Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Thu, 26 Jan 2017 12:31:26 +0100 Subject: [PATCH 03/12] oe test: test oelite.util.makedirs semantics Maybe overkill, but it's nice to know what one can expect. --- lib/oelite/cmd/test.py | 50 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/lib/oelite/cmd/test.py b/lib/oelite/cmd/test.py index 460fad8c..7014d5ed 100644 --- a/lib/oelite/cmd/test.py +++ b/lib/oelite/cmd/test.py @@ -25,6 +25,53 @@ def tearDown(self): os.chdir("/") shutil.rmtree(self.wd) + def test_makedirs(self): + """Test the semantics of oelite.util.makedirs""" + + makedirs = oelite.util.makedirs + touch = oelite.util.touch + self.assertIsNone(makedirs("x")) + self.assertIsNone(makedirs("x")) + self.assertIsNone(makedirs("y/")) + self.assertIsNone(makedirs("y/")) + self.assertIsNone(makedirs("x/y/z")) + # One can create multiple leaf directories in one go; mkdir -p + # behaves the same way. + self.assertIsNone(makedirs("z/.././z//w//../v")) + self.assertTrue(os.path.isdir("z/w")) + self.assertTrue(os.path.isdir("z/v")) + + self.assertIsNone(touch("x/a")) + with self.assertRaises(OSError) as cm: + makedirs("x/a") + self.assertEqual(cm.exception.errno, errno.ENOTDIR) + with self.assertRaises(OSError) as cm: + makedirs("x/a/z") + self.assertEqual(cm.exception.errno, errno.ENOTDIR) + + self.assertIsNone(os.symlink("a", "x/b")) + with self.assertRaises(OSError) as cm: + makedirs("x/b") + self.assertEqual(cm.exception.errno, errno.ENOTDIR) + with self.assertRaises(OSError) as cm: + makedirs("x/b/z") + self.assertEqual(cm.exception.errno, errno.ENOTDIR) + + self.assertIsNone(os.symlink("../y", "x/c")) + self.assertIsNone(makedirs("x/c")) + self.assertIsNone(makedirs("x/c/")) + + self.assertIsNone(os.symlink("nowhere", "broken")) + with self.assertRaises(OSError) as cm: + makedirs("broken") + self.assertEqual(cm.exception.errno, errno.ENOENT) + + self.assertIsNone(os.symlink("loop1", "loop2")) + self.assertIsNone(os.symlink("loop2", "loop1")) + with self.assertRaises(OSError) as cm: + makedirs("loop1") + self.assertEqual(cm.exception.errno, errno.ELOOP) + class MakedirsRaceTest(OEliteTest): def child(self): signal.alarm(2) # just in case of infinite recursion bugs @@ -75,11 +122,12 @@ def tearDown(self): os.kill(pid, signal.SIGKILL) os.waitpid(pid, 0) super(MakedirsRaceTest, self).tearDown() - def run(options, args, config): suite = unittest.TestSuite() suite.addTest(MakedirsRaceTest()) + suite.addTest(OEliteTest('test_makedirs')) + if options.show: for t in suite: print str(t), "--", t.shortDescription() From 0d509abfda0ee5d9df4bf72cc13548778f4ab8d1 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Mon, 30 Jan 2017 22:03:10 +0100 Subject: [PATCH 04/12] oelite/signal.py: move self test to 'oe test' command This gives it a better chance of actually getting run once in a while. Should we ever go for Py3 (3.2+, https://docs.python.org/3.2/library/subprocess.html), oelite.signal.restore_defaults becomes redundant but harmless. --- lib/oelite/cmd/test.py | 34 ++++++++++++++++++++++++++++++++++ lib/oelite/signal.py | 31 ------------------------------- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/lib/oelite/cmd/test.py b/lib/oelite/cmd/test.py index 7014d5ed..23c32ad3 100644 --- a/lib/oelite/cmd/test.py +++ b/lib/oelite/cmd/test.py @@ -9,6 +9,7 @@ import unittest import oelite.util +import oelite.signal description = "Run tests of internal utility functions" def add_parser_options(parser): @@ -123,10 +124,43 @@ def tearDown(self): os.waitpid(pid, 0) super(MakedirsRaceTest, self).tearDown() +class SigPipeTest(OEliteTest): + def run_sub(self, preexec_fn): + from subprocess import PIPE, Popen + + sub = Popen(["yes"], stdout=PIPE, stderr=PIPE, + preexec_fn = preexec_fn) + # Force a broken pipe. + sub.stdout.close() + err = sub.stderr.read() + ret = sub.wait() + return (ret, err) + + @unittest.skipIf(sys.version_info >= (3, 2), "Python is new enough") + def test_no_restore(self): + """Check that subprocesses inherit the SIG_IGNORE disposition for SIGPIPE.""" + (ret, err) = self.run_sub(None) + # This should terminate with a write error; we assume that + # 'yes' is so well-behaved that it both exits with a non-zero + # exit code as well as prints an error message containing + # strerror(errno). + self.assertGreater(ret, 0) + self.assertIn(os.strerror(errno.EPIPE), err) + + def test_restore(self): + """Check that oelite.signal.restore_defaults resets the SIGPIPE disposition.""" + (ret, err) = self.run_sub(oelite.signal.restore_defaults) + # This should terminate due to SIGPIPE, and not get a chance + # to write to stderr. + self.assertEqual(ret, -signal.SIGPIPE) + self.assertEqual(err, "") + def run(options, args, config): suite = unittest.TestSuite() suite.addTest(MakedirsRaceTest()) suite.addTest(OEliteTest('test_makedirs')) + suite.addTest(SigPipeTest('test_no_restore')) + suite.addTest(SigPipeTest('test_restore')) if options.show: for t in suite: diff --git a/lib/oelite/signal.py b/lib/oelite/signal.py index a7fd30cf..582aea44 100644 --- a/lib/oelite/signal.py +++ b/lib/oelite/signal.py @@ -30,34 +30,3 @@ def restore_defaults(): signal.signal(signal.SIGXFZ, signal.SIG_DFL) if hasattr(signal, "SIGXFSZ"): signal.signal(signal.SIGXFSZ, signal.SIG_DFL) - -def test_restore(): - import os - import subprocess - import errno - - PIPE=subprocess.PIPE - sub = subprocess.Popen(["yes"], stdout=PIPE, stderr=PIPE) - # Force a broken pipe. - sub.stdout.close() - err = sub.stderr.read() - # This should terminate with a write error; we assume that 'yes' - # is so well-behaved that it both exits with a non-zero exit code - # as well as prints an error message containing strerror(errno). - ret = sub.wait() - assert(ret > 0) - assert(os.strerror(errno.EPIPE) in err) - - sub = subprocess.Popen(["yes"], stdout=PIPE, stderr=PIPE, preexec_fn = restore_defaults) - # Force a broken pipe. - sub.stdout.close() - err = sub.stderr.read() - # This should terminate due to SIGPIPE, and not get a chance to write to stderr. - ret = sub.wait() - assert(ret == -signal.SIGPIPE) - assert(err == "") - -if __name__ == "__main__": - # To run: - # meta/core/lib$ python -m oelite.signal - test_restore() From 1231699993e3a31cf77d9b942c80f0878332f687 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Mon, 30 Jan 2017 22:32:57 +0100 Subject: [PATCH 05/12] oelite/compat.py: move cloexec tests to 'oe test' command --- lib/oelite/cmd/test.py | 27 +++++++++++++++++++++++++++ lib/oelite/compat.py | 22 ---------------------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/lib/oelite/cmd/test.py b/lib/oelite/cmd/test.py index 23c32ad3..81533c04 100644 --- a/lib/oelite/cmd/test.py +++ b/lib/oelite/cmd/test.py @@ -1,4 +1,5 @@ import errno +import fcntl import os import select import shutil @@ -10,6 +11,7 @@ import oelite.util import oelite.signal +import oelite.compat description = "Run tests of internal utility functions" def add_parser_options(parser): @@ -73,6 +75,30 @@ def test_makedirs(self): makedirs("loop1") self.assertEqual(cm.exception.errno, errno.ELOOP) + def test_cloexec(self): + open_cloexec = oelite.compat.open_cloexec + dup_cloexec = oelite.compat.dup_cloexec + + def has_cloexec(fd): + flags = fcntl.fcntl(fd, fcntl.F_GETFD) + return (flags & fcntl.FD_CLOEXEC) != 0 + + fd = open_cloexec("/dev/null", os.O_RDONLY) + self.assertGreaterEqual(fd, 0) + self.assertTrue(has_cloexec(fd)) + + fd2 = os.dup(fd) + self.assertGreaterEqual(fd2, 0) + self.assertFalse(has_cloexec(fd2)) + self.assertIsNone(os.close(fd2)) + + fd2 = dup_cloexec(fd) + self.assertGreaterEqual(fd2, 0) + self.assertTrue(has_cloexec(fd2)) + self.assertIsNone(os.close(fd2)) + + self.assertIsNone(os.close(fd)) + class MakedirsRaceTest(OEliteTest): def child(self): signal.alarm(2) # just in case of infinite recursion bugs @@ -161,6 +187,7 @@ def run(options, args, config): suite.addTest(OEliteTest('test_makedirs')) suite.addTest(SigPipeTest('test_no_restore')) suite.addTest(SigPipeTest('test_restore')) + suite.addTest(OEliteTest('test_cloexec')) if options.show: for t in suite: diff --git a/lib/oelite/compat.py b/lib/oelite/compat.py index 8451f4b2..f384aaa3 100755 --- a/lib/oelite/compat.py +++ b/lib/oelite/compat.py @@ -57,25 +57,3 @@ def open_cloexec_fallback(filename, flag, mode=0777): except KeyError: dup_cloexec = dup_cloexec_fallback - -def has_cloexec(fd): - flags = fcntl.fcntl(fd, fcntl.F_GETFD) - return (flags & fcntl.FD_CLOEXEC) != 0 - -def test_open_cloexec(): - fd = open_cloexec("/dev/null", os.O_RDONLY) - assert(has_cloexec(fd)) - os.close(fd) - fd = open_cloexec("/dev/null", os.O_WRONLY) - assert(has_cloexec(fd)) - os.close(fd) - -def test_dup_cloexec(): - fd = dup_cloexec(sys.stdin.fileno()) - assert(has_cloexec(fd)) - os.close(fd) - - -if __name__ == "__main__": - test_open_cloexec() - test_dup_cloexec() From 629598cff34316983463a348e62d5aa023095b8a Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Fri, 27 Jan 2017 01:03:21 +0100 Subject: [PATCH 06/12] oelite/util.py: introduce hash_file and chunk_iter helpers We get the sha1 or md5 digest of files in a couple of places by doing m = hashlib.md5() with open("foo") as f: m.update(f.read()) h = m.hexdigest() But that .read() call can be rather expensive if the file in question is a 100M tarball (opencv, I'm looking at you); it temporarily bumps our memory usage quite a lot, and it's also terribly inefficient (lots of page faults and completely blowing the cpu cache and TLB first by populating the gigantic buffer, then again when reading through it). So create a generator which simply yields a given file in chunks. Then the above would be: m = hashlib.md5() for chunk in iter_chunks("foo"): m.update(chunk) h = m.hexdigest() but since we do that middle part a lot, create another helper that does that and returns the passed-in hasher (we sometimes feed more than just one file to the same hasher, so we don't finish up by calling .hexdigest). Then the above could be simply h = oelite.util.hash_file(hashlib.md5(), "foo").hexdigest() --- lib/oelite/util.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/oelite/util.py b/lib/oelite/util.py index 3253b80a..34eda1a2 100644 --- a/lib/oelite/util.py +++ b/lib/oelite/util.py @@ -265,3 +265,17 @@ def timing_info(msg, start): msg += pretty_time(delta) info(msg) return + +def chunk_iter(fn, chunk_size = 32768): + with open(fn) as f: + while True: + chunk = f.read(chunk_size) + if chunk: + yield chunk + else: + return + +def hash_file(hasher, fn): + for chunk in chunk_iter(fn): + hasher.update(chunk) + return hasher From f9b9b338e545f4f4e78e8ebf6ebac30f5475837b Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Mon, 30 Jan 2017 23:23:45 +0100 Subject: [PATCH 07/12] oe test: add hash_file test cases --- lib/oelite/cmd/test.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lib/oelite/cmd/test.py b/lib/oelite/cmd/test.py index 81533c04..1f64d3c9 100644 --- a/lib/oelite/cmd/test.py +++ b/lib/oelite/cmd/test.py @@ -1,5 +1,6 @@ import errno import fcntl +import hashlib import os import select import shutil @@ -99,6 +100,26 @@ def has_cloexec(fd): self.assertIsNone(os.close(fd)) + def test_hash_file(self): + testv = [(0, "d41d8cd98f00b204e9800998ecf8427e", "da39a3ee5e6b4b0d3255bfef95601890afd80709"), + (1, "0cc175b9c0f1b6a831c399e269772661", "86f7e437faa5a7fce15d1ddcb9eaeaea377667b8"), + (1000, "cabe45dcc9ae5b66ba86600cca6b8ba8", "291e9a6c66994949b57ba5e650361e98fc36b1ba"), + (1000000, "7707d6ae4e027c70eea2a935c2296f21", "34aa973cd4c4daa4f61eeb2bdbad27316534016f")] + hash_file = oelite.util.hash_file + + for size, md5, sha1 in testv: + # open and say "aaaa...." :-) + with tempfile.NamedTemporaryFile() as tmp: + self.assertIsNone(tmp.write("a"*size)) + self.assertIsNone(tmp.flush()) + self.assertEqual(os.path.getsize(tmp.name), size) + + h = hash_file(hashlib.md5(), tmp.name).hexdigest() + self.assertEqual(h, md5) + + h = hash_file(hashlib.sha1(), tmp.name).hexdigest() + self.assertEqual(h, sha1) + class MakedirsRaceTest(OEliteTest): def child(self): signal.alarm(2) # just in case of infinite recursion bugs @@ -188,6 +209,7 @@ def run(options, args, config): suite.addTest(SigPipeTest('test_no_restore')) suite.addTest(SigPipeTest('test_restore')) suite.addTest(OEliteTest('test_cloexec')) + suite.addTest(OEliteTest('test_hash_file')) if options.show: for t in suite: From 4cb03b238c91edcb70078842c5360a27364d9637 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Mon, 30 Jan 2017 23:38:33 +0100 Subject: [PATCH 08/12] oelite/fetch/fetch.py: use hash_file helper in mirror() method --- lib/oelite/fetch/fetch.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/oelite/fetch/fetch.py b/lib/oelite/fetch/fetch.py index 0242326a..f3a363f3 100644 --- a/lib/oelite/fetch/fetch.py +++ b/lib/oelite/fetch/fetch.py @@ -328,13 +328,11 @@ def mirror(self, d, mirror): dst = os.path.join(mirror, src[len(self.ingredients)+1:]) if os.path.exists(dst): m = hashlib.md5() - with open(src, "r") as srcfile: - m.update(srcfile.read()) - src_md5 = m.hexdigest() + oelite.util.hash_file(m, src) + src_md5 = m.hexdigest() m = hashlib.md5() - with open(dst, "r") as dstfile: - m.update(dstfile.read()) - dst_md5 = m.hexdigest() + oelite.util.hash_file(m, dst) + dst_md5 = m.hexdigest() if src_md5 != dst_md5: print "Mirror inconsistency:", dst print "%s != %s"%(src_md5, dst_md5) From de6a3ae4285a32f9e9aada4774e3175878babec4 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Mon, 30 Jan 2017 23:41:16 +0100 Subject: [PATCH 09/12] oelite/fetch/url.py: use hash_file helper This makes signature verification (hence do_fetch) somewhat cheaper for large ingredients tar balls (I've measured about 10x for the largest ones). --- lib/oelite/fetch/url.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/oelite/fetch/url.py b/lib/oelite/fetch/url.py index c3d69d02..129efaf4 100644 --- a/lib/oelite/fetch/url.py +++ b/lib/oelite/fetch/url.py @@ -83,7 +83,7 @@ def grabbedsignature(self): def localsignature(self): m = hashlib.sha1() - m.update(open(self.localpath, "r").read()) + oelite.util.hash_file(m, self.localpath) return m.hexdigest() def get_proxies(self, d): From 9d2217477a357ff5bc9c652a0e9d35c8eeae8462 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Mon, 30 Jan 2017 23:49:48 +0100 Subject: [PATCH 10/12] oelite/fetch/local.py: use hash_file helper --- lib/oelite/fetch/local.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/oelite/fetch/local.py b/lib/oelite/fetch/local.py index a43f2367..789fc871 100644 --- a/lib/oelite/fetch/local.py +++ b/lib/oelite/fetch/local.py @@ -1,5 +1,6 @@ import oelite.fetch import oelite.path +import oelite.util import os import hashlib @@ -35,6 +36,6 @@ def signature(self): if os.path.isdir(self.localpath): raise oelite.fetch.NoSignature(self.uri, "can't compute directory signature") m = hashlib.sha1() - m.update(open(self.localpath, "r").read()) + oelite.util.hash_file(m, self.localpath) self._signature = m.digest() return self._signature From 75b2f04aa95dff138e171df98a1b45229cbbc817 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 31 Jan 2017 00:00:46 +0100 Subject: [PATCH 11/12] oelite/fetch/fetch.py: remove orphaned code The last calls of the write_checksum and verify_checksum methods vanished with 6f3677d6dd (class/fetch, lib/oelite: Improve fetch support). That commit also introduced the cache() method, but as far as I can tell, that has never had a caller; moreover, no fetcher has ever implemented it, so it's not really clear what its semantics should be. --- lib/oelite/fetch/fetch.py | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/lib/oelite/fetch/fetch.py b/lib/oelite/fetch/fetch.py index f3a363f3..97f69600 100644 --- a/lib/oelite/fetch/fetch.py +++ b/lib/oelite/fetch/fetch.py @@ -228,29 +228,6 @@ def signature(self): print "%s: no checksum known for %s"%(self.recipe, url) return "" - def cache(self): - if not "cache" in dir(self.fetcher): - return True - return self.fetcher.cache() - - def write_checksum(self, filepath): - md5path = filepath + ".md5" - checksum = hashlib.md5() - with open(filepath) as f: - checksum.update(f.read()) - with open(md5path, "w") as f: - f.write(checksum.digest()) - - def verify_checksum(self, filepath): - md5path = filepath + ".md5" - if not os.path.exists(md5path): - return None - checksum = hashlib.md5() - with open(filepath) as f: - checksum.update(f.read()) - with open(md5path) as f: - return f.readline().strip() == checksum.digest() - def fetch(self): if not "fetch" in dir(self.fetcher): return True From 0e7996bb45715cf3280395f7fc40e227eab5f391 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Tue, 31 Jan 2017 00:28:56 +0100 Subject: [PATCH 12/12] oelite/fetch/url.py: use oelite.util.makedirs helper As further prep for parallel do_fetch / do_unpack, don't open-code the old and racy version of oelite.util.makedirs. --- lib/oelite/fetch/url.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/oelite/fetch/url.py b/lib/oelite/fetch/url.py index 129efaf4..75670d25 100644 --- a/lib/oelite/fetch/url.py +++ b/lib/oelite/fetch/url.py @@ -119,8 +119,7 @@ def grab(url, filename, timeout=120, retry=5, proxies=None, passive_ftp=True): d = os.path.dirname(filename) f = os.path.basename(filename) - if not os.path.exists(d): - os.makedirs(d) + oelite.util.makedirs(d) # Use mkstemp to create and open a guaranteed unique file. We use # the file descriptor as wget's stdout. We must download to the