quic: attempting to fix non-linux QUIC builds#18027
quic: attempting to fix non-linux QUIC builds#18027mattklein123 merged 7 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
alyssawilk
force-pushed
the
quic_build
branch
from
272e7d6 to
7a0b917
Compare
|
api_compat failure is a known failure (which I can merge past) |
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
| #pragma GCC diagnostic ignored "-Wsuggest-override" | ||
| #endif | ||
|
|
||
| #if defined(__linux__) |
There was a problem hiding this comment.
Does this fix the build? There are other places in this file refers to quic classes.
Why not excluding this header file completely in the BUILD if the platform is not Linux?
There was a problem hiding this comment.
right, sorry, I had commented out most of the file in a prior build which got lost.
So I can add that back, moving the endif to the bottom of the file, or we could have one file for setting UDP_GSO_BATCH_WRITER_COMPILETIME_SUPPORT and a second file for use of it.
which would you prefer?
There was a problem hiding this comment.
I'm okay either way, maybe the latter is easier to understand?
source/server/listener_impl.cc
Outdated
| #include "source/common/quic/gso_defines.h" | ||
| #endif // ENVOY_ENABLE_QUIC | ||
|
|
||
| #ifdef UDP_GSO_BATCH_WRITER_COMPILETIME_SUPPORT |
There was a problem hiding this comment.
If you #ifdef UDP_GSO_BATCH_WRITER_COMPILETIME_SUPPORT in udp_gso_batch_writer.h, this ifdef is not needed.
There was a problem hiding this comment.
well right now the file is compile-selected out in the build.
we could leave it in the build and #ifdef out the whole contents, but this seems cleaner to me?
There was a problem hiding this comment.
also I'm super baffled by windows breakage
source/server/listener_impl.cc(39): fatal error C1083: Cannot open include file: 'source/common/quic/udp_gso_batch_writer.h': No such file or directory
but it should only see that if UDP_GSO_BATCH_WRITER_COMPILETIME_SUPPORT is 1 which should only happen on linux? What am missing? :-/
There was a problem hiding this comment.
oops nm, PEBKAC, fixed.
There was a problem hiding this comment.
Agreed this is cleaner. I'm fine either way.
BTW, udp_gso_batch_writer.h is already ifdef'ing out the whole contents, isn't it?
There was a problem hiding this comment.
uggggh, you're totally right, which means I've been attacking the wrong problem from the start.
Thanks for saving me the commit + mobile update + realizing this didn't solve my problem
trying a new workaround per your offline suggestions!
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
source/server/listener_impl.cc
Outdated
| #include "source/common/quic/udp_gso_batch_writer.h" | ||
| #endif | ||
|
|
||
|
|
4 participants
Upstream HTTP/3 android build is breaking becasue the udp gso libraries are bazel-selected out but the includes aren't:
https://github.com/envoyproxy/envoy-mobile/pull/1717/checks?check_run_id=3487450075
I think this will fix (or at least get us to the next compile error)
Risk Level: low
Testing: n/a
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: hopefully fixing android build