Re: [clang] 6654719 - [CMake] Fix logic error: NOT LIBCLANG_BUILD_STATIC does not imply PIC
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 > > Sent: Wednesday, 29 April 2020 16:37 > > To: Hans Wennborg > > Cc: Cristian Adam ; David Zarzycki via cfe-commits > > > comm...@lists.llvm.org>; Nico Weber > > 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 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 > > > > > 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: > > &g
RE: [clang] 6654719 - [CMake] Fix logic error: NOT LIBCLANG_BUILD_STATIC does not imply PIC
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 > Sent: Wednesday, 29 April 2020 16:37 > To: Hans Wennborg > Cc: Cristian Adam ; David Zarzycki via cfe-commits comm...@lists.llvm.org>; Nico Weber > 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 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 > > > > 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@lists.llvm.org > > > > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
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 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 > > > 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@lists.llvm.org > > > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang] 6654719 - [CMake] Fix logic error: NOT LIBCLANG_BUILD_STATIC does not imply PIC
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 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 > > 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@lists.llvm.org > > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang] 6654719 - [CMake] Fix logic error: NOT LIBCLANG_BUILD_STATIC does not imply PIC
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 > 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@lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [clang] 6654719 - [CMake] Fix logic error: NOT LIBCLANG_BUILD_STATIC does not imply PIC
On Sun, Apr 26, 2020 at 1:17 PM David Zarzycki via cfe-commits 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@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 6654719 - [CMake] Fix logic error: NOT LIBCLANG_BUILD_STATIC does not imply PIC
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 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@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
