Hi Hans, I'm a bit suspicious against the part
> This also revertes the part of r369938 which checked that -working-directory works. in this revert. You do: > + SmallString<128> Buf; > + if (!llvm::sys::fs::current_path(Buf)) > + Buf = "."; > + CDB << "{ \"directory\": \"" << escape(Buf) << "\""; But if I look at r369938 it seems like before r369938 it looked like - SmallString<128> Buf; - if (llvm::sys::fs::current_path(Buf)) - Buf = "."; - CDB << "{ \"directory\": \"" << escape(Buf) << "\""; Note the difference in the condition, where you do if (!llvm::sys::fs::current_path(Buf)) but before it was if (llvm::sys::fs::current_path(Buf)) We noticed this since clang -MJ test.json -O2 -nostdinc empty.c -S -o - && cat test.json started to behave differently after this revert. The "directory: " part in the output suddenly became ".". /Mikael On 2019-09-05 10:43, Hans Wennborg via cfe-commits wrote: > Author: hans > Date: Thu Sep 5 01:43:00 2019 > New Revision: 371027 > > URL: > https://protect2.fireeye.com/url?k=a390ce47-ff1a05b3-a3908edc-86cd58c48020-98ad75d673bc6e26&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D371027%26view%3Drev > Log: > Revert r361885 "[Driver] Fix -working-directory issues" > > This made clang unable to open files using relative paths on network shares on > Windows (PR43204). On the bug it was pointed out that > createPhysicalFileSystem() > is not terribly mature, and using it is risky. Reverting for now until there's > a clear way forward. > >> Currently the `-working-directory` option does not actually impact the >> working >> directory for all of the clang driver, it only impacts how files are looked >> up >> to make sure they exist. This means that that clang passes the wrong paths >> to -fdebug-compilation-dir and -coverage-notes-file. >> >> This patch fixes that by changing all the places in the driver where we >> convert >> to absolute paths to use the VFS, and then calling >> setCurrentWorkingDirectory on >> the VFS. This also changes the default VFS for `Driver` to use a virtualized >> working directory, instead of changing the process's working directory. >> >> Differential Revision: >> https://protect2.fireeye.com/url?k=4c8bdb04-100110f0-4c8b9b9f-86cd58c48020-5e9c5a0eee8ac746&q=1&u=https%3A%2F%2Freviews.llvm.org%2FD62271 > > This also revertes the part of r369938 which checked that -working-directory > works. > > Modified: > cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td > cfe/trunk/lib/Driver/Driver.cpp > cfe/trunk/lib/Driver/ToolChains/Clang.cpp > cfe/trunk/test/Driver/gen-cdb-fragment.c > cfe/trunk/test/Driver/working-directory.c > > Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td > URL: > https://protect2.fireeye.com/url?k=dc8ae2d2-80002926-dc8aa249-86cd58c48020-73e94a1494d49e2b&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Finclude%2Fclang%2FBasic%2FDiagnosticDriverKinds.td%3Frev%3D371027%26r1%3D371026%26r2%3D371027%26view%3Ddiff > ============================================================================== > --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Thu Sep 5 > 01:43:00 2019 > @@ -91,8 +91,6 @@ def err_no_external_assembler : Error< > "there is no external assembler that can be used on this platform">; > def err_drv_unable_to_remove_file : Error< > "unable to remove file: %0">; > -def err_drv_unable_to_set_working_directory : Error < > - "unable to set working directory: %0">; > def err_drv_command_failure : Error< > "unable to execute command: %0">; > def err_drv_invalid_darwin_version : Error< > > Modified: cfe/trunk/lib/Driver/Driver.cpp > URL: > https://protect2.fireeye.com/url?k=64c4a345-384e68b1-64c4e3de-86cd58c48020-5b165b055bfa7d40&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Flib%2FDriver%2FDriver.cpp%3Frev%3D371027%26r1%3D371026%26r2%3D371027%26view%3Ddiff > ============================================================================== > --- cfe/trunk/lib/Driver/Driver.cpp (original) > +++ cfe/trunk/lib/Driver/Driver.cpp Thu Sep 5 01:43:00 2019 > @@ -133,7 +133,7 @@ Driver::Driver(StringRef ClangExecutable > > // Provide a sane fallback if no VFS is specified. > if (!this->VFS) > - this->VFS = llvm::vfs::createPhysicalFileSystem().release(); > + this->VFS = llvm::vfs::getRealFileSystem(); > > Name = llvm::sys::path::filename(ClangExecutable); > Dir = llvm::sys::path::parent_path(ClangExecutable); > @@ -1010,11 +1010,6 @@ Compilation *Driver::BuildCompilation(Ar > } > } > > - // Check for working directory option before accessing any files > - if (Arg *WD = Args.getLastArg(options::OPT_working_directory)) > - if (VFS->setCurrentWorkingDirectory(WD->getValue())) > - Diag(diag::err_drv_unable_to_set_working_directory) << WD->getValue(); > - > // FIXME: This stuff needs to go into the Compilation, not the driver. > bool CCCPrintPhases; > > @@ -1996,11 +1991,20 @@ bool Driver::DiagnoseInputExistence(cons > if (Value == "-") > return true; > > - if (getVFS().exists(Value)) > + SmallString<64> Path(Value); > + if (Arg *WorkDir = Args.getLastArg(options::OPT_working_directory)) { > + if (!llvm::sys::path::is_absolute(Path)) { > + SmallString<64> Directory(WorkDir->getValue()); > + llvm::sys::path::append(Directory, Value); > + Path.assign(Directory); > + } > + } > + > + if (getVFS().exists(Path)) > return true; > > if (IsCLMode()) { > - if (!llvm::sys::path::is_absolute(Twine(Value)) && > + if (!llvm::sys::path::is_absolute(Twine(Path)) && > llvm::sys::Process::FindInEnvPath("LIB", Value)) > return true; > > @@ -2026,12 +2030,12 @@ bool Driver::DiagnoseInputExistence(cons > if (getOpts().findNearest(Value, Nearest, IncludedFlagsBitmask, > ExcludedFlagsBitmask) <= 1) { > Diag(clang::diag::err_drv_no_such_file_with_suggestion) > - << Value << Nearest; > + << Path << Nearest; > return false; > } > } > > - Diag(clang::diag::err_drv_no_such_file) << Value; > + Diag(clang::diag::err_drv_no_such_file) << Path; > return false; > } > > > Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp > URL: > https://protect2.fireeye.com/url?k=29c8d80a-754213fe-29c89891-86cd58c48020-d3513617254de7d4&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Flib%2FDriver%2FToolChains%2FClang.cpp%3Frev%3D371027%26r1%3D371026%26r2%3D371027%26view%3Ddiff > ============================================================================== > --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original) > +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Thu Sep 5 01:43:00 2019 > @@ -608,15 +608,16 @@ getFramePointerKind(const ArgList &Args, > } > > /// Add a CC1 option to specify the debug compilation directory. > -static void addDebugCompDirArg(const ArgList &Args, ArgStringList &CmdArgs, > - const llvm::vfs::FileSystem &VFS) { > +static void addDebugCompDirArg(const ArgList &Args, ArgStringList &CmdArgs) { > if (Arg *A = Args.getLastArg(options::OPT_fdebug_compilation_dir)) { > CmdArgs.push_back("-fdebug-compilation-dir"); > CmdArgs.push_back(A->getValue()); > - } else if (llvm::ErrorOr<std::string> CWD = > - VFS.getCurrentWorkingDirectory()) { > - CmdArgs.push_back("-fdebug-compilation-dir"); > - CmdArgs.push_back(Args.MakeArgString(*CWD)); > + } else { > + SmallString<128> cwd; > + if (!llvm::sys::fs::current_path(cwd)) { > + CmdArgs.push_back("-fdebug-compilation-dir"); > + CmdArgs.push_back(Args.MakeArgString(cwd)); > + } > } > } > > @@ -882,8 +883,13 @@ static void addPGOAndCoverageFlags(const > else > OutputFilename = llvm::sys::path::filename(Output.getBaseInput()); > SmallString<128> CoverageFilename = OutputFilename; > - if (llvm::sys::path::is_relative(CoverageFilename)) > - (void)D.getVFS().makeAbsolute(CoverageFilename); > + if (llvm::sys::path::is_relative(CoverageFilename)) { > + SmallString<128> Pwd; > + if (!llvm::sys::fs::current_path(Pwd)) { > + llvm::sys::path::append(Pwd, CoverageFilename); > + CoverageFilename.swap(Pwd); > + } > + } > llvm::sys::path::replace_extension(CoverageFilename, "gcno"); > CmdArgs.push_back(Args.MakeArgString(CoverageFilename)); > > @@ -2013,14 +2019,13 @@ void Clang::DumpCompilationDatabase(Comp > CompilationDatabase = std::move(File); > } > auto &CDB = *CompilationDatabase; > - auto CWD = D.getVFS().getCurrentWorkingDirectory(); > - if (!CWD) > - CWD = "."; > - CDB << "{ \"directory\": \"" << escape(*CWD) << "\""; > + SmallString<128> Buf; > + if (!llvm::sys::fs::current_path(Buf)) > + Buf = "."; > + CDB << "{ \"directory\": \"" << escape(Buf) << "\""; > CDB << ", \"file\": \"" << escape(Input.getFilename()) << "\""; > CDB << ", \"output\": \"" << escape(Output.getFilename()) << "\""; > CDB << ", \"arguments\": [\"" << escape(D.ClangExecutable) << "\""; > - SmallString<128> Buf; > Buf = "-x"; > Buf += types::getTypeName(Input.getType()); > CDB << ", \"" << escape(Buf) << "\""; > @@ -4464,7 +4469,7 @@ void Clang::ConstructJob(Compilation &C, > CmdArgs.push_back("-fno-autolink"); > > // Add in -fdebug-compilation-dir if necessary. > - addDebugCompDirArg(Args, CmdArgs, D.getVFS()); > + addDebugCompDirArg(Args, CmdArgs); > > addDebugPrefixMapArg(D, Args, CmdArgs); > > @@ -6192,7 +6197,7 @@ void ClangAs::ConstructJob(Compilation & > DebugInfoKind = (WantDebug ? codegenoptions::LimitedDebugInfo > : codegenoptions::NoDebugInfo); > // Add the -fdebug-compilation-dir flag if needed. > - addDebugCompDirArg(Args, CmdArgs, C.getDriver().getVFS()); > + addDebugCompDirArg(Args, CmdArgs); > > addDebugPrefixMapArg(getToolChain().getDriver(), Args, CmdArgs); > > > Modified: cfe/trunk/test/Driver/gen-cdb-fragment.c > URL: > https://protect2.fireeye.com/url?k=98fca8be-c476634a-98fce825-86cd58c48020-8d98a9a164425ae2&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Ftest%2FDriver%2Fgen-cdb-fragment.c%3Frev%3D371027%26r1%3D371026%26r2%3D371027%26view%3Ddiff > ============================================================================== > --- cfe/trunk/test/Driver/gen-cdb-fragment.c (original) > +++ cfe/trunk/test/Driver/gen-cdb-fragment.c Thu Sep 5 01:43:00 2019 > @@ -15,14 +15,6 @@ > // RUN: %clang -target x86_64-apple-macos10.15 -S %s -o - > -gen-cdb-fragment-path %t.cdb > // RUN: ls %t.cdb | FileCheck --check-prefix=CHECK-LS %s > > -// Working directory arg is respected. > -// RUN: rm -rf %t.cdb > -// RUN: mkdir %t.cdb > -// RUN: %clang -target x86_64-apple-macos10.15 -working-directory %t.cdb -c > %s -o - -gen-cdb-fragment-path "." > -// RUN: ls %t.cdb | FileCheck --check-prefix=CHECK-LS %s > -// RUN: cat %t.cdb/*.json | FileCheck --check-prefix=CHECK-CWD %s > -// CHECK-CWD: "directory": "{{.*}}.cdb" > - > // -### does not emit the CDB fragment > // RUN: rm -rf %t.cdb > // RUN: mkdir %t.cdb > > Modified: cfe/trunk/test/Driver/working-directory.c > URL: > https://protect2.fireeye.com/url?k=e7c50cb3-bb4fc747-e7c54c28-86cd58c48020-1a2070795ddb99be&q=1&u=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%2Fcfe%2Ftrunk%2Ftest%2FDriver%2Fworking-directory.c%3Frev%3D371027%26r1%3D371026%26r2%3D371027%26view%3Ddiff > ============================================================================== > --- cfe/trunk/test/Driver/working-directory.c (original) > +++ cfe/trunk/test/Driver/working-directory.c Thu Sep 5 01:43:00 2019 > @@ -1,11 +1,3 @@ > // RUN: %clang -### -working-directory /no/such/dir/ input 2>&1 | FileCheck > %s > -// RUN: %clang -### -working-directory %p/Inputs no_such_file.cpp -c 2>&1 | > FileCheck %s --check-prefix=CHECK_NO_FILE > -// RUN: %clang -### -working-directory %p/Inputs pchfile.cpp -c 2>&1 | > FileCheck %s --check-prefix=CHECK_WORKS > > -// CHECK: unable to set working directory: /no/such/dir/ > - > -// CHECK_NO_FILE: no such file or directory: 'no_such_file.cpp' > - > -// CHECK_WORKS: "-coverage-notes-file" > "{{[^"]+}}test{{/|\\\\}}Driver{{/|\\\\}}Inputs{{/|\\\\}}pchfile.gcno" > -// CHECK_WORKS: "-working-directory" > "{{[^"]+}}test{{/|\\\\}}Driver{{/|\\\\}}Inputs" > -// CHECK_WORKS: "-fdebug-compilation-dir" > "{{[^"]+}}test{{/|\\\\}}Driver{{/|\\\\}}Inputs" > +//CHECK: no such file or directory: '/no/such/dir/input' > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://protect2.fireeye.com/url?k=06063816-5a8cf3e2-0606788d-86cd58c48020-4a392f810e67be81&q=1&u=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fcfe-commits > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits