[PATCH] D59632: [llvm] [cmake] Add additional headers only if they exist
Shoaib Meenai via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 3 17:29:16 PDT 2019
smeenai added inline comments.
================
Comment at: llvm/cmake/modules/LLVMProcessSources.cmake:35
+ foreach(file ${hds})
+ if(EXISTS ${file})
+ list(APPEND filtered ${file})
----------------
smeenai wrote:
> mgorny wrote:
> > smeenai wrote:
> > > I'd add a comment explaining this is for broken symlinks, otherwise I'd be wondering why you were checking for the existence of a file you just globbed...
> > >
> > > Actually, is `EXISTS` resolving symlinks documented anywhere? It's unclear if it would return the existence of the symlink itself or the target of the symlink.
> > > I'd add a comment explaining this is for broken symlinks, otherwise I'd be wondering why you were checking for the existence of a file you just globbed...
> >
> > Good idea.
> >
> > > Actually, is EXISTS resolving symlinks documented anywhere? It's unclear if it would return the existence of the symlink itself or the target of the symlink.
> >
> > To be honest, I don't see it in CMake docs. I think I presumed that behavior from shell's `-f` operator but there's indeed some potential ambiguity here.
> Looks like it just calls `access`, which should resolve symlinks.
I submitted https://gitlab.kitware.com/cmake/cmake/merge_requests/3189 to CMake to clarify this.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59632/new/
https://reviews.llvm.org/D59632
More information about the llvm-commits
mailing list