Skip to content

bazel: update rules_foreign_cc#17445

Merged
lizan merged 5 commits intoenvoyproxy:mainfrom
keith:ks/bazel-update-rules_foreign_cc
Aug 16, 2021
Merged

bazel: update rules_foreign_cc#17445
lizan merged 5 commits intoenvoyproxy:mainfrom
keith:ks/bazel-update-rules_foreign_cc

Conversation

@keith
Copy link
Member

@keith keith commented Jul 22, 2021

This update is required for this commit
bazel-contrib/rules_foreign_cc@c41020e
in order to support Apple Silicon. Since our last update there have been
some breaking API changes. I followed these instructions to migrate:

https://github.com/bazelbuild/rules_foreign_cc/releases/tag/0.3.0
https://github.com/bazelbuild/rules_foreign_cc/releases/tag/0.4.0

Signed-off-by: Keith Smiley keithbsmiley@gmail.com

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Jul 22, 2021
@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #17445 was opened by keith.

see: more, trace.

Copy link
Member Author

Choose a reason for hiding this comment

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

this was never overridden so I inlined it with the new name

Copy link
Member Author

Choose a reason for hiding this comment

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

Doing this in 1 string uses 1 make command like before, I'm not sure if it matters but I kept it the same

Copy link
Member Author

Choose a reason for hiding this comment

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

note there is a change here with this rm -f first because I was seeing this:

cp: /private/var/tmp/_bazel_ksmiley/4096f01a2eae006dd7eb0d708a9f3935/sandbox/darwin-sandbox/1854/execroot/envoy/bazel-out/darwin_arm64-fastbuild/bin/external/envoy/bazel/foreign_cc/ares/include/ares_dns.h and /private/var/tmp/_bazel_ksmiley/4096f01a2eae006dd7eb0d708a9f3935/sandbox/darwin-sandbox/1854/execroot/envoy/external/com_github_c_ares_c_ares/include/ares_dns.h are identical (not copied).

maybe this isn't needed on macOS or with the new version of rules_foreign_cc? either way it seems safe for now

@keith keith force-pushed the ks/bazel-update-rules_foreign_cc branch from e2957bf to d611317 Compare July 22, 2021 00:47
@wrowe
Copy link
Contributor

wrowe commented Jul 22, 2021

It's not immediately apparent what caused the regression here based on reviewing your patch diff, why exactly rules_foreign_cc was promoted to or lost from data-plane deps;

verifying dependencies...
Validating build dependency structure...
Validating test-only dependencies...
Validating data-plane dependencies...
Dependency validation failed, please check metadata in bazel/repository_locations.bzl
Observed dataplane core deps {'com_googlesource_code_re2', 'com_googlesource_googleurl', 'com_github_circonus_labs_libcircllhist', 'com_github_google_tcmalloc', 'opencensus_proto', 'io_bazel_rules_go', 'com_github_cyan4973_xxhash', 'com_github_jbeder_yaml_cpp', 'com_github_nlohmann_json', 'com_github_grpc_grpc', 'com_google_googleapis', 'boringssl_fips', 'com_github_zlib_ng_zlib_ng', 'com_github_nodejs_http_parser', 'com_github_nghttp2_nghttp2', 'com_googlesource_quiche', 'com_google_absl', 'rules_foreign_cc', 'com_github_c_ares_c_ares', 'com_github_gperftools_gperftools', 'com_google_protobuf', 'com_github_cncf_udpa', 'boringssl', 'com_github_tencent_rapidjson', 'com_envoyproxy_protoc_gen_validate', 'net_zlib', 'com_github_fmtlib_fmt', 'com_github_libevent_libevent', 'com_github_gabime_spdlog'} is not covered by "use_category" implied core deps {'bazel_skylib', 'com_github_openzipkin_zipkinapi', 'com_googlesource_code_re2', 'com_github_bazelbuild_buildtools', 'com_googlesource_googleurl', 'com_github_circonus_labs_libcircllhist', 'com_github_google_tcmalloc', 'opencensus_proto', 'prometheus_metrics_model', 'io_bazel_rules_go', 'com_github_cyan4973_xxhash', 'com_github_jbeder_yaml_cpp', 'com_github_nlohmann_json', 'com_github_grpc_grpc', 'boringssl_fips', 'com_google_googleapis', 'rules_proto', 'com_github_zlib_ng_zlib_ng', 'com_github_nghttp2_nghttp2', 'com_google_absl', 'com_googlesource_quiche', 'com_github_c_ares_c_ares', 'com_github_gperftools_gperftools', 'com_google_protobuf', 'com_github_cncf_udpa', 'boringssl', 'com_github_tencent_rapidjson', 'net_zlib', 'com_envoyproxy_protoc_gen_validate', 'com_github_nodejs_http_parser', 'com_github_libevent_libevent', 'com_github_gabime_spdlog', 'com_github_fmtlib_fmt', 'opentelemetry_proto'}: {'rules_foreign_cc'} are missing
##[error]Bash exited with code '1'.
Finishing: Verify dependency information

@moderation
Copy link
Contributor

I've been running the latest version in my environment for a while and took a slightly different approach. I didn't test for Windows. Posting here just in case it helps with your implementation.

bazel/dependency_imports.bzl

-load("@rules_foreign_cc//:workspace_definitions.bzl", "rules_foreign_cc_dependencies")                                           
+load("@rules_foreign_cc//foreign_cc:repositories.bzl", "rules_foreign_cc_dependencies")                                          
-    rules_foreign_cc_dependencies()                                                                          
+    rules_foreign_cc_dependencies(register_default_tools = False, register_built_tools = False)              
diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl                   
index a0463ab77..4bdbd742c 100644                                                          
--- a/bazel/envoy_build_system.bzl                                                         
+++ b/bazel/envoy_build_system.bzl                                                         
@@ -1,6 +1,6 @@                                                                            
 # The main Envoy bazel file. Load this file for all Envoy-specific build macros           
 # and rules that you'd like to use in your BUILD files.                                   
-load("@rules_foreign_cc//tools/build_defs:cmake.bzl", "cmake_external")                   
+load("@rules_foreign_cc//foreign_cc:cmake.bzl", "cmake")                                  
 load(":envoy_binary.bzl", _envoy_cc_binary = "envoy_cc_binary")                           
 load(":envoy_internal.bzl", "envoy_external_dep_path")                                    
 load(                                                                                     
@@ -87,7 +87,7 @@ envoy_directory_genrule = rule(                                          
                                                                                           
 # External CMake C++ library targets should be specified with this function. This defaults
 # to building the dependencies with ninja                                                 
-def envoy_cmake_external(                                                                 
+def envoy_cmake(                                                                          
         name,                                                                             
         cache_entries = {},                                                               
         debug_cache_entries = {},                                                         
@@ -123,7 +123,7 @@ def envoy_cmake_external(                                              
     else:                                                                                 
         pf = postfix_script                                                               
                                                                                           
-    cmake_external(                                                                       
+    cmake(                                                                                
         name = name,                                                                      
         cache_entries = select({                                                          
             "@envoy//bazel:dbg_build": cache_entries_debug,                               
diff --git a/bazel/foreign_cc/BUILD b/bazel/foreign_cc/BUILD
index b24046ea9..9ee119436 100644
--- a/bazel/foreign_cc/BUILD
+++ b/bazel/foreign_cc/BUILD
@@ -1,6 +1,6 @@
 load("@rules_cc//cc:defs.bzl", "cc_library")
-load("//bazel:envoy_build_system.bzl", "envoy_cmake_external", "envoy_package")
-load("@rules_foreign_cc//tools/build_defs:configure.bzl", "configure_make")
+load("//bazel:envoy_build_system.bzl", "envoy_cmake", "envoy_package")
+load("@rules_foreign_cc//foreign_cc:configure.bzl", "configure_make")
 
 licenses(["notice"])  # Apache 2
 
@@ -21,7 +21,7 @@ configure_make(
     lib_source = "@com_github_gperftools_gperftools//:all",
     linkopts = ["-lpthread"],
     make_commands = ["make install-libLTLIBRARIES install-perftoolsincludeHEADERS"],
-    static_libraries = select({
+    out_static_libs = select({
         "//bazel:debug_tcmalloc": ["libtcmalloc_debug.a"],
         "//conditions:default": ["libtcmalloc_and_profiler.a"],
     }),
@@ -52,7 +52,7 @@ configure_make(
     lib_source = "@com_github_luajit_luajit//:all",
     make_commands = [],
     out_include_dir = "include/luajit-2.1",
-    static_libraries = select({
+    out_static_libs = select({
         "//bazel:windows_x86_64": ["lua51.lib"],
         "//conditions:default": ["libluajit-5.1.a"],
     }),
@@ -72,11 +72,11 @@ configure_make(
     lib_source = "@com_github_moonjit_moonjit//:all",
     make_commands = [],
     out_include_dir = "include/moonjit-2.2",
-    static_libraries = ["libluajit-5.1.a"],
+    out_static_libs = ["libluajit-5.1.a"],
     tags = ["skip_on_windows"],
 )
 
-envoy_cmake_external(
+envoy_cmake(
     name = "ares",
     cache_entries = {
         "CARES_BUILD_TOOLS": "no",
@@ -91,17 +91,13 @@ envoy_cmake_external(
         "//bazel:apple": ["-lresolv"],
         "//conditions:default": [],
     }),
-    postfix_script = select({
-        "//bazel:windows_x86_64": "cp -L $EXT_BUILD_ROOT/external/com_github_c_ares_c_ares/src/lib/nameser.h $INSTALLDIR/include/nameser.h && cp -L $EXT_BUILD_ROOT/external/com_github_c_ares_c_ares/include/ares_dns.h $INSTALLDIR/include/ares_dns.h",
-        "//conditions:default": "cp -L $EXT_BUILD_ROOT/external/com_github_c_ares_c_ares/include/ares_dns.h $INSTALLDIR/include/ares_dns.h",
-    }),
-    static_libraries = select({
+    out_static_libs = select({
         "//bazel:windows_x86_64": ["cares.lib"],
         "//conditions:default": ["libcares.a"],
     }),
 )
 
-envoy_cmake_external(
+envoy_cmake(
     name = "curl",
     cache_entries = {
         "BUILD_CURL_EXE": "off",
@@ -142,7 +138,7 @@ envoy_cmake_external(
     defines = ["CURL_STATICLIB"],
     generate_crosstool_file = True,
     lib_source = "@com_github_curl//:all",
-    static_libraries = select({
+    out_static_libs = select({
         "//bazel:windows_x86_64": ["libcurl.lib"],
         "//conditions:default": ["libcurl.a"],
     }),
@@ -154,7 +150,7 @@ envoy_cmake_external(
     ],
 )
 
-envoy_cmake_external(
+envoy_cmake(
     name = "event",
     cache_entries = {
         "EVENT__DISABLE_OPENSSL": "on",
@@ -171,7 +167,7 @@ envoy_cmake_external(
         "_GNU_SOURCE": "on",
     },
     lib_source = "@com_github_libevent_libevent//:all",
-    static_libraries = select({
+    out_static_libs = select({
         # macOS organization of libevent is different from Windows/Linux.
         # Including libevent_core is a requirement on those platforms, but
         # results in duplicate symbols when built on macOS.
@@ -192,7 +188,7 @@ envoy_cmake_external(
     }),
 )
 
-envoy_cmake_external(
+envoy_cmake(
     name = "llvm",
     cache_entries = {
         # Disable both: BUILD and INCLUDE, since some of the INCLUDE
@@ -231,7 +227,7 @@ envoy_cmake_external(
         "ASMFLAGS": "-UDEBUG",
     },
     lib_source = "@org_llvm_llvm//:all",
-    static_libraries = select({
+    out_static_libs = select({
         "//conditions:default": [
             # Order from llvm-config --libnames asmparser core debuginfodwarf
             # engine lto mcparser mirparser orcjit passes runtimedyld
@@ -292,7 +288,7 @@ envoy_cmake_external(
     alwayslink = True,
 )
 
-envoy_cmake_external(
+envoy_cmake(
     name = "nghttp2",
     cache_entries = {
         "ENABLE_LIB_ONLY": "on",
@@ -305,13 +301,13 @@ envoy_cmake_external(
     debug_cache_entries = {"ENABLE_DEBUG": "on"},
     defines = ["NGHTTP2_STATICLIB"],
     lib_source = "@com_github_nghttp2_nghttp2//:all",
-    static_libraries = select({
+    out_static_libs = select({
         "//bazel:windows_x86_64": ["nghttp2.lib"],
         "//conditions:default": ["libnghttp2.a"],
     }),
 )
 
-envoy_cmake_external(
+envoy_cmake(
     name = "wamr",
     cache_entries = {
         "LLVM_DIR": "$EXT_BUILD_DEPS/copy_llvm/llvm/lib/cmake/llvm",
@@ -329,7 +325,7 @@ envoy_cmake_external(
     deps = [":llvm"],
 )
 
-envoy_cmake_external(
+envoy_cmake(
     name = "wavm",
     binaries = ["wavm"],
     cache_entries = {
@@ -349,7 +345,7 @@ envoy_cmake_external(
         "ASMFLAGS": "-UDEBUG",
     },
     lib_source = "@com_github_wavm_wavm//:all",
-    static_libraries = select({
+    out_static_libs = select({
         "//conditions:default": [
             "libWAVM.a",
             "libWAVMUnwind.a",
@@ -359,7 +355,7 @@ envoy_cmake_external(
     deps = [":llvm"],
 )
 
-envoy_cmake_external(
+envoy_cmake(
     name = "zlib",
     cache_entries = {
         "CMAKE_CXX_COMPILER_FORCED": "on",
@@ -392,7 +388,7 @@ envoy_cmake_external(
         "//bazel:zlib_ng": "@com_github_zlib_ng_zlib_ng//:all",
         "//conditions:default": "@net_zlib//:all",
     }),
-    static_libraries = select({
+    out_static_libs = select({
         "//bazel:windows_x86_64": ["zlib.lib"],
         "//conditions:default": ["libz.a"],
     }),

@wrowe wrowe added the waiting label Jul 28, 2021
@keith keith force-pushed the ks/bazel-update-rules_foreign_cc branch from d611317 to df48283 Compare August 4, 2021 23:04
@keith keith force-pushed the ks/bazel-update-rules_foreign_cc branch 3 times, most recently from 664651a to 0efbaeb Compare August 4, 2021 23:07
@keith
Copy link
Member Author

keith commented Aug 5, 2021

fix for the deps issue #17598

This update is required for this commit
bazel-contrib/rules_foreign_cc@c41020e
in order to support Apple Silicon. Since our last update there have been
some breaking API changes. I followed these instructions to migrate:

https://github.com/bazelbuild/rules_foreign_cc/releases/tag/0.3.0
https://github.com/bazelbuild/rules_foreign_cc/releases/tag/0.4.0

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith keith force-pushed the ks/bazel-update-rules_foreign_cc branch from b6f87da to 5976edc Compare August 8, 2021 19:44
keith added 4 commits August 9, 2021 17:58
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
Linux CI fails to build ninja, punting for now

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith keith force-pushed the ks/bazel-update-rules_foreign_cc branch from b95dae6 to 3742b8d Compare August 11, 2021 16:36
@keith
Copy link
Member Author

keith commented Aug 11, 2021

Yay green. @moderation can you take a look?

@moderation
Copy link
Contributor

/lgtm deps

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Aug 11, 2021
Copy link
Member

@rojkov rojkov left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!
@lizan could you please check and probably merge it?
/cc @wrowe

@wrowe
Copy link
Contributor

wrowe commented Aug 16, 2021

Reviewing Monday

@lizan lizan merged commit 5e48a19 into envoyproxy:main Aug 16, 2021
@keith keith deleted the ks/bazel-update-rules_foreign_cc branch August 17, 2021 17:53
mpuncel added a commit to mpuncel/envoy that referenced this pull request Aug 19, 2021
* main:
  Fix for fuzz tests failing due to invalid corpus paths (envoyproxy#17767)
  kafka: fix integration test (envoyproxy#17764)
  Fix typo in cluster.proto (envoyproxy#17755)
  cluster manager: add drainConnections() API (envoyproxy#17747)
  kafka broker filter: move to contrib (envoyproxy#17750)
  quiche: switch external dependency to github (envoyproxy#17732)
  quiche: implement stream idle timeout in codec (envoyproxy#17674)
  Update c-ares to 1.17.2 (envoyproxy#17704)
  Fix dns resolve fuzz bug (envoyproxy#17107)
  Remove members that shadow members of the base class (envoyproxy#17713)
  thrift proxy: missing parts from the previous PR (envoyproxy#17668)
  thrift-proxy: cleanup ConnectionManager::ActiveRpc (envoyproxy#17734)
  listener: extra warning for deprecated use_proxy_proto field (envoyproxy#17736)
  kafka: add support for metadata request in mesh-filter (envoyproxy#17597)
  upstream: add all host map to priority set for fast host searching (envoyproxy#17290)
  Remove the support for `hidden_envoy_deprecated_per_filter_config` (envoyproxy#17725)
  tls: SDS support for private key providers (envoyproxy#16737)
  bazel: update rules_foreign_cc (envoyproxy#17445)

Signed-off-by: Michael Puncel <mpuncel@squareup.com>
junr03 pushed a commit that referenced this pull request Aug 19, 2021
This reverts commit 5e48a19.

Signed-off-by: Jose Nino <jnino@lyft.com>
junr03 added a commit that referenced this pull request Aug 20, 2021
Signed-off-by: Jose Nino <jnino@lyft.com>
keith added a commit to keith/envoy that referenced this pull request Aug 21, 2021
…nvoyproxy#17780)"

This reverts commit 26dae5d.

This issue was fixed by bazel-contrib/rules_foreign_cc@da8952e

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
htuch pushed a commit that referenced this pull request Aug 26, 2021
…17799)

This reverts commit 26dae5d.

This issue was fixed by bazel-contrib/rules_foreign_cc@da8952e

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
keith added a commit to keith/envoy that referenced this pull request Aug 27, 2021
)" (envoyproxy#17780)" (envoyproxy#17799)"

This reverts commit 4b3f4a5.

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
mattklein123 pushed a commit that referenced this pull request Aug 27, 2021
)" (#17799)" (#17875)

This reverts commit 4b3f4a5.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants