Skip to content

Breaking change: make protobuf comply with the C++ layering check#15212

Merged
1 commit merged into
mainfrom
test_579216715
Jan 3, 2024
Merged

Breaking change: make protobuf comply with the C++ layering check#15212
1 commit merged into
mainfrom
test_579216715

Conversation

@copybara-service

@copybara-service copybara-service Bot commented Dec 27, 2023

Copy link
Copy Markdown

Breaking change: make protobuf comply with the C++ layering check

This check enforces that each C++ build target has the correct dependencies for
all headers that it includes. We have many targets that were not correct with
respect to this check, so I fixed them up.

I also cleaned up the C++ targets related to the well-known types. I created a
cc_proto_library() target for each one and removed the :wkt_cc_protos target,
since this was necessary to satisfy the layering check. I deleted the
//src/google/protobuf:protobuf_nowkt target and deprecated :protobuf_nowkt,
because the distinction between the :protobuf and :protobuf_nowkt targets was
not really correct. Neither one exposed the headers for the well-known types in
a way that was valid with respect to the layering check, and the idea of
bundling all the well-known types together is not idiomatic in Bazel anyway.
This is a breaking change, because the //:protobuf target no longer bundles the
well-known types. From now on they should be accessed through the new
//:*_cc_proto aliases in our top-level package.

I renamed the :port_def target to :port, which simplifies things a bit by
matching our internal name.

The original motivation for this change was that to move utf8_range onto our CI
infrastructure, we needed to make its dependency rules_fuzzing compatible with
Bazel 6. The rules_fuzzing project builds with the layering check, and I found
that the process of upgrading it to Bazel 6 made it take a dependency on
protobuf, which caused it to break due to layering violations. I was able to
work around this, but it would still be nice to comply with the layering check
so that we don't have to worry about this kind of thing in the future.

@copybara-service copybara-service Bot force-pushed the test_579216715 branch 7 times, most recently from 2468643 to 5ae17bc Compare January 3, 2024 21:04
@copybara-service copybara-service Bot changed the title Enable layering check in BUILD files Enable layering check in our CI tests Jan 3, 2024
@copybara-service copybara-service Bot force-pushed the test_579216715 branch 4 times, most recently from 0b8e94b to 71b2233 Compare January 3, 2024 23:04
@copybara-service copybara-service Bot changed the title Enable layering check in our CI tests Breaking change: make protobuf comply with the C++ layering check Jan 3, 2024
This check enforces that each C++ build target has the correct dependencies for
all headers that it includes. We have many targets that were not correct with
respect to this check, so I fixed them up.

I also cleaned up the C++ targets related to the well-known types. I created a
cc_proto_library() target for each one and removed the :wkt_cc_protos target,
since this was necessary to satisfy the layering check. I deleted the
//src/google/protobuf:protobuf_nowkt target and deprecated :protobuf_nowkt,
because the distinction between the :protobuf and :protobuf_nowkt targets was
not really correct. Neither one exposed the headers for the well-known types in
a way that was valid with respect to the layering check, and the idea of
bundling all the well-known types together is not idiomatic in Bazel anyway.
This is a breaking change, because the //:protobuf target no longer bundles the
well-known types. From now on they should be accessed through the new
//:*_cc_proto aliases in our top-level package.

I renamed the :port_def target to :port, which simplifies things a bit by
matching our internal name.

The original motivation for this change was that to move utf8_range onto our CI
infrastructure, we needed to make its dependency rules_fuzzing compatible with
Bazel 6. The rules_fuzzing project builds with the layering check, and I found
that the process of upgrading it to Bazel 6 made it take a dependency on
protobuf, which caused it to break due to layering violations. I was able to
work around this, but it would still be nice to comply with the layering check
so that we don't have to worry about this kind of thing in the future.

PiperOrigin-RevId: 595516736
@copybara-service copybara-service Bot closed this pull request by merging all changes into main in a7b0421 Jan 3, 2024
@copybara-service copybara-service Bot deleted the test_579216715 branch January 3, 2024 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants