Re: [Scons-dev] fix for partial reads from cache

2020-10-27 Thread Raven Kopelman 
Yes to the pull, but I don't know if I'll be able to invest the time to
make it meet your project requirements...

https://github.com/SCons/scons/pull/3819

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

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


___
Scons-dev mailing list
Scons-dev@scons.org
https://pairlist2.pair.net/mailman/listinfo/scons-dev

Re: [Scons-dev] fix for partial reads from cache

2020-10-27 Thread Bill Deegan 
Any chance you can make a pull request for this?

On Tue, Oct 27, 2020 at 1:02 PM Raven Kopelman 
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.kopel...@safe.com | www.safe.com
>
> 
>
>
> On Tue, Oct 27, 2020 at 1:00 PM Raven Kopelman 
> 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.kopel...@safe.com | www.safe.com
>>
>> 
>>
> ___
> Scons-dev mailing list
> Scons-dev@scons.org
> https://pairlist2.pair.net/mailman/listinfo/scons-dev
>
___
Scons-dev mailing list
Scons-dev@scons.org
https://pairlist2.pair.net/mailman/listinfo/scons-dev

Re: [Scons-dev] fix for partial reads from cache

2020-10-27 Thread 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.kopel...@safe.com | www.safe.com




On Tue, Oct 27, 2020 at 1:00 PM Raven Kopelman 
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.kopel...@safe.com | www.safe.com
>
> 
>
___
Scons-dev mailing list
Scons-dev@scons.org
https://pairlist2.pair.net/mailman/listinfo/scons-dev

[Scons-dev] fix for partial reads from cache

2020-10-27 Thread Raven Kopelman 
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.kopel...@safe.com | www.safe.com


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())
___
Scons-dev mailing list
Scons-dev@scons.org
https://pairlist2.pair.net/mailman/listinfo/scons-dev