[Scons-dev] fix for partial reads from cache

[Raven Kopelman]

Looks like attachment is not easy to access; here's plaintext patch

diff -ru -x '*.pyc' SCons.orig/CacheDir.py
/usr/local/lib/python2.7/dist-packages/scons/SCons/CacheDir.py
--- SCons.orig/CacheDir.py 2020-10-23 16:12:44.773349285 -0700
+++ /usr/local/lib/python2.7/dist-packages/scons/SCons/CacheDir.py
2020-10-23 16:19:48.676570382 -0700
@@ -56,7 +56,20 @@
         if fs.islink(cachefile):
             fs.symlink(fs.readlink(cachefile), t.get_internal_path())
         else:
-            env.copy_from_cache(cachefile, t.get_internal_path())
+            try:
+                env.copy_from_cache(cachefile, t.get_internal_path())
+            except:
+                try:
+                    # In case file was partially retrieved (and now
corrupt)
+                    # delete it to avoid poisoning commands like 'ar' that
+                    # read from the initial state of the file they are
writing
+                    # to.
+                    t.fs.unlink(t.get_internal_path())
+                except:
+                    pass
+
+                raise
+
             try:
                 os.utime(cachefile, None)
             except OSError:
@@ -71,7 +84,7 @@
     cd = env.get_CacheDir()
     cachedir, cachefile = cd.cachepath(t)
     if t.fs.exists(cachefile):
-        return "Retrieved `%s' from cache" % t.get_internal_path()
+        return "Retrieving `%s' from cache" % t.get_internal_path()
     return None

 CacheRetrieve = SCons.Action.Action(CacheRetrieveFunc, CacheRetrieveString)
diff -ru -x '*.pyc' SCons.orig/Taskmaster.py
/usr/local/lib/python2.7/dist-packages/scons/SCons/Taskmaster.py
--- SCons.orig/Taskmaster.py 2020-10-23 16:12:44.785349378 -0700
+++ /usr/local/lib/python2.7/dist-packages/scons/SCons/Taskmaster.py
2020-10-23 15:55:41.521758762 -0700
@@ -243,10 +243,13 @@
                 cached_targets.append(t)
             if len(cached_targets) < len(self.targets):
                 # Remove targets before building. It's possible that we
-                # partially retrieved targets from the cache, leaving
-                # them in read-only mode. That might cause the command
+                # retrieved a subset of targets from the cache, leaving
+                # them in an inconsistent state. That might cause the
command
                 # to fail.
                 #
+                # Note that retrieve_from_cache() ensures no single target
can
+                # be partially retrieved (file left in corrupt state).
+                #
                 for t in cached_targets:
                     try:
                         t.fs.unlink(t.get_internal_path())

--
*Raven Kopelman* | Team Lead, Senior Developer

Safe Software Inc.
*T* 604.501.9985 x 331 | *F* 604.501.9965
raven.kopelman at safe.com | www.safe.com

<http://www.safe.com/emailsignature>


On Tue, Oct 27, 2020 at 1:00 PM Raven Kopelman <raven.kopelman at safe.com>
wrote:

> Hi scons devs,
>
> Here's a fix to clean up reads from the cache that fail part way through.
> Absolutely experienced by us in production (non-local shared cache).  This
> may be what my last UUID temp name fix was really trying to address, but I
> can imagine edge cases that the UUID fix could still improve.
>
> This could also have been fixed in Taskmaster.py near my comment change,
> but the reasons for cleanup seemed different enough to keep them distinct.
>
> Also cleaned up the logged message tense, as message is logged before the
> cache retrieval begins.
>
> Thanks!
> --
> *Raven Kopelman* | Team Lead, Senior Developer
>
> Safe Software Inc.
> *T* 604.501.9985 x 331 | *F* 604.501.9965
> raven.kopelman at safe.com | www.safe.com
>
> <http://www.safe.com/emailsignature>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://pairlist2.pair.net/pipermail/scons-dev/attachments/20201027/24bca35c/attachment.html>