tstellar added inline comments.
================
Comment at: clang/CMakeLists.txt:97
# Seek installed Lit.
- find_program(LLVM_LIT
- NAMES llvm-lit lit.py lit
- PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
- DOC "Path to lit.py")
+ if (NOT LLVM_EXTERNAL_LIT)
+ find_program(LLVM_EXTERNAL_LIT
----------------
kwk wrote:
> mgorny wrote:
> > I don't think you need to do this if you rename the var, `find_program()`
> > doesn't seem to do any searching when the var is already set.
> >
> > I've tested with a dummy CMakeLists:
> >
> > ```
> > set(FOO /bin/true)
> > find_program(FOO NAMES foo)
> > message(FATAL_ERROR ${FOO})
> > ```
> >
> > gives `/bin/true` for me.
> > I don't think you need to do this if you rename the var, `find_program()`
> > doesn't seem to do any searching when the var is already set.
>
> This might be true but it is counter intuitive to assume that `find_program`
> does nothing. So for readability I'd keep the `if (NOT LLVM_EXTERNAL_LIT)`. I
> know it is not needed but with the `if` it won't look like
> `find_program(LLVM_EXTERNAL_LIT` is the only/central place to set the
> variable `LLVM_EXTERNAL_LIT`. With the guard around it, it becomes more
> obvious that this is more of a fallback.
@mgorny The [[ http://devdoc.net/linux/cmake-3.9.6/command/find_program.html |
documentation ]] confirms this too:
"Once one of the calls succeeds the result variable will be set and stored in
the cache so that no call will search again."
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138258/new/
https://reviews.llvm.org/D138258
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
