[gentoo-catalyst] [PATCH] stagebase: fix path to snapshot hash file

[gentoo-catalyst] [PATCH] stagebase: fix path to snapshot hash file

[Mike Frysinger]

When we write the hash, we do so by using:
	snapshot_cache_path + / + catalyst-hash

But when we read it, we do so by:
	snapshot_cache_path + catalyst-hash

If the path lacks a trailing /, then we never read the correct hash
file.  The current helper returns -1 on missing file errors which is
compared against the existing hash.  In essence, we always trigger a
cache miss.

Add the missing / to the read logic.
---
 catalyst/base/stagebase.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index f81c51b..a2a8520 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -817,7 +817,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
 		if "snapcache" in self.settings["options"]:
 			snapshot_cache_hash=\
 				read_from_clst(self.settings["snapshot_cache_path"]+\
-				"catalyst-hash")
+				"/" + "catalyst-hash")
 			unpack_info['mode'] = self.decompressor.determine_mode(
 				unpack_info['source'])
 
-- 
2.5.2

[gentoo-catalyst] [PATCH] stagebase: replace read_from_clst with snakeoil

[Mike Frysinger]

The read_from_clst function is used in one place, and this function is
an overwrought "read all the data from a file" function.  Replace it
with a snakeoil function.
---
 catalyst/base/stagebase.py |  7 +++----
 catalyst/support.py        | 15 ---------------
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index a2a8520..652dd9d 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -16,8 +16,7 @@ from DeComp.compress import CompressMap
 from catalyst.defaults import (SOURCE_MOUNT_DEFAULTS, TARGET_MOUNT_DEFAULTS,
 	PORT_LOGDIR_CLEAN)
 from catalyst.support import (CatalystError, msg, file_locate, normpath,
-	cmd, warn, list_bashify, read_makeconf, read_from_clst, ismount,
-	file_check)
+	cmd, warn, list_bashify, read_makeconf, ismount, file_check)
 from catalyst.base.targetbase import TargetBase
 from catalyst.base.clearbase import ClearBase
 from catalyst.base.genbase import GenBase
@@ -816,8 +815,8 @@ class StageBase(TargetBase, ClearBase, GenBase):
 		print "unpack(), target_portdir = " + target_portdir
 		if "snapcache" in self.settings["options"]:
 			snapshot_cache_hash=\
-				read_from_clst(self.settings["snapshot_cache_path"]+\
-				"/" + "catalyst-hash")
+				fileutils.readfile(self.settings["snapshot_cache_path"]+\
+				"/" + "catalyst-hash", True)
 			unpack_info['mode'] = self.decompressor.determine_mode(
 				unpack_info['source'])
 
diff --git a/catalyst/support.py b/catalyst/support.py
index 960a91d..c629025 100644
--- a/catalyst/support.py
+++ b/catalyst/support.py
@@ -13,21 +13,6 @@ from catalyst.defaults import verbosity, valid_config_file_values
 BASH_BINARY             = "/bin/bash"
 
 
-def read_from_clst(path):
-	line = ''
-	myline = ''
-	try:
-		myf = open(path, "r")
-	except Exception:
-		return -1
-		#raise CatalystError("Could not open file " + path)
-	for line in myf.readlines():
-		#line = line.replace("\n", "") # drop newline
-		myline = myline + line
-	myf.close()
-	return myline
-
-
 def list_bashify(mylist):
 	if type(mylist)==types.StringType:
 		mypack=[mylist]
-- 
2.5.2

[gentoo-catalyst] [PATCH v2] stagebase: fix path to snapshot hash file

[Mike Frysinger]

When we write the hash, we do so by using:
	snapshot_cache_path + / + catalyst-hash

But when we read it, we do so by:
	snapshot_cache_path + catalyst-hash

If the path lacks a trailing /, then we never read the correct hash
file.  The current helper returns -1 on missing file errors which is
compared against the existing hash.  In essence, we always trigger a
cache miss.

Clean up the code to properly create the path and use that var in both
places to prevent future breakage.
---
v2
	- do it better like an all star

 catalyst/base/stagebase.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
index f81c51b..7d069fa 100644
--- a/catalyst/base/stagebase.py
+++ b/catalyst/base/stagebase.py
@@ -815,9 +815,9 @@ class StageBase(TargetBase, ClearBase, GenBase):
 		print self.settings["chroot_path"]
 		print "unpack(), target_portdir = " + target_portdir
 		if "snapcache" in self.settings["options"]:
-			snapshot_cache_hash=\
-				read_from_clst(self.settings["snapshot_cache_path"]+\
-				"catalyst-hash")
+			snapshot_cache_hash_path = pjoin(
+				self.settings['snapshot_cache_path'], 'catalyst-hash')
+			snapshot_cache_hash = read_from_clst(snapshot_cache_hash_path)
 			unpack_info['mode'] = self.decompressor.determine_mode(
 				unpack_info['source'])
 
@@ -862,8 +862,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
 				print unpack_errmsg %(unpack_info)
 
 			if "snapcache" in self.settings["options"]:
-				myf=open(self.settings["snapshot_cache_path"] +
-					"/" + "catalyst-hash","w")
+				myf = open(snapshot_cache_hash_path, 'w')
 				myf.write(self.settings["snapshot_path_hash"])
 				myf.close()
 			else:
-- 
2.5.2

Re: [gentoo-catalyst] [PATCH] stagebase: fix path to snapshot hash file

[Brian Dolbec]

On Thu,  8 Oct 2015 18:52:57 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> When we write the hash, we do so by using:
> 	snapshot_cache_path + / + catalyst-hash
> 
> But when we read it, we do so by:
> 	snapshot_cache_path + catalyst-hash
> 
> If the path lacks a trailing /, then we never read the correct hash
> file.  The current helper returns -1 on missing file errors which is
> compared against the existing hash.  In essence, we always trigger a
> cache miss.
> 
> Add the missing / to the read logic.
> ---
>  catalyst/base/stagebase.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
> index f81c51b..a2a8520 100644
> --- a/catalyst/base/stagebase.py
> +++ b/catalyst/base/stagebase.py
> @@ -817,7 +817,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
>  		if "snapcache" in self.settings["options"]:
>  			snapshot_cache_hash=\
>  				read_from_clst(self.settings["snapshot_cache_path"]+\
> -				"catalyst-hash")
> +				"/" + "catalyst-hash")
>  			unpack_info['mode'] =
> self.decompressor.determine_mode( unpack_info['source'])
>  


works for me.  It was probably a bug introduced by me removing trailing
slashes in pretty much all variables.  I kept running into issues with
slashes causing problems.  Sometimes they were needed other times they
caused failures.

It was easier to remove them by default then add them only when needed.
-- 
Brian Dolbec <dolsen>

Re: [gentoo-catalyst] [PATCH v2] stagebase: fix path to snapshot hash file

[Brian Dolbec]

On Thu,  8 Oct 2015 20:23:05 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> When we write the hash, we do so by using:
> 	snapshot_cache_path + / + catalyst-hash
> 
> But when we read it, we do so by:
> 	snapshot_cache_path + catalyst-hash
> 
> If the path lacks a trailing /, then we never read the correct hash
> file.  The current helper returns -1 on missing file errors which is
> compared against the existing hash.  In essence, we always trigger a
> cache miss.
> 
> Clean up the code to properly create the path and use that var in both
> places to prevent future breakage.
> ---
> v2
> 	- do it better like an all star
> 
>  catalyst/base/stagebase.py | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/catalyst/base/stagebase.py b/catalyst/base/stagebase.py
> index f81c51b..7d069fa 100644
> --- a/catalyst/base/stagebase.py
> +++ b/catalyst/base/stagebase.py
> @@ -815,9 +815,9 @@ class StageBase(TargetBase, ClearBase, GenBase):
>  		print self.settings["chroot_path"]
>  		print "unpack(), target_portdir = " + target_portdir
>  		if "snapcache" in self.settings["options"]:
> -			snapshot_cache_hash=\
> -
> read_from_clst(self.settings["snapshot_cache_path"]+\
> -				"catalyst-hash")
> +			snapshot_cache_hash_path = pjoin(
> +
> self.settings['snapshot_cache_path'], 'catalyst-hash')
> +			snapshot_cache_hash =
> read_from_clst(snapshot_cache_hash_path) unpack_info['mode'] =
> self.decompressor.determine_mode( unpack_info['source'])
>  
> @@ -862,8 +862,7 @@ class StageBase(TargetBase, ClearBase, GenBase):
>  				print unpack_errmsg %(unpack_info)
>  
>  			if "snapcache" in self.settings["options"]:
> -
> myf=open(self.settings["snapshot_cache_path"] +
> -					"/" + "catalyst-hash","w")
> +				myf = open(snapshot_cache_hash_path,
> 'w') myf.write(self.settings["snapshot_path_hash"])
>  				myf.close()
>  			else:

looks good.  There shouldn't be too many like this left now.  I did as
many as I could when changing the related code.  But I know there was
still more.

-- 
Brian Dolbec <dolsen>