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

[David Zarzycki via cfe-commits]

Hi Cristian,

That's alright. No worries. Good luck with the move (especially during the pandemic). My three stage test of LLVM+clang+lld+libcxx+libcxxabi is almost done, and I'll commit your suggested fix soon.

Dave

On Wed, Apr 29, 2020, at 10:53 AM, Cristian Adam wrote:
> 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
> > > > >
> > >
>