[PATCH] D108366: [clang][deps] Make resource directory deduction configurable

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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