MantisBT - CMake
View Issue Details
0010122CMakeCMakepublic2010-01-12 04:142012-03-19 09:21
Yaakov (Cygwin Ports) 
Brad King 
normalmajoralways
closedfixed 
CMake-2-8 
CMake 2.8.4CMake 2.8.4 
0010122: Patches for Cygwin
Currently CMake defines WIN32 on Cygwin. The problem with that is most CMake packages assume that WIN32 means MinGW or MSVC and don't consider Cygwin at all. This leads to mistaken assumptions, from using GDI instead of X11 to not including an install() command because there is no set location for anything in Windows.

I have built over 120 different source packages with (my patched) CMake, including most of KDE4, and have found that NOT defining WIN32 on Cygwin is much more accurate. Even the very few cases where WIN32 and CYGWIN would be handled similarly, this is nothing compared to the number of patches that would be required for every WIN32 AND NOT CYGWIN.

I'm attaching a patch for CMake 2.8 that removes the WIN32 define, creates DLLs with the Cygwin-specific "cyg" prefix as libtool does, and other related changes. I would be glad to answer any questions you have on this patch.
A few other patches, as well:

1) Fix the KDE3 module .la file to match a libtool-generated one:

http://cygwin-ports.svn.sourceforge.net/viewvc/cygwin-ports/ports/trunk/devel/cmake/2.6-KDE3Macros.patch [^]

2) On Cygwin, /bin is mounted to /usr/bin and /lib to /usr/lib, meaning their contents are identical. We still want to use /usr as the prefix, but this can confuse things. (This patch probably cannot be applied as is.)

http://cygwin-ports.svn.sourceforge.net/viewvc/cygwin-ports/ports/trunk/devel/cmake/2.8-usr-prefix.patch [^]

TIA for your consideration.
No tags attached.
related to 0010136closed Brad King CMake should prefer /usr over / 
related to 0010210closed Brad King CMake uses incorrect SONAME naming policy under Cygwin and MinGW standards 
related to 0013050closed Brad King FindOpenGL not working on Cygwin 
patch 2.8-cygwin.patch (6,549) 2010-01-12 04:14
https://public.kitware.com/Bug/file/2756/2.8-cygwin.patch
patch cygwin-no-win32-2010-01-13.patch (4,119) 2010-01-13 14:20
https://public.kitware.com/Bug/file/2760/cygwin-no-win32-2010-01-13.patch
patch cygwin-no-win32-2010-01-13-take2.patch (3,536) 2010-01-13 15:49
https://public.kitware.com/Bug/file/2761/cygwin-no-win32-2010-01-13-take2.patch
patch 2.8.1-cygwin.patch (2,652) 2010-05-26 22:10
https://public.kitware.com/Bug/file/3146/2.8.1-cygwin.patch
Issue History
2010-01-12 04:14Yaakov (Cygwin Ports)New Issue
2010-01-12 04:14Yaakov (Cygwin Ports)File Added: 2.8-cygwin.patch
2010-01-12 07:52Bill HoffmanNote Added: 0019095
2010-01-12 07:53Bill HoffmanStatusnew => assigned
2010-01-12 07:53Bill HoffmanAssigned To => Bill Hoffman
2010-01-12 13:45Yaakov (Cygwin Ports)Note Added: 0019101
2010-01-12 16:04Bill HoffmanNote Added: 0019103
2010-01-12 16:21Yaakov (Cygwin Ports)Note Added: 0019104
2010-01-12 16:33Bill HoffmanNote Added: 0019105
2010-01-13 13:00Brad KingNote Added: 0019116
2010-01-13 13:01Brad KingNote Added: 0019117
2010-01-13 13:12Brad KingNote Added: 0019118
2010-01-13 13:17Brad KingRelationship addedrelated to 0010136
2010-01-13 13:29Brad KingNote Added: 0019119
2010-01-13 13:30Brad KingNote Added: 0019120
2010-01-13 13:45Alex NeundorfNote Added: 0019122
2010-01-13 13:50Brad KingNote Added: 0019123
2010-01-13 13:50Alex NeundorfNote Added: 0019124
2010-01-13 13:55Brad KingNote Added: 0019125
2010-01-13 14:13Brad KingNote Added: 0019126
2010-01-13 14:20Brad KingFile Added: cygwin-no-win32-2010-01-13.patch
2010-01-13 14:22Brad KingNote Added: 0019127
2010-01-13 14:37Yaakov (Cygwin Ports)Note Added: 0019128
2010-01-13 14:40Brad KingNote Added: 0019129
2010-01-13 14:47Alex NeundorfNote Added: 0019130
2010-01-13 15:13Yaakov (Cygwin Ports)Note Added: 0019131
2010-01-13 15:34Yaakov (Cygwin Ports)Note Added: 0019132
2010-01-13 15:49Yaakov (Cygwin Ports)File Added: cygwin-no-win32-2010-01-13-take2.patch
2010-01-13 15:51Yaakov (Cygwin Ports)Note Added: 0019133
2010-01-13 16:55Yaakov (Cygwin Ports)Note Added: 0019134
2010-01-18 14:22Brad KingNote Added: 0019167
2010-01-18 14:39Yaakov (Cygwin Ports)Note Added: 0019168
2010-01-18 15:03Brad KingNote Added: 0019169
2010-01-18 15:24Yaakov (Cygwin Ports)Note Added: 0019170
2010-01-18 15:37Brad KingNote Added: 0019171
2010-01-18 15:39Brad KingNote Edited: 0019171
2010-01-18 15:46Yaakov (Cygwin Ports)Note Added: 0019172
2010-01-18 16:22Brad KingNote Added: 0019173
2010-01-18 20:40Yaakov (Cygwin Ports)Note Added: 0019176
2010-01-19 09:01Brad KingNote Added: 0019189
2010-01-21 13:28Yaakov (Cygwin Ports)Note Added: 0019244
2010-01-21 15:09Brad KingNote Added: 0019246
2010-01-21 18:47Yaakov (Cygwin Ports)Note Added: 0019255
2010-02-01 10:04Brad KingRelationship addedrelated to 0010210
2010-02-24 16:36Yaakov (Cygwin Ports)Note Added: 0019610
2010-02-25 14:05Brad KingNote Added: 0019619
2010-02-25 14:29Yaakov (Cygwin Ports)Note Added: 0019620
2010-02-25 15:28Brad KingNote Added: 0019625
2010-02-25 15:33Brad KingNote Added: 0019626
2010-02-25 16:28Yaakov (Cygwin Ports)Note Added: 0019633
2010-02-25 16:59Bill HoffmanNote Added: 0019635
2010-02-25 22:01Yaakov (Cygwin Ports)Note Added: 0019639
2010-02-26 09:44Brad KingNote Added: 0019644
2010-05-26 22:10Yaakov (Cygwin Ports)File Added: 2.8.1-cygwin.patch
2010-05-26 22:19Yaakov (Cygwin Ports)Note Added: 0020833
2010-05-27 09:02Bill HoffmanNote Added: 0020834
2010-05-27 13:40Yaakov (Cygwin Ports)Note Added: 0020844
2010-05-27 14:50Brad KingNote Added: 0020846
2010-06-20 18:04Yaakov (Cygwin Ports)Note Added: 0021099
2010-06-21 09:22Brad KingNote Added: 0021103
2010-12-10 09:32Bill HoffmanTarget Version => CMake 2.8.4
2010-12-10 13:01Bill HoffmanNote Added: 0023872
2010-12-10 14:30Yaakov (Cygwin Ports)Note Added: 0023889
2010-12-14 15:46Brad KingAssigned ToBill Hoffman => Brad King
2010-12-14 15:48Brad KingNote Added: 0023969
2010-12-17 14:49Brad KingNote Added: 0024243
2011-01-07 09:48David ColeNote Added: 0024505
2011-01-07 09:48David ColeStatusassigned => resolved
2011-01-07 09:48David ColeFixed in Version => CMake 2.8.4
2011-01-07 09:48David ColeResolutionopen => fixed
2011-05-02 14:45David ColeNote Added: 0026322
2011-05-02 14:45David ColeStatusresolved => closed
2012-03-19 09:21Brad KingRelationship addedrelated to 0013050

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   
This is still on my TODO list, sorry I haven't had time to get to it.

Currently the plan is to create a CMake Policy for each change in behavior (cyg prefix and !WIN32 separately). Read up on them here:

  http://www.cmake.org/Wiki/CMake_Policies [^]
  http://www.cmake.org/Wiki/CMake_Policies_Design_Discussion [^]
  http://www.cmake.org/cmake/help/cmake-2-8-docs.html#section_Policies [^]
  http://www.cmake.org/cmake/help/cmake-2-8-docs.html#command:cmake_policy [^]
(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   
Qt4 has been part of the Cygwin distro since last December:

http://cygwin.com/ml/cygwin-xfree-announce/2009-12/msg00003.html [^]
(0020846)
Brad King   
2010-05-27 14:50   
I've published the CMAKE_EXE_EXPORTS_*_FLAG part on next:

http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=cd3a4f00 [^]
(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   
WIN32 and CMAKE_SHARED_MODULE_PREFlX.

Patch for 2.8.3: http://cygwin-ports.git.sourceforge.net/git/gitweb.cgi?p=cygwin-ports/ports;a=blob;f=devel/cmake/2.8.1-cygwin.patch [^]

If changing CMAKE_SHARED_MODULE_PREFIX globally is a concern, then a possible alternative would be to introduce a glib_add_module() macro which would assure that module naming is compatible with libtool's (and hence GModule). Right now, I'm not aware of another platform whose naming doesn't match libtool's except for Cygwin.
(0023969)
Brad King   
2010-12-14 15:48   
Our other projects that depended on the old module prefix naming have been fixed.

CMAKE_SHARED_MODULE_PREFIX is now 'cyg':

http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=1dcc9777 [^]
(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.