Re: [clang] 6654719 - [CMake] Fix logic error: NOT LIBCLANG_BUILD_STATIC does not imply PIC

[David Zarzycki via cfe-commits]

Okay, sounds good. For whatever it may be worth, I pasted the build failure to D75068 shortly after it landed with the hope that the original author(s) would commit a quick fix, but that didn't happen.


On Wed, Apr 29, 2020, at 10:23 AM, Hans Wennborg wrote:
> Your suggestion of
> 
> > if(LLVM_ENABLE_PIC OR (WIN32 AND NOT LIBCLANG_BUILD_STATIC))
> 
> sounds good to me. I don't know what the non-Windows non-PIC problem
> was exactly (your commit didn't mention it), so maybe it's best if you
> land that to verify it fixes the issue?
> 
> Thanks,
> Hans
> 
> On Wed, Apr 29, 2020 at 3:59 PM David Zarzycki <dave at znu.io> wrote:
> >
> > Hans,
> >
> > Non-Windows non-PIC setups were working just fine until four days ago when D75068 landed. I tried to narrowly unbreak non-Windows non-PIC systems but apparently that broke Windows. For that, I'm sorry. That being said, non-Windows systems are broken again and this breakable was foreseeable given my revert. What is your plan for non-Windows systems?
> >
> > As a part of the post-revert discussion in D75068, cristian.adam suggested that the check for WIN32 should be added back, but nobody followed through on that. What is stopping us from adding Christian's proposed change? Specifically:
> >
> > if(LLVM_ENABLE_PIC OR (WIN32 AND NOT LIBCLANG_BUILD_STATIC))
> >
> > Dave
> >
> > On Wed, Apr 29, 2020, at 9:19 AM, Hans Wennborg wrote:
> > > On Sun, Apr 26, 2020 at 1:17 PM David Zarzycki via cfe-commits
> > > <cfe-commits at lists.llvm.org> wrote:
> > > >
> > > >
> > > > Author: David Zarzycki
> > > > Date: 2020-04-26T07:16:42-04:00
> > > > New Revision: 665471907a5c072c6653a38c35f35e5d54cef220
> > > >
> > > > URL: https://github.com/llvm/llvm-project/commit/665471907a5c072c6653a38c35f35e5d54cef220
> > > > DIFF: https://github.com/llvm/llvm-project/commit/665471907a5c072c6653a38c35f35e5d54cef220.diff
> > > >
> > > > LOG: [CMake] Fix logic error: NOT LIBCLANG_BUILD_STATIC does not imply PIC
> > >
> > > But it does imply building libclang.dll.
> > >
> > > Your change broke building libclang.dll when configured with "cmake
> > > -GNinja -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang
> > > -DLLVM_TARGETS_TO_BUILD=X86 -DLLVM_ENABLE_PIC=OFF" on Windows, see the
> > > discussion here: https://reviews.llvm.org/D74564#1884556
> > >
> > > I've reverted in 209ab6d8835cd88320ceb814893759cfbda91d15.
> > >
> > > >
> > > > Added:
> > > >
> > > >
> > > > Modified:
> > > >     clang/tools/libclang/CMakeLists.txt
> > > >
> > > > Removed:
> > > >
> > > >
> > > >
> > > > ################################################################################
> > > > diff  --git a/clang/tools/libclang/CMakeLists.txt b/clang/tools/libclang/CMakeLists.txt
> > > > index bb2b14cc8e27..9368501592a9 100644
> > > > --- a/clang/tools/libclang/CMakeLists.txt
> > > > +++ b/clang/tools/libclang/CMakeLists.txt
> > > > @@ -77,7 +77,7 @@ if(MSVC)
> > > >    set(LLVM_EXPORTED_SYMBOL_FILE)
> > > >  endif()
> > > >
> > > > -if(LLVM_ENABLE_PIC OR NOT LIBCLANG_BUILD_STATIC)
> > > > +if(LLVM_ENABLE_PIC)
> > > >    set(ENABLE_SHARED SHARED)
> > > >  endif()
> > > >
> > > >
> > > >
> > > >
> > > > _______________________________________________
> > > > cfe-commits mailing list
> > > > cfe-commits at lists.llvm.org
> > > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> > >
>