Skip to content

osx support #5154

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 21, 2023
Merged

osx support #5154

merged 4 commits into from
Nov 21, 2023

Conversation

zirain
Copy link
Member

@zirain zirain commented Nov 17, 2023

this PR aim to make it possible to run make build on m1 mac pro

@zirain zirain requested a review from a team as a code owner November 17, 2023 06:35
@istio-testing istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 17, 2023
@zirain zirain added do-not-merge Block automatic merging of a PR. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 17, 2023
@istio-testing istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 17, 2023
@zirain
Copy link
Member Author

zirain commented Nov 17, 2023

/retest

kyessenov
kyessenov reviewed Nov 17, 2023
View reviewed changes
.bazelrc
@@ -14,7 +14,7 @@ build:remote --remote_timeout=7200
# ========================================

# Enable libc++ and C++20 by default.
build --config=libc++20
build:linux --config=libc++20
Copy link
Contributor

Choose a reason for hiding this comment

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

why disable on macos? we have c++20 code already I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

clang: error: invalid linker name in argument '-fuse-ld=lld'

Copy link
Contributor

Choose a reason for hiding this comment

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

you should use clang's lld on macos? what linker are you using?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the default one setting xcode.

I won't encourage anyone use proxy on macos for production, all these lines make that easier to develop on macos.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have no problems with allowing macos development. Just want to make sure we're not breaking release on linux.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do my best to make it won't happen, and there's a long time before next release, so I think it's pretty safe.

kyessenov
kyessenov reviewed Nov 17, 2023
View reviewed changes
.bazelrc
@@ -25,7 +25,8 @@ build --define path_normalization_by_default=true

# Heap profiler is supported only with gperf tcmalloc, not google tcmalloc.
# See: https://github.com/istio/istio/issues/28233
build --define tcmalloc=gperftools
build:linux --define tcmalloc=gperftools
build:macos --define tcmalloc=disabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Same - why disable gperftools?

Copy link
Member Author

Choose a reason for hiding this comment

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

always fails with following error:

configure: error: in `/private/var/tmp/_bazel_zirain/351f889f26744009d08525dc5ecb1daa/sandbox/darwin-sandbox/371/execroot/io_istio_proxy/bazel-out/darwin_arm64-fastbuild/bin/external/envoy/bazel/foreign_cc/gperftools_build.build_tmpdir':
configure: error: C compiler cannot create executables

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, it's fine but you won't get heap profile and some tools. what does tcmalloc disabled mean? what malloc is it using?

Copy link
Member Author

@zirain zirain Nov 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

envoy has same line, this's a known limitation. https://github.com/envoyproxy/envoy/blob/05457bb047b87f4cc1a25635400ddcaa7fccc923/.bazelrc#L138

I won't encourage anyone use proxy on macos for production, all these line make that easier to develop on macos.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't recall, does linux configuration automatically kick-in?

Copy link
Member Author

Choose a reason for hiding this comment

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

if not, there must be huge problem.

@zirain
Copy link
Member Author

zirain commented Nov 20, 2023

/retest

@zirain zirain removed the do-not-merge Block automatic merging of a PR. label Nov 20, 2023
@istio-testing istio-testing merged commit 3c71988 into istio:master Nov 21, 2023
@zirain zirain deleted the osx-support branch November 21, 2023 02:24
@jcetkov
Copy link

jcetkov commented Nov 28, 2023

@zirain the mac build is failing for me with

./lib/findprog-in.c:137:25: error: call to undeclared function 'eaccess'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                    if (eaccess (progpathname, X_OK) == 0)
                        ^
./lib/findprog-in.c:137:25: note: did you mean 'access'?
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/unistd.h:431:6: note: 'access' declared here
int      access(const char *, int);
         ^
./lib/findprog-in.c:211:21: error: call to undeclared function 'eaccess'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                if (eaccess (progpathname, X_OK) == 0)

seems to be related to bazel-contrib/rules_foreign_cc#859, however I'm not very proficient neither with bazel nor with c, would you have any tips how to work around this?

Seems to be some fairly recent change (bazel version change?) as I was able to build the proxy @ 1.17.8, but it's failing at HEAD as well as @1.19.4 tag.

Thank you!

@zirain
Copy link
Member Author

zirain commented Nov 28, 2023

I test these only for master on m1 mac pro, I will give a try when I have bandwidth.

@jcetkov
Copy link

jcetkov commented Nov 28, 2023

thanks! this is also m1 mac pro (ventura 13.6)

@zirain
Copy link
Member Author

zirain commented Nov 30, 2023

@jcetkov I can build release-1.19 on m1 macpro(Sonoma 14.1.1) with following change:

diff --git a/.bazelrc b/.bazelrc
index 210fd1d3..5c81f338 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -22,7 +22,8 @@ build --define path_normalization_by_default=true
 
 # Heap profiler is supported only with gperf tcmalloc, not google tcmalloc.
 # See: https://github.com/istio/istio/issues/28233
-build --define tcmalloc=gperftools
+build:linux --define tcmalloc=gperftools
+build:macos --define tcmalloc=disabled
 
 # Build with embedded V8-based WebAssembly runtime.
 build --define wasm=v8
@@ -48,3 +49,9 @@ build --cxxopt -Wformat-security
 
 # Link pthread for flatbuffers
 build --host_linkopt=-pthread
+
+# get from https://github.com/Homebrew/homebrew-core/blob/master/Formula/e/envoy.rb
+build:macos --cxxopt=-Wno-range-loop-analysis
+build:macos --host_cxxopt=-Wno-range-loop-analysis
+build:macos --cxxopt=-Wno-deprecated-declarations
+build:macos --host_cxxopt=-Wno-deprecated-declarations
diff --git a/bazel/extension_config/extensions_build_config.bzl b/bazel/extension_config/extensions_build_config.bzl
index 46565ffc..35ea09e4 100644
--- a/bazel/extension_config/extensions_build_config.bzl
+++ b/bazel/extension_config/extensions_build_config.bzl
@@ -470,7 +470,7 @@ ISTIO_ENABLED_CONTRIB_EXTENSIONS = [
     "envoy.filters.sip.router",
     "envoy.tls.key_providers.cryptomb",
     "envoy.tls.key_providers.qat",
-    "envoy.network.connection_balance.dlb",
+    # "envoy.network.connection_balance.dlb",
 ]
 
 EXTENSIONS = dict([(k,v) for k,v in ENVOY_EXTENSIONS.items() if not k in ISTIO_DISABLED_EXTENSIONS] +

for dlb issue, please take a look at envoyproxy/envoy#30951, which didn't backport.

@jcetkov
Copy link

jcetkov commented Dec 1, 2023 •
edited
Loading

@zirain I don't think this is the root of my problem, rather an update of the rules_foreign_cc in upstream envoy.
This change, which then makes me run into this

@codefromthecrypt codefromthecrypt mentioned this pull request Jan 23, 2025
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kyessenov kyessenov kyessenov approved these changes

Assignees
No one assigned
Labels
size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zirain @jcetkov @kyessenov @istio-testing