starred/librespot

Fork 0

mirror of https://github.com/librespot-org/librespot.git synced 2026-04-27 08:15:50 +03:00

[GH-ISSUE #129] Use protobuf-macros from crates.io, or remove it #101

New issue

Closed

opened 2026-02-27 19:28:50 +03:00 by kerem · 5 comments

kerem commented

2026-02-27 19:28:50 +03:00

Owner

Originally created by @plietar on GitHub (Feb 8, 2018).
Original GitHub issue: https://github.com/librespot-org/librespot/issues/129

Originally assigned to: @plietar on GitHub.

We currently use protobuf-macros (my crate), which is not on crates.io. This blocks publishing librespot-core to crates.io

Potential solutions are:

I can publish it on crates.io as it is. On the other hand the way it works on rust stable is a big hack, so I'm not a fan of this and I'd prefer to get rid of the hack.
Wait for procedural macros to land on Rust stable, rewrite protobuf-macros to use them, and publish to crates.io. This is probably not going to happen any time soon (at least 6 months IMO).
Get rid of it. It makes code that builds messages more verbose. When I started writing librespot a big portion of code was dedicated to building messages so protobuf-macros was a big improvement. Today this kind of code is fairly minor in proportion to the rest so I wouldn't mind.
Replace the protobuf library with PROST (https://github.com/danburkert/prost), which uses plain structs to store messages hence doesn't need protobuf-macros for message literals. OTOH I don't really like the rest of the library for various reasons (doesn't really respect protobuf's philosophy).
Find a protobuf library which has allows message literals but is still good in other aspects.

I'd be curious to see the impact of 3. It's probably small enough to make it the best solution.

Originally created by @plietar on GitHub (Feb 8, 2018). Original GitHub issue: https://github.com/librespot-org/librespot/issues/129 Originally assigned to: @plietar on GitHub. We currently use protobuf-macros (my crate), which is not on crates.io. This blocks publishing `librespot-core` to crates.io Potential solutions are: 1) I can publish it on crates.io as it is. On the other hand the way it works on rust stable is a big hack, so I'm not a fan of this and I'd prefer to get rid of the hack. 2) Wait for procedural macros to land on Rust stable, rewrite protobuf-macros to use them, and publish to crates.io. This is probably not going to happen any time soon (at least 6 months IMO). 3) Get rid of it. It makes code that builds messages more verbose. When I started writing librespot a big portion of code was dedicated to building messages so protobuf-macros was a big improvement. Today this kind of code is fairly minor in proportion to the rest so I wouldn't mind. 4) Replace the protobuf library with PROST (https://github.com/danburkert/prost), which uses plain structs to store messages hence doesn't need protobuf-macros for message literals. OTOH I don't really like the rest of the library for various reasons (doesn't really respect protobuf's philosophy). 5) Find a protobuf library which has allows message literals but is still good in other aspects. I'd be curious to see the impact of 3. It's probably small enough to make it the best solution.

kerem closed this issue

2026-02-27 19:28:50 +03:00

kerem commented

2026-02-27 19:28:51 +03:00

Author

Owner

@sashahilton00 commented on GitHub (Feb 8, 2018):

I haven't really looked at the protobuf side of things to have a view on this. It's up to you what we do. With regards to the message building code being more verbose, does this make it easier to pinpoint where the error is when spotify decides to update their protobuf spec, or were they just convenient when you were reverse engineering? My only view is that we should avoid option 2 if possible, since we have enough to do without having to think about rewriting bits of the code in an undetemined number of months time.

@sashahilton00 commented on GitHub (Feb 8, 2018): I haven't really looked at the protobuf side of things to have a view on this. It's up to you what we do. With regards to the message building code being more verbose, does this make it easier to pinpoint where the error is when spotify decides to update their protobuf spec, or were they just convenient when you were reverse engineering? My only view is that we should avoid option 2 if possible, since we have enough to do without having to think about rewriting bits of the code in an undetemined number of months time.

kerem commented

2026-02-27 19:28:51 +03:00

Author

Owner

@plietar commented on GitHub (Feb 8, 2018):

With regards to the message building code being more verbose, does this make it easier to pinpoint where the error is when spotify decides to update their protobuf spec

No, it makes pretty much no difference.

Take for example github.com/librespot-org/librespot@7ead896ae7/core/src/connection/handshake.rs (L84-L99)

protobuf-macros is essentially just sugar that rewrites it with:

let mut packet = ClientHello::new();
packet.mut_build_info().set_product(protocol::keyexchange::Product::PRODUCT_PARTNER);
packet.mut_build_info().set_platform(protocol::keyexchange::Platform::PLATFORM_LINUX_X86);
packet.mut_build_info().set_version(0x10800000000);
packet.mut_cryptosuites_supported().push(protocol::keyexchange::Cryptosuite::CRYPTO_SUITE_SHANNON);
packet.mut_login_crypto_hello().mut_diffie_hellman().set_gc(gc);
packet.mut_login_crypto_hello().mut_diffie_hellman().set_server_keys_known(1);
packet.set_client_nonce(util::rand_vec(&mut thread_rng(), 0x10));
packet.set_padding(vec![0x1e]);

So a little less readable but not a huge deal.

Option 3 involves rewriting all uses of protobuf_init into the explicit version, and is probably the best course of action.

@plietar commented on GitHub (Feb 8, 2018): > With regards to the message building code being more verbose, does this make it easier to pinpoint where the error is when spotify decides to update their protobuf spec No, it makes pretty much no difference. Take for example https://github.com/librespot-org/librespot/blob/7ead896ae79ddb4a8b194adb564d9dd61596f5e9/core/src/connection/handshake.rs#L84-L99 protobuf-macros is essentially just sugar that rewrites it with: ```rust let mut packet = ClientHello::new(); packet.mut_build_info().set_product(protocol::keyexchange::Product::PRODUCT_PARTNER); packet.mut_build_info().set_platform(protocol::keyexchange::Platform::PLATFORM_LINUX_X86); packet.mut_build_info().set_version(0x10800000000); packet.mut_cryptosuites_supported().push(protocol::keyexchange::Cryptosuite::CRYPTO_SUITE_SHANNON); packet.mut_login_crypto_hello().mut_diffie_hellman().set_gc(gc); packet.mut_login_crypto_hello().mut_diffie_hellman().set_server_keys_known(1); packet.set_client_nonce(util::rand_vec(&mut thread_rng(), 0x10)); packet.set_padding(vec![0x1e]); ``` So a little less readable but not a huge deal. Option 3 involves rewriting all uses of protobuf_init into the explicit version, and is probably the best course of action.

kerem commented

2026-02-27 19:28:51 +03:00

Author

Owner

@sashahilton00 commented on GitHub (Feb 8, 2018):

Thanks for clarifying. If it's just syntactic sugar, then I think we can probably lose it if it's just hindering progress otherwise.

@sashahilton00 commented on GitHub (Feb 8, 2018): Thanks for clarifying. If it's just syntactic sugar, then I think we can probably lose it if it's just hindering progress otherwise.

kerem commented

2026-02-27 19:28:51 +03:00

Author

Owner

@awiouy commented on GitHub (Feb 11, 2018):

@plietar How can I get the output of protobuf_init! to the screen or to a file?

@awiouy commented on GitHub (Feb 11, 2018): @plietar How can I get the output of `protobuf_init!` to the screen or to a file?

kerem commented

2026-02-27 19:28:51 +03:00

Author

Owner

@plietar commented on GitHub (Feb 11, 2018):

@awiouy You should be able to find the expanded version in target/debug/build/librespot-core-HASH/out/lib.rs

@plietar commented on GitHub (Feb 11, 2018): @awiouy You should be able to find the expanded version in `target/debug/build/librespot-core-HASH/out/lib.rs`

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/librespot#101

No description provided.

Rows
Columns