[clang-tools-extra] [clang-include-cleaner] Fix incorrect directory issue for writing files (PR #111375)
Byoungchan Lee via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 9 03:45:37 PDT 2024
https://github.com/bc-lee updated https://github.com/llvm/llvm-project/pull/111375
>From 23b90bba12c010e5882e09e9f6b765a7281324aa Mon Sep 17 00:00:00 2001
From: Byoungchan Lee <byoungchan.lee at gmx.com>
Date: Mon, 7 Oct 2024 22:19:38 +0900
Subject: [PATCH 1/2] [clang-include-cleaner] Fix incorrect directory issue for
writing files
If the current working directory of `clang-include-cleaner` differs from
the directory of the input files specified in the compilation database,
it doesn't adjust the input file paths properly. As a result,
`clang-include-cleaner` either writes files to the wrong directory or
fails to write files altogether.
This pull request fixes the issue by checking whether the input file
path is absolute. If it isn't, the path is concatenated with
the directory of the input file specified in the compilation database.
Fixes #110843.
---
.../include-cleaner/tool/IncludeCleaner.cpp | 24 +++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index 080099adc9a07a..1cb8b9c4ae99b7 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -173,11 +173,22 @@ class Action : public clang::ASTFrontendAction {
if (!HTMLReportPath.empty())
writeHTML();
+ // Source File's path relative to compilation database.
llvm::StringRef Path =
SM.getFileEntryRefForID(SM.getMainFileID())->getName();
assert(!Path.empty() && "Main file path not known?");
llvm::StringRef Code = SM.getBufferData(SM.getMainFileID());
+ auto Cwd = getCompilerInstance()
+ .getFileManager()
+ .getVirtualFileSystem()
+ .getCurrentWorkingDirectory();
+ if (Cwd.getError()) {
+ llvm::errs() << "Failed to get current working directory: "
+ << Cwd.getError().message() << "\n";
+ return;
+ }
+
auto Results =
analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI,
getCompilerInstance().getPreprocessor(), HeaderFilter);
@@ -201,8 +212,17 @@ class Action : public clang::ASTFrontendAction {
}
}
- if (!Results.Missing.empty() || !Results.Unused.empty())
- EditedFiles.try_emplace(Path, Final);
+ if (!Results.Missing.empty() || !Results.Unused.empty()) {
+ auto Sept = llvm::sys::path::get_separator();
+ // Adjust the path to be absolute if it is not.
+ std::string FullPath;
+ if (llvm::sys::path::is_absolute(Path))
+ FullPath = Path.str();
+ else
+ FullPath = Cwd.get() + Sept.str() + Path.str();
+
+ EditedFiles.try_emplace(FullPath, Final);
+ }
}
void writeHTML() {
>From eef397cf22e062cf024ffcd84fef7bdf98833351 Mon Sep 17 00:00:00 2001
From: Byoungchan Lee <byoungchan.lee at gmx.com>
Date: Wed, 9 Oct 2024 19:43:02 +0900
Subject: [PATCH 2/2] Fix path issue identified by @kadircet
Some Bazel systems might have a read-only file system in the build
directory. In such cases, the output file should be written to
the original source file. Adjusted the logic to find the correct
output directory based on the compilation database and
the current working directory.
---
.../include-cleaner/tool/IncludeCleaner.cpp | 95 ++++++++++++++-----
1 file changed, 73 insertions(+), 22 deletions(-)
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index 1cb8b9c4ae99b7..4481601140e3fe 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -179,16 +179,6 @@ class Action : public clang::ASTFrontendAction {
assert(!Path.empty() && "Main file path not known?");
llvm::StringRef Code = SM.getBufferData(SM.getMainFileID());
- auto Cwd = getCompilerInstance()
- .getFileManager()
- .getVirtualFileSystem()
- .getCurrentWorkingDirectory();
- if (Cwd.getError()) {
- llvm::errs() << "Failed to get current working directory: "
- << Cwd.getError().message() << "\n";
- return;
- }
-
auto Results =
analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI,
getCompilerInstance().getPreprocessor(), HeaderFilter);
@@ -212,17 +202,8 @@ class Action : public clang::ASTFrontendAction {
}
}
- if (!Results.Missing.empty() || !Results.Unused.empty()) {
- auto Sept = llvm::sys::path::get_separator();
- // Adjust the path to be absolute if it is not.
- std::string FullPath;
- if (llvm::sys::path::is_absolute(Path))
- FullPath = Path.str();
- else
- FullPath = Cwd.get() + Sept.str() + Path.str();
-
- EditedFiles.try_emplace(FullPath, Final);
- }
+ if (!Results.Missing.empty() || !Results.Unused.empty())
+ EditedFiles.try_emplace(Path, Final);
}
void writeHTML() {
@@ -300,6 +281,42 @@ std::function<bool(llvm::StringRef)> headerFilter() {
};
}
+llvm::ErrorOr<std::string> getCurrentWorkingDirectory() {
+ llvm::SmallString<256> CurrentPath;
+ if (const std::error_code EC = llvm::sys::fs::current_path(CurrentPath))
+ return llvm::ErrorOr<std::string>(EC);
+ return std::string(CurrentPath.str());
+}
+
+std::optional<std::string>
+adjustCompilationPath(const std::string &Filename, const std::string &Directory,
+ const std::string &CurrentWorkingDirectory) {
+ if (llvm::sys::path::is_absolute(Filename)) {
+ // If the file path is already absolute, use it as is.
+ return Filename;
+ }
+
+ auto Sept = llvm::sys::path::get_separator().str();
+ // First, try to find the file based on the compilation database.
+ std::string FilePath = Directory + Sept + Filename;
+ // Check if it is writable.
+ if (llvm::sys::fs::access(FilePath, llvm::sys::fs::AccessMode::Write) !=
+ std::error_code()) {
+ // If not, try to find the file based on the current working
+ // directory, as some Bazel invocations may not set the compilation
+ // invocation's filesystem as non-writable. In such cases, we can
+ // find the file based on the current working directory.
+ FilePath =
+ static_cast<std::string>(CurrentWorkingDirectory) + Sept + Filename;
+ if (llvm::sys::fs::access(FilePath, llvm::sys::fs::AccessMode::Write) !=
+ std::error_code()) {
+ llvm::errs() << "Failed to find a writable path for " << Filename << "\n";
+ return std::nullopt;
+ }
+ }
+ return FilePath;
+}
+
} // namespace
} // namespace include_cleaner
} // namespace clang
@@ -325,7 +342,32 @@ int main(int argc, const char **argv) {
}
}
- clang::tooling::ClangTool Tool(OptionsParser->getCompilations(),
+ auto &CompilationDatabase = OptionsParser->getCompilations();
+ // Mapping of edited file paths to their actual paths.
+ std::map<std::string, std::string> EditedFilesToActualFiles;
+ if (Edit) {
+ std::vector<clang::tooling::CompileCommand> Commands =
+ CompilationDatabase.getAllCompileCommands();
+ auto CurrentWorkingDirectory = getCurrentWorkingDirectory();
+ // if (CurrentWorkingDirectory.get()
+ if (auto EC = CurrentWorkingDirectory.getError()) {
+ llvm::errs() << "Failed to get current working directory: "
+ << EC.message() << "\n";
+ return 1;
+ }
+
+ for (const auto &Command : Commands) {
+ auto AdjustedFilePath = adjustCompilationPath(
+ Command.Filename, Command.Directory, CurrentWorkingDirectory.get());
+ if (!AdjustedFilePath.has_value()) {
+ // We already printed an error message.
+ return 1;
+ }
+ EditedFilesToActualFiles[Command.Filename] = AdjustedFilePath.value();
+ }
+ }
+
+ clang::tooling::ClangTool Tool(CompilationDatabase,
OptionsParser->getSourcePathList());
auto HeaderFilter = headerFilter();
@@ -336,6 +378,15 @@ int main(int argc, const char **argv) {
if (Edit) {
for (const auto &NameAndContent : Factory.editedFiles()) {
llvm::StringRef FileName = NameAndContent.first();
+ auto AdjustedFilePathIter = EditedFilesToActualFiles.find(FileName.str());
+ if (AdjustedFilePathIter != EditedFilesToActualFiles.end()) {
+ FileName = AdjustedFilePathIter->second;
+ } else {
+ llvm::errs() << "Failed to find the actual path for " << FileName
+ << "\n";
+ ++Errors;
+ }
+
const std::string &FinalCode = NameAndContent.second;
if (auto Err = llvm::writeToOutput(
FileName, [&](llvm::raw_ostream &OS) -> llvm::Error {
More information about the cfe-commits
mailing list