aganea marked 5 inline comments as done. aganea added a comment. In D80833#2071863 <https://reviews.llvm.org/D80833#2071863>, @hans wrote:
> In D80833#2071818 <https://reviews.llvm.org/D80833#2071818>, @aganea wrote: > > > I didn't change the PWD/CWD stored in `LF_BUILDINFO`, it has already been > > done by Reid a while ago: D53179 <https://reviews.llvm.org/D53179> (it > > already stores absolute paths) > > > That's surprising to me. Nico is the real export on reproducible builds, but > we generally try very hard to make builds reproducible independent of source > and build dir. Maybe there should be a flag controlling this? /Brepro? Ah, you must be using `-fdebug-compilation-dir` on Chromium I suppose, to make paths relative in .OBJs. Otherwise the default is the full path to PWD/CWD. Added a test for that. In D80833#2071969 <https://reviews.llvm.org/D80833#2071969>, @thakis wrote: > We don't store physical absolute paths in pdbs if you hold things right. Look > for /pdbsourcepath: in > http://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html . > rnk's change seems to store the main TU's dir as cwd, which likely either > takes the dir from the main TU as passed (ie if you pass an erlative path, it > stores that), or it honors /pdbsourcepath:. If that's fine as-is, that's ok. > It's different from what `pwd` returns though I think. (imagine you're in > c:\foo and you build bar\baz.cc. the cwd in the pdb will be c:\foo\bar but > the command will be relative to c:\foo if I understand things right? I might > not though.) Without `-fdebug-compilation-dir`, the **full path **is stored in `LF_BUILDINFO` (see `CodeGenOpts::DebugCompilationDir` and `CGDebugInfo::getCurrentDirname()`) With `-fdebug-compilation-dir .`, the PWD/CWD becomes relative ('.') and I've added code in LLD to call `pdbMakeAbsolute` on it, like for other PDB strings. Now `/pdbsourcepath` works as well (all this is covered in lld/test/COFF/pdb-relative-source-lines.test). However if you're in `c:\foo` and you do `clang-cl /c bar\baz.cc`, then `bar\baz.cc` will be stored as-it-is in `LF_BUILDINFO` (the PWD/CWD and the main filename are stored separately) The only absolute paths that remain are: a. the compiler path (`D:\llvm-project\buildninjaDebMSVC\bin\clang-cl.exe` in the yaml below) and b. the `-internal-isystem` paths. However that is not an issue on our end, as we're building with `-nostdinc` + nuget packages installed in a fixed dir on all users' machines. Not sure how that works for you? ================ Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:835 +static std::string renderCommandLine(ArrayRef<const char *> CommandLineArgs, + StringRef MainFile) { + std::string FlatCmdLine; ---------------- aganea wrote: > hans wrote: > > Don't we already have code somewhere that can do this quoting? E.g. the > > code that prints the cc1 args for "clang -v"? > Yes, the code below was copy-pasted from `Command::printArg`. But that's in > Clang. Should I make that code common somewhere in `llvm/lib/Support`? Replaced by `llvm::sys::flattenWindowsCommandLine`. I can't find any equivalent for Linux, I hope just aggregating the arguments with spaces in between is fine? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80833/new/ https://reviews.llvm.org/D80833 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
