On 07/03/2011 12:23 PM, Alexander Neundorf wrote:
> On Monday 20 June 2011, Brad King wrote:
>> On 06/17/2011 05:09 PM, Alexander Neundorf wrote:
>>> I improved it somewhat, so IMO it is basically working now.
>>> There is now a branch UsingCMakeLikePkgConfig on stage.
>>> Would be nice if you could have a look.
>>
>> The patch series is hard to follow because it adds and removes
>> some code/comments and adds a file too late. Please edit the
>> topic with "git rebase -i" to clean it up and push again.
>
> There is now a new branch UsingCMakeLikePkgConfig2 in the stage.
> It should have somewhat nicer commits.
Much easier, thanks.
> This is not yet ready to be merged, but it would be nice if it could get a
> review :-)
In the first commit "Make clLocalGenerator::GetTargetFlags() public"
please fix the typo "s/cl/cm/".
In the second commit "Add find-package mode, which does nothing yet"
I think we should use an enumeration for the mode rather than separate
ScriptMode and FindPackageMode booleans. It does not make sense to
have both at once, so both flags can be replaced by a single enumeration
value.
On the fourth commit, "Install a cmake.m4" perhaps it is worth posting
it to a mailing list in the KDE community on which members that requested
this functionality participate. Perhaps they can offer the help that
you request in the commit message. This can wait until the rest of the
series has matured though.
In the main commit, "Implement find-package mode of cmake", there are
some technical challenges remaining:
(1) The code
# this is ugly, and not enough for the multilib-stuff I guess
if(UNIX AND EXISTS /usr/lib64)
set(CMAKE_SIZEOF_VOID_P 8)
endif()
hints at this one. Some FindXXX modules or XXXConfig.cmake files may
switch behavior off the target compiler and architecture. Without
having a real enable_language() the files may not present accurate
information. I don't think a full enable_language can be made to work
in this mode, at least not efficiently. This problem may be a matter
of passing more arguments into the mode to tell it things like the
address size as necessary.
(2) In the case that the to-be-found package really does provide a
CMake package configuration file (FooConfig.cmake) then it may provide
the libraries as imported targets. The Foo_LIBRARIES variable will
contain only the logical names and not any real path information.
You'll have to read one of the LOCATION_<CONFIG> properties from
such imported targets to get an actual file path.
(3) This hunk:
+ // free generic one if generated
+// this->SetGlobalGenerator(0); // setting 0-pointer is not possible
+// delete gg; // this crashes inside the cmake instance
will require further investigation and/or refactoring of existing
code to address. Perhaps the changes can be included in the second
patch or an additional patch in the series.
(4) Tests should be added in another patch after this. They can wait
until the rest has matured though so we know what to test ;)
Thanks,
-Brad
_______________________________________________
cmake-developers mailing list
cmake-developers@cmake.org
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
