|
Notes |
|
|
(0019095)
|
|
Bill Hoffman
|
|
2010-01-12 07:52
|
|
|
This would break way too much code.... The patch itself has to change a bunch of code in CMake itself. Imagine all of the other C++ code around the world that would require the same changes.... Also changing the default for loadable modules could really break some stuff... I really don't see how we could apply this patch. |
|
|
|
(0019101)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-12 13:45
|
|
> This would break way too much code.... The patch itself has to change
> a bunch of code in CMake itself.
Actually, it un"break"s the incorrect assumptions about Cygwin so that packages actually build correctly which they do NOT without the patch. As I said, I have built ~120 packages with this patch, including most of KDE4, and the latter would be completely impossible without this patch.
> Imagine all of the other C++ code around the world that would require
> the same changes
I'm not sure what you mean by this. With the current stock CMake, almost every single WIN32 needs to be changed to WIN32 AND NOT CYGWIN. With this patch, most packages build OOTB.
> Also changing the default for loadable modules could really break some stuff
Libtool uses the 'cyg' prefix for both libraries and modules; NOT using 'cyg' breaks g_module_build_path() for instance.
> I really don't see how we could apply this patch.
Well, without it, there is no way that CMake-based packages will ever be included in our distro, as they will not build correctly, if at all. So if you're not prepared to accept it yet, I am willing to maintain the CMake package in the Cygwin distro with these patches. |
|
|
|
(0019103)
|
|
Bill Hoffman
|
|
2010-01-12 16:04
|
|
I am the maintainer for CMake on cygwin... :)
You maybe right about "almost every single WIN32 needs to be changed to WIN32 AND NOT CYGWIN. ". However, CMake has been doing this for 10 years. So, every project that currently uses CMake, and has been ported to CYGWIN, is expecting this to be done. If we stop doing it, I would expect all CMake projects that currently work on cygwin will stop working. We have to come up with a solution that does not break existing CMake based projects. The fact that several lines of code in the CMake C++ code had to be changed tells me that projects like VTK, ITK, and other project will need those same changes. That is not something I would like to break.
So, can you think of a way to make these changes without making a change to the c/C++ of CMake? The only thing I can think of is some sort of mode, but is less than ideal... |
|
|
|
(0019104)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-12 16:21
|
|
> However, CMake has been doing this for 10 years.
So can we make sure that it doesn't become 11 years?
> So, every project that currently uses CMake, and has been ported to CYGWIN,
> is expecting this to be done.
And how many projects have been *correctly* ported to Cygwin? Not as many as you may think.
> If we stop doing it, I would expect all CMake projects that currently work
> on cygwin will stop working.
Again, how many is that? Either packages haven't been specifically ported to Cygwin, or they try to group Cygwin with Win32, which is generally wrong.
> The fact that several lines of code in the CMake C++ code had to be changed
> tells me that projects like VTK, ITK, and other project will need those same
> changes.
Yes, any code that makes false assumptions about Cygwin needs to be changed. There's no way to work around that if you want it to work. |
|
|
|
(0019105)
|
|
Bill Hoffman
|
|
2010-01-12 16:33
|
|
|
There are plenty of projects using CMake and Cygwin, I am not going to break those projects. Perhaps this discussion should be moved to the CMake mailing list. I know it will most likely break all of the Kitware hosted open source vis, vision, and super computing projects. There are most likely plenty of researchers around the world that have derivative code based on that that will break as well. I obviously have no way to count the number of projects, but I know it is greater than one, and more than likely in the 100's. |
|
|
|
(0019116)
|
|
Brad King
|
|
2010-01-13 13:00
|
|
I've applied some parts of the patch that do not change existing behavior:
Use if(CYGWIN) instead of if(WIN32 AND UNIX)
/cvsroot/CMake/CMake/Modules/FindTclsh.cmake,v <-- Modules/FindTclsh.cmake
new revision: 1.26; previous revision: 1.25
/cvsroot/CMake/CMake/Source/CMakeLists.txt,v <-- Source/CMakeLists.txt
new revision: 1.436; previous revision: 1.435
Enable extra CodeBlocks generator on Cygwin
/cvsroot/CMake/CMake/Source/CMakeLists.txt,v <-- Source/CMakeLists.txt
new revision: 1.437; previous revision: 1.436
/cvsroot/CMake/CMake/Source/cmake.cxx,v <-- Source/cmake.cxx
new revision: 1.443; previous revision: 1.442
KWSys: Fix SharedForward on Cygwin without -mwin32
/cvsroot/CMake/CMake/Source/kwsys/SharedForward.h.in,v <-- Source/kwsys/SharedForward.h.in
new revision: 1.18; previous revision: 1.17 |
|
|
|
(0019117)
|
|
Brad King
|
|
2010-01-13 13:01
|
|
Strictly speaking this one changes behavior but I consider it a bug fix, and it shouldn't break anything since the name of the import library does not change:
Name Cygwin DLLs with SOVERSION, not VERSION
/cvsroot/CMake/CMake/Source/cmTarget.cxx,v <-- Source/cmTarget.cxx
new revision: 1.286; previous revision: 1.285 |
|
|
|
(0019118)
|
|
Brad King
|
|
2010-01-13 13:12
|
|
|
Regarding "2.8-usr-prefix.patch", we changed the prefix search order not too long ago. See issue 0009657. The reporter did not know whether "/" or "/usr" should go first, so at the time we decided to leave that order alone. The comment in the patch about FHS and not having linkable libraries in /lib looks like a good argument to adjust this order. |
|
|
|
(0019119)
|
|
Brad King
|
|
2010-01-13 13:29
|
|
|
I created issue 0010136 to deal with 2.8-usr-prefix.patch since it affects all platforms. |
|
|
|
(0019120)
|
|
Brad King
|
|
2010-01-13 13:30
|
|
Regarding this hunk:
@@ -74,7 +74,7 @@ LIST(APPEND CMAKE_SYSTEM_PROGRAM_PATH
)
LIST(APPEND CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES
- /lib /usr/lib /usr/lib32 /usr/lib64
+ /usr/lib /usr/lib32 /usr/lib64
)
The CMAKE_PLATFORM_IMPLICIT_LINK_DIRECTORIES variable records places that compilers typically look for libraries (to avoid explicit options like -L/usr/lib in link lines). Many compilers implicitly search /lib, so it belongs in this variable. It has nothing to do with CMake's search for libraries. |
|
|
|
(0019122)
|
|
Alex Neundorf
|
|
2010-01-13 13:45
|
|
We have the following code in KDE4s FindKDE4Internal.cmake:
if (WIN32)
if(CYGWIN)
message(FATAL_ERROR "Cygwin is NOT supported, use mingw or MSVC to build KDE4.")
endif(CYGWIN)
...
So, from the KDE side, building KDE4 on cygwin is officially not supported.
I also can't remember any email on the KDE lists where somebody asked for that.
So right now, if something in CMake breaks KDE4 on cygwin, I wouldn't consider this an issue, since KDE4 itself doesn't support cygwin.
Alex |
|
|
|
(0019123)
|
|
Brad King
|
|
2010-01-13 13:50
|
|
This commit supersedes the cmCacheManager.cxx change from 2.8-cygwin.patch:
Remove unused #include <windows.h>
/cvsroot/CMake/CMake/Source/cmCacheManager.cxx,v <-- Source/cmCacheManager.cxx
new revision: 1.116; previous revision: 1.115 |
|
|
|
(0019124)
|
|
Alex Neundorf
|
|
2010-01-13 13:50
|
|
About patch 1, the la-file: I have no idea how this looks on libtool, if your patch is correct, then it probably should be applied.
Alex |
|
|
|
(0019125)
|
|
Brad King
|
|
2010-01-13 13:55
|
|
I've committed 2.6-KDE3Macros.patch:
Fix KDE3 .la file format on Cygwin
/cvsroot/CMake/CMake/Modules/KDE3Macros.cmake,v <-- Modules/KDE3Macros.cmake
new revision: 1.10; previous revision: 1.9 |
|
|
|
(0019126)
|
|
Brad King
|
|
2010-01-13 14:13
|
|
I've committed another hunk from 2.8-cygwin.patch that should not break anything:
Do not find cyg*.dll on Cygwin
/cvsroot/CMake/CMake/Modules/Platform/CYGWIN.cmake,v <-- Modules/Platform/CYGWIN.cmake
new revision: 1.23; previous revision: 1.22
Import libraries are always available so we do not need to find .dll files. |
|
|
|
(0019127)
|
|
Brad King
|
|
2010-01-13 14:22
|
|
|
I've uploaded patch "cygwin-no-win32-2010-01-13.patch" which contains everything proposed in this issue that has not yet been committed. I ported the changes to the CMake development head for further discussion. |
|
|
|
(0019128)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-13 14:37
|
|
I appreciate your consideration of these patches. A lot of issues have been raised in quick succession, so let me try to deal with them one at a time:
* 2.8-usr-prefix.patch:
While technically, link libraries should not be in /lib per the FHS, you're probably correct that we can't assume all systems are compliant. As long as /usr/lib* precede /lib, then the correct /usr prefix should be used and AFAICS should be fine.
* KDE4:
> We have the following code in KDE4s FindKDE4Internal.cmake:
> if (WIN32)
> if(CYGWIN)
> message(FATAL_ERROR "Cygwin is NOT supported, use mingw or MSVC to build KDE4.")
> endif(CYGWIN)
KDE4 assumes that WIN32 means *native* Win32/GDI, and this can be seen throughout KDE4. But Cygwin's Qt4 is *NIX/X11, so building a Win32 KDE4 with a Cygwin/X11 Qt4 is clearly not going to work, hence the error.
If you do not define WIN32 on Cygwin, however, then you build KDE4 like other *NIX platforms, which actually works quite well.
> So, from the KDE side, building KDE4 on cygwin is officially not supported.
> I also can't remember any email on the KDE lists where somebody asked for
> that.
Actually, I have, in KDE bug 130219, as well as some emails to kde-buildsystem last year. Some of my patches have been committed by the KDE-Windows maintainer, with whom I've had previous exchanges back in the days of the KDE-Cygwin project.
> So right now, if something in CMake breaks KDE4 on cygwin, I wouldn't consider
> this an issue, since KDE4 itself doesn't support cygwin.
Yet. KDE is not opposed to supporting Cygwin, but they need someone (me) to make the necessary fixes to make it work. |
|
|
|
(0019129)
|
|
Brad King
|
|
2010-01-13 14:40
|
|
(1) Regarding auto-export/import:
-set(CMAKE_EXE_LINKER_FLAGS_INIT "-Wl,--enable-auto-import")
- # To simulate UNIX shared libs we export/import all DLL symbols.
- set(CMAKE_SHARED_LIBRARY_CREATE_${lang}_FLAGS "${CMAKE_SHARED_LIBRARY_CREATE_${lang}_FLAGS} -Wl,--export-all-symbols -Wl,--enable-auto-import")
- set(CMAKE_SHARED_MODULE_CREATE_${lang}_FLAGS "${CMAKE_SHARED_LIBRARY_CREATE_${lang}_FLAGS}")
The auto-import and export-all flags make Cygwin shared libraries behave like UNIX shared libraries, which seems to be in line with your goal of easy porting. Often the Cygwin linker activates auto-import anyway and then warns that it did so without the flag. What is your justification for this change?
(2) Regarding DLL_EXPORT:
- set(CMAKE_SHARED_LIBRARY_${lang}_FLAGS "") # No -fPIC on cygwin
+ set(CMAKE_SHARED_LIBRARY_${lang}_FLAGS "-DDLL_EXPORT") # No -fPIC on cygwin
What is your justification for this? Is it part of the Cygwin specification for building source packages? CMake already defines -Dmysharedlib_EXPORTS in sources compiled into mysharedlib to tell included headers they are being compiled in their DLL implementation.
(3) Regarding the "cyg" prefix for loadable modules:
Is there a place in the Cygwin spec that says plugins (that have no .dll.a and cannot be linked) should have a "cyg" prefix too? Is this change just because libtool does it? The CYGWIN.cmake hunk that changes the prefix that CMake generates could break existing projects that build plugins and then search for them at runtime with the "lib" prefix.
The DynamicLoader.cxx hunk does not affect CMake itself. KWSys is a library shared by CMake and many Kitware projects. We run all its tests in CMake because it builds everywhere the other projects could. Changing KWSys code affects many projects which are still using CMake 2.4 and CMake 2.6, both of which will continue to build Cygwin modules with the "lib" prefix. If they start looking for the plugins with "cyg" after building with "lib" they will break. These projects need to be updated to deal with this change before we can commit it to KWSys. Fortunately it does not affect CMake itself anyway.
(4) Regarding the -mwindows flag:
-set(CMAKE_CREATE_WIN32_EXE "-mwindows")
This variable is used to generate link lines for non-console Windows applications (with WinMain, activated with the WIN32_EXECUTABLE property). Is that not the purpose of -mwindows?
(5) Regarding gdi32:
-set(CMAKE_DL_LIBS "-lgdi32" )
+set(CMAKE_DL_LIBS "" )
If an application uses the Windows API (-mwin32) it should link to this library explicitly just like any other. This change should probably be made, but we'll have to keep compatibility somehow.
(6) Regarding definition of WIN32 in the CMake language:
I'll refrain from discussing this one for now. |
|
|
|
(0019130)
|
|
Alex Neundorf
|
|
2010-01-13 14:47
|
|
Re 19128:
> Yet. KDE is not opposed to supporting Cygwin, but they need someone (me) to
> make the necessary fixes to make it work.
Now I found the mails on the kde-windows list.
I don't oppose supporting cygwin for KDE4, as long as it doesn't require a lot of special handling for cygwin in KDE4, since KDE4 runs natively on Windows and also on UNIX, so I don't see a strong need for it.
Alex |
|
|
|
(0019131)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-13 15:13
|
|
> (1) Regarding auto-export/import:
> The auto-import and export-all flags make Cygwin shared libraries behave like
> UNIX shared libraries, which seems to be in line with your goal of easy
> porting. Often the Cygwin linker activates auto-import anyway and then warns
> that it did so without the flag. What is your justification for this change?
-Wl,--enable-auto-import is now the default with the latest gcc/binutils, so adding it is not incorrect, but it's no longer necessary. If you really need to support older Cygwin installations, there should be no harm in keeping it.
> (2) Regarding DLL_EXPORT:
> What is your justification for this?
For compatibility with libtool, which defines $pic_flag as "-DPIC -DDLL_EXPORT" on Cygwin.
> (3) Regarding the "cyg" prefix for loadable modules:
> Is there a place in the Cygwin spec that says plugins (that have no .dll.a
> and cannot be linked) should have a "cyg" prefix too? Is this change just
> because libtool does it?
I'm not sure what you mean by "the Cygwin spec". libtool has been using the "cyg" prefix for years. For instance, GLib's g_module_build_path() assumes this behaviour, so a CMake-built module with a "lib" prefix would not be found.
> (4) Regarding the -mwindows flag:
> -set(CMAKE_CREATE_WIN32_EXE "-mwindows")
> This variable is used to generate link lines for non-console Windows
> applications (with WinMain, activated with the WIN32_EXECUTABLE property).
> Is that not the purpose of -mwindows?
I see, I misunderstood the purpose of this variable. If CMAKE_CREATE_WIN32_EXE is only used with the WIN32_EXECUTABLE property, then it can stay.
> (5) Regarding gdi32:
> If an application uses the Windows API (-mwin32) it should link to this
> library explicitly just like any other. This change should probably be made,
> but we'll have to keep compatibility somehow.
If CMAKE_CREATE_WIN32_EXE stays as above, there should be no issue for a WIN32_EXECTUABLE, as -mwindows implies -lgdi32. |
|
|
|
(0019132)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-13 15:34
|
|
> I don't oppose supporting cygwin for KDE4, as long as it doesn't require a
> lot of special handling for cygwin in KDE4, since KDE4 runs natively on
> Windows and also on UNIX, so I don't see a strong need for it.
Some of the kdelibs patches were already committed by habacker; I'll have a better idea after 4.4 comes out what remains. kdebindings needs several patches, mostly to fix install() DESTINATIONs (which fixes both Cygwin and Win32). The rest of the modules need only very minor patches or none at all. |
|
|
|
(0019133)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-13 15:51
|
|
|
I just revised the patch for HEAD to reflect my last comments. |
|
|
|
(0019134)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-13 16:55
|
|
A clarification to my first point in comment 19131:
-Wl,--enable-auto-import is now the default, so adding it explicitly is harmless but unnecessary. If you must support older Cygwin installations (we don't), then by all means keep it.
-Wl,--export-all-symbols is a different matter. Per the GNU ld docs, this is essentially the default UNLESS either a .def file or __declspec(dllexport) is used, in which case only the symbols specified thereby are exported. If --export-all-symbols is explicitly passed, then these two are defeated and all symbols are disabled anyway. Therefore --export-all-symbols is not desirable for libraries since no way remains to restrict which symbols are to be exported.
OTOH, for executables, no symbols are exported unless -Wl,--export-all-symbols is passed, which is usually only necessary for GTK+ programs using GtkBuilder or GladeXML. Based on how this is handled on other platforms, I therefore suggest adding the following lines to CYGWIN.cmake:
SET(CMAKE_EXE_EXPORTS_C_FLAG "-Wl,--export-all-symbols")
SET(CMAKE_EXE_EXPORTS_CXX_FLAG "-Wl,--export-all-symbols") |
|
|
|
(0019167)
|
|
Brad King
|
|
2010-01-18 14:22
|
|
Thanks for your updated patch and responses.
> to support older Cygwin installations, there should be no harm in keeping -Wl,--enable-auto-import.
Okay, we'll keep it for a while because some of our users rarely update cygwin. I'll add a TODO comment to remove it.
> For compatibility with libtool, which defines $pic_flag as "-DPIC -DDLL_EXPORT" on Cygwin.
This only aids in porting a non-CMake project to CMake. Since CMake already provides its own equivalent to this feature we'd prefer that projects use it. Also, the work we're discussing here is about porting to cygwin projects that already use CMake.
> libtool has been using the "cyg" prefix for years. For instance, GLib's g_module_build_path() assumes this behaviour
I do want to make this change as it helps keep cygwin DLLs separate from MinGW DLLs. I need to think about what to do for compatibility.
> If CMAKE_CREATE_WIN32_EXE is only used with the WIN32_EXECUTABLE property, then it can stay.
Great.
> -mwindows implies -lgdi32
The gdi32 library has nothing to do with LoadLibrary or dlopen so there is no reason for it to be in CMAKE_DL_LIBS anyway. The CMAKE_DL_LIBS variable is not used by CMake; it is provided so that projects may link to "dl" in a cross-platform way. We should be able to just remove it. |
|
|
|
(0019168)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-18 14:39
|
|
> This only aids in porting a non-CMake project to CMake. Since CMake already
> provides its own equivalent to this feature we'd prefer that projects use it.
> Also, the work we're discussing here is about porting to cygwin projects that
> already use CMake.
It also affects CMake projects which are built against libtool-based projects. For instance, GLib's headers use DLL_EXPORT to define when __declspec(dllexport) or __declspec(dllimport) should be used. |
|
|
|
(0019169)
|
|
Brad King
|
|
2010-01-18 15:03
|
|
So DLL_EXPORT does not try to distinguish between dllexport and dllimport for a given library, but rather between the shared and static forms of the library? libtool defines it for all shared libraries because they should always use the shared form of their dependencies?
Is DLL_EXPORT approximately the libtool equivalent to _DLL?
http://msdn.microsoft.com/en-us/library/b0084kay%28VS.80%29.aspx [^] |
|
|
|
(0019170)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-18 15:24
|
|
> So DLL_EXPORT does not try to distinguish between dllexport and dllimport for
> a given library, but rather between the shared and static forms of the
> library? libtool defines it for all shared libraries because they should
> always use the shared form of their dependencies?
Perhaps this will help clarify:
http://sourceware.org/autobook/autobook/autobook_255.html [^]
(While the example is only #ifdef _WIN32, e.g. GLib does this #ifdef G_PLATFORM_WIN32, which means all PE/COFF platforms, including Cygwin.) |
|
|
|
(0019171)
|
|
Brad King
|
2010-01-18 15:37
(edited on: 2010-01-18 15:39) |
|
The problem with that approach is that DLL_EXPORT gets defined for all shared libraries. What happens when the glib header is included from the implementation of another shared library? It will try to dllexport glib's symbols from that library. Linkers typically automatically convert dllexport to dllimport if the symbol is not defined, but this does not work well for data symbols.
It is important to distinguish the dllexport (implementation) case from the dllimport (interface) case for each shared library independently. This means that DLL_EXPORT must have a per-library name. With CMake, one may write:
# CMakeLists.txt
add_library(mylib SHARED mylib.c mylib.h)
/* mylib.h */
#if defined(_WIN32) && defined(mylib_SHARED)
# if defined(mylib_EXPORTS) /* inside a mylib implementation source */
# define mylib_SCOPE __declspec(dllexport)
# else
# define mylib_SCOPE __declspec(dllimport)
# endif
#else
# define mylib_SCOPE extern /* or just empty */
#endif
CMake automatically defines mylib_EXPORTS while compiling translation units in mylib (like mylib.c). The mylib_SHARED macro is to be defined by the includer to indicate whether it is building/linking the shared form of the library (just like LIBHELLO_DLL_IMPORT in the above-linked).
|
|
|
|
(0019172)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-18 15:46
|
|
> The problem with that approach is that DLL_EXPORT gets defined for all shared
> libraries. What happens when the glib header is included from the
> implementation of another shared library? It will try to dllexport glib's
> symbols from that library.
I see, the example is a little confusing in that respect. Here's what GLib actually does (from <glib/gtypes.h>):
#ifndef GLIB_VAR
# ifdef G_PLATFORM_WIN32
# ifdef GLIB_STATIC_COMPILATION
# define GLIB_VAR extern
# else /* !GLIB_STATIC_COMPILATION */
# ifdef GLIB_COMPILATION
# ifdef DLL_EXPORT
# define GLIB_VAR __declspec(dllexport)
# else /* !DLL_EXPORT */
# define GLIB_VAR extern
# endif /* !DLL_EXPORT */
# else /* !GLIB_COMPILATION */
# define GLIB_VAR extern __declspec(dllimport)
# endif /* !GLIB_COMPILATION */
# endif /* !GLIB_STATIC_COMPILATION */
# else /* !G_PLATFORM_WIN32 */
# define GLIB_VAR extern
# endif /* !G_PLATFORM_WIN32 */
#endif /* GLIB_VAR */
GLIB_COMPILATION is defined only when building GLib itself, so a GLib-dependent package defines (only) DLL_EXPORT and gets an extern __declspec(dllimport) for GLib's symbols, just as it should. |
|
|
|
(0019173)
|
|
Brad King
|
|
2010-01-18 16:22
|
|
I do not see how switching on DLL_EXPORT contributes anything. In order to get to the test for it, we need
-DG_PLATFORM_WIN32 -UGLIB_STATIC_COMPILATION -DGLIB_COMPILATION
That means we need dllexport because users of the library will do the equivalent of
-DG_PLATFORM_WIN32 -UGLIB_STATIC_COMPILATION -UGLIB_COMPILATION
and get dllimport. Unless the library was built with dllexport there will not be a proper import library to provide the symbols.
So, what is the purpose of the !DLL_EXPORT case in that block? IOW, why is
# ifdef DLL_EXPORT
# define GLIB_VAR __declspec(dllexport)
# else /* !DLL_EXPORT */
# define GLIB_VAR extern
# endif /* !DLL_EXPORT */
not just
# define GLIB_VAR __declspec(dllexport)
? |
|
|
|
(0019176)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-18 20:40
|
|
GLIB_COMPILATION (and GLIB_STATIC_COMPILATION) means "I am building GLib itself" and is only defined within the GLib Makefiles. G_PLATFORM_WIN32 is defined by the GLib headers for Cygwin/MinGW/MSVC platforms and is not to be set manually.
But I think we're starting to lose the forest for the trees. I'm still not sure why CMake can't be compatible with libtool wrt a simple define, although I understand your point about CMake having its own way of doing the same thing. |
|
|
|
(0019189)
|
|
Brad King
|
|
2010-01-19 09:01
|
|
I don't want to define DLL_EXPORT for every source file in a shared library in every project just to be compatible with a few projects that use it because they were written for libtool. It needlessly complicates the command line, and would need to be done on all Windows platforms, not just cygwin. CMake already has an interface for the same purpose.
If a project wants CMake to define DLL_EXPORT instead of its default mylib_EXPORTS, it can set the DEFINE_SYMBOL property:
# http://www.cmake.org/cmake/help/cmake-2-8-docs.html#prop_tgt:DEFINE_SYMBOL [^]
set_property(TARGET mylib PROPERTY DEFINE_SYMBOL DLL_EXPORT)
It can also use the COMPILE_DEFINITIONS property, or the ADD_DEFINITIONS command. |
|
|
|
(0019244)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-21 13:28
|
|
To summarize remaining issues:
* Do not set WIN32/CMAKE_HOST_WIN32;
* Use 'cyg' prefix for MODULEs (agreed to in principle, comment 0019167);
* Unset CMAKE_DL_LIBS (agreed to in comment 0019167);
* Don't use -Wl,--export-all-symbols for libraries but do in CMAKE_EXE_EXPORTS_${lang}_FLAG (comment 0019134). |
|
|
|
(0019246)
|
|
Brad King
|
|
2010-01-21 15:09
|
|
I committed the (mostly) backwards-compatible fixes:
Fix CMAKE_DL_LIBS on Cygwin
/cvsroot/CMake/CMake/Modules/Platform/CYGWIN-GNU.cmake,v <-- Modules/Platform/CYGWIN-GNU.cmake
new revision: 1.2; previous revision: 1.1
Do not export all symbols from DLLs on Cygwin
/cvsroot/CMake/CMake/Modules/Platform/CYGWIN-GNU.cmake,v <-- Modules/Platform/CYGWIN-GNU.cmake
new revision: 1.3; previous revision: 1.2
* The --enable-auto-import flag was actually added for executables recently by issue 0009071. I left a TODO comment to determine when it is the default everywhere.
* I'm considering --export-all-symbols for CMAKE_EXE_EXPORTS_${lang}_FLAG. It would be a reasonable place for the flag but leaves no way to dllexport a narrow interface without hacking the value of the variable.
* I still need to think about how to do the 'cyg' prefix with compatibility
* I still need to think about whether/how we can do !WIN32 with compatibility |
|
|
|
(0019255)
|
|
Yaakov (Cygwin Ports)
|
|
2010-01-21 18:47
|
|
> I'm considering --export-all-symbols for CMAKE_EXE_EXPORTS_${lang}_FLAG. It
> would be a reasonable place for the flag but leaves no way to dllexport a
> narrow interface without hacking the value of the variable.
If an executable actually uses __declspec(dllexport), then ENABLE_EXPORTS isn't necessary, although I doubt that a program written with Win32 in mind would be built in this fashion. This is more for programs primarily intended for *NIX/X11 where this is not so uncommon (e.g. most GtkBuilder or GladeXML-based executables rely on its own symbols being exported). CMAKE_EXE_EXPORTS_${lang}_FLAG is set to -Wl,--export-dynamic on ELF platforms; the PE/COFF equivalent of that is -Wl,--export-all-symbols, so AFAICS this is where to set it. |
|
|
|
(0019610)
|
|
Yaakov (Cygwin Ports)
|
|
2010-02-24 16:36
|
|
|
What more can I do to help resolve the remaining issues? |
|
|
|
(0019619)
|
|
Brad King
|
|
2010-02-25 14:05
|
|
|
|
|
(0019620)
|
|
Yaakov (Cygwin Ports)
|
|
2010-02-25 14:28
|
|
I understand the CMake Policy structure but AFAICS since Policies default to OLD that would require editing every single package to fix this issue, which is only a slight improvement over what we're trying to avoid, adding AND NOT CYGWIN to almost all WIN32s.
Furthermore, even if these Policies were to default to NEW, leaving it to authors to determine to "define" Cygwin behaviour is in itself asking for trouble. I have built enough software for Cygwin that I can tell you there are many misconceptions out there about what Cygwin is and isn't. Allowing even the suggestion that Cygwin could be the same as Win32 often leads people to think it should be that way despite everything.
So I would ask you to reconsider this solution and look at fixing these outright, so that our CMake will be able to correctly build as many packages as possible OOTB. |
|
|
|
(0019625)
|
|
Brad King
|
|
2010-02-25 15:28
|
|
Changing the behavior outright would break many projects whose authors already put time into getting them to build under Cygwin. It would not be friendly to these authors to break their projects in favor of projects whose authors have put in no time for Cygwin.
Policies only default to OLD when the minimum required version of CMake is older than that in which the policy was introduced. The idea is that they slowly disappear over time as projects start requiring newer CMake versions. For example, when policies (especially CMP0003) were first introduced there was no end to the questions about it. Now that many projects start with
cmake_minimum_required(VERSION 2.6) # Sets CMP0003 to NEW
we rarely get questions and things just work. This approach allows us to change behavior while retaining compatibility with existing projects. The only cost is that projects that want the NEW behavior early need to set policies. |
|
|
|
(0019626)
|
|
Brad King
|
|
2010-02-25 15:33
|
|
A middle ground solution is to allow build-time selection of the setting of WIN32. Something like:
cmake /path/to/src -DCYGWIN_NO_WIN32=1
could tell the platform module to not set WIN32. Then you can build projects that need this behavior without modification, or with at most
cmake_minimum_required(VERSION 2.6)
set(CYGWIN_NO_WIN32 1) # Add this line
project(SomeProject)
at the top. |
|
|
|
(0019633)
|
|
Yaakov (Cygwin Ports)
|
|
2010-02-25 16:28
|
|
> Changing the behavior outright would break many projects whose authors already
> put time into getting them to build under Cygwin. It would not be friendly to
> these authors to break their projects in favor of projects whose authors have
> put in no time for Cygwin.
I'm not sure what you mean by "getting them to build under Cygwin". Projects which currently use an explicit CYGWIN conditional, provided that they actually understand what Cygwin is (which is not always the case), should not be affected by unsetting WIN32 on Cygwin. What this affects are projects who have not taken Cygwin into account at all, or just assume that it should be lumped together with Win32, in which case they are currently broken and unsetting WIN32 *fixes* them (or at least gets us much closer).
> Policies only default to OLD when the minimum required version of CMake is
> older than that in which the policy was introduced. The idea is that they
> slowly disappear over time as projects start requiring newer CMake versions.
But that still means we would need to wait for packages to require whichever new version these patches land into to build OOTB.
> A middle ground solution is to allow build-time selection of the setting of
> WIN32. Something like:
> cmake /path/to/src -DCYGWIN_NO_WIN32=1
Again, giving authors the choice of "defining" what Cygwin is and isn't is asking for trouble. Would the command-line option be overridden by an explicit set(CYGWIN_NO_WIN32 0) in a CMakeLists.txt? |
|
|
|
(0019635)
|
|
Bill Hoffman
|
|
2010-02-25 16:59
|
|
> I'm not sure what you mean by "getting them to build under Cygwin".
> Projects which currently use an explicit CYGWIN conditional, provided
> that they actually understand what Cygwin is (which is not always the
> case), should not be affected by unsetting WIN32 on Cygwin. What this
> affects are projects who have not taken Cygwin into account at all,
> or just assume that it should be lumped together with Win32, in which
> case they are currently broken and unsetting WIN32 *fixes* them (or
> at least gets us much closer).
There are projects that use if(WIN32 AND UNIX) to test for Cygwin in CMake files. Those projects would be broken. We created the policies to address the problem of CMake not breaking peoples projects.
I realize this is not exactly what you want, but with these changes it should pretty easy to port to cygwin. Just change the policy set, or the version of CMake required. And in the long term (1 year from now), it will be exactly as you want it. |
|
|
|
(0019639)
|
|
Yaakov (Cygwin Ports)
|
|
2010-02-25 22:01
|
|
|
What is the timeframe for a CMake release including all these changes? |
|
|
|
(0019644)
|
|
Brad King
|
|
2010-02-26 09:44
|
|
It would be in 2.8.2 at the earliest (we are accepting regression-only fixes into the 2.8.1-rc* series currently). That probably won't be out for several months.
Meanwhile you can tweak projects in question by setting CMAKE_MODULE_PATH to point at a directory in which you've created a custom Platform/CYGWIN.cmake file that includes ${CMAKE_ROOT}/Modules/Platform/CYGWIN.cmake and then adds
set(WIN32 0)
This should work with CMake 2.4 and higher. |
|
|
|
(0020833)
|
|
Yaakov (Cygwin Ports)
|
|
2010-05-26 22:19
|
|
Updated patch for 2.8.1 uploaded. The CMAKE_EXE_EXPORTS_*_FLAG part is critical and should be non-controversial.
Bill, the distro cmake needs an update with this patch, and cmake-gui can also be added (as a separate binary package, due to its extra Qt4 dependencies). What is your timeline for this? |
|
|
|
(0020834)
|
|
Bill Hoffman
|
|
2010-05-27 09:02
|
|
|
How can cmake-gui be added? I did not think there was a way to build qt4 on cygwin? |
|
|
|
(0020844)
|
|
Yaakov (Cygwin Ports)
|
|
2010-05-27 13:40
|
|
|
|
|
(0020846)
|
|
Brad King
|
|
2010-05-27 14:50
|
|
|
|
|
(0021099)
|
|
Yaakov (Cygwin Ports)
|
|
2010-06-20 18:04
|
|
|
I was thinking of alternative solutions to the module naming issue. Are there other platforms where CMake's library/module naming does not match that of libtool? |
|
|
|
(0021103)
|
|
Brad King
|
|
2010-06-21 09:22
|
|
I don't know of any other platforms off the top of my head, but since libtool once used 'lib' on cygwin and now uses 'cyg' it could have changed on any number of platforms since CMake support was written for them.
BTW, I made a change to KWSys to simplify the lib->cyg patch:
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=3f929475 [^]
Now only the hunk
-SET(CMAKE_SHARED_MODULE_PREFIX "lib")
+SET(CMAKE_SHARED_MODULE_PREFIX "cyg")
is left for that part. This is the hunk that can break existing project releases. |
|
|
|
(0023872)
|
|
Bill Hoffman
|
|
2010-12-10 13:01
|
|
|
Is the only issue left here to remove setting WIN32 on cygwin? |
|
|
|
(0023889)
|
|
Yaakov (Cygwin Ports)
|
|
2010-12-10 14:30
|
|
|
|
|
(0023969)
|
|
Brad King
|
|
2010-12-14 15:48
|
|
|
|
|
(0024243)
|
|
Brad King
|
|
2010-12-17 14:49
|
|
I've implemented the WIN32=0 behavior. The main commit is
http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=85c0a69a [^]
A more complete set of commits leading to this is
$ git log --graph aadee46c..85c0a69a
The default is now WIN32=0 out of the box, which breaks compatibility. In order to facilitate the transition CMake will now warn on Cygwin about the change in behavior for *all* projects that do not have
cmake_minimum_required(VERSION 2.8.4) # or greater
One may set CMAKE_LEGACY_CYGWIN_WIN32=0 or CMAKE_LEGACY_CYGWIN_WIN32=1 in the environment, CMake cache, or as a local variable to choose new or old behavior without a warning. Projects that do require 2.8.4 or greater will get WIN32=0 regardless of the legacy option, so this is not a WIN32 on/off switch. |
|
|
|
(0024505)
|
|
David Cole
|
|
2011-01-07 09:48
|
|
Yaakov, I am resolving this issue as fixed. It's been fixed and available in CMake's 'next' and 'master' branches for some days (or weeks) now.
We hope that this makes CMake 2.8.4 acceptable for use out-of-the-box at this point w.r.t. cygwin builds.
If you have any further concerns or comments to add to this issue, feel free to re-open it and add them directly here.
I'm resolving it now because to the best of our knowledge, it is now fully resolved, and I'd like to mark it as such on the Roadmap and Change Log pages here in Mantis.
Thanks. |
|
|
|
(0026322)
|
|
David Cole
|
|
2011-05-02 14:45
|
|
|
Closing resolved issues that have not been updated in more than 3 months. |
|
2.8-cygwin.patch (6,549) 2010-01-12 04:14