Opened 6 months ago

Closed 6 months ago

Last modified 6 months ago

#15580 closed enhancement (fixed)

wxGTK: 'MAC_OS_X_VERSION_MAX_ALLOWED' is not defined, evaluates to 0

Reported by: mojca Owned by: vadz
Priority: low Milestone:
Component: wxGTK Version: 3.0.0-rc1
Keywords: Cc:
Blocked By: Blocking:
Patch: yes

Description

When compiling wxWidgets 3.0.0-rc1 against wxGTK on Mac OS X I get a lot of warnings like the following:

In file included from ../include/wx/string.h:45:
../include/wx/wxcrtbase.h:569:39: warning: 'MAC_OS_X_VERSION_MAX_ALLOWED' is not defined, evaluates to 0 [-Wundef]
    #if !defined(__WXOSX_IPHONE__) && MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_2

I didn't dive deep into the code and I'm not sure what the consequences are, but the following part in wxcrtbase.h and possibly other files is a bit problematic:

#ifdef __DARWIN__
    #if !defined(__WXOSX_IPHONE__) && MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_2
        #define wxNEED_WX_MBSTOWCS
    #endif
#endif

I don't know whether wxNEED_WX_MBSTOWCS is needed in the GTK-based build on Mac. In case it is, the second if should be different. In case it isn't, the first ifdef should be testing for __WXMAC__ rather than __DARWIN__.

I also find it weird to see references to version 10.2 given that wxWidgets now doesn't work on anything older than 10.5 anyway.

Complete build log can be found here (is case anyone is interested): https://build.macports.org/builders/buildports-lion-x86_64/builds/15069

Attachments (2)

patch-wcstombs.diff download (3.2 KB) - added by mojca 6 months ago.
patch-wxdl.diff download (7.0 KB) - added by mojca 6 months ago.
simplification by removing support for Mac OS X 10.2 and earlier

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 months ago by vadz

  • Milestone 3.0 deleted
  • Priority changed from normal to low

Yes, I agree with both points, we should indeed test for __WXMAC__ before using MAC_OS_X_VERSION_XXX and we should get rid of 10.2 checks completely. The only thing I disagree with is that it's 3.0-critical, wxGTK on OS X is really not a high enough priority. But, of course, any (not too complex/extensive) patches fixing this would still be very welcome.

comment:2 Changed 6 months ago by mojca

  • Patch set

I didn't do any testing yet, but based on comments in the sources here is what I imagine the patch could be: the attached patch gets rid of some complexity by simply removing the extra functions that were only used on Mac OS X 10.2 and earlier.

Changed 6 months ago by mojca

Changed 6 months ago by mojca

simplification by removing support for Mac OS X 10.2 and earlier

comment:3 Changed 6 months ago by mojca

The second patch (also untested) adds some more simplifications by removing support for 10.2.

comment:4 Changed 6 months ago by vadz

Thanks, I'm going to test and apply this.

comment:5 Changed 6 months ago by vadz

  • Owner set to vadz
  • Status changed from new to accepted

I've uploaded a patch combining the patches above with more of similar changes here, any testing would be very welcome, TIA!

comment:6 Changed 6 months ago by VZ

(In [75024]) Always use standard mbstowcs() and wcstombs() functions.

Don't provide our own (not fully functional) definitions of them and always
use the system versions as we don't support OS X 10.2 which was the last
platform where these functions didn't exist/work.

See #15580.

comment:7 Changed 6 months ago by VZ

(In [75025]) Remove our own dlxxx() functions emulation for OS X <= 10.3.

We don't support so old versions of OS X any more anyhow, so simplify the code
by using dlopen() &c directly instead of using our own wx_dlopen() with custom
implementation for OS X.

See #15580.

comment:8 Changed 6 months ago by VZ

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [75028]) Get rid of compile- and run-time checks for OS X < 10.5.

As 10.5 is the minimal supported version, it doesn't make sense to check for
it (or even earlier versions) during the build or run-time.

Closes #15580.

comment:9 Changed 6 months ago by SC

(In [75050]) CoreText is always available on 10.5+, so using all this code unconditionally, committing Vadim's suggestions with two extensions, see #15580

comment:10 Changed 6 months ago by mojca

After applying these patches I stumbled into other somewhat related problems:

warning: 'wxOSX_USE_COCOA_OR_IPHONE' is not defined, evaluates to 0 [-Wundef]

I opened a new ticket #15600.

In any case thank you very much for this huge cleanup.

Note: See TracTickets for help on using tickets.