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

[Cristian Adam via cfe-commits]

Hi, 

Thank you David for not reverting my 3rd attempt to get libclang to build statically on Windows.

In my defense the commit landed on a Saturday, and while I usually hack on weekends, but now I'm involved in moving to a new home and I wasn't able to reply to your message.

I'm sorry for the trouble of getting the revert and failed build. I thought I had everything covered this time...

Cheers
Cristian.

> -----Original Message-----
> From: David Zarzycki <dave at znu.io>
> Sent: Wednesday, 29 April 2020 16:37
> To: Hans Wennborg <hans at chromium.org>
> Cc: Cristian Adam <cristian.adam at qt.io>; David Zarzycki via cfe-commits <cfe-
> commits at lists.llvm.org>; Nico Weber <thakis at chromium.org>
> Subject: Re: [clang] 6654719 - [CMake] Fix logic error: NOT
> LIBCLANG_BUILD_STATIC does not imply PIC
> 
> 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/665471907a5c072c6653
> > > > > a38c35f35e5d54cef220
> > > > > DIFF:
> > > > > https://github.com/llvm/llvm-project/commit/665471907a5c072c6653
> > > > > a38c35f35e5d54cef220.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
> > > >
> >