-
Notifications
You must be signed in to change notification settings - Fork 215
Fix for the compiler path in compile_commands.json file #1138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -14,6 +14,7 @@ | |||||
import java.io.InputStream; | ||||||
import java.nio.charset.StandardCharsets; | ||||||
import java.util.ArrayList; | ||||||
import java.util.Arrays; | ||||||
import java.util.Collection; | ||||||
import java.util.HashMap; | ||||||
import java.util.LinkedHashSet; | ||||||
|
@@ -70,7 +71,7 @@ public final class CompilationDatabaseGenerator { | |||||
* Checked on each build | ||||||
* Used before we look up the environment | ||||||
*/ | ||||||
private Map<ITool, String> toolMap = new HashMap<>(); | ||||||
private Map<String, String> toolMap = new HashMap<>(); | ||||||
private IProject project; | ||||||
private IConfiguration configuration; | ||||||
private ICSourceEntry[] srcEntries; | ||||||
|
@@ -229,18 +230,19 @@ private List<CompilationDatabaseInformation> populateObjList(IProject project, I | |||||
outputLocation + "", inputStrings, sourceLocation, outputLocation); //$NON-NLS-1$ | ||||||
|
||||||
IBuildMacroProvider provider = ManagedBuildManager.getBuildMacroProvider(); | ||||||
String compilerName = CompilationDatabaseGenerator.getCompilerName(tool); | ||||||
String commandLine = cmdLInfo.getCommandLine(); | ||||||
commandLine = commandLine.replace(compilerName, "").trim(); //$NON-NLS-1$ | ||||||
String compilerPath = findCompilerInPath(tool, config); | ||||||
String compilerName = CompilationDatabaseGenerator.getCompilerName(commandLine); | ||||||
jonahgraham marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
String compilerArguments = getCompilerArgs(commandLine); | ||||||
String compilerPath = findCompilerInPath(tool, config, compilerName); | ||||||
String resolvedOptionFileContents; | ||||||
if (compilerPath != null && !compilerPath.isEmpty()) { | ||||||
resolvedOptionFileContents = provider.resolveValueToMakefileFormat(compilerPath + " " + commandLine, //$NON-NLS-1$ | ||||||
resolvedOptionFileContents = provider.resolveValueToMakefileFormat( | ||||||
compilerPath + " " + compilerArguments, //$NON-NLS-1$ | ||||||
"", " ", //$NON-NLS-1$//$NON-NLS-2$ | ||||||
IBuildMacroProvider.CONTEXT_FILE, | ||||||
new FileContextData(sourceLocation, outputLocation, null, tool)); | ||||||
} else { | ||||||
resolvedOptionFileContents = provider.resolveValueToMakefileFormat(commandLine, "", " ", //$NON-NLS-1$//$NON-NLS-2$ | ||||||
resolvedOptionFileContents = provider.resolveValueToMakefileFormat(compilerArguments, "", " ", //$NON-NLS-1$//$NON-NLS-2$ | ||||||
IBuildMacroProvider.CONTEXT_FILE, | ||||||
new FileContextData(sourceLocation, outputLocation, null, tool)); | ||||||
|
||||||
|
@@ -438,15 +440,15 @@ public boolean visit(IResourceProxy proxy) throws CoreException { | |||||
|
||||||
} | ||||||
|
||||||
private String findCompilerInPath(ITool tool, IConfiguration config) { | ||||||
if (toolMap.containsKey(tool)) { | ||||||
return "\"" + toolMap.get(tool) + "\""; //$NON-NLS-1$//$NON-NLS-2$ | ||||||
private String findCompilerInPath(ITool tool, IConfiguration config, String compilerName) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
String cacheKey = compilerName; | ||||||
if (toolMap.containsKey(cacheKey)) { | ||||||
return "\"" + toolMap.get(cacheKey) + "\""; //$NON-NLS-1$//$NON-NLS-2$ | ||||||
} | ||||||
String compilerName = CompilationDatabaseGenerator.getCompilerName(tool); | ||||||
File pathToCompiler = new File(compilerName); | ||||||
|
||||||
if (pathToCompiler.isAbsolute()) { | ||||||
toolMap.put(tool, compilerName); | ||||||
toolMap.put(cacheKey, compilerName); | ||||||
return "\"" + compilerName + "\""; //$NON-NLS-1$ //$NON-NLS-2$ | ||||||
} | ||||||
ICConfigurationDescription cfg = ManagedBuildManager.getDescriptionForConfiguration(config); | ||||||
|
@@ -458,7 +460,7 @@ private String findCompilerInPath(ITool tool, IConfiguration config) { | |||||
IPath resolvedPath = PathUtil.findProgramLocation(compilerName, variable.getValue()); | ||||||
if (resolvedPath != null) { | ||||||
String path = resolvedPath.toString(); | ||||||
toolMap.put(tool, path); | ||||||
toolMap.put(cacheKey, path); | ||||||
return "\"" + path + "\""; //$NON-NLS-1$ //$NON-NLS-2$ | ||||||
} else { | ||||||
return null; // Only one PATH so can exit early | ||||||
|
@@ -469,13 +471,20 @@ private String findCompilerInPath(ITool tool, IConfiguration config) { | |||||
return null; | ||||||
} | ||||||
|
||||||
private static String getCompilerName(ITool tool) { | ||||||
String compilerCommand = tool.getToolCommand(); | ||||||
String[] arguments = CommandLineUtil.argumentsToArray(compilerCommand); | ||||||
private static String getCompilerName(String commandLine) { | ||||||
String[] arguments = CommandLineUtil.argumentsToArray(commandLine); | ||||||
if (arguments.length == 0) { | ||||||
return ""; //$NON-NLS-1$ | ||||||
} | ||||||
return arguments[0]; | ||||||
} | ||||||
|
||||||
private static String getCompilerArgs(String commandLine) { | ||||||
String[] arguments = CommandLineUtil.argumentsToArray(commandLine); | ||||||
if (arguments.length <= 1) { | ||||||
return ""; //$NON-NLS-1$ | ||||||
} | ||||||
return String.join(" ", Arrays.copyOfRange(arguments, 1, arguments.length)); //$NON-NLS-1$ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an invalid way of converting a list to a command line string. For example org.eclipse.cdt.utils.CommandLineUtil.argumentsToString(Collection, boolean) may or may not actually match the requirements. I hope that it does as it would be simpler than implementing a new case. If argumentsToString does not meet requirements, I recommend adding a new method to CommandLineUtil and the appropriate tests to CommandLineUtilTest |
||||||
} | ||||||
|
||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we now have a key of a string it is much less obvious what that key is, so please document it. Perhaps a line like this in the javadoc:
Map of compiler name (as calculated by {@link #getCompilerName(String)}) to the absolute path of the compiler.