Skip to content
This repository has been archived by the owner on Feb 4, 2020. It is now read-only.

Commit

Permalink
Fix source file duplication in command line
Browse files Browse the repository at this point in the history
Previously, if the /Tp or /Tc option were passed, clcache would
duplicate the source file in the command line, making MSVC
think that it was compiling two separate files (/Tcexample.c 
and example.c), when it was really compiling one.

The problem with this was that MSVC only allows some
options if it is compiling one file, which caused some
invocations of clcache to fail.
  • Loading branch information
xoviat authored and frerich committed Oct 12, 2017
1 parent b880939 commit 3ef6643
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 15 deletions.
34 changes: 24 additions & 10 deletions clcache.py
Expand Up @@ -1004,6 +1004,7 @@ def findCompilerBinary():


def printTraceStatement(msg):
# type: (str) -> None
if "CLCACHE_LOG" in os.environ:
scriptDir = os.path.realpath(os.path.dirname(sys.argv[0]))
with OUTPUT_LOCK:
Expand Down Expand Up @@ -1252,15 +1253,21 @@ def parseArgumentsAndInputFiles(cmdline):

@staticmethod
def analyze(cmdline):
# type: List[str] -> Tuple[List[Tuple[str, str]], List[str]]
options, inputFiles = CommandLineAnalyzer.parseArgumentsAndInputFiles(cmdline)
# Use an override pattern to shadow input files that have
# already been specified in the function above
inputFiles = {inputFile: '' for inputFile in inputFiles}
compl = False
if 'Tp' in options:
inputFiles += options['Tp']
inputFiles.update({inputFile: '/Tp' for inputFile in options['Tp']})
compl = True
if 'Tc' in options:
inputFiles += options['Tc']
inputFiles.update({inputFile: '/Tc' for inputFile in options['Tc']})
compl = True

# Now collect the inputFiles into the return format
inputFiles = list(inputFiles.items())
if not inputFiles:
raise NoSourceFileError()

Expand Down Expand Up @@ -1293,7 +1300,7 @@ def analyze(cmdline):
objectFiles = [tmp]
if objectFiles is None:
# Generate from .c/.cpp filenames
objectFiles = [os.path.join(prefix, basenameWithoutExtension(f)) + '.obj' for f in inputFiles]
objectFiles = [os.path.join(prefix, basenameWithoutExtension(f)) + '.obj' for f, _ in inputFiles]

printTraceStatement("Compiler source files: {}".format(inputFiles))
printTraceStatement("Compiler object file: {}".format(objectFiles))
Expand Down Expand Up @@ -1617,19 +1624,26 @@ def processCompileRequest(cache, compiler, args):
printOutAndErr(out, err)
return exitCode

def filterSourceFiles(cmdLine, sourceFiles):
# type: (List[str], List[Tuple[str, str]]) -> Iterator[str]
setOfSources = set(sourceFile for sourceFile, _ in sourceFiles)
skippedArgs = ('/Tc', '/Tp', '-Tp', '-Tc')
yield from (
arg for arg in cmdLine
if not (arg in setOfSources or arg.startswith(skippedArgs))
)

def scheduleJobs(cache, compiler, cmdLine, environment, sourceFiles, objectFiles):
baseCmdLine = []
setOfSources = set(sourceFiles)
for arg in cmdLine:
if not (arg in setOfSources or arg.startswith("/MP")):
baseCmdLine.append(arg)
# type: (Any, str, List[str], Any, List[Tuple[str, str]], List[str]) -> int
# Filter out all source files from the command line to form baseCmdLine
baseCmdLine = [arg for arg in filterSourceFiles(cmdLine, sourceFiles) if not arg.startswith('/MP')]

exitCode = 0
cleanupRequired = False
with concurrent.futures.ThreadPoolExecutor(max_workers=jobCount(cmdLine)) as executor:
jobs = []
for srcFile, objFile in zip(sourceFiles, objectFiles):
jobCmdLine = baseCmdLine + [srcFile]
for (srcFile, srcLanguage), objFile in zip(sourceFiles, objectFiles):
jobCmdLine = baseCmdLine + [srcLanguage + srcFile]
jobs.append(executor.submit(
processSingleSource,
compiler, jobCmdLine, srcFile, objFile, environment))
Expand Down
3 changes: 3 additions & 0 deletions integrationtests.py
Expand Up @@ -806,13 +806,16 @@ def testHitsViaMpConcurrent(self):
self.assertEqual(stats.numCacheEntries(), 2)

def testOutput(self):
# type: () -> None
with cd(os.path.join(ASSETS_DIR, "parallel")), tempfile.TemporaryDirectory() as tempDir:
sources = glob.glob("*.cpp")
clcache.Cache(tempDir)
customEnv = self._createEnv(tempDir)
cmd = CLCACHE_CMD + ["/nologo", "/EHsc", "/c"]
mpFlag = "/MP" + str(len(sources))
out = subprocess.check_output(cmd + [mpFlag] + sources, env=customEnv).decode("ascii")
# print the output so that it shows up in py.test
print(out)

for s in sources:
self.assertEqual(out.count(s), 1)
Expand Down
35 changes: 30 additions & 5 deletions unittests.py
Expand Up @@ -488,6 +488,30 @@ def testNestedResponseFiles(self):
)


class TestFilterSourceFiles(unittest.TestCase):
def _assertFiltered(self, cmdLine, files, filteredCmdLine):
# type: (List[str], List[Tuple[str, str]]) -> List[str]
files = clcache.filterSourceFiles(cmdLine, files)
self.assertEqual(list(files), filteredCmdLine)

def testFilterSourceFiles(self):
self._assertFiltered(
['/c', '/EP', '/FoSome.obj', 'main.cpp'], [('main.cpp', '')],
['/c', '/EP', '/FoSome.obj'])
self._assertFiltered(
['/c', '/EP', '/FoSome.obj', 'main.cpp'], [('main.cpp', '/Tp')],
['/c', '/EP', '/FoSome.obj'])
self._assertFiltered(
['/c', '/EP', '/FoSome.obj', 'main.cpp'], [('main.cpp', '/Tc')],
['/c', '/EP', '/FoSome.obj'])
self._assertFiltered(
['/c', '/EP', '/FoSome.obj', '/Tcmain.cpp'], [('main.cpp', '')],
['/c', '/EP', '/FoSome.obj'])
self._assertFiltered(
['/c', '/EP', '/FoSome.obj', '/Tcmain.cpp'], [('main.cpp', '-Tc')],
['/c', '/EP', '/FoSome.obj'])


class TestAnalyzeCommandLine(unittest.TestCase):
def _testSourceFilesOk(self, cmdLine):
try:
Expand All @@ -504,13 +528,14 @@ def _testFailure(self, cmdLine, expectedExceptionClass):
self.assertRaises(expectedExceptionClass, lambda: CommandLineAnalyzer.analyze(cmdLine))

def _testFull(self, cmdLine, expectedSourceFiles, expectedOutputFile):
# type: (List[str], List[Tuple[str, str]], List[str]) -> None
sourceFiles, outputFile = CommandLineAnalyzer.analyze(cmdLine)
self.assertEqual(sourceFiles, expectedSourceFiles)
self.assertEqual(outputFile, expectedOutputFile)

def _testFo(self, foArgument, expectedObjectFilepath):
self._testFull(['/c', foArgument, 'main.cpp'],
["main.cpp"], [expectedObjectFilepath])
[("main.cpp", '')], [expectedObjectFilepath])

def _testFi(self, fiArgument):
self._testPreprocessingOutfile(['/c', '/P', fiArgument, 'main.cpp'])
Expand All @@ -528,7 +553,7 @@ def testEmpty(self):
self._testFailure([], NoSourceFileError)

def testSimple(self):
self._testFull(["/c", "main.cpp"], ["main.cpp"], ["main.obj"])
self._testFull(["/c", "main.cpp"], [("main.cpp", "")], ["main.obj"])

def testNoSource(self):
# No source file has priority over other errors, for consistency
Expand All @@ -547,7 +572,7 @@ def testNoSource(self):
def testOutputFileFromSourcefile(self):
# For object file
self._testFull(['/c', 'main.cpp'],
['main.cpp'], ['main.obj'])
[('main.cpp', '')], ['main.obj'])
# For preprocessor file
self._testFailure(['/c', '/P', 'main.cpp'], CalledForPreprocessingError)

Expand Down Expand Up @@ -634,9 +659,9 @@ def testPreprocessingFi(self):
def testTpTcSimple(self):
# clcache can handle /Tc or /Tp as long as there is only one of them
self._testFull(['/c', '/TcMyCcProgram.c'],
['MyCcProgram.c'], ['MyCcProgram.obj'])
[('MyCcProgram.c', '/Tc')], ['MyCcProgram.obj'])
self._testFull(['/c', '/TpMyCxxProgram.cpp'],
['MyCxxProgram.cpp'], ['MyCxxProgram.obj'])
[('MyCxxProgram.cpp', '/Tp')], ['MyCxxProgram.obj'])

def testLink(self):
self._testFailure(["main.cpp"], CalledForLinkError)
Expand Down

0 comments on commit 3ef6643

Please sign in to comment.