Skip to content

build: fix mac build on xcode 12#17393

Closed
ramaraochavali wants to merge 3 commits intoenvoyproxy:mainfrom
ramaraochavali:fix/mac_build
Closed

build: fix mac build on xcode 12#17393
ramaraochavali wants to merge 3 commits intoenvoyproxy:mainfrom
ramaraochavali:fix/mac_build

Conversation

@ramaraochavali
Copy link
Copy Markdown
Contributor

Commit Message: fix mac build on xcode 12
Additional Description:
Build with Xcode 12 on Mac fails with the following error as abseil while constructing map uses non reference range loop.


source/extensions/filters/network/postgres_proxy/postgres_decoder.cc:502:19: error: loop variable 'sp' of type 'const absl::string_view' creates a copy from type 'const absl::string_view' [-Werror,-Wrange-loop-analysis]
  for (const auto sp : splitter) {
                  ^
source/extensions/filters/network/postgres_proxy/postgres_decoder.cc:502:8: note: use reference type 'const absl::string_view &' to prevent copying
  for (const auto sp : splitter) {
       ^~~~~~~~~~~~~~~

Risk Level: Low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
@PiotrSikora
Copy link
Copy Markdown
Contributor

FWIW, I believe this is a false-positive (see: https://reviews.llvm.org/D73007), so adding -Wno-range-loop-analysis on macOS might be better than this expensive fix.

@codefromthecrypt
Copy link
Copy Markdown
Contributor

codefromthecrypt commented Jul 19, 2021

see also #13184 as it is similar

@codefromthecrypt
Copy link
Copy Markdown
Contributor

Homebrew/homebrew-core#81490 will try to work around this until merged as currently homebrew is busted

@htuch
Copy link
Copy Markdown
Member

htuch commented Jul 20, 2021

@ramaraochavali what is the status of this? Should we close or revise as per comments?

@ramaraochavali
Copy link
Copy Markdown
Contributor Author

I am fine with the workaround of removing -Wrange-loop-analysis for mac. Is this the right place to disable

"-Wrange-loop-analysis",
?

Is that fine with every one @rebello95 ?

@codefromthecrypt
Copy link
Copy Markdown
Contributor

project team: (for lack of knowing how escalations work @mattklein123 )

I don't know how severity works here, but this should be a dev 1. only thing worse would be all platforms unable to build (dev 0). That os/x doesn't build due to this means no one can upgrade homebrew to envoy 1.19. To understand the full context is basically knowing that many projects rely on github actions, who have finally stopped installing ancient xcode.

Note: this is also hurting the homebrew project as it is causing broken builds on the existing versions. In other words envoy's config is causing stalls on unrelated forumulas such as go. Can someone with deep knowledge on the build please pitch in and resolve this!

@mattklein123 mattklein123 self-assigned this Jul 20, 2021
@mattklein123
Copy link
Copy Markdown
Member

@ramaraochavali yes let's just disable range-loop-analysis on macOS. cc @keith or @lizan for help on how to do that or just ask them in the #bazel slack room. Thank you!

/wait

@keith
Copy link
Copy Markdown
Member

keith commented Jul 20, 2021

#17428

keith added a commit to keith/envoy that referenced this pull request Jul 20, 2021
The version of clang that ships with Xcode 12 has false positives with
this warning that might be fixed by https://reviews.llvm.org/D73007

In the meantime we can disable it entirely as discussed on
envoyproxy#17393

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@mattklein123
Copy link
Copy Markdown
Member

Thanks Keith! Closing this one.

@ramaraochavali ramaraochavali deleted the fix/mac_build branch July 21, 2021 03:19
@ramaraochavali
Copy link
Copy Markdown
Contributor Author

makes sense. Will look forward for the other PR getting merged :-)

snowp pushed a commit that referenced this pull request Jul 21, 2021
The version of clang that ships with Xcode 12 has false positives with
this warning that might be fixed by https://reviews.llvm.org/D73007

In the meantime we can disable it entirely as discussed on
#17393

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
The version of clang that ships with Xcode 12 has false positives with
this warning that might be fixed by https://reviews.llvm.org/D73007

In the meantime we can disable it entirely as discussed on
envoyproxy#17393

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants