Opened 12 months ago

Last modified 8 months ago

#15600 confirmed enhancement

wxGTK on Mac: #if defined (__DARWIN__) should be replaced with __WXOSX___ at some places

Reported by: mojca Owned by:
Priority: low Milestone:
Component: wxGTK Version: 3.0.0
Keywords: Cc:
Blocked By: Blocking:
Patch: no

Description

When testing the patches for #15580 I stumbled across the following problem.

The following code in src/common/wxcrt.cpp is problematic for example:

#if defined(__DARWIN__)
    #include "wx/osx/core/cfref.h"
    #include <CoreFoundation/CFLocale.h>
    #include "wx/osx/core/cfstring.h"
    #include <xlocale.h>
#endif

and I believe that the following modification or something similar should be made:

--- a/src/common/wxcrt.cpp
+++ b/src/common/wxcrt.cpp
@@ -66,7 +66,7 @@
     #define wxSET_ERRNO(value) errno = value
 #endif
 
-#if defined(__DARWIN__)
+#if defined(__WXOSX__)
     #include "wx/osx/core/cfref.h"
     #include <CoreFoundation/CFLocale.h>
     #include "wx/osx/core/cfstring.h"

or maybe WXMAC (not sure which one should be used). A similar problem exists in src/common/utilscmn.cpp for example and possibly at other places as well. I need a bit more testing to see what else I'm missing.

The problem arises when compiling wxWidgets against GTK. Some other compile warnings are listed below:

In file included from ../src/common/wxcrt.cpp:72:
../include/wx/osx/core/cfstring.h:49:5: warning: 'wxOSX_USE_COCOA_OR_IPHONE' is not defined, evaluates to 0 [-Wundef]
#if wxOSX_USE_COCOA_OR_IPHONE
    ^

In file included from ../src/osx/core/mimetype.cpp:25:
In file included from ../include/wx/osx/private.h:4:
In file included from ../include/wx/osx/core/private.h:20:
../include/wx/osx/core/cfstring.h:49:5: warning: 'wxOSX_USE_COCOA_OR_IPHONE' is not defined, evaluates to 0 [-Wundef]
#if wxOSX_USE_COCOA_OR_IPHONE
    ^

In file included from ../src/osx/core/evtloop_cf.cpp:37:
In file included from ../include/wx/osx/private.h:4:
In file included from ../include/wx/osx/core/private.h:20:
../include/wx/osx/core/cfstring.h:49:5: warning: 'wxOSX_USE_COCOA_OR_IPHONE' is not defined, evaluates to 0 [-Wundef]
#if wxOSX_USE_COCOA_OR_IPHONE
    ^

In file included from ../src/osx/core/stdpaths_cf.cpp:32:
../include/wx/osx/core/cfstring.h:49:5: warning: 'wxOSX_USE_COCOA_OR_IPHONE' is not defined, evaluates to 0 [-Wundef]
#if wxOSX_USE_COCOA_OR_IPHONE
    ^

In file included from ../src/osx/core/utilsexc_base.cpp:41:
../include/wx/osx/core/cfstring.h:49:5: warning: 'wxOSX_USE_COCOA_OR_IPHONE' is not defined, evaluates to 0 [-Wundef]
#if wxOSX_USE_COCOA_OR_IPHONE
    ^

In file included from ../src/osx/core/utilsexc_base.cpp:42:
../include/wx/osx/core/private.h:51:22: warning: 'wxOSX_USE_IPHONE' is not defined, evaluates to 0 [-Wundef]
#if ( !wxUSE_GUI && !wxOSX_USE_IPHONE ) || wxOSX_USE_COCOA_OR_CARBON
                     ^

In file included from ../src/osx/core/mimetype.cpp:25:
../include/wx/osx/private.h:6:5: warning: 'wxOSX_USE_IPHONE' is not defined, evaluates to 0 [-Wundef]
#if wxOSX_USE_IPHONE
    ^

../src/common/utilscmn.cpp:562:5: warning: 'wxOSX_USE_COCOA_OR_CARBON' is not defined, evaluates to 0 [-Wundef]
#if wxOSX_USE_COCOA_OR_CARBON
    ^

In file included from ../src/osx/core/sockosx.cpp:20:
../include/wx/osx/core/cfstring.h:49:5: warning: 'wxOSX_USE_COCOA_OR_IPHONE' is not defined, evaluates to 0 [-Wundef]
#if wxOSX_USE_COCOA_OR_IPHONE
    ^

Not that this is a serious problem in practice since I can hardly imagine anyone wanting to use GTK instead of Cocoa, but this should be an easy fix. I'm testing this along with wxGTK 2.8 builds which are needed for programs that haven't been ported to 2.9/3.0 yet.

Change History (10)

comment:1 Changed 11 months ago by vadz

  • Version changed from 3.0.0-rc1 to 3.0.0

Just changing version as these tickets are still present in 3.0.0. final.

comment:2 Changed 8 months ago by vadz

  • Status changed from new to confirmed

I don't want to change this blindly, but if you could please make a patch replacing the wrong tests with the ones allowing wxGTK to build, I could definitely apply it. TIA!

comment:3 Changed 8 months ago by mojca

Just to be on the safe side: what is the difference between WXOSX and WXMAC?

comment:4 Changed 8 months ago by mojca

Also, what is the point in the following? Why/when is testing for DARWIN not sufficient?

./include/wx/atomic.h:#elif defined(__WXMAC__) || defined(__DARWIN__)

comment:5 Changed 8 months ago by csomor

__DARWIN__ is also true for the 'old' cocoa port __WXCOCOA__
__WXOSX__ is true for all 'new' osx ports __WXOSX_IPHONE__, __WXOSX_CARBON__, __WXOSX_COCOA__, __WXMAC__ is the same, left in for compatibility reaons

comment:6 Changed 8 months ago by vadz

There is no difference between __WXOSX__ and __WXMAC__ any more (since we don't support OS 9, for which the latter used to be defined but not the former). And __DARWIN__ is always defined under wxOSX too now since there are no other MacOS platforms any more.

comment:7 Changed 8 months ago by csomor

formerly wxMac supported also pre-darwin systems

comment:8 Changed 8 months ago by mojca

I checked, but I'm not too familiar with the code to be able to judge which instances of __DARWIN__ need to be fixed. I know this patch:

--- a/src/common/wxcrt.cpp
+++ b/src/common/wxcrt.cpp
@@ -66,7 +66,7 @@
     #define wxSET_ERRNO(value) errno = value
 #endif
 
-#if defined(__DARWIN__)
+#if defined(__WXOSX__)
     #include "wx/osx/core/cfref.h"
     #include <CoreFoundation/CFLocale.h>
     #include "wx/osx/core/cfstring.h"

for the rest I don't dare to change anything. Some remaining errors are related to compilation of files which should only be compiled for non-GTK builds, but I'm not even sure how to prevent the compilation of those files in the first place.

The same code as the patch above in WX_2_8_BRANCH is in src/common/intl.cpp.

comment:9 Changed 8 months ago by mojca

I have an impression that some files should not even be compiled on GTK-based installation, but I'm not sure how to fix or test that and whether that is true in the first place.

This is a complete list of related warnings:

In file included from ../wxWidgets/include/wx/osx/core/private.h:20,
                 from ../wxWidgets/include/wx/osx/private.h:4,
                 from ../wxWidgets/src/osx/core/mimetype.cpp:25:
../wxWidgets/include/wx/osx/core/cfstring.h:49:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:74:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:79:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
In file included from ../wxWidgets/include/wx/osx/private.h:4,
                 from ../wxWidgets/src/osx/core/mimetype.cpp:25:
../wxWidgets/include/wx/osx/core/private.h:51:22: warning: "wxOSX_USE_IPHONE" is not defined

In file included from ../wxWidgets/src/osx/core/cfstring.cpp:22:
../wxWidgets/include/wx/osx/core/cfstring.h:49:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:74:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:79:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/src/osx/core/cfstring.cpp:696:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined

In file included from ../wxWidgets/include/wx/osx/core/private.h:20,
                 from ../wxWidgets/include/wx/osx/private.h:4,
                 from ../wxWidgets/src/osx/core/evtloop_cf.cpp:37:
../wxWidgets/include/wx/osx/core/cfstring.h:49:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:74:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:79:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
In file included from ../wxWidgets/include/wx/osx/private.h:4,
                 from ../wxWidgets/src/osx/core/evtloop_cf.cpp:37:
../wxWidgets/include/wx/osx/core/private.h:51:22: warning: "wxOSX_USE_IPHONE" is not defined
In file included from ../wxWidgets/src/osx/core/evtloop_cf.cpp:37:
../wxWidgets/include/wx/osx/private.h:6:5: warning: "wxOSX_USE_IPHONE" is not defined
../wxWidgets/include/wx/osx/private.h:8:7: warning: "wxOSX_USE_CARBON" is not defined
../wxWidgets/include/wx/osx/private.h:10:7: warning: "wxOSX_USE_COCOA" is not defined
In file included from ../wxWidgets/src/osx/core/mimetype.cpp:25:
../wxWidgets/include/wx/osx/private.h:6:5: warning: "wxOSX_USE_IPHONE" is not defined
../wxWidgets/include/wx/osx/private.h:8:7: warning: "wxOSX_USE_CARBON" is not defined
../wxWidgets/include/wx/osx/private.h:10:7: warning: "wxOSX_USE_COCOA" is not defined

In file included from ../wxWidgets/src/osx/core/stdpaths_cf.cpp:32:
../wxWidgets/include/wx/osx/core/cfstring.h:49:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:74:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:79:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined

In file included from ../wxWidgets/src/osx/core/utilsexc_base.cpp:41:
../wxWidgets/include/wx/osx/core/cfstring.h:49:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:74:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:79:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
In file included from ../wxWidgets/src/osx/core/utilsexc_base.cpp:42:
../wxWidgets/include/wx/osx/core/private.h:51:22: warning: "wxOSX_USE_IPHONE" is not defined
../wxWidgets/src/osx/core/utilsexc_base.cpp:56:22: warning: "wxOSX_USE_IPHONE" is not defined

../wxWidgets/src/common/utilscmn.cpp:562:5: warning: "wxOSX_USE_COCOA_OR_CARBON" is not defined

In file included from ../wxWidgets/src/osx/core/sockosx.cpp:20:
../wxWidgets/include/wx/osx/core/cfstring.h:49:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:74:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined
../wxWidgets/include/wx/osx/core/cfstring.h:79:5: warning: "wxOSX_USE_COCOA_OR_IPHONE" is not defined

Another patch that I found based on these warnings is

--- a/src/common/utilscmn.cpp
+++ b/src/common/utilscmn.cpp
@@ -558,7 +558,7 @@ bool wxGetEnvMap(wxEnvVariableHashMap *map)
    // Now this routine wil give false for OpenVMS
    // TODO : should we do something with logicals?
     char **env=NULL;
-#elif defined(__DARWIN__)
+#elif defined(__WXOSX__)
 #if wxOSX_USE_COCOA_OR_CARBON
     // Under Mac shared libraries don't have access to the global environ
     // variable so use this Mac-specific function instead as advised by

but some other files should probably not be compiled at all, like src/osx/utilsexc_base.cpp for example, but probably others as well. I don't know how to properly remove those files, so that they don't end up in Makefile.in. I see build/bakefiles/files.bkl, but I don't know how to patch it properly.

Based on your other replies it looks as if the following should be fixed (no need to test for WXMAC):

./include/wx/atomic.h:#elif defined(__WXMAC__) || defined(__DARWIN__)

and WXMAC could probably be globally replaced by WXOSX? (Except for leaving it defined for compatibility reasons of course.) Currently it doesn't really look as if one was meant to be replaced by the other. They are both well represented in the code.

comment:10 Changed 8 months ago by SC

(In [76072]) cleanup, see #15600

Note: See TracTickets for help on using tickets.