Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alpine: Fix build with Xcode 16/clang 16 (macOS 15) #25819

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

markmentovai
Copy link
Contributor

Alpine’s configure script contains a check to detect the parameter type accepted by qsort’s comparison function. It does this by doing a trial compilation of a program with a comparison function that accepts two void* parameters, which is the modern form specified by the standard, and checking whether compilation succeeded. If compilation fails, it assumes the historic pre-standard form, which uses char* parameters, and configures Alpine accordingly.

The program for the test comparison is written in an archaic style that, among other things, does not explicitly declare the comparison function’s return type. This can be the subject of a compiler warning, -Wimplicit-int.

Clang 16, included in Xcode 16, enables -Werror=implicit-int by default[1], as does GCC 14[2]. Because this condition is now an error, the trial compilation began failing under these compilers, causing Alpine to be configured to use char* parameters in qsort comparison functions. Because this does not match qsort’s actual declaration, where comparison functions are expected to use void* parameters, this would result in a build failure.

The program used to detect the proper type to use for qsort’s comparison functions is updated to avoid the -Wimplicit-int problem.

[1] https://releases.llvm.org/16.0.0/tools/clang/docs/ReleaseNotes.html#potentially-breaking-changes
[2] https://gcc.gnu.org/gcc-14/porting_to.html#warnings-as-errors

Description

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 15.0 24A335 arm64
Xcode 16.0 16A242d

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL in commit message?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

Alpine’s configure script contains a check to detect the parameter type
accepted by qsort’s comparison function. It does this by doing a trial
compilation of a program with a comparison function that accepts two
void* parameters, which is the modern form specified by the standard,
and checking whether compilation succeeded. If compilation fails, it
assumes the historic pre-standard form, which uses char* parameters, and
configures Alpine accordingly.

The program for the test comparison is written in an archaic style that,
among other things, does not explicitly declare the comparison
function’s return type. This can be the subject of a compiler warning,
-Wimplicit-int.

Clang 16, included in Xcode 16, enables -Werror=implicit-int by
default[1], as does GCC 14[2]. Because this condition is now an error,
the trial compilation began failing under these compilers, causing
Alpine to be configured to use char* parameters in qsort comparison
functions. Because this does not match qsort’s actual declaration, where
comparison functions are expected to use void* parameters, this would
result in a build failure.

The program used to detect the proper type to use for qsort’s comparison
functions is updated to avoid the -Wimplicit-int problem.

[1] https://releases.llvm.org/16.0.0/tools/clang/docs/ReleaseNotes.html#potentially-breaking-changes
[2] https://gcc.gnu.org/gcc-14/porting_to.html#warnings-as-errors
@macportsbot
Copy link

Notifying maintainers:
@jerryyhom for port alpine.
@jcvernaleo for port alpine.

@macportsbot macportsbot added type: bugfix maintainer: open Affects an openmaintainer port labels Sep 18, 2024
Copy link
Contributor

@jcvernaleo jcvernaleo left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Thanks!

OK

@jerryyhom
Copy link
Contributor

Please submit patch to upstream for everyone's benefit.

@markmentovai
Copy link
Contributor Author

markmentovai commented Sep 19, 2024

Should have said so, but this patch came from upstream, from the master branch.

https://repo.or.cz/alpine.git/commit/0c44e2ae8b861e06139cfc6377cc59db0f12f781

alpinemail/alpine@0c44e2a

@reneeotten
Copy link
Contributor

@markmentovai I guess this patch is actually correct on all systems and perhaps should be picked up also when a build previously succeeded? If so, as you know, one should increase the "revision" - or is there a reason not do so here?

@markmentovai
Copy link
Contributor Author

@reneeotten there's no need to bump the revision. When attempting to build with compilers that need this patch (clang ≥ 16, GCC ≥ 14), if this patch is not applied, the configure step will misconfigure the build such that the build step won't be able to complete successfully.

If this patch is used with a compiler that doesn't strictly need it (clang < 16, GCC < 14), there won't be any difference to how the configure step configures the build, and there won't be any difference in the output of the build step. Under these circumstances, no revision bump is necessary (or desirable).

So the patch is correct on all systems, has no impact on the build output, and only serves to overcome a warning that's been upgraded to an error in clang 16 (thus Xcode 16 and macOS 15).

@reneeotten reneeotten merged commit 091c14f into macports:master Sep 20, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer: open Affects an openmaintainer port type: bugfix
Development

Successfully merging this pull request may close these issues.

5 participants