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

feat: migrate to quick-protobuf #3312

Merged
merged 79 commits into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
79 commits
Select commit Hold shift + click to select a range
c250f68
Add generated directory
kckeiks Jan 7, 2023
cac57a0
Add generated code
kckeiks Jan 7, 2023
44b3465
Add ci
kckeiks Jan 7, 2023
4dca4d0
Try adding quotes
kckeiks Jan 7, 2023
6b19385
Add input variable
kckeiks Jan 12, 2023
9d4b864
Move inputs
kckeiks Jan 12, 2023
c9853f7
Move inputs 2
kckeiks Jan 12, 2023
9716fe5
Glob into variable
kckeiks Jan 12, 2023
1d1dff0
Glob into variable 2
kckeiks Jan 12, 2023
b2aabcd
Use glob
kckeiks Jan 12, 2023
559a1d3
Remove quotes
kckeiks Jan 12, 2023
d62b50d
Remove refs
kckeiks Jan 12, 2023
b894ac7
Test glob pattern
kckeiks Jan 12, 2023
391a892
Bad commit
thomaseizinger Jan 12, 2023
2991994
Remove explicit echo
thomaseizinger Jan 12, 2023
8a25c84
Fail CI on untracked files
thomaseizinger Jan 12, 2023
ccc4541
Revert "Bad commit"
thomaseizinger Jan 12, 2023
f464e3b
Cache installed `pb-rs` binary
thomaseizinger Jan 12, 2023
d2ed2c6
Show bad proto files to the user
thomaseizinger Jan 12, 2023
c6dd4e4
Don't duplicate command
thomaseizinger Jan 12, 2023
367a1a4
Only run on files in `generated/` directory
thomaseizinger Jan 13, 2023
8896f81
Resolve conflict
kckeiks Jan 21, 2023
78a483f
Remove prost from core
kckeiks Jan 22, 2023
78be5d5
Fix core tests
kckeiks Jan 22, 2023
430a9fb
Fmt code
kckeiks Jan 22, 2023
d7b6b27
Merge remote-tracking branch 'origin/add-quickprotobuf-take2' into ad…
kckeiks Jan 22, 2023
f1967b8
Include generated in proto mod
kckeiks Jan 22, 2023
76dd1d1
Remove prost from autonat
kckeiks Jan 22, 2023
840887a
Update changelog
kckeiks Jan 22, 2023
2a97315
Merge branch 'master' into add-quickprotobuf-take2
kckeiks Jan 28, 2023
448765a
Rename to quick-protobuf-codec
kckeiks Jan 28, 2023
651b9c9
Remove prost-codec
kckeiks Jan 28, 2023
a6b63ac
Don't use `Cow`
thomaseizinger Jan 31, 2023
2e2bbe7
Fix core
kckeiks Feb 2, 2023
59ff24d
Run fmt
kckeiks Feb 2, 2023
a8762f8
Remove prost from identify
kckeiks Feb 2, 2023
fc13432
Fix autonat
kckeiks Feb 2, 2023
4bd8ab0
Update changelogs
kckeiks Feb 2, 2023
55d6f4d
Update docs
kckeiks Feb 4, 2023
b5cfe29
Remove prost from floodsub
kckeiks Feb 4, 2023
c2eaefd
Remove prost from gossipsub
kckeiks Feb 5, 2023
fa5fa4d
Update changelog in identify
kckeiks Feb 5, 2023
86579c3
Remove prost from kad
kckeiks Feb 7, 2023
f6552da
Clean up relay
kckeiks Feb 9, 2023
326ddee
Remove prost from rendezvous
kckeiks Feb 9, 2023
9ea8ce4
Remove prost from noise
kckeiks Feb 13, 2023
a2f3cd4
Remove prost from plaintext
kckeiks Feb 13, 2023
dad962a
Remove prost from webrtc
kckeiks Feb 14, 2023
469699a
Clean up
kckeiks Feb 14, 2023
02d642b
Fix cargo check with all features
kckeiks Feb 14, 2023
929d96c
Fix webrtc tests
kckeiks Feb 14, 2023
2a1ff8c
Fix clippy errors
kckeiks Feb 14, 2023
c38e9e3
Fix clippy errors take 2
kckeiks Feb 14, 2023
003de70
Fix CI
kckeiks Feb 14, 2023
8b3398d
Merge branch 'master' into add-quickprotobuf-take2
kckeiks Feb 14, 2023
75c6715
Merge branch 'master' into add-quickprotobuf-take2
kckeiks Feb 14, 2023
ee44340
Merge branch 'master' into add-quickprotobuf-take2
kckeiks Feb 21, 2023
4f7605a
Fix pb-rs version
kckeiks Feb 21, 2023
23526e6
Use long option name
kckeiks Feb 21, 2023
c42a5a8
Address changelog feedback
kckeiks Feb 21, 2023
ece12c1
Remove line
kckeiks Feb 21, 2023
67e657c
Merge branch 'add-quickprotobuf-take2' of github.com:kckeiks/rust-lib…
kckeiks Feb 21, 2023
3acc5f9
Merge branch 'master' into add-quickprotobuf-take2
kckeiks Feb 24, 2023
90ebad6
Zeroize Data
kckeiks Feb 24, 2023
66a2af5
Remove protoc installation from ci
kckeiks Feb 24, 2023
c014e1e
Remove protoc installation from interop-tests
kckeiks Feb 24, 2023
a4335ce
Revert ci changes
kckeiks Feb 24, 2023
8531ac4
Fmt relay
kckeiks Feb 24, 2023
6cf4fe5
Merge branch 'master' into add-quickprotobuf-take2
kckeiks Feb 24, 2023
5ca8747
Remove transparent
kckeiks Feb 24, 2023
98e302b
Add protoc installation to ci
kckeiks Feb 25, 2023
b0a41ec
Remove display impl
kckeiks Feb 25, 2023
da3e088
Box error
kckeiks Feb 25, 2023
2b1b274
Change DecodeError
kckeiks Feb 25, 2023
08ce33f
Merge branch 'master' into add-quickprotobuf-take2
kckeiks Mar 1, 2023
dfc55b8
Make an error type that does not expose anything
kckeiks Mar 1, 2023
99cee6e
Merge branch 'master' into add-quickprotobuf-take2
thomaseizinger Mar 2, 2023
f939e06
Merge branch 'master' into add-quickprotobuf-take2
mxinden Mar 2, 2023
d2be0bf
Merge branch 'master' into add-quickprotobuf-take2
mxinden Mar 2, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -293,3 +293,30 @@ jobs:
echo "PR title is too long (greater than 72 characters)"
exit 1
fi

check-proto-files:
name: Check for changes in proto files
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3

- uses: Swatinem/rust-cache@359a70e43a0bb8a13953b04a90f76428b4959bb6 # v2.2.0

- run: cargo install --version 0.10.0 pb-rs --locked

- name: Glob match
uses: tj-actions/glob@v16
id: glob
with:
files: |
**/generated/*.proto

- name: Generate proto files
run: pb-rs --dont_use_cow ${{ steps.glob.outputs.paths }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reasoning for not using Cow? I am assuming code simplicity? I find it a neat feature, though I would assume that the performance gain is not worth the additional complexity in most cases. Can we easily use Cow on a case-by-case basis in the future?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem / feature is that when using Cow, the deserialised data structure retains a reference to the byte buffer.

This doesn't play well with asynchronous-codec where Encoder passes you a byte buffer and expects you to create an owned type, without references to the original buffer.

I think Encoder would have to undergo some serious refactoring to make this possible. Perhaps now that GATs are stable, this is actually a viable interface. For the time being, we opted to not go down this route.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reasoning for not using Cow? I am assuming code simplicity? I find it a neat feature, though I would assume that the performance gain is not worth the additional complexity in most cases. Can we easily use Cow on a case-by-case basis in the future?

We would be able to use Cow on a case-by-case basis if we find a way to run this CI check on all proto files, without having to enumerate them all so we don't forget adding new ones but also have a list of files that we want to enable Cow for.


- name: Ensure generated files are unmodified # https://stackoverflow.com/a/5737794
run: |
git_status=$(git status --porcelain)

echo $git_status
test -z "$git_status"
159 changes: 29 additions & 130 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ members = [
"misc/multistream-select",
"misc/rw-stream-sink",
"misc/keygen",
"misc/prost-codec",
"misc/quick-protobuf-codec",
"misc/quickcheck-ext",
"muxers/mplex",
"muxers/yamux",
Expand Down
2 changes: 2 additions & 0 deletions core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

- Move `ConnectionId` to `libp2p-swarm`. See [PR 3221].
- Move `PendingPoint` to `libp2p-swarm` and make it crate-private. See [PR 3221].
- Migrate from `prost` to `quick-protobuf`. This removes `protoc` dependency. See [PR 3312].

[PR 3312]: https://github.com/libp2p/rust-libp2p/pull/3312
[PR 3221]: https://github.com/libp2p/rust-libp2p/pull/3221

# 0.38.0
Expand Down
5 changes: 1 addition & 4 deletions core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ multistream-select = { version = "0.12.1", path = "../misc/multistream-select" }
p256 = { version = "0.12.0", default-features = false, features = ["ecdsa", "std"], optional = true }
parking_lot = "0.12.0"
pin-project = "1.0.0"
prost = "0.11"
quick-protobuf = "0.8"
once_cell = "1.17.1"
rand = "0.8"
rw-stream-sink = { version = "0.3.0", path = "../misc/rw-stream-sink" }
Expand All @@ -54,9 +54,6 @@ quickcheck = { package = "quickcheck-ext", path = "../misc/quickcheck-ext" }
rmp-serde = "1.0"
serde_json = "1.0"

[build-dependencies]
prost-build = "0.11"

[features]
secp256k1 = [ "libsecp256k1" ]
ecdsa = [ "p256" ]
Expand Down
Loading