Skip to content

bazel: update DEPS on googleurl #17794

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

Merged
wrowe merged 5 commits into from
Sep 1, 2021
Merged

bazel: update DEPS on googleurl #17794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f2124db
update DEPS on googleurl
RenjieTang Aug 20, 2021
0fd85f5
try to fix MSVC complier issue
RenjieTang Aug 30, 2021
5e81f1f
try to supress msvc 2397 warning
RenjieTang Aug 30, 2021
935c892
update to a later version
RenjieTang Sep 1, 2021
c13c88e
fix some MSVC compiler errors
RenjieTang Sep 1, 2021
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
66 changes: 54 additions & 12 deletions bazel/external/googleurl.patch
View file
Open in desktop
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,55 @@ index 0cd36dc..8c4cbd4 100644
#else
#define STACK_UNINITIALIZED
#endif
@@ -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

# 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.

diff --git a/url/BUILD b/url/BUILD
index f2ec8da..4e2d55b 100644
index f2ec8da..951039b 100644
--- a/url/BUILD
+++ b/url/BUILD
@@ -52,3 +52,27 @@ cc_library(
Expand Down
8 changes: 4 additions & 4 deletions bazel/repository_locations.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,13 +821,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"],

@moderation moderation Aug 23, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was wondering with the recent switch of Quiche deps from quiche.googlesource.com (#17732) I was wondering if this dependency can change too? It doesn't look like the envoy-integration branch is being updated at the Github location - https://github.com/google/quiche/commits/envoy-integration

@RenjieTang RenjieTang Aug 23, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think we can probably move googleurl into Github too. @danzh2010 for more comments.

@RenjieTang RenjieTang Aug 24, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I chatted with Dan, and it seems that @dio has been in charge of this dep.
I currently need to update this dep to do some URL sanitization work. After this PR, @dio can decide how he wants to proceed with the repo location.

@dio dio Sep 17, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@danzh2010 @RenjieTang @moderation I think we need to move to GitHub, but that needs Google's decision? Since it will be hosted along with https://github.com/google/quiche (or if it is still only all about envoy, judging from the name: "envoy-integration", probably we may host it on envoyproxy org)? cc. @htuch @yanavlasov.

PS. Sorry for the very late reply was out for a long time.

@mattklein123 mattklein123 Sep 17, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 to move to GH like we did for quiche, but let's discuss on the unrevert PR?

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 added googleurl_578e68d9ae4c25e37bc0d23460f3e2f183fe526a.tar.gz
View file
Open in desktop
Binary file not shown.