diff --git a/clcache.py b/clcache.py index cd299de8..45c6eb30 100644 --- a/clcache.py +++ b/clcache.py @@ -91,6 +91,14 @@ def normalizeBaseDir(baseDir): return None +class IncludeNotFoundException(Exception): + pass + + +class IncludeChangedException(Exception): + pass + + class CacheLockException(Exception): pass @@ -118,7 +126,7 @@ def setManifest(self, manifestHash, manifest): ensureDirectoryExists(self.manifestSectionDir) with open(self.manifestPath(manifestHash), 'w') as outFile: # Converting namedtuple to JSON via OrderedDict preserves key names and keys order - json.dump(manifest._asdict(), outFile, indent=2) + json.dump(manifest._asdict(), outFile, sort_keys=True, indent=2) def getManifest(self, manifestHash): fileName = self.manifestPath(manifestHash) @@ -138,7 +146,7 @@ class ManifestRepository(object): # invalidation, such that a manifest that was stored using the old format is not # interpreted using the new format. Instead the old file will not be touched # again due to a new manifest hash and is cleaned away after some time. - MANIFEST_FILE_FORMAT_VERSION = 3 + MANIFEST_FILE_FORMAT_VERSION = 4 def __init__(self, manifestsRootDir): self._manifestsRootDir = manifestsRootDir @@ -184,8 +192,22 @@ def getManifestHash(compilerBinary, commandLine, sourceFile): return getFileHash(sourceFile, additionalData) @staticmethod - def getIncludesContentHashForFiles(listOfIncludesAbsolute): - listOfIncludesHashes = [getFileHash(filepath) for filepath in listOfIncludesAbsolute] + def getIncludesContentHashForFiles(includes): + listOfIncludesHashes = [] + includeMissing = False + + for path in sorted(includes.keys()): + try: + fileHash = getFileHash(path) + if fileHash != includes[path]: + raise IncludeChangedException() + listOfIncludesHashes.append(fileHash) + except FileNotFoundError: + includeMissing = True + + if includeMissing: + raise IncludeNotFoundException() + return ManifestRepository.getIncludesContentHashForHashes(listOfIncludesHashes) @staticmethod @@ -1224,12 +1246,14 @@ def clearCache(cache): print('Cache cleared') -# Returns pair - list of includes and new compiler output. +# Returns pair: +# 1. set of include filepaths +# 2. new compiler output # Output changes if strip is True in that case all lines with include # directives are stripped from it -def parseIncludesList(compilerOutput, sourceFile, strip): +def parseIncludesSet(compilerOutput, sourceFile, strip): newOutput = [] - includesSet = set([]) + includesSet = set() # Example lines # Note: including file: C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\INCLUDE\limits.h @@ -1256,9 +1280,9 @@ def parseIncludesList(compilerOutput, sourceFile, strip): elif strip: newOutput.append(line) if strip: - return sorted(includesSet), ''.join(newOutput) + return includesSet, ''.join(newOutput) else: - return sorted(includesSet), compilerOutput + return includesSet, compilerOutput def addObjectToCache(stats, cache, cachekey, artifacts): @@ -1294,17 +1318,34 @@ def postprocessObjectEvicted(cache, objectFile, cachekey, compilerResult): return compilerResult -def postprocessHeaderChangedMiss( - cache, objectFile, manifestSection, manifest, manifestHash, includesContentHash, compilerResult): +def createManifest(manifestHash, includePaths): + baseDir = normalizeBaseDir(os.environ.get('CLCACHE_BASEDIR')) + + includes = {path:getFileHash(path) for path in includePaths} + includesContentHash = ManifestRepository.getIncludesContentHashForFiles(includes) cachekey = Cache.getDirectCacheKey(manifestHash, includesContentHash) + + # Create new manifest + if baseDir: + relocatableIncludePaths = { + collapseBasedirToPlaceholder(path, baseDir):contentHash + for path, contentHash in includes.items() + } + manifest = Manifest(relocatableIncludePaths, {}) + else: + manifest = Manifest(includes, {}) + manifest.includesContentToObjectMap[includesContentHash] = cachekey + return manifest, cachekey + + +def postprocessHeaderChangedMiss( + cache, objectFile, manifestSection, manifestHash, sourceFile, compilerResult, stripIncludes): returnCode, compilerOutput, compilerStderr = compilerResult + includePaths, compilerOutput = parseIncludesSet(compilerOutput, sourceFile, stripIncludes) removedItems = [] if returnCode == 0 and os.path.exists(objectFile): - while len(manifest.includesContentToObjectMap) >= MAX_MANIFEST_HASHES: - _, objectHash = manifest.includesContentToObjectMap.popitem() - removedItems.append(objectHash) - manifest.includesContentToObjectMap[includesContentHash] = cachekey + manifest, cachekey = createManifest(manifestHash, includePaths) with cache.lock, cache.statistics as stats: stats.registerHeaderChangedMiss() @@ -1313,27 +1354,19 @@ def postprocessHeaderChangedMiss( cache.compilerArtifactsRepository.removeObjects(stats, removedItems) manifestSection.setManifest(manifestHash, manifest) - return compilerResult + return returnCode, compilerOutput, compilerStderr def postprocessNoManifestMiss( - cache, objectFile, manifestSection, manifestHash, baseDir, sourceFile, compilerResult, stripIncludes): + cache, objectFile, manifestSection, manifestHash, sourceFile, compilerResult, stripIncludes): returnCode, compilerOutput, compilerStderr = compilerResult - listOfIncludes, compilerOutput = parseIncludesList(compilerOutput, sourceFile, stripIncludes) + includePaths, compilerOutput = parseIncludesSet(compilerOutput, sourceFile, stripIncludes) manifest = None cachekey = None if returnCode == 0 and os.path.exists(objectFile): - # Store compile output and manifest - if baseDir: - relocatableIncludePaths = [collapseBasedirToPlaceholder(path, baseDir) for path in listOfIncludes] - manifest = Manifest(relocatableIncludePaths, {}) - else: - manifest = Manifest(listOfIncludes, {}) - includesContentHash = ManifestRepository.getIncludesContentHashForFiles(listOfIncludes) - cachekey = Cache.getDirectCacheKey(manifestHash, includesContentHash) - manifest.includesContentToObjectMap[includesContentHash] = cachekey + manifest, cachekey = createManifest(manifestHash, includePaths) with cache.lock, cache.statistics as stats: stats.registerSourceChangedMiss() @@ -1469,32 +1502,44 @@ def processDirect(cache, objectFile, compiler, cmdLine, sourceFile): manifestHash = ManifestRepository.getManifestHash(compiler, cmdLine, sourceFile) manifestSection = cache.manifestRepository.section(manifestHash) with cache.lock: + createNewManifest = False manifest = manifestSection.getManifest(manifestHash) if manifest is not None: # NOTE: command line options already included in hash for manifest name - includesContentHash = ManifestRepository.getIncludesContentHashForFiles( - [expandBasedirPlaceholder(include, baseDir) for include in manifest.includeFiles]) - cachekey = manifest.includesContentToObjectMap.get(includesContentHash) - if cachekey is not None: + try: + includesContentHash = ManifestRepository.getIncludesContentHashForFiles({ + expandBasedirPlaceholder(path, baseDir):contentHash + for path, contentHash in manifest.includeFiles.items() + }) + + cachekey = manifest.includesContentToObjectMap.get(includesContentHash) + assert cachekey is not None if cache.compilerArtifactsRepository.section(cachekey).hasEntry(cachekey): return processCacheHit(cache, objectFile, cachekey) else: postProcessing = lambda compilerResult: postprocessObjectEvicted( cache, objectFile, cachekey, compilerResult) - else: + except IncludeChangedException: + createNewManifest = True postProcessing = lambda compilerResult: postprocessHeaderChangedMiss( - cache, objectFile, manifestSection, manifest, manifestHash, includesContentHash, compilerResult) + cache, objectFile, manifestSection, manifestHash, sourceFile, compilerResult, stripIncludes) + except IncludeNotFoundException: + # register nothing. This is probably just a compile error + postProcessing = None else: - origCmdLine = cmdLine - stripIncludes = False - if '/showIncludes' not in cmdLine: - cmdLine = ['/showIncludes'] + origCmdLine - stripIncludes = True + createNewManifest = True postProcessing = lambda compilerResult: postprocessNoManifestMiss( - cache, objectFile, manifestSection, manifestHash, baseDir, sourceFile, compilerResult, stripIncludes) + cache, objectFile, manifestSection, manifestHash, sourceFile, compilerResult, stripIncludes) + + if createNewManifest: + stripIncludes = False + if '/showIncludes' not in cmdLine: + cmdLine.insert(0, '/showIncludes') + stripIncludes = True compilerResult = invokeRealCompiler(compiler, cmdLine, captureOutput=True) - compilerResult = postProcessing(compilerResult) + if postProcessing: + compilerResult = postProcessing(compilerResult) printTraceStatement("Finished. Exit code {0:d}".format(compilerResult[0])) return compilerResult diff --git a/integrationtests.py b/integrationtests.py index 39ae2f75..c062b9e9 100644 --- a/integrationtests.py +++ b/integrationtests.py @@ -312,6 +312,76 @@ def testNoDirect(self): self.assertEqual(output, "2") +class TestHeaderMiss(unittest.TestCase): + # When a required header disappears, we must fall back to real compiler + # complaining about the miss + def testRequiredHeaderDisappears(self): + with cd(os.path.join(ASSETS_DIR, "header-miss")): + compileCmd = CLCACHE_CMD + ["/nologo", "/EHsc", "/c", "main.cpp"] + + with open("info.h", "w") as header: + header.write("#define INFO 1337\n") + subprocess.check_call(compileCmd) + + os.remove("info.h") + + # real compiler fails + process = subprocess.Popen(compileCmd, stdout=subprocess.PIPE) + stdout, _ = process.communicate() + self.assertEqual(process.returncode, 2) + self.assertTrue("C1083" in stdout.decode(clcache.CL_DEFAULT_CODEC)) + + # When a header included by another header becomes obsolete and disappers, + # we must fall back to real compiler. + def testObsoleteHeaderDisappears(self): + # A includes B + with cd(os.path.join(ASSETS_DIR, "header-miss-obsolete")): + compileCmd = CLCACHE_CMD + ["/I.", "/nologo", "/EHsc", "/c", "main.cpp"] + cache = clcache.Cache() + + with open("A.h", "w") as header: + header.write('#define INFO 1337\n') + header.write('#include "B.h"\n') + with open("B.h", "w") as header: + header.write('#define SOMETHING 1\n') + + subprocess.check_call(compileCmd) + + with cache.statistics as stats: + headerChangedMisses1 = stats.numHeaderChangedMisses() + hits1 = stats.numCacheHits() + misses1 = stats.numCacheMisses() + + # Make include B.h obsolete + with open("A.h", "w") as header: + header.write('#define INFO 1337\n') + header.write('\n') + os.remove("B.h") + + subprocess.check_call(compileCmd) + + with cache.statistics as stats: + headerChangedMisses2 = stats.numHeaderChangedMisses() + hits2 = stats.numCacheHits() + misses2 = stats.numCacheMisses() + + self.assertEqual(headerChangedMisses2, headerChangedMisses1+1) + self.assertEqual(misses2, misses1+1) + self.assertEqual(hits2, hits1) + + # Ensure the new manifest was stored + subprocess.check_call(compileCmd) + + with cache.statistics as stats: + headerChangedMisses3 = stats.numHeaderChangedMisses() + hits3 = stats.numCacheHits() + misses3 = stats.numCacheMisses() + + self.assertEqual(headerChangedMisses3, headerChangedMisses2) + self.assertEqual(misses3, misses2) + self.assertEqual(hits3, hits2+1) + + class TestRunParallel(unittest.TestCase): def _zeroStats(self): subprocess.check_call(CLCACHE_CMD + ["-z"]) @@ -604,6 +674,8 @@ def testHitsViaMpConcurrent(self): self.assertEqual(stats.numCacheHits(), 2) self.assertEqual(stats.numCacheMisses(), 2) self.assertEqual(stats.numCacheEntries(), 2) + + class TestBasedir(unittest.TestCase): def testBasedir(self): with cd(os.path.join(ASSETS_DIR, "basedir")), tempfile.TemporaryDirectory() as tempDir: diff --git a/tests/integrationtests/header-miss-obsolete/.gitignore b/tests/integrationtests/header-miss-obsolete/.gitignore new file mode 100644 index 00000000..3d1cf01d --- /dev/null +++ b/tests/integrationtests/header-miss-obsolete/.gitignore @@ -0,0 +1,5 @@ +*.obj + +# headers auto-generated by tests +A.h +B.h diff --git a/tests/integrationtests/header-miss-obsolete/main.cpp b/tests/integrationtests/header-miss-obsolete/main.cpp new file mode 100644 index 00000000..f6983593 --- /dev/null +++ b/tests/integrationtests/header-miss-obsolete/main.cpp @@ -0,0 +1,8 @@ +#include +#include "A.h" + +int main() +{ + std::cout << INFO << std::endl; + return 0; +} diff --git a/tests/integrationtests/header-miss/.gitignore b/tests/integrationtests/header-miss/.gitignore new file mode 100644 index 00000000..278b3d7d --- /dev/null +++ b/tests/integrationtests/header-miss/.gitignore @@ -0,0 +1,2 @@ +# This file is auto-generated by tests +info.h diff --git a/tests/integrationtests/header-miss/main.cpp b/tests/integrationtests/header-miss/main.cpp new file mode 100644 index 00000000..b6d0bb6e --- /dev/null +++ b/tests/integrationtests/header-miss/main.cpp @@ -0,0 +1,8 @@ +#include +#include "info.h" + +int main() +{ + std::cout << INFO << std::endl; + return 0; +} diff --git a/unittests.py b/unittests.py index 3b5596a6..08bb6e50 100644 --- a/unittests.py +++ b/unittests.py @@ -812,7 +812,7 @@ def _readSampleFileNoIncludes(self): def testParseIncludesNoStrip(self): sample = self._readSampleFileDefault() - includesSet, newCompilerOutput = clcache.parseIncludesList( + includesSet, newCompilerOutput = clcache.parseIncludesSet( sample['CompilerOutput'], r'C:\Projects\test\smartsqlite\src\version.cpp', strip=False) @@ -826,7 +826,7 @@ def testParseIncludesNoStrip(self): def testParseIncludesStrip(self): sample = self._readSampleFileDefault() - includesSet, newCompilerOutput = clcache.parseIncludesList( + includesSet, newCompilerOutput = clcache.parseIncludesSet( sample['CompilerOutput'], r'C:\Projects\test\smartsqlite\src\version.cpp', strip=True) @@ -841,7 +841,7 @@ def testParseIncludesStrip(self): def testParseIncludesNoIncludes(self): sample = self._readSampleFileNoIncludes() for stripIncludes in [True, False]: - includesSet, newCompilerOutput = clcache.parseIncludesList( + includesSet, newCompilerOutput = clcache.parseIncludesSet( sample['CompilerOutput'], r"C:\Projects\test\myproject\main.cpp", strip=stripIncludes) @@ -851,7 +851,7 @@ def testParseIncludesNoIncludes(self): def testParseIncludesGerman(self): sample = self._readSampleFileDefault(lang="de") - includesSet, _ = clcache.parseIncludesList( + includesSet, _ = clcache.parseIncludesSet( sample['CompilerOutput'], r"C:\Projects\test\smartsqlite\src\version.cpp", strip=False)