Skip to content

Conversation

kvakvs
Copy link
Collaborator

@kvakvs kvakvs commented Sep 7, 2025

Problem

Compile errors appear when building wxWidgets on MacOS using .github/scripts/build-macos-wxwidgets.sh with latest Clang 17.

Solution

  1. Replaced via sed: #include <fp.h> to #include <math.h> in the wxWidgets/src/png/pngpriv.h
  2. Commented out via sed: #define fdopen in wxWidgets/src/zlib/zutil.h
  3. Minor changes (addressing issues found in sh file by shellcheck tool).

Other Solutions

The problem caused by the changes in standard library in Clang 17. Alternative solution is installing and using previous Clang 16 via Homebrew or somehow else.

@kvakvs kvakvs self-assigned this Sep 7, 2025
@kvakvs kvakvs added the team:PS Assigned to OTP team PS label Sep 7, 2025
Copy link
Contributor

github-actions bot commented Sep 7, 2025

CT Test Results

  1 files   11 suites   3m 52s ⏱️
 95 tests  91 ✅ 4 💤 0 ❌
111 runs  107 ✅ 4 💤 0 ❌

Results for commit 3b4ad7e.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@kvakvs kvakvs requested a review from kikofernandez September 7, 2025 01:44
@kvakvs kvakvs force-pushed the wx-png-compile branch 2 times, most recently from 0490806 to 07f47d8 Compare September 7, 2025 02:17
@IngelaAndin IngelaAndin requested a review from dgud September 8, 2025 06:31
kikofernandez
kikofernandez previously approved these changes Sep 8, 2025
Copy link
Contributor

@kikofernandez kikofernandez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change sounds good.
I would wait for @dgud input, since he is the main maintainer of the wx app

@dgud
Copy link
Contributor

dgud commented Sep 8, 2025

I don't like patching wxWidgets, a better variant would be to use system png and zlib there are configure flags for that.
if system libraries for png and zlib exists.

Copy link
Contributor

@IngelaAndin IngelaAndin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes a suggested by @dgud

@kvakvs
Copy link
Collaborator Author

kvakvs commented Sep 10, 2025

Now it is just one line of change, i kept the download bit, but commented, and the option i removed is also commented. Local build succeeded. Will see what the CI has to say.

@kvakvs
Copy link
Collaborator Author

kvakvs commented Sep 15, 2025

Apparently changing it to use system libraries and removing the patching did not work as expected, and there's fp.h include error again. Even if system libraries are possibly fixed, the build machine doesn't have them up to date.

@dgud
Copy link
Contributor

dgud commented Sep 15, 2025

It didn't use system libraries for png and jpeg :-(

configure: WARNING: system png library not found or too old, will use built-in instead
checking for png.h... (cached) no
checking whether png.c file exists... yes
configure: WARNING: system jpeg library not found, will use built-in instead
checking for jpeglib.h... no

@kvakvs kvakvs force-pushed the wx-png-compile branch 2 times, most recently from 45a01d1 to 01c119e Compare September 23, 2025 12:30
@dgud
Copy link
Contributor

dgud commented Sep 24, 2025

Rebase to maint and squash commits

@kvakvs
Copy link
Collaborator Author

kvakvs commented Sep 24, 2025

Source branch renamed, this now continues in #10239

@kvakvs kvakvs closed this Sep 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:PS Assigned to OTP team PS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants