Skip to content
This repository was archived by the owner on Feb 4, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f0227bb
BUG: fix source file duplication in command liine
xoviat Oct 7, 2017
40cd447
BUG: improve handling of specified sources
xoviat Oct 7, 2017
cc8a52b
BUG: revert f0227bb
xoviat Oct 7, 2017
ba1f7d1
BUG: revise analyze method to detect language
xoviat Oct 9, 2017
3df60a6
MAINT: cleanup zip iterator
xoviat Oct 9, 2017
662f7ed
BUG: fix syntax error
xoviat Oct 9, 2017
bbdd257
MAINT: cleanup formBaseCommandLine
xoviat Oct 9, 2017
94218fd
:art:
xoviat Oct 9, 2017
f69b709
MAINT: cleanup source file filtering
xoviat Oct 9, 2017
df74660
MAINT: improve jobCmdLine readability
xoviat Oct 9, 2017
0aac12b
TST: fix existing unittests
xoviat Oct 9, 2017
ce9de5e
appveyor: define CLCACHE_LOG
xoviat Oct 10, 2017
ec0fae1
TST: printTraceStatement: print caller line
xoviat Oct 10, 2017
2e10786
BUG: fix printTraceStatement
xoviat Oct 10, 2017
019ae2f
FIX: switch to OrderedDict
xoviat Oct 10, 2017
6c3a855
FIX: move import
xoviat Oct 10, 2017
56be5e2
FIX: check whether cmdLine arg startswith /MP
xoviat Oct 11, 2017
5de5fcc
FIX: revert use of OrderedDict
xoviat Oct 11, 2017
725410e
FIX: appveyor: revert CLCACHE_LOG
xoviat Oct 11, 2017
19e7126
TST: revert changes
xoviat Oct 11, 2017
9079f32
TST: unit: correct comparisons
xoviat Oct 11, 2017
7fb4480
TST: integration: print output
xoviat Oct 11, 2017
4a7ee82
TST: unit: add some reasonable tests for filterSourceFiles
xoviat Oct 11, 2017
34fcb5d
TST: unit: fix
xoviat Oct 11, 2017
9ebb89b
FIX: revert changes to printTraceStatement
xoviat Oct 12, 2017
084b23b
FIX: remove unused import
xoviat Oct 12, 2017
275ae15
FIX: update ??? to type: Any
xoviat Oct 12, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions clcache.py
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,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 @@ -1243,15 +1244,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())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be that the failure in tests is introduced here because inputFiles.items() does not always return the items in the same order? either sorting them or using OrderedDict in line 1253 could help.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if it didn't fix the tests I think it still makes sense to keep the OrderedDict. The output of analyze() should return the same order across executions 1) to keep the tests passing and 2) to generate the same hash for the compilation task (if it is computed after, I am not sure about this). I don't know enough about this part of the code, to be checked with @frerich.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test failures have nothing to do with the order, and if I understand correctly, the hash is calculated per-file ("This causes clcache to reinvoke itself recursively for each of the source files"), making the order irrelevant. There is no reason to tell Python to preserve the order when it doesn't matter.

if not inputFiles:
raise NoSourceFileError()

Expand Down Expand Up @@ -1284,7 +1291,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 @@ -1612,19 +1619,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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