[Scons-dev] fix for partial reads from cache

[Bill Deegan]

Any chance you can make a pull request for this?

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

> 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>
>>
> _______________________________________________
> Scons-dev mailing list
> Scons-dev at scons.org
> https://pairlist2.pair.net/mailman/listinfo/scons-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://pairlist2.pair.net/pipermail/scons-dev/attachments/20201027/592a5c10/attachment-0001.html>