dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land.
LGTM. ================ Comment at: clang/test/ClangScanDeps/resource_directory.c:3 +// RUN: cp %S/Inputs/resource_directory/* %t +// RUN: sed -e "s|CLANG|/our/custom/bin/clang|g" -e "s|DIR|%/t|g" %S/Inputs/resource_directory/cdb_tu.json > %t/cdb.json + ---------------- jansvoboda11 wrote: > dexonsmith wrote: > > I don't love this hardcoded path to clang which //could// exist on some > > machine. > > > > Could you use `/dev/null` for the path to clang? > > Or `/dev/null/bin/clang`? > This is not a concern anymore. It used to be, since the deduction of resource > directory based on the compiler path was only kicking in if the > `-print-resource-dir` invocation failed (e.g. the compiler executable didn't > exist). Since we're now explicitly passing `--resource-dir-recipe > modify-compiler-path`, it doesn't matter if the compiler executable exists on > the filesystem or not. We're always going to do simple string manipulation in > this case. > > For the other test-case (`--resource-dir-recipe invoke-compiler`), we're > actually using an executable that does exist on the filesystem. Okay. As long as existence on disk of `/custom/compiler/resources` and/or `/our/custom/clang` won't change behaviour of the test, I think the test is fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108366/new/ https://reviews.llvm.org/D108366 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
