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

deps: update absl, googleapis, cel-cpp #35514

Closed
wants to merge 10 commits into from

Conversation

kyessenov
Copy link
Contributor

Commit Message: update absl, googleapis, cel-cpp
Risk Level: low

Change-Id: I45d346443dea2ba767c6c929805af65d7af05a93
Signed-off-by: Kuat Yessenov <[email protected]>
Change-Id: Ida8a42d5de3cd8c24504dcdedbacb70a7c172046
Signed-off-by: Kuat Yessenov <[email protected]>
@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Jul 30, 2024
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).
envoyproxy/dependency-shepherds assignee is @moderation

🐱

Caused by: #35514 was opened by kyessenov.

see: more, trace.

@soulxu
Copy link
Member

soulxu commented Jul 31, 2024

/assign @yanavlasov

@soulxu
Copy link
Member

soulxu commented Jul 31, 2024

/wait-any

@phlax
Copy link
Member

phlax commented Jul 31, 2024

@kyessenov this adds 100s of lines of warnings to most of our builds

can we either fix this upstream before merging or patch out the warnings if they are not important

@kyessenov
Copy link
Contributor Author

@phlax, yeah I can fix upstream. There's work to do here with various static asserts too.

Change-Id: I1fa5cc0418b5b267f77cf021844685aa96521487
Change-Id: Ifa9f8e1da03c7404044de12a3a825929420a9dcf
Signed-off-by: Kuat Yessenov <[email protected]>
@kyessenov kyessenov requested a review from alyssawilk as a code owner July 31, 2024 19:30
Change-Id: I97525ca09a9e043017466455e371eb6745c1da37
Signed-off-by: Kuat Yessenov <[email protected]>
Change-Id: I2c5f1bb8d3331eb35c78746efea1d2b48151983d
Signed-off-by: Kuat Yessenov <[email protected]>
Change-Id: I452b4cb002335ac7ef334ab6dd295f0d938997ab
Change-Id: Icc5580dfc89124e4b97c33f75e8a40b1106c60cd
Signed-off-by: Kuat Yessenov <[email protected]>
@moderation
Copy link
Contributor

Deps looks good and will check back when CI errors are fixed

@yanavlasov
Copy link
Contributor

/wait

@abeyad
Copy link
Contributor

abeyad commented Aug 1, 2024

@kyessenov regarding the iOS/macOS failures, if I change the Abseil library to be the previous one, i.e.

version = "20230802.1",
sha256 = "987ce98f02eefbaf930d6e38ab16aa05737234d7afbab2d5c4ea7adbe50c28ed",

then it builds properly. It seems like this version of the Abseil library has issues in its macOS/iOS target build?

@kyessenov
Copy link
Contributor Author

@abeyad macOS build failure is distinct - it's something about alignment difference. I am not sure how iOS build error is triggered - is it using clang 14 or some strange compiler version?

@abeyad
Copy link
Contributor

abeyad commented Aug 1, 2024

@abeyad macOS build failure is distinct - it's something about alignment difference. I am not sure how iOS build error is triggered - is it using clang 14 or some strange compiler version?

I'm using clang 15 locally on my mac and it fails to build with the same error as the iOS job. Note that I'm using the option --config=mobile-test-ios (which results in these options:

envoy/mobile/.bazelrc

Lines 69 to 71 in 770fa7b

build:ios --define=manual_stamp=manual_stamp
build:ios --test_timeout=390,750,1500,5700
build:ios --define=envoy_full_protos=enabled
).

@kyessenov
Copy link
Contributor Author

Sigh, I don't have a Mac. And it's also quic code. Any way to proceed debugging besides Googling for some other reports of a build failure?

Change-Id: I14ccd0ae23629055cb30a3ea86a303a7f9b7731e
@yanavlasov
Copy link
Contributor

I see two problems:

  1. Link failure with undefined _FLAGS_v in quic::QuicCryptoClientConfig::CachedState::Initialize - IOS and Android are likely the same problem. My guess it is produced by QUIC_DVLOG(1) but, I'm unsure what needs to be logged to make it work. I'll ping the QUIC team to assist.
  2. Alignment error in CEL for macOS build. Not sure about this one.

@yanavlasov
Copy link
Contributor

/wait

Change-Id: I3de3c8c8b04d8269d1961e082cf33886aca19b03
@kyessenov
Copy link
Contributor Author

/retest

1 similar comment
@kyessenov
Copy link
Contributor Author

/retest

@phlax
Copy link
Member

phlax commented Sep 3, 2024

ci fails look real

/wait

@asedeno
Copy link
Contributor

asedeno commented Sep 5, 2024

For the macOS failure: google/cel-cpp#831

@asedeno
Copy link
Contributor

asedeno commented Sep 5, 2024

The macOS build failure should be fixed by bumping CEL to google/cel-cpp@aafa026.

That's going to require bumping protobuf too though.

@asedeno
Copy link
Contributor

asedeno commented Sep 9, 2024

I'd like to decouple the absl update from the cel update, just to untangle the various build failures. Was 20240116.2 chosen over 20240722.0 for any particular reason? It was pretty new when this PR was opened.

[edit:] ah, I see that 20240722.0 was released August 1st, after this PR was opened.

@phlax
Copy link
Member

phlax commented Sep 9, 2024

@asedeno i tried updating absl on its own previously and iirc came to the conclusion that it would need a CEL update also - at least without patching

@asedeno
Copy link
Contributor

asedeno commented Sep 9, 2024

@phlax, I'm a bit late to this party and also arriving to the same conclusion. I might poke at it some more just to satisfy my own curiosity.

Copy link

github-actions bot commented Oct 9, 2024

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Oct 9, 2024
Copy link

This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot closed this Oct 16, 2024
@asedeno asedeno mentioned this pull request Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps Approval required for changes to Envoy's external dependencies stale stalebot believes this issue/PR has not been touched recently waiting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants