From 97d776476c6845ca9736486c31b58c780b4a659b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela=20Pastor?= Date: Thu, 12 Oct 2017 13:05:45 -0700 Subject: [PATCH 1/6] Extract file names to constants --- clcache.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/clcache.py b/clcache.py index 860c28dd..6c53d3d1 100644 --- a/clcache.py +++ b/clcache.py @@ -354,6 +354,10 @@ def forPath(path): class CompilerArtifactsSection(object): + OBJECT_FILE = 'object' + STDOUT_FILE = 'output.txt' + STDERR_FILE = 'stderr.txt' + def __init__(self, compilerArtifactsSectionDir): self.compilerArtifactsSectionDir = compilerArtifactsSectionDir self.lock = CacheLock.forPath(self.compilerArtifactsSectionDir) @@ -365,7 +369,7 @@ def cacheEntries(self): return childDirectories(self.compilerArtifactsSectionDir, absolute=False) def cachedObjectName(self, key): - return os.path.join(self.cacheEntryDir(key), "object") + return os.path.join(self.cacheEntryDir(key), CompilerArtifactsSection.OBJECT_FILE) def hasEntry(self, key): return os.path.exists(self.cacheEntryDir(key)) @@ -374,16 +378,16 @@ def setEntry(self, key, artifacts): ensureDirectoryExists(self.cacheEntryDir(key)) if artifacts.objectFilePath is not None: copyOrLink(artifacts.objectFilePath, self.cachedObjectName(key)) - self._setCachedCompilerConsoleOutput(key, 'output.txt', artifacts.stdout) + self._setCachedCompilerConsoleOutput(key, CompilerArtifactsSection.STDOUT_FILE, artifacts.stdout) if artifacts.stderr != '': - self._setCachedCompilerConsoleOutput(key, 'stderr.txt', artifacts.stderr) + self._setCachedCompilerConsoleOutput(key, CompilerArtifactsSection.STDERR_FILE, artifacts.stderr) def getEntry(self, key): assert self.hasEntry(key) return CompilerArtifacts( self.cachedObjectName(key), - self._getCachedCompilerConsoleOutput(key, 'output.txt'), - self._getCachedCompilerConsoleOutput(key, 'stderr.txt') + self._getCachedCompilerConsoleOutput(key, CompilerArtifactsSection.STDOUT_FILE), + self._getCachedCompilerConsoleOutput(key, CompilerArtifactsSection.STDERR_FILE) ) def _getCachedCompilerConsoleOutput(self, key, fileName): From 0bf5533cdd294f4f472611cf4faed6002dfdd6d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela=20Pastor?= Date: Thu, 12 Oct 2017 13:18:30 -0700 Subject: [PATCH 2/6] Compute file path in-place in CompilerArtifactsSection.setEntry() This will help refactoring to write first to a temporary folder and replace. --- clcache.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/clcache.py b/clcache.py index 6c53d3d1..d9a19beb 100644 --- a/clcache.py +++ b/clcache.py @@ -375,32 +375,32 @@ def hasEntry(self, key): return os.path.exists(self.cacheEntryDir(key)) def setEntry(self, key, artifacts): - ensureDirectoryExists(self.cacheEntryDir(key)) + cacheEntryDir = self.cacheEntryDir(key) + ensureDirectoryExists(cacheEntryDir) if artifacts.objectFilePath is not None: - copyOrLink(artifacts.objectFilePath, self.cachedObjectName(key)) - self._setCachedCompilerConsoleOutput(key, CompilerArtifactsSection.STDOUT_FILE, artifacts.stdout) + copyOrLink(artifacts.objectFilePath, os.path.join(cacheEntryDir, CompilerArtifactsSection.OBJECT_FILE)) + self._setCachedCompilerConsoleOutput(key, os.path.join(cacheEntryDir, CompilerArtifactsSection.STDOUT_FILE), artifacts.stdout) if artifacts.stderr != '': - self._setCachedCompilerConsoleOutput(key, CompilerArtifactsSection.STDERR_FILE, artifacts.stderr) + self._setCachedCompilerConsoleOutput(key, os.path.join(cacheEntryDir, CompilerArtifactsSection.STDERR_FILE), artifacts.stderr) def getEntry(self, key): assert self.hasEntry(key) + cacheEntryDir = self.cacheEntryDir(key) return CompilerArtifacts( - self.cachedObjectName(key), - self._getCachedCompilerConsoleOutput(key, CompilerArtifactsSection.STDOUT_FILE), - self._getCachedCompilerConsoleOutput(key, CompilerArtifactsSection.STDERR_FILE) + os.path.join(cacheEntryDir, CompilerArtifactsSection.OBJECT_FILE), + self._getCachedCompilerConsoleOutput(key, os.path.join(cacheEntryDir, CompilerArtifactsSection.STDOUT_FILE)), + self._getCachedCompilerConsoleOutput(key, os.path.join(cacheEntryDir, CompilerArtifactsSection.STDERR_FILE)) ) - def _getCachedCompilerConsoleOutput(self, key, fileName): + def _getCachedCompilerConsoleOutput(self, key, path): try: - outputFilePath = os.path.join(self.cacheEntryDir(key), fileName) - with open(outputFilePath, 'rb') as f: + with open(path, 'rb') as f: return f.read().decode(CACHE_COMPILER_OUTPUT_STORAGE_CODEC) except IOError: return '' - def _setCachedCompilerConsoleOutput(self, key, fileName, output): - outputFilePath = os.path.join(self.cacheEntryDir(key), fileName) - with open(outputFilePath, 'wb') as f: + def _setCachedCompilerConsoleOutput(self, key, path, output): + with open(path, 'wb') as f: f.write(output.encode(CACHE_COMPILER_OUTPUT_STORAGE_CODEC)) From 35f50989a8c70750bcd77e3107cca7b768a9d827 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela=20Pastor?= Date: Thu, 12 Oct 2017 13:20:23 -0700 Subject: [PATCH 3/6] Remove unused parameter from interface --- clcache.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clcache.py b/clcache.py index d9a19beb..018cff73 100644 --- a/clcache.py +++ b/clcache.py @@ -379,27 +379,27 @@ def setEntry(self, key, artifacts): ensureDirectoryExists(cacheEntryDir) if artifacts.objectFilePath is not None: copyOrLink(artifacts.objectFilePath, os.path.join(cacheEntryDir, CompilerArtifactsSection.OBJECT_FILE)) - self._setCachedCompilerConsoleOutput(key, os.path.join(cacheEntryDir, CompilerArtifactsSection.STDOUT_FILE), artifacts.stdout) + self._setCachedCompilerConsoleOutput(os.path.join(cacheEntryDir, CompilerArtifactsSection.STDOUT_FILE), artifacts.stdout) if artifacts.stderr != '': - self._setCachedCompilerConsoleOutput(key, os.path.join(cacheEntryDir, CompilerArtifactsSection.STDERR_FILE), artifacts.stderr) + self._setCachedCompilerConsoleOutput(os.path.join(cacheEntryDir, CompilerArtifactsSection.STDERR_FILE), artifacts.stderr) def getEntry(self, key): assert self.hasEntry(key) cacheEntryDir = self.cacheEntryDir(key) return CompilerArtifacts( os.path.join(cacheEntryDir, CompilerArtifactsSection.OBJECT_FILE), - self._getCachedCompilerConsoleOutput(key, os.path.join(cacheEntryDir, CompilerArtifactsSection.STDOUT_FILE)), - self._getCachedCompilerConsoleOutput(key, os.path.join(cacheEntryDir, CompilerArtifactsSection.STDERR_FILE)) + self._getCachedCompilerConsoleOutput(os.path.join(cacheEntryDir, CompilerArtifactsSection.STDOUT_FILE)), + self._getCachedCompilerConsoleOutput(os.path.join(cacheEntryDir, CompilerArtifactsSection.STDERR_FILE)) ) - def _getCachedCompilerConsoleOutput(self, key, path): + def _getCachedCompilerConsoleOutput(self, path): try: with open(path, 'rb') as f: return f.read().decode(CACHE_COMPILER_OUTPUT_STORAGE_CODEC) except IOError: return '' - def _setCachedCompilerConsoleOutput(self, key, path, output): + def _setCachedCompilerConsoleOutput(self, path, output): with open(path, 'wb') as f: f.write(output.encode(CACHE_COMPILER_OUTPUT_STORAGE_CODEC)) From 423809291ff5d726c7d01c57a8feaa8ee5851dbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela=20Pastor?= Date: Thu, 12 Oct 2017 13:30:28 -0700 Subject: [PATCH 4/6] Write cache entry to a temporaryDir and replace This guarantees that the whole cache entry is replaced atomically. This precents cache corruption when the clcache process is terminated in the middle of a write. --- clcache.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/clcache.py b/clcache.py index 018cff73..4d5d2d0b 100644 --- a/clcache.py +++ b/clcache.py @@ -376,12 +376,16 @@ def hasEntry(self, key): def setEntry(self, key, artifacts): cacheEntryDir = self.cacheEntryDir(key) - ensureDirectoryExists(cacheEntryDir) + # Write new files to a temporary directory + tempEntryDir = cacheEntryDir + '.new' + ensureDirectoryExists(tempEntryDir) if artifacts.objectFilePath is not None: - copyOrLink(artifacts.objectFilePath, os.path.join(cacheEntryDir, CompilerArtifactsSection.OBJECT_FILE)) - self._setCachedCompilerConsoleOutput(os.path.join(cacheEntryDir, CompilerArtifactsSection.STDOUT_FILE), artifacts.stdout) + copyOrLink(artifacts.objectFilePath, os.path.join(tempEntryDir, CompilerArtifactsSection.OBJECT_FILE)) + self._setCachedCompilerConsoleOutput(os.path.join(tempEntryDir, CompilerArtifactsSection.STDOUT_FILE), artifacts.stdout) if artifacts.stderr != '': - self._setCachedCompilerConsoleOutput(os.path.join(cacheEntryDir, CompilerArtifactsSection.STDERR_FILE), artifacts.stderr) + self._setCachedCompilerConsoleOutput(os.path.join(tempEntryDir, CompilerArtifactsSection.STDERR_FILE), artifacts.stderr) + # Replace the full cache entry atomically + os.replace(tempEntryDir, cacheEntryDir) def getEntry(self, key): assert self.hasEntry(key) From 0368719c33656485ed3d030d5091bf50c1e2e63c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela=20Pastor?= Date: Thu, 12 Oct 2017 14:22:59 -0700 Subject: [PATCH 5/6] Split long lines --- clcache.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/clcache.py b/clcache.py index 4d5d2d0b..43ad547b 100644 --- a/clcache.py +++ b/clcache.py @@ -378,12 +378,17 @@ def setEntry(self, key, artifacts): cacheEntryDir = self.cacheEntryDir(key) # Write new files to a temporary directory tempEntryDir = cacheEntryDir + '.new' + # Remove any possible left-over in tempEntryDir from previous executions + rmtree(tempEntryDir, ignore_errors=True) ensureDirectoryExists(tempEntryDir) if artifacts.objectFilePath is not None: - copyOrLink(artifacts.objectFilePath, os.path.join(tempEntryDir, CompilerArtifactsSection.OBJECT_FILE)) - self._setCachedCompilerConsoleOutput(os.path.join(tempEntryDir, CompilerArtifactsSection.STDOUT_FILE), artifacts.stdout) + copyOrLink(artifacts.objectFilePath, + os.path.join(tempEntryDir, CompilerArtifactsSection.OBJECT_FILE)) + self._setCachedCompilerConsoleOutput(os.path.join(tempEntryDir, CompilerArtifactsSection.STDOUT_FILE), + artifacts.stdout) if artifacts.stderr != '': - self._setCachedCompilerConsoleOutput(os.path.join(tempEntryDir, CompilerArtifactsSection.STDERR_FILE), artifacts.stderr) + self._setCachedCompilerConsoleOutput(os.path.join(tempEntryDir, CompilerArtifactsSection.STDERR_FILE), + artifacts.stderr) # Replace the full cache entry atomically os.replace(tempEntryDir, cacheEntryDir) From 26f01f0757b60bbbbfb3f812f498ef97b9820a60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela=20Pastor?= Date: Thu, 12 Oct 2017 14:28:01 -0700 Subject: [PATCH 6/6] Make {get,set}CachedCompilerConsoleOutput free functions --- clcache.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/clcache.py b/clcache.py index 43ad547b..43aff165 100644 --- a/clcache.py +++ b/clcache.py @@ -124,6 +124,17 @@ def atomicWrite(fileName): os.replace(tempFileName, fileName) +def getCachedCompilerConsoleOutput(path): + try: + with open(path, 'rb') as f: + return f.read().decode(CACHE_COMPILER_OUTPUT_STORAGE_CODEC) + except IOError: + return '' + +def setCachedCompilerConsoleOutput(path, output): + with open(path, 'wb') as f: + f.write(output.encode(CACHE_COMPILER_OUTPUT_STORAGE_CODEC)) + class IncludeNotFoundException(Exception): pass @@ -384,11 +395,11 @@ def setEntry(self, key, artifacts): if artifacts.objectFilePath is not None: copyOrLink(artifacts.objectFilePath, os.path.join(tempEntryDir, CompilerArtifactsSection.OBJECT_FILE)) - self._setCachedCompilerConsoleOutput(os.path.join(tempEntryDir, CompilerArtifactsSection.STDOUT_FILE), - artifacts.stdout) + setCachedCompilerConsoleOutput(os.path.join(tempEntryDir, CompilerArtifactsSection.STDOUT_FILE), + artifacts.stdout) if artifacts.stderr != '': - self._setCachedCompilerConsoleOutput(os.path.join(tempEntryDir, CompilerArtifactsSection.STDERR_FILE), - artifacts.stderr) + setCachedCompilerConsoleOutput(os.path.join(tempEntryDir, CompilerArtifactsSection.STDERR_FILE), + artifacts.stderr) # Replace the full cache entry atomically os.replace(tempEntryDir, cacheEntryDir) @@ -397,21 +408,10 @@ def getEntry(self, key): cacheEntryDir = self.cacheEntryDir(key) return CompilerArtifacts( os.path.join(cacheEntryDir, CompilerArtifactsSection.OBJECT_FILE), - self._getCachedCompilerConsoleOutput(os.path.join(cacheEntryDir, CompilerArtifactsSection.STDOUT_FILE)), - self._getCachedCompilerConsoleOutput(os.path.join(cacheEntryDir, CompilerArtifactsSection.STDERR_FILE)) + getCachedCompilerConsoleOutput(os.path.join(cacheEntryDir, CompilerArtifactsSection.STDOUT_FILE)), + getCachedCompilerConsoleOutput(os.path.join(cacheEntryDir, CompilerArtifactsSection.STDERR_FILE)) ) - def _getCachedCompilerConsoleOutput(self, path): - try: - with open(path, 'rb') as f: - return f.read().decode(CACHE_COMPILER_OUTPUT_STORAGE_CODEC) - except IOError: - return '' - - def _setCachedCompilerConsoleOutput(self, path, output): - with open(path, 'wb') as f: - f.write(output.encode(CACHE_COMPILER_OUTPUT_STORAGE_CODEC)) - class CompilerArtifactsRepository(object): def __init__(self, compilerArtifactsRootDir):