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

macos: disable gcc availability workaround as needed #15508

Closed
wants to merge 10 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Nov 7, 2024

Homebrew gcc 14.2.0_1 fixed the issue, and the workaround is no longer
needed. Not only not needed, but the workaround is breaking builds with
the fixed gcc.

Auto-detect the upstream fix and stop applying the local workaround if
detected.

Assisted-by: Bo Anderson
Ref: Homebrew/homebrew-core#194778 (comment)
Follow-up to e91fcba #14155

@vszakats vszakats added build CI Continuous Integration appleOS specific to an Apple operating system labels Nov 7, 2024
@vszakats
Copy link
Member Author

vszakats commented Nov 7, 2024

@carlocab
Copy link

carlocab commented Nov 7, 2024

There is no way to detect if gcc is 14.2.0 or 14.2.0_1, so this patch drops the local workaround for both 14.2.0 releases.

You are probably referring to ways to check using the preprocessor, but, just to be sure, there is a way to do this if you're willing to do it at configure-time. For example:

❯ brew info --json=v2 gcc | jq '.formulae[].revision'
1

will allow you to check if you're dealing with 14.2.0_1 or just 14.2.0. But checking this is a bit overkill, as you point out.

@vszakats
Copy link
Member Author

vszakats commented Nov 7, 2024

Well, our CI steps around the affected combination by chance, hence the green results.

@vszakats
Copy link
Member Author

vszakats commented Nov 7, 2024

There is no way to detect if gcc is 14.2.0 or 14.2.0_1, so this patch drops the local workaround for both 14.2.0 releases.

You are probably referring to ways to check using the preprocessor, but, just to be sure, there is a way to do this if you're willing to do it at configure-time. For example:

❯ brew info --json=v2 gcc | jq '.formulae[].revision'
1

will allow you to check if you're dealing with 14.2.0_1 or just 14.2.0. But checking this is a bit overkill, as you point out.

Thanks for jumping in @carlocab! This could be a solution, but it seems
much work (and extra runtime in every future run) for the added benefit.

(AFAIR this command also touched the network by default, which
cause some issues in here: curl/curl-for-win@7e1f968
That patch seemed to have fixed it.)

It's probably fine to accept this small snag. curl doesn't seem to
have many gcc users on macOS and Homebrew will bring up most
to revision 1. (Unless they are stuck on Monterey and unwilling to
rebuild from source for this.)

@vszakats
Copy link
Member Author

vszakats commented Nov 7, 2024

I'm thinking to add macro(s) to force-disable (enable?) this hack. This adds an escape hatch for 14.2.0 and for other cases we didn't enticipate.

@vszakats vszakats changed the title macos: disable availability workaround for gcc 14.2.0+ macos: update availability workaround for gcc 14.2.0+ Nov 7, 2024
@vszakats
Copy link
Member Author

vszakats commented Nov 7, 2024

Updated with auto-detection following the suggestion of @Bo98. Thanks!

It checks out locally with 14.2.0 and with 14.2.0_1 in CI here:
https://github.com/curl/curl-for-win/actions/runs/11729083807/job/32673934382

Now I'm thinking to delete the manual overrides added earlier today.

@vszakats vszakats changed the title macos: update availability workaround for gcc 14.2.0+ macos: disable availability workaround as needed Nov 7, 2024
Homebrew gcc 14.2.0_1 fixed the issue, and the workaround is no longer
needed. Not only not needed, but the workaround breaks the fixed
version. There is no way to detect if gcc is 14.2.0 or 14.2.0_1, so
this patch drops the local workaround for both 14.2.0 releases.

It means the issue will happen with 14.2.0, in which case the solution
is to update to 14.2.0_1.

Sadly it doesn't seem possible to fix both 14.2.0 and 14.2.0_1. (Perhaps
unless doing an elaborate env detection, but that seems overkill for
this single transition.)

This issue did not yet hit CI, because the macos image is still coming
with 14.2.0, but will hit once it's bumped to 14.2.0_1.

Follow-up to e91fcba curl#14155
Thanks to Bo Anderson @Bo98 for the solution.
Linux gcc:
```
curl_setup.h:66:18: error: missing binary operator before token "("
   66 |    !__has_feature(attribute_availability))
      |                  ^
```
https://github.com/curl/curl/actions/runs/11729267034/job/32674506717?pr=15508#step:35:36

msvc:
```
lib\curl_setup.h(66,18): error C1012: unmatched parenthesis: missing ')'
```
https://github.com/curl/curl/actions/runs/11729267029/job/32674516200?pr=15508#step:9:20

clang compilers seem to be fine without this in all tested CI envs.
@vszakats vszakats force-pushed the mac-gcc-stop-avail-hack branch from d7e9864 to 3debd14 Compare November 8, 2024 09:53
@vszakats vszakats changed the title macos: disable availability workaround as needed macos: disable gcc availability workaround as needed Nov 8, 2024
@vszakats vszakats closed this in 354f3f9 Nov 8, 2024
@vszakats vszakats deleted the mac-gcc-stop-avail-hack branch November 8, 2024 10:16
vszakats added a commit to curl/curl-for-win that referenced this pull request Nov 8, 2024
…ci skip]

Enable ECH with AWS-LC in test and dev builds.

- ECH: enable support for the AWS-LC backend
  curl/curl@1cd745a
  curl/curl#15499

- macos: disable gcc availability workaround as needed
  curl/curl@354f3f9
  curl/curl#15508
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appleOS specific to an Apple operating system build CI Continuous Integration
Development

Successfully merging this pull request may close these issues.

2 participants