|
Notes |
|
|
(0004346)
|
|
Brad King
|
|
2006-06-26 11:38
|
|
I see comments in the files about working around problems with CMake versions before 2.4. Since the files will be included in 2.4 and higher please remove the work-arounds. You may also want to look at all the fancy find options available in the 2.4 FIND_* commands.
|
|
|
|
(0004347)
|
|
Brad King
|
|
2006-06-26 11:39
|
|
|
I'm assigning this bug to myself. |
|
|
|
(0004389)
|
|
Jan Woetzel
|
|
2006-06-30 08:49
|
|
|
working on the issues... Jan. |
|
|
|
(0004396)
|
|
Jan Woetzel
|
|
2006-07-03 19:38
|
|
-Requested CMake backward comp. issues solved
-Incdirs "-I" is replaced by "-isystem" on Linux now (like done with "|sed" in FindwxWindows.cmake
Q:
Which fancy options of FIND_* do you mean?
(NO_DEFAULT_PATH, PATH_SUFFIXES, ...?)
We don't use PATH_SUFFIXES with intention to avoid mixing different build trees and configurations and trees.
TODO/sidenote:
-CMake source itself needs adaptions when really replacing FindwxWidgets by teh atatched files because of variable name change WXWINDOWS_* --> WXWIDGETS_*.
In particular
-the main CMake WX test
-Source/WXDialog and needs some (minor) work.
Best,
Jan.
|
|
|
|
(0004400)
|
|
Jan Woetzel
|
|
2006-07-04 12:05
|
|
I managed to fix teh wxWIndows -< wxWidgets changes inside teh CMake tree.
WXDialog compiles again.
Shall I send you a ZIP with patch files?
Jan. |
|
|
|
(0004403)
|
|
Jan Woetzel
|
|
2006-07-04 15:09
|
|
This bug is related to bug report 0003482
Sorry for splitting.
All required changes are inside the patch.
Jan.
|
|
|
|
(0004415)
|
|
Brad King
|
|
2006-07-07 17:08
|
|
I've tried the patch and it looks good so far. Here are more comments.
The case between the name of the FindwxWidgets.cmake file and the names of the variables it sets should be consistent:
FIND_PACKAGE(wxWidgets)
IF(wxWidgets_FOUND)
....
ENDIF(wxWidgets_FOUND)
The basic interface to use wxWidgets should be just
FIND_PACKAGE(wxWidgets REQUIRED)
not
INCLUDE (${CMAKE_ROOT}/Modules/UsewxWidgets.cmake )
Any options like WXWIDGETS_USE_MONOLITHIC should be part of the find module. The "Use" part of the code in UsewxWidgets.cmake can stay, so that the user might do something like
FIND_PACKAGE(wxWidgets REQUIRED)
INCLUDE(${wxWidgets_USE_FILE})
where wxWidgets_USE_FILE is set by the find script to the locaiton of the use file.
|
|
|
|
(0004417)
|
|
Jan Woetzel
|
|
2006-07-07 19:00
|
|
(1)
Q: uppercase/case sensitive ?
I think prefix of all flags should be uppercase.
Because
-all other FindXxx.cmake Modules seem to set uppercase XXX_FOUND
-there should be consistency between
XXX_FOUND, XXX_LIBRARIES in the case to avoid mixing of XXX_FOO and Xxx_BAR
Thus
FIND_PACKAGE(wxWidgets)
IF(WXIDGETS_FOUND)
is correct, I think
Or am I missing something?
(2) I agree wth you Find/Use/Options suggestions.
I'm working on FIND_PACKAGE(... REQUIRED components) to handle teh Options.
E.g.
FIND_PACKAGE(wxWidgets REQUIRED core base gl net ...)
INCLUDE(${WXWIDGTES_USE_FILE}) # uppercase
Working on it.
Jan.
|
|
|
|
(0004418)
|
|
Jan Woetzel
|
|
2006-07-08 10:46
|
|
This topic is related to bug 0003494 because I want o use the <Foo>_FIND_COMPONENTS feature for final usage as
FIND_PACKAGE(wxWidgets REQUIRED base net gl xml...<any comp handled by foreach>)
INCLUDE(${WXWIDGETS_USE_FILE}
and
# no components listedm, thus use "defautl" set liek std
FIND_PACKAGE(wxWidgets)
INCLUDE(${WXWIDGETS_USE_FILE}
Jan.
|
|
|
|
(0004445)
|
|
Brad King
|
|
2006-07-11 17:18
|
|
Now that bug 3494 is fixed you should be able to address issue 2.
For issue 1 (case), I've been trying to change the convention away from all-uppercase and instead make ALL the XXX_* variables match the case of the file name FindXXX.cmake. Therefore the vars should be wxWidgets_FOUND, wxWidgets_LIBS, etc. The reason is that when packages are converted to CMake they may provide a FooConfig.cmake file and then the variables will be in mixed-case because that is the way FIND_PACKAGE works.
|
|
|
|
(0004447)
|
|
Jan Woetzel
|
|
2006-07-11 19:24
|
|
(1) OK. I will
- convert to mixed case
- integrate FIND_PACKAGE REQUIRED components feature
(2)
Please add a note to CMake\Modules\readme.txt about the desired behavior of mixed case of variable names and filenames.
|
|
|
|
(0004455)
|
|
Jan Woetzel
|
|
2006-07-16 10:57
|
|
OK, here we go - finished.
-Converted case WXWIDGETS -> wxWidgets
-removed some preifxed CMAKE_WXWIDGETS_<FOO> -> wxWidgtes_<FOO>, e.g.
-renamed WXWIDGTES_CONFIG_EXE to wxWidgtes_CONFIG_EXECUTABEL for consistency with FindwxWindows and Modules/readme.txt conventions
-simple FIND_PACKAGE REQUIRED components support in addition to wxWidgets_USE_LIBS added
-magick component "std" added which expand to some standard libs for consistency between Windows and Linux and abstraction of monolithic/multilib build
-Added <Foo>_FIND_COMPONENTS docu to FIND_PACKAGE command and fixed minor typos
-Adapted CMake Teats/UseWX and WXDialog to the changes.
CMake+Tests#WXDIalog compile on my machoine.
Everything seems to work reasonably icnluding
CMake itself, its Test, in particular UseWX and WXDialog.
The patch is against CVS and contains all changes.
Thus you need to apply only the last patch.
Best,
Jan.
|
|
|
|
(0004458)
|
|
Jan Woetzel
|
|
2006-07-16 11:13
|
|
|
Increased severity because the patch fixes bug 0003453. |
|
|
|
(0004460)
|
|
Jan Woetzel
|
|
2006-07-16 15:01
|
|
Forgot two more additional changes,
-dropped one debug MESSAGE,
-deprecated warning message is STATUS on top of Use_wxWindows.cmake, now.
|
|
|
|
(0004474)
|
|
Brad King
|
|
2006-07-18 13:37
|
|
I notice that wxWidgets_LINK_DIRECTORIES is meant for defining rpaths on UNIX and not for linking. Perhaps it should be renamed wxWidgets_RUNTIME_LIBRARY_DIRS. Then on UNIX it can contain RPATH information and on Windows it can contain the directories with the .dll files that go with the .lib files being linked in wxWidgets_LIBRARIES.
|
|
|
|
(0004476)
|
|
Brad King
|
|
2006-07-18 13:48
|
|
In Source/WXDialog, the FIND_PACKAGE command should be more like:
FIND_PACKAGE(wxWidgets REQUIRED)
There should also be something to take into account the possibility that CMake is getting built by CMake 2.2 where the find command will fail since WXWINDOWS_FOUND will be set instead. Perhaps we should check the version of CMake used to build and not provide the BUILD_WX_DIALOG option unless the CMake version is new enough.
Also the WXDialog target should be renamed to something with CMake or CMakeSetup in its name. Perhaps the old wxCMakeSetup name would work.
|
|
|
|
(0004484)
|
|
Jan Woetzel
|
|
2006-07-18 15:16
|
|
Brad,
thanks for your attention.
> BUILD_WXDialog
> --renamed to something with CMake or CMakeSetup in its name
BUILD_WXDialog
is consistent with the existing
BUILD_MFCDialog and
BUILD_CursesDialog naming.
(1) Do you agree?
> Source/WXDialog/CMakeLists.txt:
> FIND_PACKAGE(wxWidgets REQUIRED)
With REQUIRED a FATAL_ERROR will be thrown on missing wx when BUILD_WXDialog is enabled. Then not even the command line "cmake" will be built. I think the behavior "go as far as you can get" may be useful for bootstrapping.
(2) If you think REQUIRED is the desired behavior feel free to add it.
>CMake 2.2 where the find command will fail since >WXWINDOWS_FOUND
Yes, but I think that workaround has minor priority because:
-WXDialog has major problems with UNICODE WX builds and should be considered "experimental"
-only expert users will want to play with WXDialog
-These experts may live with a two step loop of bootstrapping from very old cmake 2.0/2.2 to first build command line cmake 2.4. This cmake 2.4 can be used to configure+build again with WXDialog 2.4.
Not perfectly, but a step of improvement for getting closer to ideal.
Please note that I'm busy next weeks and won't be able to contribute additional major changes.
|
|
|
|
(0004485)
|
|
Jan Woetzel
|
|
2006-07-18 15:27
|
|
>wxWidgets_LINK_DIRECTORIES is meant for defining rpaths
>on UNIX and not for linking.
> renamed wxWidgets_RUNTIME_LIBRARY_DIRS.
No,
wxWidgets_LINK_DIRECTORIES should contain .so/.a on Linux and .lib (not .dll) on Windows.
It is designed for LINK_DIRECTORIES usage on Linux and Windows and RPATH usage on Linux.
Should we rename
wxWidgets_LINK_DIRECTORIES -->
wxWidgets_LIBRARY_DIRS
accoridng to your recent Modules/readme.txt changes?
Thanks, Jan.
|
|
|
|
(0004486)
|
|
Brad King
|
|
2006-07-18 15:29
|
|
(1) The option BUILD_WXDialog is fine.
I mostly meant that the executable should be called wxCMakeSetup because \"WXDialog\" is not very descriptive when installed.
(2) If the user enables the option BUILD_WXDialog and the build succeeds he/she would expect there to be a wxCMakeSetup. Currently this silently fails.
We could instead take the ccmake approach and find wxWidgets QUIETly, and if found then provide the BUILD_WXDialog option. This will probably be the right way in the long term.
Since it is still experimental and an advanced option we should probably go with the REQUIRED version because the person enabling it is a developer and will understand. |
|
|
|
(0004487)
|
|
Brad King
|
|
2006-07-18 15:33
|
|
> Should we rename wxWidgets_LINK_DIRECTORIES --> wxWidgets_LIBRARY_DIRS accoridng to your recent Modules/readme.txt changes?
Yes, but...
Why would LINK_DIRECTORIES be needed if the full paths to the libraries are provided? The link dirs should not be needed at all.
Runtime dirs are useful for setting up test environments.
|
|
|
|
(0004488)
|
|
Jan Woetzel
|
|
2006-07-18 16:08
|
|
wxWidgets_LINK_DIRECTORIES / wxWidgets_LIBRARY_DIRS
CMake projects don't need them.
However, we use them
- for RPATH on Linux and
- to collect libdirs to configure a "foo-config" shell script for compatibility with handwritten Makefiles to be used like `foo-config --libs` etc.
Slightly off-topic, but...
- I'm not familiar with an rpath equivalent on Windows.
Our .DLLs are either found via PATH environment variable or current directory of .exe on MS WIndows.
Thus we try to let EXECUTABLE_OUTPUT_PATH and LIBRARY_OUTPT_PATH be the same for all our BUILD_SHARED Windows projects.
Q:
Is there a smarter way to specify "typical" .dll runtime search locations on Windows and if how does CMake support this (with or without install) - any keywords for me?
|
|
|
|
(0004489)
|
|
Brad King
|
|
2006-07-18 16:23
|
|
The usage of wxWidgets_RUNTIME_LIBRARY_DIRS would be with RPATH or LD_LIBRARY_PATH on Linux and with PATH on Windows.
|
|
|
|
(0004490)
|
|
Brad King
|
|
2006-07-18 16:24
|
|
The requirement to set EXECUTABLE_OUTPUT_PATH and LIBRARY_OUTPUT_PATH to be the same is due to a bad decision made early in CMake development. Really the .dll file should be created in EXECUTABLE_OUTPUT_PATH and the .lib in LIBRARY_OUTPUT_PATH. I'd like to change it but that is not on-topic for this bug.
|
|
|
|
(0004492)
|
|
Brad King
|
|
2006-07-18 17:17
|
|
As stated in my changes to Modules/readme.txt the final variable used to report include directories should be wxWidgets_INCLUDE_DIRS (not the plural name). wxWidgets_INCLUDE_DIR should be used only as the argument to a FIND_PATH command if one is needed.
The -I and -isystem options from wx-config should all be pulled out into wxWidgets_INCLUDE_DIRS. Unless they actually show up in the include path known to cmake the headers will not be included in dependency scanning. If you still want to add the -isystem options to the CXX flags to get them to show up on the command line first that way then fine, but see my note in bug 0003453 about things not working that way.
|
|
|
|
(0004493)
|
|
Jan Woetzel
|
|
2006-07-18 17:57
|
|
OK, to conclude the open issues and make progress:
(1) WXDialog/CMakeLists.txt. REQUIRED ?
- I'm OK with
FIND_PACKAGE(wWidgets REQUIRED)
(2) rename wxWidgets_LINK_DIRECTORIES --> wxWidgets_LIBRARY_DIRS ?
- I'm OK with renaming for consistency with Modules/readme.txt
(3) rename wxWidgets_INCLUDE_DIR
-> wxWidgets_INCLUDE_DIRS
- I'm Ok with reanimg it for consistency with Modules/readme.txt
(4) rename wxWidgets_LINK_DIRECTORIES --> wxWidgets_RUNTIME_LIBRARY_DIRS or add support for wxWidgets_RUNTIME_LIBRARY_DIRS ?
- On Linux compile time and runtime dir is actually the same (both .so).
- On Windows we don\\\\\\\'t distinguishing between compile time .lib dir and runtime .dll dir.
So we shouldn\\\\\\\'t raise the users hope that we would...
- Implementing the distinguishing between .lib and .dll dir may is not planned and may be harder than expected to assert consistency between .lib and .dll
-As no other scipt seems to support
<Foo>_RUNTIME_LIBRARY_DIRS yet I doubt we should somehow fake that we do.
Thus: we should simply drop the idea of wxWidgets_RUNTIME_LIBRARY_DIRSfor now, I think.
(5) rename executable WXDialog --> wxCMakeSetup or similar?
- I agree, we have two choices:
(a) ADD_EXECUTABLE(wxCMakeSetup WXDialog.cpp ...) or
(b) INSTALL with different name.
We shoudl choose (a) because it\\\\\\\'s simpler and cleaner in the long term.
However, let\\\\\\\'s postpone that until WXDialog stabilizes and ignore it fow now. The developers won\\\\\\\'t care about names and users donÄt yet use it.
(6) remove deprecated code
- When you apply the patch:
Please remove \\\\\\\"Source\\\\\\\\WXDialog\\\\\\\\bin\\\\\\\" because it contains only deprecated duplicate scripts. User+developers may be confused about which find wx script to use.
They should be educated to use only Modules/FindwWidgets.cmake and developers should fix problems *only* there.
(7) -isystem and wxWidgets_INCLUDE_DIRS ?
It would be ideal to write a test if the compiler support -isystem at all.
However, a simple \"is g++ and NOT APPLE\" logic should work for now, right?
I don\'t have MacOS/APPLE at hand for testing.
I have to check the precedence/order of -I and -isystem.
We really want to keep -isystem because we compile our projects with \"-pedantic\" by default and don\'t want to see the WX warnings noise.
If you agree wuth 1-6 I will submit a new patch against CMake CVS including the changes.
I\'m uncertain about (7) because I can\'t test it without a mac at hand..
Can you verify that an additional IF(NOT APLLE) after checking for \"g++\" near -isystem is enough ?
Jan.
|
|
|
|
(0004494)
|
|
Jan Woetzel
|
|
2006-07-18 18:09
|
|
more about (7):
IF( CMAKE_COMPILER_IS_GNUCC AND CMAKE_COMPILER_IS_GNUCXX )
IF (NOT APPLE)
< replace by -isystem>
should eork, right?
Jan.
|
|
|
|
(0004496)
|
|
Jan Woetzel
|
|
2006-07-19 02:25
|
|
even more about (7)
- We can add an option wxWidgets_USE_ISYSTEM that defaults to false.
Thsu isystem repalcement will be used onyl if teh user explicitly requests it.
IF (HAVE_ISYSTEM)
IF( CMAKE_COMPILER_IS_GNUCC AND CMAKE_COMPILER_IS_GNUCXX )
IF (NOT APPLE)
<replace by -isystem>
OK?
|
|
|
|
(0004498)
|
|
Brad King
|
|
2006-07-19 10:18
|
|
Your solution for (7) looks good. I'll try it with your next patch for the other changes.
As far as warnings in wx headers, GCC really needs a "#pragma warning push/pop" directive like MSVC in order to block warnings from specific headers. I wonder if they'll ever add it.
|
|
|
|
(0004499)
|
|
Jan Woetzel
|
|
2006-07-19 11:26
|
|
OK, next try with final_3_CMake.patch.
Contains the fixes 1-7 including 7 for
FindwxWidgets and FindwxWindows
gcc pragma warning would be great, indeed.
Jan.
|
|
|
|
(0004528)
|
|
Brad King
|
|
2006-07-21 15:43
|
|
I've now tested this on Windows, Linux, and OSX and it looks pretty good.
I've applied the patch to CVS:
/cvsroot/CMake/CMake/Modules/FindwxWidgets.cmake,v <-- FindwxWidgets.cmake
new revision: 1.3; previous revision: 1.2
/cvsroot/CMake/CMake/Modules/FindwxWindows.cmake,v <-- FindwxWindows.cmake
new revision: 1.19; previous revision: 1.18
/cvsroot/CMake/CMake/Modules/Use_wxWindows.cmake,v <-- Use_wxWindows.cmake
new revision: 1.5; previous revision: 1.4
/cvsroot/CMake/CMake/Modules/UsewxWidgets.cmake,v <-- UsewxWidgets.cmake
new revision: 1.4; previous revision: 1.3
/cvsroot/CMake/CMake/Source/CMakeLists.txt,v <-- CMakeLists.txt
new revision: 1.297; previous revision: 1.296
/cvsroot/CMake/CMake/Source/WXDialog/CMakeLists.txt,v <-- CMakeLists.txt
new revision: 1.24; previous revision: 1.23
/cvsroot/CMake/CMake/Source/WXDialog/bin/FindUPX.cmake,v <-- FindUPX.cmake
new revision: 1.2; previous revision: 1.1
/cvsroot/CMake/CMake/Source/WXDialog/bin/FindwxW.cmake,v <-- FindwxW.cmake
new revision: 1.2; previous revision: 1.1
/cvsroot/CMake/CMake/Source/WXDialog/bin/FindwxWin.cmake,v <-- FindwxWin.cmake
new revision: 1.5; previous revision: 1.4
/cvsroot/CMake/CMake/Source/WXDialog/bin/UsewxW.cmake,v <-- UsewxW.cmake
new revision: 1.2; previous revision: 1.1
/cvsroot/CMake/CMake/Tests/UseWX/CMakeLists.txt,v <-- CMakeLists.txt
new revision: 1.3; previous revision: 1.2
/cvsroot/CMake/CMake/Tests/UseWX/WX.cxx,v <-- WX.cxx
new revision: 1.3; previous revision: 1.2
We'll include this in 2.4.3.
|
|
|
|
(0004529)
|
|
Brad King
|
|
2006-07-21 15:44
|
|
|
Thanks for the contribution! |
|
FindwxWidgets.zip (7,090) 1969-12-31 19:00
FindwxWidgets.cmake (21,779) 1969-12-31 19:00
CMake.patch (38,789) 1969-12-31 19:00