Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote: On 06/08/2012 12:59 PM, Stephen Kelly wrote: Fixed now (I hope), thanks, This change is now in master in preparation for 2.8.9. Thanks for your work and persistence on this! FYI, I rewrote the topic history prior to merging to clean up all the cruft from dashboard fixes and the change in overall approach that we made midway through. It is now three commits: http://cmake.org/gitweb?p=cmake.git;a=commit;h=31d7a0f2 http://cmake.org/gitweb?p=cmake.git;a=commit;h=55d7aa4c http://cmake.org/gitweb?p=cmake.git;a=commit;h=bd349630 I wrote a new commit message for each one to record the thinking that went into it after our discussions in this thread. The net change made by the topic is identical to your original. Great. Looking forward to using it :). Thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 06/08/2012 12:59 PM, Stephen Kelly wrote: Fixed now (I hope), thanks, This change is now in master in preparation for 2.8.9. Thanks for your work and persistence on this! FYI, I rewrote the topic history prior to merging to clean up all the cruft from dashboard fixes and the change in overall approach that we made midway through. It is now three commits: http://cmake.org/gitweb?p=cmake.git;a=commit;h=31d7a0f2 http://cmake.org/gitweb?p=cmake.git;a=commit;h=55d7aa4c http://cmake.org/gitweb?p=cmake.git;a=commit;h=bd349630 I wrote a new commit message for each one to record the thinking that went into it after our discussions in this thread. The net change made by the topic is identical to your original. -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote:
On Wed, Jun 6, 2012 at 3:48 AM, Stephen Kelly
steve...@gmail.com wrote:
This seems to have caused todays failures:
http://open.cdash.org/buildSummary.php?buildid=2337242
* Does the test pass if you replace
if(NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 3.4)
with
if(NOT CMAKE_${lang}_COMPILER_VERSION VERSION_LESS 3.4)
That fixes it. The test does check_cxx_source_compiles. Inside the
try_compile it will enable only CXX and not C so testing
CMAKE_C_COMPILER_VERSION does not make sense.
Great. That is confirmed now.
I still have an error with Watcom that I don't know how to fix, which I
think is the last error on this topic:
http://open.cdash.org/testDetails.php?test=148809149build=2343558
My guess was that the CMAKE_CXX_COMPILE_OPTIONS_DLL flag is not being
populated or used, though I don't see why.
Can someone with access to that box debug it a bit please?
Thanks,
Steve.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 06/08/2012 04:50 AM, Stephen Kelly wrote:
I still have an error with Watcom that I don't know how to fix, which I
think is the last error on this topic:
http://open.cdash.org/testDetails.php?test=148809149build=2343558
My guess was that the CMAKE_CXX_COMPILE_OPTIONS_DLL flag is not being
populated or used, though I don't see why.
Can someone with access to that box debug it a bit please?
The compiler flags are fine. This compiler just doesn't export
inline class members from DLLs, it appears. Use the patch below.
-Brad
diff --git a/Tests/PositionIndependentTargets/pic_lib.cpp
b/Tests/PositionIndependentTargets/pic_lib.cpp
index ec351b4..b8b25a3 100644
--- a/Tests/PositionIndependentTargets/pic_lib.cpp
+++ b/Tests/PositionIndependentTargets/pic_lib.cpp
@@ -3,8 +3,10 @@
class PIC_TEST_EXPORT Dummy
{
- int dummy()
- {
-return 0;
- }
+ int dummy();
};
+
+int Dummy::dummy()
+{
+ return 0;
+}
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote: On 06/08/2012 04:50 AM, Stephen Kelly wrote: I still have an error with Watcom that I don't know how to fix, which I think is the last error on this topic: http://open.cdash.org/testDetails.php?test=148809149build=2343558 My guess was that the CMAKE_CXX_COMPILE_OPTIONS_DLL flag is not being populated or used, though I don't see why. Can someone with access to that box debug it a bit please? The compiler flags are fine. This compiler just doesn't export inline class members from DLLs, it appears. Use the patch below. Done, thanks! -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On Fri, Jun 8, 2012 at 8:28 AM, Stephen Kelly steve...@gmail.com wrote: Brad King wrote: On 06/08/2012 04:50 AM, Stephen Kelly wrote: I still have an error with Watcom that I don't know how to fix, which I think is the last error on this topic: http://open.cdash.org/testDetails.php?test=148809149build=2343558 My guess was that the CMAKE_CXX_COMPILE_OPTIONS_DLL flag is not being populated or used, though I don't see why. Can someone with access to that box debug it a bit please? The compiler flags are fine. This compiler just doesn't export inline class members from DLLs, it appears. Use the patch below. Done, thanks! -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers There's also this build warning: http://open.cdash.org/viewBuildError.php?type=1buildid=2342611 Please eradicate that, and then this topic will be ready for merging. Thx, David -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
David Cole wrote: There's also this build warning: http://open.cdash.org/viewBuildError.php?type=1buildid=2342611 Please eradicate that, and then this topic will be ready for merging. Any idea what causes it? I don't see why only that line would cause such a warning. It should be from every compilation or none... Any idea what the fix is? Thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 06/08/2012 10:08 AM, Stephen Kelly wrote:
David Cole wrote:
There's also this build warning:
http://open.cdash.org/viewBuildError.php?type=1buildid=2342611
Please eradicate that, and then this topic will be ready for merging.
Any idea what causes it?
I don't see why only that line would cause such a warning. It should be from
every compilation or none...
Any idea what the fix is?
That dashboard build uses bootstrap so CMake is built with itself.
Therefore the new build flags appear during CMake's own build which
includes the test module in KWSys. That is the only shared library
built as part of CMake, so only that shows the warning.
The problem is in
Modules/Platform/CYGWIN-GNU.cmake
Modules/Platform/Windows-GNU.cmake
which currently have
set(CMAKE_SHARED_LIBRARY_${lang}_FLAGS ) # No -fPIC on cygwin
and
set(CMAKE_SHARED_LIBRARY_${lang}_FLAGS ) # No -fPIC on Windows
to override the Modules/Compiler/GNU.cmake settings.
They should be updated to override the PIC/PIE compile
option variables too.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote: On 06/08/2012 10:08 AM, Stephen Kelly wrote: David Cole wrote: There's also this build warning: http://open.cdash.org/viewBuildError.php?type=1buildid=2342611 Please eradicate that, and then this topic will be ready for merging. Any idea what causes it? I don't see why only that line would cause such a warning. It should be from every compilation or none... Any idea what the fix is? That dashboard build uses bootstrap so CMake is built with itself. Therefore the new build flags appear during CMake's own build which includes the test module in KWSys. That is the only shared library built as part of CMake, so only that shows the warning. Fixed, thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 06/08/2012 11:16 AM, Stephen Kelly wrote: David Cole wrote: Please eradicate that, and then this topic will be ready for merging. It looks like the unit test is not being executed on the FarAway linux continuous build. That started yesterday, and as it is continuous (unclean?), I thought a clean build today would resolve it, but it is still not being executed today. Can we find out why that is? This change: http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f65b5083 means that the test won't run if CMAKE_CXX_COMPILER_VERSION is not set while CMake itself is configuring. The value will only be set if CMake is built by another CMake that is 2.8.8 or later. Currently our minimum required version is 2.8.2, so anything in between will not run this test. -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote: On 06/08/2012 11:16 AM, Stephen Kelly wrote: David Cole wrote: Please eradicate that, and then this topic will be ready for merging. It looks like the unit test is not being executed on the FarAway linux continuous build. That started yesterday, and as it is continuous (unclean?), I thought a clean build today would resolve it, but it is still not being executed today. Can we find out why that is? This change: http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f65b5083 means that the test won't run if CMAKE_CXX_COMPILER_VERSION is not set while CMake itself is configuring. The value will only be set if CMake is built by another CMake that is 2.8.8 or later. Currently our minimum required version is 2.8.2, so anything in between will not run this test. Makes sense. Fixed now (I hope), thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On Fri, Jun 8, 2012 at 12:52 PM, Brad King brad.k...@kitware.com wrote: On 06/08/2012 11:16 AM, Stephen Kelly wrote: David Cole wrote: Please eradicate that, and then this topic will be ready for merging. It looks like the unit test is not being executed on the FarAway linux continuous build. That started yesterday, and as it is continuous (unclean?), I thought a clean build today would resolve it, but it is still not being executed today. Can we find out why that is? This change: http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=f65b5083 means that the test won't run if CMAKE_CXX_COMPILER_VERSION is not set while CMake itself is configuring. The value will only be set if CMake is built by another CMake that is 2.8.8 or later. Currently our minimum required version is 2.8.2, so anything in between will not run this test. -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers And most of the dashboards are driven by pre-2.8.8, so this means that this change unintentionally/accidentally stopped it from running on a bunch of dashboards. You can see where it stopped running today where all the -1 values are in the Test/Pass column on the dashboard. Bleee. ;-) David -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Stephen Kelly wrote:
Brad King wrote:
On 06/05/2012 01:23 PM, Stephen Kelly wrote:
Brad King wrote:
You can't try_compile inside a platform file.
I'm not sure I'm trying to?
I thought you meant you would add the try_compile to the platform file
to decide whether to report -fPIE.
Do you mean in the tests, or do you mean the POSITION_INDEPENDENT_CODE
feature should be disabled for older GCC?
I mean that
set(CMAKE_${lang}_COMPILE_OPTIONS_PIE -fPIE)
should not be done if the compiler does not have -fPIE.
Therefore Modules/Compiler/GNU.cmake needs to test the
compiler version to decide whether -fPIE is available.
I see. Done for GNU.cmake now at least.
This seems to have caused todays failures:
http://open.cdash.org/buildSummary.php?buildid=2337242
That build has CMAKE_C_COMPILER_VERSION == 4.6.1:
http://open.cdash.org/testDetails.php?test=148901720build=2337242
but it seems that -fPIE is not added:
http://open.cdash.org/testDetails.php?test=148901721build=2337242
It doesn't make sense for me to attempt to debug that because I don't have
access to the box.
Can someone who has access to one of the boces that is failing like that
debug it please?
Things to check are:
* Does the test fail when you run it
* Does the test pass when you remove the change to GNU.cmake
* Does the test pass if you replace
if(NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 3.4)
with
if(NOT CMAKE_${lang}_COMPILER_VERSION VERSION_LESS 3.4)
(I've just patched the code to use that anyway, but please try both to
confirm if that is the reason for the failure without waiting for tomorrow)
* Does the try_compile source dir correctly contain
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
* Does the CMAKE_CXX_COMPILE_OPTIONS_PIE contain -fPIE when checked in the
unit test?
Thanks,
Steve.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On Wed, Jun 6, 2012 at 3:48 AM, Stephen Kelly steve...@gmail.com wrote:
This seems to have caused todays failures:
http://open.cdash.org/buildSummary.php?buildid=2337242
* Does the test pass if you replace
if(NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 3.4)
with
if(NOT CMAKE_${lang}_COMPILER_VERSION VERSION_LESS 3.4)
That fixes it. The test does check_cxx_source_compiles. Inside the
try_compile it will enable only CXX and not C so testing
CMAKE_C_COMPILER_VERSION does not make sense.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 06/05/2012 11:40 AM, Stephen Kelly wrote: http://open.cdash.org/testDetails.php?test=148807497build=2335525 I tried outputting the OUTPUT resulting from the try_compile, but that does not seem to actually create the output as expected, even though the releveant commit is being tested (http://cmake.org/gitweb?p=cmake.git;a=commit;h=84d29a5358e4fa01583e2aef1e8e8097e187045f ). Can someone with access to that box find out what is going on? CMakeError.log contains: cc1plus: error: unrecognized option `-fPIE' It is a very old OS X: $ sw_vers ProductName:Mac OS X ProductVersion: 10.3.9 BuildVersion: 7W98 $ g++ --version g++ (GCC) 3.3 20030304 (Apple Computer, Inc. build 1666) and so probably doesn't understand PIE. -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote: On 06/05/2012 11:54 AM, Stephen Kelly wrote: I can add a try_compile for -fPIE on APPLE (though I wonder if it would work), but still, I expect to see something like that in the OUTPUT variable from check_cxx_source_compiles... The OUTPUT variable is internal to the macro and not documented as holding anything when the call returns. A quick glance doesn't tell me why the value isn't leaking out as an implementation detail though. You can't try_compile inside a platform file. I'm not sure I'm trying to? It's too early in the init process because the platform files are needed to generate inside the try_compile. You would have to run your own execute_process and invoke the compiler directly. I suggest checking the compiler version rather than testing the flag. Just checking CMAKE_C_COMPILER_VERSION might be enough in this case. Do you mean in the tests, or do you mean the POSITION_INDEPENDENT_CODE feature should be disabled for older GCC? Thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 06/05/2012 01:23 PM, Stephen Kelly wrote:
Brad King wrote:
You can't try_compile inside a platform file.
I'm not sure I'm trying to?
I thought you meant you would add the try_compile to the platform file
to decide whether to report -fPIE.
Do you mean in the tests, or do you mean the POSITION_INDEPENDENT_CODE
feature should be disabled for older GCC?
I mean that
set(CMAKE_${lang}_COMPILE_OPTIONS_PIE -fPIE)
should not be done if the compiler does not have -fPIE.
Therefore Modules/Compiler/GNU.cmake needs to test the
compiler version to decide whether -fPIE is available.
Likely other toolchains will need similar checks.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote:
On 06/05/2012 01:23 PM, Stephen Kelly wrote:
Brad King wrote:
You can't try_compile inside a platform file.
I'm not sure I'm trying to?
I thought you meant you would add the try_compile to the platform file
to decide whether to report -fPIE.
Do you mean in the tests, or do you mean the POSITION_INDEPENDENT_CODE
feature should be disabled for older GCC?
I mean that
set(CMAKE_${lang}_COMPILE_OPTIONS_PIE -fPIE)
should not be done if the compiler does not have -fPIE.
Therefore Modules/Compiler/GNU.cmake needs to test the
compiler version to decide whether -fPIE is available.
I see. Done for GNU.cmake now at least.
Likely other toolchains will need similar checks.
We'll have to deal with those as they come I guess.
Thanks,
Steve.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Stephen Kelly wrote: Brad King wrote: The current implementation can be refactored in a way that should make Xcode easy. Then simply use the patch below for Xcode. Great, thanks, all done. I'll merge it into next next week when I can look at the dashboard. I've merged it now. Thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote:
On 05/30/2012 02:22 PM, Stephen Kelly wrote:
Brad King wrote:
Also the code near calls to GetShouldUseOldFlags is not
indented with our convention.
I've had a look, and I don't see the breaks from the convention. Can you
say what commits and hunks?
In the makefile generator:
I see cmGlobalUnixMakefileGenerator3::AddCXXCompileCommand gave me the wrong
idea I think.
and similarly in Ninja. The } else { construct is not consistent
with style in the rest of our code.
Both fixed.
I haven't tried to implement the XCode patch
I might be able to try making one at some point in the next few weeks,
but if someone else can do that, it would be great.
Thanks for your patience with so many rounds of review.
No problem. Thank you too.
I think the topic is in good shape other than for Xcode.
I'll try to look at Xcode when I get a chance.
Sounds great.
Another change in my newest push is that I've moved the storage of the
shared library flags from inside the
} // end if in try compile
to inside the
} // end for each language
I noticed when running the unit tests that I was getting the policy warning
otherwise because the flags were not actually being stored.
Thanks,
Steve.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 05/31/2012 05:30 AM, Stephen Kelly wrote:
I see cmGlobalUnixMakefileGenerator3::AddCXXCompileCommand gave me the wrong
idea I think.
I never noticed that one, thanks.
and similarly in Ninja. The } else { construct is not consistent
with style in the rest of our code.
Both fixed.
See the patch below for what I meant.
Another change in my newest push is that I've moved the storage of the
shared library flags from inside the
} // end if in try compile
to inside the
} // end for each language
I noticed when running the unit tests that I was getting the policy warning
otherwise because the flags were not actually being stored.
Good catch.
Thanks,
-Brad
diff --git a/Source/cmMakefileTargetGenerator.cxx
b/Source/cmMakefileTargetGenerator.cxx
index a071a2a..451283b 100644
--- a/Source/cmMakefileTargetGenerator.cxx
+++ b/Source/cmMakefileTargetGenerator.cxx
@@ -265,7 +265,9 @@ std::string cmMakefileTargetGenerator::GetFlags(const
std::string l)
if (this-LocalGenerator-GetShouldUseOldFlags(shared, l))
{
this-LocalGenerator-AddSharedFlags(flags, lang, shared);
-} else {
+ }
+else
+ {
// Add position independendent flags, if needed.
if (this-Target-GetPropertyAsBool(POSITION_INDEPENDENT_CODE))
{
diff --git a/Source/cmNinjaTargetGenerator.cxx
b/Source/cmNinjaTargetGenerator.cxx
index edd17f7..2191422 100644
--- a/Source/cmNinjaTargetGenerator.cxx
+++ b/Source/cmNinjaTargetGenerator.cxx
@@ -149,7 +149,9 @@ cmNinjaTargetGenerator::ComputeFlagsForObject(cmSourceFile
*source,
if (this-LocalGenerator-GetShouldUseOldFlags(shared, language))
{
this-LocalGenerator-AddSharedFlags(flags, language.c_str(), shared);
- } else {
+}
+ else
+{
if (this-Target-GetPropertyAsBool(POSITION_INDEPENDENT_CODE))
{
// Add position independendent flags, if needed.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 05/30/2012 02:45 PM, Brad King wrote:
Thanks for your patience with so many rounds of review.
I think the topic is in good shape other than for Xcode.
I'll try to look at Xcode when I get a chance.
The current implementation can be refactored in a way that
should make Xcode easy. Instead of hard-coding duplicate
code in the Makefile and Ninja generators to call the
cmLocalGenerator::GetShouldUseOldFlags
cmLocalGenerator::AddSharedFlags
cmLocalGenerator::AddPositionIndependentFlags
methods factor that logic out into a new
cmLocalGenerator::AddCMP0018Flags(
std::string flags,
cmTarget* target,
std::string const language);
method. Then the above-three methods can be made *private*
inside cmLocalGenerator. Please squash this approach back
into the current commits in the topic.
Then simply use the patch below for Xcode.
Thanks,
-Brad
diff --git a/Source/cmGlobalXCodeGenerator.cxx
b/Source/cmGlobalXCodeGenerator.cxx
index 522f3da..5a5ce01 100644
--- a/Source/cmGlobalXCodeGenerator.cxx
+++ b/Source/cmGlobalXCodeGenerator.cxx
@@ -1594,14 +1594,14 @@ void
cmGlobalXCodeGenerator::CreateBuildSettings(cmTarget target,
if(strcmp(lang, CXX) == 0)
{
this-CurrentLocalGenerator-AddLanguageFlags(cflags, C, configName);
- this-CurrentLocalGenerator-AddSharedFlags(cflags, lang, shared);
+ this-CurrentLocalGenerator-AddCMP0018Flags(flags, target, C);
}
// Add language-specific flags.
this-CurrentLocalGenerator-AddLanguageFlags(flags, lang, configName);
// Add shared-library flags if needed.
-this-CurrentLocalGenerator-AddSharedFlags(flags, lang, shared);
+this-CurrentLocalGenerator-AddCMP0018Flags(flags, target, lang);
}
else if(binary)
{
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote: The current implementation can be refactored in a way that should make Xcode easy. Then simply use the patch below for Xcode. Great, thanks, all done. I'll merge it into next next week when I can look at the dashboard. Thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote: On 05/29/2012 12:16 PM, Stephen Kelly wrote: Done, though I left some notes in the commit for now. It seems the existing variable is used for more than just PIC equivalents. Thanks for going through all the platforms! I think the only non-PIC instances are for Watcom and Embarcadero where options -bd and -tD, respectively, tell the compiler that the object file is intended for inclusion in a DLL, and for SCO where -belf is required for shared libraries. Did you notice any others? RISCos.cmake and ULTRIX.cmake contains '-G 0' and HP-UX-HP.cmake contains +Z. I don't know what they are really for. I've also pushed again. Thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On Wed, May 30, 2012 at 6:20 AM, Stephen Kelly steve...@gmail.com wrote: RISCos.cmake and ULTRIX.cmake contains '-G 0' and HP-UX-HP.cmake contains +Z. I don't know what they are really for. HP's +Z option is for PIC: http://h21007.www2.hp.com/portal/download/files/unprot/aCxx/Online_Help/libs.htm#shlibcomp so this can be subsumed into CMAKE_C_COMPILE_OPTIONS_PIC. The RISCos and ULTRIX -G n option sets the small data/bss cut-off for MIPS Gp-Relative Addressing: http://gcc.gnu.org/onlinedocs/gcc/MIPS-Options.html#index-G-1738 I'm not familiar enough with that platform to know why -G 0 is needed for shared libraries. I don't know if anyone actually uses CMake on that platform though so we could just drop the flag when the policy is set to NEW and wait for a contributor to address it. In updating the documentation of the policy I think you misread my unspecified means to select phrase. I meant a method that is not specified rather than if the policy is not specified. See below for new wording. Commit 098cb871 adds trailing whitespace in cmLocalGenerator.h. In Windows-Embarcadero and Windows-wcl386 you misspelled the new variable. Use s/SHARED_LIBRARY/COMPILE_OPTIONS/ to fix it. My comment about Fortran in CMakeCXXInformation meant that you should update CMakeFortranInformation with the equivalent change that you already made in CMakeCXXInformation. The new AddSharedDllFlags can be replaced by a call to the AppendFeatureOptions method with DLL as the last argument. Also the call to it in the cmMakefileTargetGenerator does not compile because language is not a variable in scope. Thanks, -Brad CMake 2.8.8 and lower compiled sources in SHARED and MODULE libraries using the value of the undocumented CMAKE_SHARED_LIBRARY_Lang_FLAGS platform variable. The variable contained platform-specific flags needed to compile objects for shared libraries. Typically it included a flag such as -fPIC for position independent code but also included other flags needed on certain platforms. CMake 2.8.9 and higher prefer instead to use the POSITION_INDEPENDENT_CODE target property to determine what targets should be position independent, and new undocumented platform variables to select flags while ignoring CMAKE_SHARED_LIBRARY_Lang_FLAGS completely. The default for either approach... -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
What a mess. Sorry about that. Updated now. Thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 05/30/2012 12:26 PM, Stephen Kelly wrote:
What a mess. Sorry about that.
Updated now.
Great, that looks pretty good. A couple more comments:
The KWStyle test fails due to a few lines too long. Use:
$ git log master.. -p --pickaxe-regex -S.{80} -- Source
to look for hunks that add them. Some day I'll put that
in the local pre-commit check.
Also the code near calls to GetShouldUseOldFlags is not
indented with our convention.
The commit that adds CMAKE_POSITION_INDEPENDENT_CODE
for initialization should add it to cmDocumentVariables.
Why is the test added as a Module. test?
Remaining tasks include:
(1) The Xcode generator calls AddSharedFlags. I do not think
the VS IDE generators ever used the old variable at all. FYI,
it seems that calls to AddSharedFlags have been slowly
disappearing over the years for various reasons, such as here:
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=65b6a8f5
Your CMP0018, once implemented for Xcode, will remove the
last of the calls to AddSharedFlags when set to NEW :)
(2) CMAKE_SHARED_LIBRARY_${lang}_FLAGS is still used in
cmLocalGenerator::GetTargetFlags for compiling executable
sources. I don't think that's actually correct though.
It was added here:
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=7cef36c6#patch3
but that code path was never really used until the Ninja
generator and cmake --find-package started using
GetTargetFlags. I think we can just drop this use of the
variable.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote: Also the code near calls to GetShouldUseOldFlags is not indented with our convention. I've had a look, and I don't see the breaks from the convention. Can you say what commits and hunks? I haven't tried to implement the XCode patch, but I removed the use from the cmLocalGenerator codepath. I might be able to try making one at some point in the next few weeks, but if someone else can do that, it would be great. Thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 05/30/2012 02:22 PM, Stephen Kelly wrote:
Brad King wrote:
Also the code near calls to GetShouldUseOldFlags is not
indented with our convention.
I've had a look, and I don't see the breaks from the convention. Can you say
what commits and hunks?
In the makefile generator:
+if (this-LocalGenerator-GetShouldUseOldFlags(shared, l))
+ {
+ this-LocalGenerator-AddSharedFlags(flags, lang, shared);
+ } else {
+ // Add position independendent flags, if needed.
+ if (this-Target-GetPropertyAsBool(POSITION_INDEPENDENT_CODE))
+{
+this-LocalGenerator-AddPositionIndependentFlags(flags,
+ lang,
+ targetType);
+}
+ if (shared)
+{
+this-LocalGenerator-AppendFeatureOptions(flags, lang, DLL);
+}
+ }
and similarly in Ninja. The } else { construct is not consistent
with style in the rest of our code.
I haven't tried to implement the XCode patch
I might be able to try making one at some point in the next few weeks, but
if someone else can do that, it would be great.
Thanks for your patience with so many rounds of review.
I think the topic is in good shape other than for Xcode.
I'll try to look at Xcode when I get a chance.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote:
On 05/25/2012 04:47 PM, Stephen Kelly wrote:
I'm also not completely certain about the change to the try_compile
behavior in the branch. Could there be any reason to use
set(CMAKE_POSITION_INDEPENDENT_CODE ON) but want to do a try_compile
without -fPI{C,E} ? My guess is no, because one would do the try_compile
before conditionally setting that property, I guess.
Since project's don't currently set CMAKE_POSITION_INDEPENDENT_CODE
there is no compatibility problem. I think it is okay to honor that
for try_compile. It is more representative of how the sources will
actually be compiled in the calling project.
The current topic looks pretty good so far. There is some more work
to be done. Please see the block at the bottom of this message for
the documentation I recommend using for CMP0018.
Done, in a new force push.
Look at the logic in cmLocalGenerator::AppendFeatureOptions for
handling of _COMPILE_OPTIONS_ values. We need to use
ExpandListArgument to ensure that options are separated as
CMake-style lists instead of whitespace. That is the convention
I'm establishing for COMPILE_OPTIONS platform variables. It will
not make a difference for single-argument options like -fPIC but
could if some platforms have multi-argument options.
Before merging we will need to add the equivalent to the hunk
+ set(CMAKE_${lang}_COMPILE_OPTIONS_PIC -fPIC)
+ set(CMAKE_${lang}_COMPILE_OPTIONS_PIE -fPIE)
on all necessary platforms. The policy can't be per-platform.
You already have a TODO comment for this.
Done, though I left some notes in the commit for now. It seems the existing
variable is used for more than just PIC equivalents.
Thanks,
Steve.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 05/29/2012 12:16 PM, Stephen Kelly wrote:
Done, though I left some notes in the commit for now. It seems the existing
variable is used for more than just PIC equivalents.
Thanks for going through all the platforms!
I think the only non-PIC instances are for Watcom and Embarcadero
where options -bd and -tD, respectively, tell the compiler that
the object file is intended for inclusion in a DLL, and for SCO
where -belf is required for shared libraries. Did you notice any
others?
The solution is to generalize the policy further. It's short
description Ignore CMAKE_SHARED_LIBRARY_Lang_FLAGS variable. is
already correct. The OLD behavior should continue to be the same,
using the modified variable value and ignoring all new variables.
The NEW behavior must be generalized to use multiple new platform
variables, one for each purpose originally served by the old
variable. So far we've identified:
- CMAKE_${lang}_COMPILE_OPTIONS_PI{C,E}, controlled by the
POSITION_INDEPENDENT_CODE target property. You already did
this one.
- CMAKE_${lang}_COMPILE_OPTIONS_DLL, used for any object intended
for inclusion in a dynamic library e.g. -bd, -tD, or -belf.
This is new to our design. For now we can just hard-code its
use on SHARED and MODULE libraries but some day we could add a
property for it so it can be set in an OBJECT library whose
objects will be used in a shared library.
- Any others you noticed?
Please also try updating the CMP0018 documentation accordingly.
Other comments on the topic: Use of ExpandListArgument and
EscapeForShell looks good, thanks. I think it may be simpler
to use GetSafeDefinition for the _COMPILE_OPTIONS_ variable
lookups so they can simply be left unset on platforms that do
not need them. Then we can remove CMAKE_SHARED_LIBRARY_C_FLAGS
from platforms where it is empty anyway.
Your change to CMakeCXXInformation will be needed in Fortran too.
Thanks,
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 05/25/2012 04:47 PM, Stephen Kelly wrote:
I'm also not completely certain about the change to the try_compile behavior
in the branch. Could there be any reason to use
set(CMAKE_POSITION_INDEPENDENT_CODE ON) but want to do a try_compile without
-fPI{C,E} ? My guess is no, because one would do the try_compile before
conditionally setting that property, I guess.
Since project's don't currently set CMAKE_POSITION_INDEPENDENT_CODE
there is no compatibility problem. I think it is okay to honor that
for try_compile. It is more representative of how the sources will
actually be compiled in the calling project.
The current topic looks pretty good so far. There is some more work
to be done. Please see the block at the bottom of this message for
the documentation I recommend using for CMP0018.
Look at the logic in cmLocalGenerator::AppendFeatureOptions for
handling of _COMPILE_OPTIONS_ values. We need to use
ExpandListArgument to ensure that options are separated as
CMake-style lists instead of whitespace. That is the convention
I'm establishing for COMPILE_OPTIONS platform variables. It will
not make a difference for single-argument options like -fPIC but
could if some platforms have multi-argument options.
Before merging we will need to add the equivalent to the hunk
+ set(CMAKE_${lang}_COMPILE_OPTIONS_PIC -fPIC)
+ set(CMAKE_${lang}_COMPILE_OPTIONS_PIE -fPIE)
on all necessary platforms. The policy can't be per-platform.
You already have a TODO comment for this.
Thanks,
-Brad
Ignore CMAKE_SHARED_LIBRARY_Lang_FLAGS variable.
CMake 2.8.8 and lower compiled sources in SHARED and MODULE libraries
using the value of the undocumented CMAKE_SHARED_LIBRARY_Lang_FLAGS
platform variable that contained flag to request position independent
code, such as -fPIC. CMake 2.8.9 and higher prefer instead to use the
POSITION_INDEPENDENT_CODE target property to determine what targets
should be position independent, and unspecified means to select the
flags while ignoring CMAKE_SHARED_LIBRARY_Lang_FLAGS.
The default for either approach produces identical compilation flags,
but if a project modifies CMAKE_SHARED_LIBRARY_Lang_FLAGS from its
original value this policy determines which approach to use. The OLD
behavior for this policy is to ignore the POSITION_INDEPENDENT_CODE
property for all targets and use the modified value of
CMAKE_SHARED_LIBRARY_Lang_FLAGS for SHARED and MODULE libraries.
The NEW behavior for this policy is to ignore
CMAKE_SHARED_LIBRARY_Lang_FLAGS whether it is modified or not and
honor the POSITION_INDEPENDENT_CODE target property.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Hi,
I've re-pushed to my gitorious clone.
Brad King wrote:
On 05/17/2012 08:36 AM, Stephen Kelly wrote:
Done and re-pushed to my clone. I still have to write the unit tests, but
I think it can be reviewed again at this point.
Great start. Here are some comments.
The Update the documentation of IMPORTED_LOCATION to match the code.
commit
has an incorrect message. You're modifying the
MAP_IMPORTED_CONFIG_CONFIG
docs. Also, the original wording the commit changes is correct under the
assumption that the property is set, and if the property is not set then
why
should its docs apply? Anyway, I think it can be made even more clear
than your proposed wording by using
If this property is set and no matching configurations...
Merged to next as a separate branch.
-
The LanguageToCachedSharedLibFlags ivar would be better named
LanguageToOriginalSharedLibFlags IMO. Also please add a comment
where they are set to explain why (just ref the policy).
Done.
The proposed CMP0018 summary should prefer POSITION_INDEPENDENT_CODE
instead of COMPILE_OPTIONS, no? The property is part of the public
interface the user can set/get. The COMPILE_OPTIONS values are impl
details. Also the documentation appears to be cut-n-pasted from another
policy. Please fill in the details.
Done. Looks like I missed step 3 or 'copy/paste/modify' :)
The docs of POSITION_INDEPENDENT_CODE are incorrect. The last sentence
should be This property is true by default for SHARED and MODULE library
targets and false otherwise..
Done.
Your logic should use GetSafeDefinition instead of GetDefinition before
storing the result in a std::string because the latter returns NULL if
not set.
Given that the variable being replaced is fetched as a RequiredDefinition, I
used that for now. I guess I can change it, but I think it makes sense as a
RequiredDefinition.
AddPositionIndependentFlags shouldn't care whether the library is
shared or not, right?
My vague understanding up to now has been that static libraries should not
get the flag:
http://www.gnu.org/software/libtool/manual/html_node/Static-libraries.html
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28811
http://stackoverflow.com/questions/1589977/linking-a-shared-library-against-
a-static-library-must-the-static-library-be-co
I'm not certain though...
If the property is true then we want the flags.
Perhaps:
std::string picFlags;
if (targetType == cmTarget::EXECUTABLE)
{
std::string flagsVar = CMAKE_;
flagsVar += lang;
flagsVar += _COMPILE_OPTIONS_PIE;
picFlags = this-Makefile-GetSafeDefinition(flagsVar.c_str());
}
if(picFlags.empty())
{
std::string flagsVar = CMAKE_;
flagsVar += lang;
flagsVar += _COMPILE_OPTIONS_PIC;
picFlags = this-Makefile-GetSafeDefinition(flagsVar.c_str());
}
if(!picFlags.empty())
{
this-AppendFlags(flags, picFlags.c_str());
}
Done, though I don't know if static libraries should be excluded here.
There is a lot more to adding a policy than just
GetPolicyStatus(cmPolicies::CMP0018) == cmPolicies::OLD
Look at some of the other policies. You need to handle all the possible
values. If it is WARN then you need to produce the warning and treat as
OLD behavior. See the switch() statements used for others.
Done.
I'm also not completely certain about the change to the try_compile behavior
in the branch. Could there be any reason to use
set(CMAKE_POSITION_INDEPENDENT_CODE ON) but want to do a try_compile without
-fPI{C,E} ? My guess is no, because one would do the try_compile before
conditionally setting that property, I guess.
Thanks,
Steve.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 05/25/2012 04:47 PM, Stephen Kelly wrote:
I've re-pushed to my gitorious clone.
Thanks. I'll review when I get a chance.
AddPositionIndependentFlags shouldn't care whether the library is
shared or not, right?
My vague understanding up to now has been that static libraries should not
get the flag
It should be up to the project. I might create some static libs
as dependencies of a shared lib. When linking the shared lib it
will bring in the objects from the static libs so they should have
been built with -fPIC. If I set POSITION_INDEPENDENT_CODE on a
target I expect to get -fPI{C,E} no matter the target type.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Brad King wrote:
On 05/25/2012 04:47 PM, Stephen Kelly wrote:
I've re-pushed to my gitorious clone.
Thanks. I'll review when I get a chance.
Something else that occured to me was that there's no way currently in the
branch to use both POSITION_INDEPENDENT_CODE and set FLAGS for all
SHARED_LIBRARYs and set the policy to NEW.
Should a CMAKE_${TARGET_TYPE}_${LANG}_COMPILE_OPTIONS be added in this
branch too?
AddPositionIndependentFlags shouldn't care whether the library is
shared or not, right?
My vague understanding up to now has been that static libraries should
not get the flag
It should be up to the project. I might create some static libs
as dependencies of a shared lib. When linking the shared lib it
will bring in the objects from the static libs so they should have
been built with -fPIC. If I set POSITION_INDEPENDENT_CODE on a
target I expect to get -fPI{C,E} no matter the target type.
Right.
Thanks,
Steve.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 05/17/2012 08:36 AM, Stephen Kelly wrote:
Done and re-pushed to my clone. I still have to write the unit tests, but I
think it can be reviewed again at this point.
Great start. Here are some comments.
The Update the documentation of IMPORTED_LOCATION to match the code. commit
has an incorrect message. You're modifying the MAP_IMPORTED_CONFIG_CONFIG
docs. Also, the original wording the commit changes is correct under the
assumption that the property is set, and if the property is not set then why
should its docs apply? Anyway, I think it can be made even more clear than
your proposed wording by using
If this property is set and no matching configurations...
-
The LanguageToCachedSharedLibFlags ivar would be better named
LanguageToOriginalSharedLibFlags IMO. Also please add a comment
where they are set to explain why (just ref the policy).
The proposed CMP0018 summary should prefer POSITION_INDEPENDENT_CODE
instead of COMPILE_OPTIONS, no? The property is part of the public
interface the user can set/get. The COMPILE_OPTIONS values are impl
details. Also the documentation appears to be cut-n-pasted from another
policy. Please fill in the details.
The docs of POSITION_INDEPENDENT_CODE are incorrect. The last sentence
should be This property is true by default for SHARED and MODULE library
targets and false otherwise..
Your logic should use GetSafeDefinition instead of GetDefinition before
storing the result in a std::string because the latter returns NULL if
not set.
AddPositionIndependentFlags shouldn't care whether the library is
shared or not, right? If the property is true then we want the flags.
Perhaps:
std::string picFlags;
if (targetType == cmTarget::EXECUTABLE)
{
std::string flagsVar = CMAKE_;
flagsVar += lang;
flagsVar += _COMPILE_OPTIONS_PIE;
picFlags = this-Makefile-GetSafeDefinition(flagsVar.c_str());
}
if(picFlags.empty())
{
std::string flagsVar = CMAKE_;
flagsVar += lang;
flagsVar += _COMPILE_OPTIONS_PIC;
picFlags = this-Makefile-GetSafeDefinition(flagsVar.c_str());
}
if(!picFlags.empty())
{
this-AppendFlags(flags, picFlags.c_str());
}
There is a lot more to adding a policy than just
GetPolicyStatus(cmPolicies::CMP0018) == cmPolicies::OLD
Look at some of the other policies. You need to handle all the possible
values. If it is WARN then you need to produce the warning and treat as
OLD behavior. See the switch() statements used for others.
Thanks,
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 05/13/2012 04:24 PM, Stephen Kelly wrote:
I've added a 'WIP: Experiment with backwards compatibility.' patch to my
gitorious clone.
+ // TODO: Is there a 'not set' state for properties?
+ // We should handle that differently to boolean False.
There is a not set state for properties. Use the raw GetProperty
method. If it returns a NULL pointer then the property is not set.
I think it should be possible to do this by either setting
a default in the CMake files, or handling the setting of properties of names
matching CMAKE_SHARED_LIBRARY_${lang}_FLAGS specially to also store a
default on first call (which will come from the CMake internal files, I
guess?)
That gives me an idea on how to add a policy for this without triggering
on every shared library. Trigger it on projects that change the flags.
Since POSITION_INDEPENDENT_CODE's default behavior is the same as the
current default CMAKE_SHARED_LIBRARY_${lang}_FLAGS there is no problem
for projects that do not change the flags.
Start by implementing POSITION_INDEPENDENT_CODE and initialize it to
true for shared libraries just as in your original proposal. There is
no need for special not set behavior as in my previous suggestion.
After each language is enabled (cmGlobalGenerator::EnableLanguage)
save the initial CMAKE_SHARED_LIBRARY_${lang}_FLAGS in a C++ structure.
Then in the generator before calling AddSharedFlags, compare the value
of the variable to the saved value to see if it changed. If not, then
just ignore the value and use the POSITION_INDEPENDENT_CODE as normal
(make sure CMAKE_SHARED_LIBRARY_${lang}_FLAGS expands to empty).
If the project did change the value then trigger the policy. The OLD
behavior is to use the flags and ignore POSITION_INDEPENDENT_CODE. The
NEW behavior is to use POSITION_INDEPENDENT_CODE and ignore the flags.
I remembered that a while back I started a convention for feature-flag
mapping. Instead of variable names like CMAKE_..._${lang}_FLAGS
containing space-separated values, we can use names like
CMAKE_${lang}_COMPILE_OPTIONS_... containing ;-seprated values. The
name and value form is consistent with plans I mention here:
http://www.cmake.org/Bug/view.php?id=6493#c21559
for a future alternative to the COMPILE_FLAGS property. The name
CMAKE_${lang}_COMPILE_OPTIONS_IPO is the first one, for the
INTERPROCEDURAL_OPTIMIZATION property. For the
POSITION_INDEPENDENT_CODE property we could use:
set(CMAKE_${lang}_COMPILE_OPTIONS_PIC -fPIC)
set(CMAKE_${lang}_COMPILE_OPTIONS_PIE -fPIE)
For executables CMake can first look for PIE and then for PIC.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 5/5/2012 4:27 PM, Stephen Kelly wrote:
Ok. Then factoring out how it is set is the first step. Looking at the
output of 'git grep -i \?pic\b' and 'git grep -wi +Z', there are many
different ways of telling the compiler that we want this property. Most of
them are ancient unix which I can't test. Should I change those at all?
This issue is the subject of a TODO comment in my wip branch on this topic.
They all set the CMAKE_SHARED_LIBRARY_${lang}_FLAGS one way or
another. AFAIK CMake platform files use that variable exclusively
for -fPIC or equivalent flag. It is possible that projects set
the value to add their own flags though. I also know there are
projects that *read* the variable to get the flag so it can be
used in a static library (working around the lack of the feature
you're adding).
I think the most compatible way to do this is:
(1) Define a new platform variable named specifically to hold this
flag, like CMAKE_${lang}_PIC_FLAG or something.
(2) Document the property as using that flag *and* suppressing
use of CMAKE_SHARED_LIBRARY_${lang}_FLAGS altogether.
(3) Leave the property undefined even for shared libs and let
the project set the property to get new behavior.
I'd love to add a policy to get new behavior by default in a
future version of CMake. However, I don't know if we can add it
without creating a warning on every shared library in every project.
Perhaps instead it could switch off the minimum required version.
That's pretty subtle though.
https://gitorious.org/~steveire/cmake/steveires-cmake/commits/position-independent-targets
Good start. I'd like to amend my suggested property name to be
POSITION_INDEPENDENT_CODE.
How should this interact with OBJECT library support? If I compile
a set of objects meant for use in an executable shouldn't they use
-fPIE instead of -fPIC? CMake cannot know where $TARGET_OBJECTS:...
will be used when it compiles an object library.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
Stephen Kelly wrote:
Brad King wrote:
On 5/3/2012 12:02 PM, Stephen Kelly wrote:
* Make set(CMAKE_POSITION_INDEPENDENT_BINARIES True) set the appropriate
flags.
This is the right choice IMO, though the variable should just
initialize a POSITION_INDEPENDENT target property. The target
property would then map to the right flag. You'll need to factor
out and generalize the CMAKE_SHARED_LIBRARY_${lang}_FLAGS platform
information variable:
http://cmake.org/gitweb?p=cmake.git;a=blob;f=Modules/Compiler/GNU.cmake;hb=v2.8.8#l24
currently used to add -fPIC to compilation of objects in shared
libraries. Make its use based on the new property, and simply
make the property true by default for shared libraries.
-Brad
Ok. Then factoring out how it is set is the first step. Looking at the
output of 'git grep -i \?pic\b' and 'git grep -wi +Z', there are many
different ways of telling the compiler that we want this property. Most of
them are ancient unix which I can't test. Should I change those at all?
This issue is the subject of a TODO comment in my wip branch on this topic.
https://gitorious.org/~steveire/cmake/steveires-cmake/commits/position-
independent-targets
I'd appreciate feedback on the approach at this point. I will also add tests
before pushing to next.
Thanks,
Steve.
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] Adding logic to CMake for -fPIE and -fPIC
On 5/3/2012 12:02 PM, Stephen Kelly wrote:
* Make set(CMAKE_POSITION_INDEPENDENT_BINARIES True) set the appropriate flags.
This is the right choice IMO, though the variable should just
initialize a POSITION_INDEPENDENT target property. The target
property would then map to the right flag. You'll need to factor
out and generalize the CMAKE_SHARED_LIBRARY_${lang}_FLAGS platform
information variable:
http://cmake.org/gitweb?p=cmake.git;a=blob;f=Modules/Compiler/GNU.cmake;hb=v2.8.8#l24
currently used to add -fPIC to compilation of objects in shared
libraries. Make its use based on the new property, and simply
make the property true by default for shared libraries.
-Brad
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
