Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates Protobuf to v27 and protovalidate to v0.7.1, and fixes all of the resulting compilation and conformance failures.
As one would expect, there was a tremendous amount of troubleshooting involved in this thankfully-relatively-small PR. Here's my log of what happened. I'll try to be succinct, but I want to capture all of the details so my reasoning can be understood in the future.
First, I tried to update protobuf. This led to pulling a newer version of absl. The version of cel-cpp we use did not compile with this version of absl.
Next, I tried to update cel-cpp. However, the latest version of cel-cpp is broken on macOS for two separate reasons 1, 2.
After taking a break to work on other protovalidate implementations I returned and tried another approach. This time, instead of updating cel-cpp, I just patched it to work with newer absl. Thankfully, this proved surprisingly viable. The
cel_cpp.patch
file now contains this fix too.Unfortunately, compilation was broken in CI on a non-sense compiler error:
It seemed likely to be a compiler issue, thus I was stalled again.
For some reason it finally occurred to me that I probably should just simply update the compiler. In a stroke of accidental rubber-ducking luck, I noticed that GitHub's
ubuntu-latest
had yet to actually move toubuntu-24.04
, which has a vastly more up-to-date C++ toolchain than the olderubuntu-22.04
. This immediately fixed the problem.E-mail validation is hard. In other languages we fall back on standard library functionality, but C++ puts us at a hard impasse; the C++ standard library hardly concerns itself with application-level functionality like SMTP standards. Anyway, I channeled my frustration at the lack of a consistent validation scheme for e-mail, which culminated into [Feature Request] Rigerously specify e-mail address validation protovalidate#236.
For the new failing test cases, we needed to improve the validation of localpart in C++. Lacking any specific reference point, I decided it would be acceptable if the C++ version started adopting ideas from WHATWG HTML email validation. It doesn't move the
localpart
validation to entirely work like WHATWG HTML email validation, as our version still has our specific checks, but now we are a strict subset in protovalidate-cc, so we can remove our additional checks later if we can greenlight adopting the WHATWG HTML standard.The remaining test failures are all related to ignoring validation rules and presence. The following changes were made:
ignore
option is now taken into account in addition to the legacyskipped
andignore_empty
options.IGNORE_IF_DEFAULT_VALUE
Map
types. I haven't traced down why, buthas_presence
seems to always be true for fields of syntheticMap
types in the C++ implementation. (Except in proto3?)And with that I think we will have working Editions support.