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

Rust reflection: simplify dependencies, fix Android build compatibility #8512

Merged
merged 11 commits into from
Feb 4, 2025

Conversation

dextero
Copy link
Contributor

@dextero dextero commented Jan 30, 2025

This set of changes:

  • Removes some of the dependencies of Rust reflection API,
  • Updates the bitflags dependency of flatbuffers crate to 2.8,
  • Makes flatc mark the generated Rust bitflags types as copyable and comparable (which was implicit in bitflags 1.*)

This fixes some of the build issues we encountered when building the library in Android tree, which has strong opinions on which versions of dependencies can be used.

cc @candysonya

@github-actions github-actions bot added c++ codegen Involving generating code from schema rust labels Jan 30, 2025
@dextero dextero marked this pull request as draft January 30, 2025 13:23
@candysonya
Copy link
Contributor

Thanks so much for the change and importing it into Android source tree!

Marcin Radomski added 2 commits January 30, 2025 13:37
num crate is a wrapper over num-traits and a few other crates, that
reexports the APIs from all of them. We only need num-traits.

Signed-off-by: Marcin Radomski <[email protected]>
We only use it to get intmax_t for deriving alignment, which is an alias
for `core::ffi::c_long` [1]. We can use that directly instead.

[1] https://docs.rs/stdint/1.0.0/stdint/type.intmax_t.html

Signed-off-by: Marcin Radomski <[email protected]>
Marcin Radomski added 6 commits January 30, 2025 14:53
It's used to format a string used for debugging only, so we might as
well use the builtin Debug representation of a string.

Signed-off-by: Marcin Radomski <[email protected]>
Otherwise it limits the use of structs generated for reflection.fbs
in Rust reflection API.

Signed-off-by: Marcin Radomski <[email protected]>
from_bits_unchecked was replaced with safe from_bits_retain.

Signed-off-by: Marcin Radomski <[email protected]>
With flatc --rust ../../../reflection/reflection.fbs

Signed-off-by: Marcin Radomski <[email protected]>
@dextero dextero marked this pull request as ready for review January 30, 2025 14:57
Found by Buildifire presubmit:

  Function "sh_binary" is not global anymore and needs to be loaded from
  "@rules_shell//shell:sh_binary.bzl".

Signed-off-by: Marcin Radomski <[email protected]>
In bitflags v2, the debug string representation of enum values is
different than it was in v1:
  Blue -> Color(Blue)
  (empty) -> LongEnum(0x0)

This change adjusts the expected test value.

Signed-off-by: Marcin Radomski <[email protected]>
dbaileychess
dbaileychess previously approved these changes Feb 3, 2025
grpc-swift 1.4.1 depends on swift-nio-ssl 2.14.0+ [1]. swift-nio-ssl 2.29.1
published on 2025-01-30, introduced some code [2] that uses a "switch
expression syntax" supported since Swift 5.9 [3]. Attempts to compile it with
Swift 5.8 cause build errors.

swift-nio-ssl project doesn't seem to support Swift 5.8. A commit from
2024-10-29 removes a "deprecated reference to a Swift 5.8 pipeline" [4].

swift-nio-ssl 2.29.0 is the last version that can be compiled with Swift
5.8. This commit pins it to that exact version.

[1] https://github.com/grpc/grpc-swift/blob/66e27d7e84a2f51df6b8d5c4c3649639cfe478c1/Package.swift#L33
[2] apple/swift-nio-ssl@3cb4d5a#diff-bc1db1321ff689c2819245dcce1a3080554f0fc13f81b8d326c97e7d42717c8fR54
[3] https://github.com/swiftlang/swift-evolution/blob/main/proposals/0380-if-switch-expressions.md
[4] apple/swift-nio-ssl@8a6b89d
@dbaileychess dbaileychess merged commit a285e7e into google:master Feb 4, 2025
47 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ codegen Involving generating code from schema rust swift
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants