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

Revert "Revert "bazel: update DEPS on googleurl (#17794)" (#17958)" #18192

Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 79 additions & 17 deletions bazel/external/googleurl.patch
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,22 @@
# project using clang-cl. Tracked in https://github.com/envoyproxy/envoy/issues/11974.

diff --git a/base/compiler_specific.h b/base/compiler_specific.h
index 0cd36dc..8c4cbd4 100644
index 6651220..a469c19 100644
--- a/base/compiler_specific.h
+++ b/base/compiler_specific.h
@@ -7,10 +7,6 @@

#include "build/build_config.h"

-#if defined(COMPILER_MSVC) && !defined(__clang__)
-#error "Only clang-cl is supported on Windows, see https://crbug.com/988071"
-#endif
-
// Annotate a variable indicating it's ok if the variable is not used.
// (Typically used to silence a compiler warning when the assignment
// is important for some other reason.)
@@ -55,8 +51,12 @@
// prevent code folding, see gurl_base::debug::Alias.
// This is a wrapper around `__has_cpp_attribute`, which can be used to test for
// the presence of an attribute. In case the compiler does not support this
// macro it will simply evaluate to 0.
@@ -75,8 +71,12 @@
// prevent code folding, see NO_CODE_FOLDING() in base/debug/alias.h.
// Use like:
// void NOT_TAIL_CALLED FooBar();
-#if defined(__clang__) && __has_attribute(not_tail_called)
Expand All @@ -30,18 +30,18 @@ index 0cd36dc..8c4cbd4 100644
#else
#define NOT_TAIL_CALLED
#endif
@@ -226,7 +226,9 @@
@@ -273,7 +273,9 @@
#endif
#endif

-#if defined(__clang__) && __has_attribute(uninitialized)
+#if defined(__clang__)
+#if defined(__has_attribute)
+#if __has_attribute(uninitialized)
// Attribute "uninitialized" disables -ftrivial-auto-var-init=pattern for
// the specified variable.
// Library-wide alternative is
@@ -257,6 +259,8 @@
@@ -304,6 +306,8 @@
// E.g. platform, bot, benchmark or test name in patch description or next to
// the attribute.
#define STACK_UNINITIALIZED __attribute__((uninitialized))
Expand All @@ -50,13 +50,74 @@ index 0cd36dc..8c4cbd4 100644
#else
#define STACK_UNINITIALIZED
#endif

# TODO(dio): Consider to remove the following patch when we have IDN-free optional build for URL
# library from the upstream Chromium project. This is tracked in:
# https://github.com/envoyproxy/envoy/issues/14743.

@@ -365,8 +369,12 @@ inline constexpr bool AnalyzerAssumeTrue(bool arg) {
#endif // defined(__clang_analyzer__)

// Use nomerge attribute to disable optimization of merging multiple same calls.
-#if defined(__clang__) && __has_attribute(nomerge)
+#if defined(__clang__)
+#if defined(__has_attribute)
+#if __has_attribute(nomerge)
#define NOMERGE [[clang::nomerge]]
+#endif
+#endif
#else
#define NOMERGE
#endif
@@ -392,8 +400,12 @@ inline constexpr bool AnalyzerAssumeTrue(bool arg) {
// See also:
// https://clang.llvm.org/docs/AttributeReference.html#trivial-abi
// https://libcxx.llvm.org/docs/DesignDocs/UniquePtrTrivialAbi.html
-#if defined(__clang__) && __has_attribute(trivial_abi)
+#if defined(__clang__)
+#if defined(__has_attribute)
+#if __has_attribute(trivial_abi)
#define TRIVIAL_ABI [[clang::trivial_abi]]
+#endif
+#endif
#else
#define TRIVIAL_ABI
#endif
@@ -401,8 +413,12 @@ inline constexpr bool AnalyzerAssumeTrue(bool arg) {
// Marks a member function as reinitializing a moved-from variable.
// See also
// https://clang.llvm.org/extra/clang-tidy/checks/bugprone-use-after-move.html#reinitialization
-#if defined(__clang__) && __has_attribute(reinitializes)
+#if defined(__clang__)
+#if defined(__has_attribute)
+#if __has_attribute(reinitializes)
#define REINITIALIZES_AFTER_MOVE [[clang::reinitializes]]
+#endif
+#endif
#else
#define REINITIALIZES_AFTER_MOVE
#endif
diff --git a/base/containers/checked_iterators.h b/base/containers/checked_iterators.h
index b5fe925..553a772 100644
--- a/base/containers/checked_iterators.h
+++ b/base/containers/checked_iterators.h
@@ -217,7 +217,7 @@ using CheckedContiguousConstIterator = CheckedContiguousIterator<const T>;

} // namespace base

-#if defined(_LIBCPP_VERSION) && !defined(OS_NACL)
+#if defined(_LIBCPP_VERSION) && !defined(OS_NACL) && !defined(SKIP_CONTIGUOUS_ITERATOR_CPP17)
// Specialize both std::__is_cpp17_contiguous_iterator and std::pointer_traits
// for CCI in case we compile with libc++ outside of NaCl. The former is
// required to enable certain algorithm optimizations (e.g. std::copy can be a
@@ -237,10 +237,6 @@ using CheckedContiguousConstIterator = CheckedContiguousIterator<const T>;
// [3] https://wg21.link/pointer.traits.optmem
namespace std {

-template <typename T>
-struct __is_cpp17_contiguous_iterator<::gurl_base::CheckedContiguousIterator<T>>
- : true_type {};
-
Copy link
Author

Choose a reason for hiding this comment

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

Deleted. Only difference between #17958

Copy link
Member

Choose a reason for hiding this comment

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

Any more info on this deletion? What impact does it have / are we tracking fixing this upstream somehow?

Copy link
Member

Choose a reason for hiding this comment

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

It fixes it since the android error is trying to specialize something that doesn't exist. I'm surprise this builds at all though, maybe it's just a performance optimization?

template <typename T>
struct pointer_traits<::gurl_base::CheckedContiguousIterator<T>> {
using pointer = ::gurl_base::CheckedContiguousIterator<T>;
diff --git a/url/BUILD b/url/BUILD
index f2ec8da..4e2d55b 100644
index f2ec8da..df69661 100644
--- a/url/BUILD
+++ b/url/BUILD
@@ -52,3 +52,27 @@ cc_library(
Expand Down Expand Up @@ -87,3 +148,4 @@ index f2ec8da..4e2d55b 100644
+ "//polyfills",
+ ]
+)

8 changes: 4 additions & 4 deletions bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -834,13 +834,13 @@ REPOSITORY_LOCATIONS_SPEC = dict(
project_name = "Chrome URL parsing library",
project_desc = "Chrome URL parsing library",
project_url = "https://quiche.googlesource.com/googleurl",
# Static snapshot of https://quiche.googlesource.com/quiche/+archive/ef0d23689e240e6c8de4c3a5296b209128c87373.tar.gz.
version = "ef0d23689e240e6c8de4c3a5296b209128c87373",
sha256 = "d769283fed1319bca68bae8bdd47fbc3a7933999329eee850eff1f1ea61ce176",
# Static snapshot of https://quiche.googlesource.com/quiche/+archive/561705e0066ff11e6cb97b8092f1547835beeb92.tar.gz.
version = "561705e0066ff11e6cb97b8092f1547835beeb92",
sha256 = "7ce00768fea1fa4c7bf658942f13e41c9ba30e9cff931a6cda2f9fd02289f673",
urls = ["https://storage.googleapis.com/quiche-envoy-integration/googleurl_{version}.tar.gz"],
use_category = ["controlplane", "dataplane_core"],
extensions = [],
release_date = "2020-07-30",
release_date = "2021-08-31",
cpe = "N/A",
),
com_google_cel_cpp = dict(
Expand Down
Binary file not shown.