Drop wxOSX/Cocoa 32 bit builds (PR #22495)

36 views
Skip to first unread message

VZ

unread,
Jun 6, 2022, 6:47:58 PM6/6/22
to wx-...@googlegroups.com, Subscribed

See #22227.


You can view, comment on, or merge this pull request online at:

  https://github.com/wxWidgets/wxWidgets/pull/22495

Commit Summary

  • ddd6b34 Drop support for 32-bit wxOSX/Cocoa build
  • 58354ea Remove Cocoa type declarations from wx/defs.h

File Changes

(10 files)

Patch Links:


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495@github.com>

VZ

unread,
Jun 6, 2022, 7:52:35 PM6/6/22
to wx-...@googlegroups.com, Push

@vadz pushed 1 commit.

  • 6211d53 Remove Cocoa type declarations from wx/defs.h


View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495/push/10087535452@github.com>

Stefan Csomor

unread,
Jun 8, 2022, 2:02:15 AM6/8/22
to wx-...@googlegroups.com, Subscribed

@vadz Thanks for the effort, but while we get rid of 32 bit on macOS, I'd rather want to keep CGFloat exposed, there still is some iOS Hardware around that runs iOS 12 with arm7v and watchOS also still has a partially 32 bit world. And __LP64__ should be there IMHO as well, but any macOS only code of course can get rid of ! __LP64__


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495/c1149497229@github.com>

VZ

unread,
Jun 8, 2022, 8:11:07 AM6/8/22
to wx-...@googlegroups.com, Subscribed

Sorry but why do you want to keep it exposed in public wx headers? Wouldn't any code using iOS hardware include the necessary (CoreGraphics?) headers anyhow? Typically we try to avoid polluting the global namespace with the platform-dependent symbols as much as possible to avoid bad surprises when recompiling the code on another platform, so it seems pretty strange to intentionally do it here.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495/c1149832954@github.com>

Stefan Csomor

unread,
Jun 8, 2022, 8:39:12 AM6/8/22
to wx-...@googlegroups.com, Subscribed

@vadz I see your point about the possible collision with another platform, but CGFloat wouldn't exist in the global space when not compiled on an Apple Platform anyway, or am I overlooking something ? being able to use native types in some public API helps writing cleaner code without having to always cast. But if having it in public hurts more, then so be it.
The non LP64 #defines should stay for every Apple platforms except macOS. This would lead to wrong code when compiled on one of the 32 bit platforms we still can run on.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495/c1149861239@github.com>

VZ

unread,
Jun 8, 2022, 8:49:17 AM6/8/22
to wx-...@googlegroups.com, Subscribed

@vadz I see your point about the possible collision with another platform, but CGFloat wouldn't exist in the global space when not compiled on an Apple Platform anyway,

But this is the danger -- suppose somebody uses CGFloat as an identifier in their own code (admittedly unlikely with this particular one, but in general we had plenty of problems like this). Their code would compile just fine under MSW and Linux but fail with tons of errors under Mac due to this identifier suddenly becoming a type.

But if having it in public hurts more, then so be it.

I don't think it's a huge problem in this case, CG is a relatively unique prefix, but I just don't see why would we want to do it, and what do we gain from it? Just CGFloat on its own is not going to be enough to do anything useful, anybody using it should include CG headers anyhow.

Also, if we keep the public CGFloat redeclaration, do we keep NS[U]Integer one too? Because this one seems even more fragile and I'd like to get rid of it even more.

The non LP64 #defines should stay for every Apple platforms except macOS. This would lead to wrong code when compiled on one of the 32 bit platforms we still can run on.

Sorry, do you mean those in include/wx/osx/config_xcode.h or something else?


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495/c1149872116@github.com>

Stefan Csomor

unread,
Jun 8, 2022, 10:05:52 AM6/8/22
to wx-...@googlegroups.com, Subscribed

Also, if we keep the public CGFloat redeclaration, do we keep NS[U]Integer one too? Because this one seems even more fragile and I'd like to get rid of it even more.

I've revived an old OS, compiling and running 32 bit iOS, works fine, so go ahead - except for below

The non LP64 #defines should stay for every Apple platforms except macOS. This would lead to wrong code when compiled on one of the 32 bit platforms we still can run on.

Sorry, do you mean those in include/wx/osx/config_xcode.h or something else?

yes, this file should remain unchanged

Thanks,

Stefan


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495/c1149965211@github.com>

Stefan Csomor

unread,
Jun 8, 2022, 10:18:43 AM6/8/22
to wx-...@googlegroups.com, Subscribed

@csomor requested changes on this pull request.


In include/wx/osx/config_xcode.h:

> -#ifdef __LP64__
 #define SIZEOF_VOID_P 8
 #define SIZEOF_LONG 8
 #define SIZEOF_SIZE_T 8
-#else
-#define SIZEOF_VOID_P 4
-#define SIZEOF_LONG 4
-#define SIZEOF_SIZE_T 4
-#endif

please leave this unchanged


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495/review/999852033@github.com>

VZ

unread,
Jun 8, 2022, 10:59:45 AM6/8/22
to wx-...@googlegroups.com, Subscribed

@vadz commented on this pull request.


In include/wx/osx/config_xcode.h:

> -#ifdef __LP64__
 #define SIZEOF_VOID_P 8
 #define SIZEOF_LONG 8
 #define SIZEOF_SIZE_T 8
-#else
-#define SIZEOF_VOID_P 4
-#define SIZEOF_LONG 4
-#define SIZEOF_SIZE_T 4
-#endif

Done, thanks.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495/review/999929602@github.com>

VZ

unread,
Jun 8, 2022, 11:00:55 AM6/8/22
to wx-...@googlegroups.com, Subscribed

Merged #22495 into master.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495/issue_event/6770756391@github.com>

Sergey Fedorov

unread,
Nov 7, 2023, 11:03:28 PM11/7/23
to wx-...@googlegroups.com, Subscribed

What was the point to deliberately break 32-bit builds on one system?


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495/c1800977464@github.com>

Lauri Nurmi

unread,
Nov 8, 2023, 3:50:57 AM11/8/23
to wx-...@googlegroups.com, Subscribed

What was the point to deliberately break 32-bit builds on one system?

If you read the commit messages of this pull request, and discussions in #22227, you can deduce that the builds were not deliberately broken; they got broken because of the lack of need and interest in testing 32-bit builds. As pointed out, the fixes needed wouldn't have been trivial, and nobody was interested enough to volunteer fixing the builds, or volunteer paying someone else to fix them.


Reply to this email directly, view it on GitHub, or unsubscribe.

You are receiving this because you are subscribed to this thread.Message ID: <wxWidgets/wxWidgets/pull/22495/c1801344119@github.com>

Reply all
Reply to author
Forward
0 new messages