-
Notifications
You must be signed in to change notification settings - Fork 628
feat: add boost mqtt5 #6938
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
base: main
Are you sure you want to change the base?
feat: add boost mqtt5 #6938
Conversation
|
Hello @bazelbuild/bcr-maintainers, modules (boost.algorithm, boost.align, boost.any, boost.array, boost.asio, boost.assert, boost.atomic, boost.beast, boost.bimap, boost.bind, boost.chrono, boost.circular_buffer, boost.concept_check, boost.config, boost.container, boost.container_hash, boost.context, boost.conversion, boost.core, boost.coroutine, boost.date_time, boost.describe, boost.detail, boost.dll, boost.dynamic_bitset, boost.endian, boost.exception, boost.filesystem, boost.foreach, boost.format, boost.function, boost.function_types, boost.functional, boost.fusion, boost.geometry, boost.graph, boost.heap, boost.icl, boost.integer, boost.interprocess, boost.intrusive, boost.io, boost.iostreams, boost.iterator, boost.json, boost.lambda, boost.lexical_cast, boost.log, boost.logic, boost.math, boost.move, boost.mp11, boost.mpl, boost.multi_array, boost.multi_index, boost.multiprecision, boost.numeric_conversion, boost.optional, boost.parameter, boost.pfr, boost.phoenix, boost.polygon, boost.pool, boost.predef, boost.preprocessor, boost.process, boost.program_options, boost.property_map, boost.property_tree, boost.proto, boost.ptr_container, boost.qvm, boost.random, boost.range, boost.ratio, boost.rational, boost.regex, boost.scope, boost.scope_exit, boost.serialization, boost.signals2, boost.smart_ptr, boost.sort, boost.spirit, boost.stacktrace, boost.static_assert, boost.static_string, boost.system, boost.test, boost.thread, boost.throw_exception, boost.tokenizer, boost.tti, boost.tuple, boost.type_index, boost.type_traits, boost.typeof, boost.units, boost.unordered, boost.url, boost.utility, boost.uuid, boost.variant, boost.variant2, boost.vmd, boost.winapi, boost.xpressive) have been updated in this PR. |
|
Hello @Vertexwahn, modules you maintain (boost, boost.algorithm, boost.align, boost.any, boost.array, boost.asio, boost.assert, boost.assign, boost.atomic, boost.beast, boost.bimap, boost.bind, boost.callable_traits, boost.charconv, boost.chrono, boost.circular_buffer, boost.compat, boost.concept_check, boost.config, boost.container, boost.container_hash, boost.context, boost.conversion, boost.core, boost.coroutine, boost.coroutine2, boost.crc, boost.date_time, boost.describe, boost.detail, boost.dll, boost.dynamic_bitset, boost.endian, boost.exception, boost.filesystem, boost.foreach, boost.format, boost.function, boost.function_types, boost.functional, boost.fusion, boost.geometry, boost.graph, boost.hash2, boost.heap, boost.hof, boost.icl, boost.integer, boost.interprocess, boost.intrusive, boost.io, boost.iostreams, boost.iterator, boost.json, boost.lambda, boost.lambda2, boost.leaf, boost.lexical_cast, boost.lockfree, boost.log, boost.logic, boost.math, boost.move, boost.mp11, boost.mpl, boost.multi_array, boost.multi_index, boost.multiprecision, boost.mysql, boost.numeric_conversion, boost.optional, boost.parameter, boost.pfr, boost.phoenix, boost.polygon, boost.pool, boost.predef, boost.preprocessor, boost.process, boost.program_options, boost.property_map, boost.property_tree, boost.proto, boost.ptr_container, boost.qvm, boost.random, boost.range, boost.ratio, boost.rational, boost.regex, boost.scope, boost.scope_exit, boost.serialization, boost.signals2, boost.smart_ptr, boost.sort, boost.spirit, boost.stacktrace, boost.static_assert, boost.static_string, boost.system, boost.test, boost.thread, boost.throw_exception, boost.timer, boost.tokenizer, boost.tti, boost.tuple, boost.type_index, boost.type_traits, boost.typeof, boost.units, boost.unordered, boost.url, boost.utility, boost.uuid, boost.variant, boost.variant2, boost.vmd, boost.winapi, boost.xpressive) have been updated in this PR. |
|
Hello @cdelguercio, modules you maintain (boost, boost.assign) have been updated in this PR. |
|
Hello @kekxv, modules you maintain (boost.charconv, boost.compat, boost.mysql) have been updated in this PR. |
|
Hello @vtsao-openai, modules you maintain (boost.crc, boost.heap, boost.lockfree, boost.test, boost.timer, boost.url) have been updated in this PR. |
|
Hello @lalten, modules you maintain (boost.dll, boost.log) have been updated in this PR. |
|
Hello @hofbi, modules you maintain (boost.process) have been updated in this PR. |
|
Hello @kgreenek, modules you maintain (boost, boost.units) have been updated in this PR. |
|
Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (boost.mqtt5) have been updated in this PR. |
|
cc @martin4861 |
|
@bazel-io skip_check unstable_url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds a new version 1.89.0.bcr.3 for a large number of Boost modules. The changes appear to be consistent with the Bazel Central Registry policies for adding new module versions. However, the pull request title "feat: add boost mqtt5" is misleading as the boost.mqtt5 module is not included in the changes. Please consider updating the title to accurately reflect the scope of this PR, for example, "feat: Update Boost modules to 1.89.0.bcr.3". This will improve the clarity and maintainability of the repository's history.
| local_defines = select({ | ||
| "@platforms//os:windows": ["BOOST_OS_WINDOWS"], | ||
| "//conditions:default": [], | ||
| }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BOOST_OS_WINDOWS macro is typically defined by including headers from boost.config. Since @boost.config is already a dependency of this target, this explicit definition seems redundant and could hide potential configuration issues. If this is a necessary workaround, please add a comment explaining why it's required. Otherwise, it should be removed to rely on boost.config's standard platform detection mechanism.
| # For tests. Note that presubmit tests `@boost.heap//...` from an | ||
| # external workspace so these should not be `dev_dependency = True`. | ||
| bazel_dep(name = "boost.container", version = "1.89.0.bcr.3") | ||
| bazel_dep(name = "boost.foreach", version = "1.89.0.bcr.3") | ||
| bazel_dep(name = "boost.test", version = "1.89.0.bcr.3") | ||
| bazel_dep(name = "platforms", version = "1.0.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These test-only dependencies are declared as regular dependencies. This means that any consumer of boost.heap will also transitively depend on boost.container, boost.foreach, boost.test, etc., which is generally undesirable. The BCR style guide recommends using a bcr_test_module for tests to isolate test-only dependencies. Please consider refactoring the tests into a bcr_test_module to avoid polluting the dependency graph of consumers.
References
- The style guide strongly suggests using a test module (
bcr_test_module) to verify targets. This helps isolate test-only dependencies so they are not imposed on consumers of the module. (link)
|
@fmeum @meteorcloudy @kotlaja please add presubmit auto label. |
|
The size of such PRs remains rather concerning. Will every single update to any boost module require such 300+ file PRs? |
Adding a new boost module at the same semver as other existing modules should only require touching that module plus the |
|
How can I resolve circular dependencies without updating another modules? |
The script produces new versions of everything, I don't know how easy it is to adapt it to the situation. It should be doable by hand though: Just add the new module (which the script doesn't support yet anyway) and then manually copy over the latest |
75695f0 to
3b5fbb3
Compare
|
Hello @bazelbuild/bcr-maintainers, modules (boost.spirit) have been updated in this PR. |
|
Hello @Vertexwahn, modules you maintain (boost, boost.spirit) have been updated in this PR. |
|
Hello @cdelguercio, @kgreenek, modules you maintain (boost) have been updated in this PR. |
|
Hello @bazelbuild/bcr-maintainers, modules (boost.spirit, boost.test) have been updated in this PR. |
|
Hello @Vertexwahn, modules you maintain (boost, boost.spirit, boost.test) have been updated in this PR. |
|
Hello @vtsao-openai, modules you maintain (boost.test) have been updated in this PR. |
I try to update minimum modules as you say, but I found out boost.spirit depends on many boost modules which depends on boost meta package, so I guess I should update almost all of boost module to new boost meta package in the end. |
|
@Vertexwahn Do you have any thought to handle this problem? |
|
@wep21 I don't think the error you are seeing is caused by the dependency cycle - at least I don't see how that could happen Perhaps the module file of boost.spirit was updated by the script in place? |
Currently, I've updated the module file by hand. |
Signed-off-by: wep21 <[email protected]>
Signed-off-by: wep21 <[email protected]>
Signed-off-by: wep21 <[email protected]>
Signed-off-by: wep21 <[email protected]>
9740ed1 to
90682d1
Compare
|
From ci error, it appears that the dependency chain error occurs only with Bazel 7. Is this a bug of bazel 7? |
|
Other versions also fail when other errors - not sure what's going on, I'll retry the build. |
|
Uh oh!
There was an error while loading. Please reload this page.