From 52fe08c43ed63860cdd137e9e43a899599880491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela?= Date: Wed, 2 Nov 2016 17:52:33 +0100 Subject: [PATCH 1/6] Make ASSETS_DIR an absolute path This change makes easier to write integration tests that change the cmd. --- integrationtests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrationtests.py b/integrationtests.py index 0cbe38ed..c876bb7f 100644 --- a/integrationtests.py +++ b/integrationtests.py @@ -24,7 +24,7 @@ PYTHON_BINARY = sys.executable CLCACHE_SCRIPT = os.path.join(os.path.dirname(os.path.realpath(__file__)), "clcache.py") -ASSETS_DIR = os.path.join("tests", "integrationtests") +ASSETS_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "tests", "integrationtests") # pytest-cov note: subprocesses are coverage tested by default with some limitations # "For subprocess measurement environment variables must make it from the main process to the From 4ec76a180d0aa6db596158545f7b8d0b73145162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela?= Date: Wed, 26 Oct 2016 16:35:59 +0200 Subject: [PATCH 2/6] Add more integration tests to TestBasedir Show a few scenarios where using CLCACHE_BASEDIR should produce hits and misses. --- integrationtests.py | 121 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 93 insertions(+), 28 deletions(-) diff --git a/integrationtests.py b/integrationtests.py index c876bb7f..4dc9c809 100644 --- a/integrationtests.py +++ b/integrationtests.py @@ -1073,37 +1073,102 @@ def testHitsViaMpConcurrent(self): class TestBasedir(unittest.TestCase): - def testBasedir(self): - with cd(os.path.join(ASSETS_DIR, "basedir")), tempfile.TemporaryDirectory() as tempDir: - # First, create two separate build directories with the same sources - for buildDir in ["builddir_a", "builddir_b"]: - shutil.rmtree(buildDir, ignore_errors=True) - os.mkdir(buildDir) + def setUp(self): + self.projectDir = os.path.join(ASSETS_DIR, "basedir") + self.tempDir = tempfile.TemporaryDirectory() + self.clcacheDir = os.path.join(self.tempDir.name, "clcache") + self.savedCwd = os.getcwd() + + os.chdir(self.tempDir.name) + + # First, create two separate build directories with the same sources + for buildDir in ["builddir_a", "builddir_b"]: + shutil.copytree(self.projectDir, buildDir) + + self.cache = clcache.Cache(self.clcacheDir) + + def tearDown(self): + os.chdir(self.savedCwd) + self.tempDir.cleanup() + + def _runCompiler(self, cppFile, extraArgs=None): + cmd = CLCACHE_CMD + ["/nologo", "/EHsc", "/c"] + if extraArgs: + cmd.extend(extraArgs) + cmd = cmd + [cppFile] + env = dict(os.environ, CLCACHE_DIR=self.clcacheDir, CLCACHE_BASEDIR=os.getcwd()) + self.assertEqual(subprocess.call(cmd, env=env), 0) + + def expectHit(self, runCompiler): + # Build once in one directory + with cd("builddir_a"): + runCompiler[0]() + with self.cache.statistics as stats: + self.assertEqual(stats.numCacheMisses(), 1) + self.assertEqual(stats.numCacheHits(), 0) - shutil.copy("main.cpp", buildDir) - shutil.copy("constants.h", buildDir) + # Build again in a different directory, this should hit now because of CLCACHE_BASEDIR + with cd("builddir_b"): + runCompiler[1]() + with self.cache.statistics as stats: + self.assertEqual(stats.numCacheMisses(), 1) + self.assertEqual(stats.numCacheHits(), 1) - cache = clcache.Cache(tempDir) + def expectMiss(self, runCompiler): + # Build once in one directory + with cd("builddir_a"): + runCompiler[0]() + with self.cache.statistics as stats: + self.assertEqual(stats.numCacheMisses(), 1) + self.assertEqual(stats.numCacheHits(), 0) + + # Build again in a different directory, this should hit now because of CLCACHE_BASEDIR + with cd("builddir_b"): + runCompiler[1]() + with self.cache.statistics as stats: + self.assertEqual(stats.numCacheMisses(), 2) + self.assertEqual(stats.numCacheHits(), 0) + + def testBasedirRelativePaths(self): + def runCompiler(): + self._runCompiler("main.cpp") + self.expectHit([runCompiler, runCompiler]) + + def testBasedirAbsolutePaths(self): + def runCompiler(): + self._runCompiler(os.path.join(os.getcwd(), "main.cpp")) + self.expectHit([runCompiler, runCompiler]) + + def testBasedirIncludeArg(self): + def runCompiler(): + self._runCompiler("main.cpp", ["/I{}".format(os.getcwd())]) + self.expectHit([runCompiler, runCompiler]) + + def testBasedirIncludeSlashes(self): + runCompiler1 = lambda: self._runCompiler("main.cpp", ["/I{}/".format(os.getcwd())]) + runCompiler2 = lambda: self._runCompiler("main.cpp", ["/I{}".format(os.getcwd())]) + self.expectHit([runCompiler1, runCompiler2]) + + def testBasedirIncludeArgDifferentCapitalization(self): + def runCompiler(): + self._runCompiler("main.cpp", ["/I{}".format(os.getcwd().upper())]) + self.expectHit([runCompiler, runCompiler]) + + def testBasedirDefineArg(self): + def runCompiler(): + self._runCompiler("main.cpp", ["/DRESOURCES_DIR={}".format(os.getcwd())]) + self.expectMiss([runCompiler, runCompiler]) + + def testBasedirRelativeIncludeArg(self): + basedir = os.getcwd() + + def runCompiler(cppFile="main.cpp"): + cmd = CLCACHE_CMD + ["/nologo", "/EHsc", "/c", "/I."] + cmd = cmd + [cppFile] + env = dict(os.environ, CLCACHE_DIR=self.clcacheDir, CLCACHE_BASEDIR=basedir) + self.assertEqual(subprocess.call(cmd, env=env), 0) - cmd = CLCACHE_CMD + ["/nologo", "/EHsc", "/c", "main.cpp"] - - # Build once in one directory - with cd("builddir_a"): - env = dict(os.environ, CLCACHE_DIR=tempDir, CLCACHE_BASEDIR=os.getcwd()) - self.assertEqual(subprocess.call(cmd, env=env), 0) - with cache.statistics as stats: - self.assertEqual(stats.numCacheMisses(), 1) - self.assertEqual(stats.numCacheHits(), 0) - - shutil.rmtree("builddir_a", ignore_errors=True) - - # Build again in a different directory, this should hit now because of CLCACHE_BASEDIR - with cd("builddir_b"): - env = dict(os.environ, CLCACHE_DIR=tempDir, CLCACHE_BASEDIR=os.getcwd()) - self.assertEqual(subprocess.call(cmd, env=env), 0) - with cache.statistics as stats: - self.assertEqual(stats.numCacheMisses(), 1) - self.assertEqual(stats.numCacheHits(), 1) + self.expectMiss([runCompiler, runCompiler]) class TestCleanCache(unittest.TestCase): From 4b3cdd96d059fddda13fb895a70cbc3e682f5287 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela?= Date: Wed, 26 Oct 2016 17:12:57 +0200 Subject: [PATCH 3/6] Handle CLCACHE_BASEDIR in manifest hash computation In direct mode the command line arguments are hashed to compute the manifest hash. CLCACHE_BASEDIR should be used to unify arguments in order to produce hits between different invocations. This should only be applied to arguments that specify where to find the source code to the compiler or we risk creating hits for compiler tasks that generate different object files. For example, these two invocations should reuse the object in the cache: "cl.exe /IC:\Project1\include\lib1 main.cpp" with CLCACHE_BASEDIR=C:\Project1 "cl.exe /IC:\Project2\include\lib1 main.cpp" with CLCACHE_BASEDIR=C:\Project2 But these two should not reuse it: "cl.exe /IC:\Project1\include\lib1 /DFOLDER=C:\Project1\ main.cpp" with CLCACHE_BASEDIR=C:\Project1 "cl.exe /IC:\Project2\include\lib1 /DFOLDER=C:\Project2\ main.cpp" with CLCACHE_BASEDIR=C:\Project2 In this example the /D argument does not specify the compiler where to find the source code but it may specify a default path (for example). --- clcache.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/clcache.py b/clcache.py index e9d77f42..ddc30f75 100644 --- a/clcache.py +++ b/clcache.py @@ -232,9 +232,21 @@ def getManifestHash(compilerBinary, commandLine, sourceFile): # One of the few exceptions to this rule is the /MP switch, which only # defines how many compiler processes are running simultaneusly. commandLine = [arg for arg in commandLine if not arg.startswith("/MP")] + arguments, inputFiles = CommandLineAnalyzer.parseArgumentsAndInputFiles(commandLine) + collapseBasedirInCmdPath = lambda path: collapseBasedirToPlaceholder(os.path.normcase(os.path.abspath(path))) + + commandLineArgs = [] + projectSpecificArgs = ("AI", "I", "FU") + for k in sorted(arguments.keys()): + if k in projectSpecificArgs: + commandLineArgs.extend(["/" + k + collapseBasedirInCmdPath(arg) for arg in arguments[k]]) + else: + commandLineArgs.extend(["/" + k + arg for arg in arguments[k]]) + + commandLineArgs.extend(collapseBasedirInCmdPath(arg) for arg in inputFiles) additionalData = "{}|{}|{}".format( - compilerHash, commandLine, ManifestRepository.MANIFEST_FILE_FORMAT_VERSION) + compilerHash, commandLineArgs, ManifestRepository.MANIFEST_FILE_FORMAT_VERSION) return getFileHash(sourceFile, additionalData) @staticmethod From ea01aef08f4aae928dd1cb1b46f7dceaf538dd00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela?= Date: Thu, 3 Nov 2016 10:00:36 +0100 Subject: [PATCH 4/6] Remove trailing separator in normalizeBaseDir The include paths in the command line may not include a trailing separator. normalizedBaseDir should not include a trailing separator in order to match. --- clcache.py | 4 ++-- unittests.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/clcache.py b/clcache.py index ddc30f75..1ab8b8fb 100644 --- a/clcache.py +++ b/clcache.py @@ -97,8 +97,8 @@ def childDirectories(path, absolute=True): def normalizeBaseDir(baseDir): if baseDir: baseDir = os.path.normcase(baseDir) - if not baseDir.endswith(os.path.sep): - baseDir += os.path.sep + if baseDir.endswith(os.path.sep): + baseDir = baseDir[0:-1] return baseDir else: # Converts empty string to None diff --git a/unittests.py b/unittests.py index 03058c3d..97d1e5e2 100644 --- a/unittests.py +++ b/unittests.py @@ -69,14 +69,14 @@ def testNormalizeBaseDir(self): # Note: raw string literals cannot end in an odd number of backslashes # https://docs.python.org/3/faq/design.html#why-can-t-raw-strings-r-strings-end-with-a-backslash # So we consistenly use basic literals - self.assertEqual(clcache.normalizeBaseDir("c:"), "c:\\") - self.assertEqual(clcache.normalizeBaseDir("c:\\projects"), "c:\\projects\\") + self.assertEqual(clcache.normalizeBaseDir("c:"), "c:") + self.assertEqual(clcache.normalizeBaseDir("c:\\projects"), "c:\\projects") - self.assertEqual(clcache.normalizeBaseDir("C:\\"), "c:\\") - self.assertEqual(clcache.normalizeBaseDir("C:\\Projects\\"), "c:\\projects\\") + self.assertEqual(clcache.normalizeBaseDir("C:\\"), "c:") + self.assertEqual(clcache.normalizeBaseDir("C:\\Projects\\"), "c:\\projects") - self.assertEqual(clcache.normalizeBaseDir("c:\\projects with space"), "c:\\projects with space\\") - self.assertEqual(clcache.normalizeBaseDir("c:\\projects with ö"), "c:\\projects with ö\\") + self.assertEqual(clcache.normalizeBaseDir("c:\\projects with space"), "c:\\projects with space") + self.assertEqual(clcache.normalizeBaseDir("c:\\projects with ö"), "c:\\projects with ö") def testFilesBeneathSimple(self): with cd(os.path.join(ASSETS_DIR, "files-beneath")): From 69dfb04664439b8823a6384d2cf1834b23db05e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela?= Date: Mon, 7 Nov 2016 16:00:07 +0100 Subject: [PATCH 5/6] Minor cleanup: variable names --- clcache.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/clcache.py b/clcache.py index 1ab8b8fb..dba1e4ee 100644 --- a/clcache.py +++ b/clcache.py @@ -235,18 +235,18 @@ def getManifestHash(compilerBinary, commandLine, sourceFile): arguments, inputFiles = CommandLineAnalyzer.parseArgumentsAndInputFiles(commandLine) collapseBasedirInCmdPath = lambda path: collapseBasedirToPlaceholder(os.path.normcase(os.path.abspath(path))) - commandLineArgs = [] - projectSpecificArgs = ("AI", "I", "FU") + commandLine = [] + argumentsWithPaths = ("AI", "I", "FU") for k in sorted(arguments.keys()): - if k in projectSpecificArgs: - commandLineArgs.extend(["/" + k + collapseBasedirInCmdPath(arg) for arg in arguments[k]]) + if k in argumentsWithPaths: + commandLine.extend(["/" + k + collapseBasedirInCmdPath(arg) for arg in arguments[k]]) else: - commandLineArgs.extend(["/" + k + arg for arg in arguments[k]]) + commandLine.extend(["/" + k + arg for arg in arguments[k]]) - commandLineArgs.extend(collapseBasedirInCmdPath(arg) for arg in inputFiles) + commandLine.extend(collapseBasedirInCmdPath(arg) for arg in inputFiles) additionalData = "{}|{}|{}".format( - compilerHash, commandLineArgs, ManifestRepository.MANIFEST_FILE_FORMAT_VERSION) + compilerHash, commandLine, ManifestRepository.MANIFEST_FILE_FORMAT_VERSION) return getFileHash(sourceFile, additionalData) @staticmethod From 7d45cb3fafb19313a07a0f2d2e18f970e3c008d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Si=C3=B1uela?= Date: Mon, 7 Nov 2016 16:06:16 +0100 Subject: [PATCH 6/6] Update comment --- clcache.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/clcache.py b/clcache.py index dba1e4ee..fe117214 100644 --- a/clcache.py +++ b/clcache.py @@ -227,10 +227,12 @@ def getManifestHash(compilerBinary, commandLine, sourceFile): compilerHash = getCompilerHash(compilerBinary) # NOTE: We intentionally do not normalize command line to include - # preprocessor options. In direct mode we do not perform - # preprocessing before cache lookup, so all parameters are important. - # One of the few exceptions to this rule is the /MP switch, which only - # defines how many compiler processes are running simultaneusly. + # preprocessor options. In direct mode we do not perform preprocessing + # before cache lookup, so all parameters are important. One of the few + # exceptions to this rule is the /MP switch, which only defines how many + # compiler processes are running simultaneusly. Arguments that specify + # the compiler where to find the source files are parsed to replace + # ocurrences of CLCACHE_BASEDIR by a placeholder. commandLine = [arg for arg in commandLine if not arg.startswith("/MP")] arguments, inputFiles = CommandLineAnalyzer.parseArgumentsAndInputFiles(commandLine) collapseBasedirInCmdPath = lambda path: collapseBasedirToPlaceholder(os.path.normcase(os.path.abspath(path)))