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

Remove compatibility overrides for protobuf error collectors after update of grpc/protobuf #1408

Closed
hzeller opened this issue May 15, 2024 · 1 comment
Labels
code-maintenance Everything w.r.t. code quality, maintenance, TODOs

Comments

@hzeller
Copy link
Member

hzeller commented May 15, 2024

The classes that act as ErrorCollector for protocol buffer changed the interface; Currently they are formulated so that they work with old and new protocol buffer versions, which is needed until we've updated grpc (which is also providing the protocol buffer dependencey; GRPC update is currently stuck on grpc/grpc#36132 ).

The files in question are

  • third_party/xls/common/file/filesystem.cc
  • third_party/xls/tools/proto_to_dslx.cc

They have TODOs pointing to this issue.

After grpc is updated, fix these TODOs.

@hzeller hzeller added the code-maintenance Everything w.r.t. code quality, maintenance, TODOs label May 15, 2024
copybara-service bot pushed a commit that referenced this issue May 15, 2024
The protobuffer error collector interfaces changed to accept
string_views instead.
Since these are virtual methods, we need to override
both in the transition period (but not add any 'override' to
it as this depends on the available version we override).

Once we are on latest protobuf (i.e. once we can update grpc),
this will automatically compile with the new version.
After that, we can remove the old AddError()/AddWarning()
methods.

Issues: #1408
PiperOrigin-RevId: 634099367
@hzeller
Copy link
Member Author

hzeller commented May 28, 2024

It looks like the c++20 issue in question is fixed upstream in grpc, now we just need to wait for it to be backported to then integrate 1.64.x (x > 0).

copybara-service bot pushed a commit that referenced this issue Jun 26, 2024
They need to be updated in tandem as both rely on a newer version
of protobuf that is incompatible with the old.

The patches in or-tools are not needed anymore as
  * They have fully migrated to absl logging and removed a compatibility
    flag
  * The switching of _not_ using some dependencies works and don't
    have to patched out manually anymore.
    (we set these in our .bazelrc).
    Subsequently: also remove scip and glpk from our dependency
    files as they are not needed anymore.

Affects issue #1408 as the new protobuf version means we don't need
compatibility workarounds anymore (but will be addressd in a
separate change).

PiperOrigin-RevId: 647054928
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-maintenance Everything w.r.t. code quality, maintenance, TODOs
Projects
None yet
Development

No branches or pull requests

1 participant