[PR #293] [MERGED] Simplify librespot-protocol build caching. #864

New issue

Closed

opened 2026-02-27 20:00:14 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 20:00:14 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/librespot-org/librespot/pull/293
Author: @willstott101
Created: 2/23/2019
Status: ✅ Merged
Merged: 9/11/2019
Merged by: @ashthespy

Base: master ← Head: protobuf-codegen-pure

📝 Commits (1)

97f61ec Protobuf 2.4.0, generate all proto files every time, but only write when changed, supporting poor souls with crlf line ending conversion.

📊 Changes

5 files changed (+50 additions, -97 deletions)

View changed files

📝 Cargo.lock (+1 -0)
📝 protocol/Cargo.toml (+1 -0)
📝 protocol/build.rs (+46 -85)
➖ protocol/files.rs (+0 -10)
📝 protocol/src/lib.rs (+2 -2)

📄 Description

Whilst on my laptop the github desktop app cloned this repo with crlf line endings. This threw off the crc checking in the protobuf build script. Triggering a rebuild using protoc...

I'm on Windows and installing protoc is a fiddle. With the aim of very slowly making librespot easier to use and distribute on Windows I switched to using the protobuf-codegen-pure crate.

When playing with this I found out the existing string replacement technique could end up corrupting files.rs (if one checksum was a substring of another). That may only be possible when manually changing the checksums to trigger builds though...

Regardless I decided to just get rid of the shell/cksum/protoc dependencies altogether, and have a completely rust based build system for the protobuf files.

~~When trying to get rid of the shell script I moved the CRCs to the lib.rs file. I'm also now using an existing crate to calculate the checksum as there's no need to be compatible with cksum.~~

Now this PR removes the checksum system in favour of trusting cargo to run build.rs at the right times, and always re-generating the rust files. However the regenerated files are only written to disc (triggering a proper rebuild) when those resulting files have actually changed (whole-sale string comparison). This avoids all kinds of caching problems, and makes the code simpler IMO.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/librespot-org/librespot/pull/293 **Author:** [@willstott101](https://github.com/willstott101) **Created:** 2/23/2019 **Status:** ✅ Merged **Merged:** 9/11/2019 **Merged by:** [@ashthespy](https://github.com/ashthespy) **Base:** `master` ← **Head:** `protobuf-codegen-pure` --- ### 📝 Commits (1) - [`97f61ec`](https://github.com/librespot-org/librespot/commit/97f61ec2a85afa184a8becd7d3e85292bdb35562) Protobuf 2.4.0, generate all proto files every time, but only write when changed, supporting poor souls with crlf line ending conversion. ### 📊 Changes **5 files changed** (+50 additions, -97 deletions) <details> <summary>View changed files</summary> 📝 `Cargo.lock` (+1 -0) 📝 `protocol/Cargo.toml` (+1 -0) 📝 `protocol/build.rs` (+46 -85) ➖ `protocol/files.rs` (+0 -10) 📝 `protocol/src/lib.rs` (+2 -2) </details> ### 📄 Description Whilst on my laptop the github desktop app cloned this repo with crlf line endings. This threw off the crc checking in the protobuf build script. Triggering a rebuild using protoc... I'm on Windows and installing protoc is a fiddle. With the aim of very slowly making librespot easier to use and distribute on Windows I switched to using the `protobuf-codegen-pure` crate. When playing with this I found out the existing string replacement technique could end up corrupting files.rs (if one checksum was a substring of another). That may only be possible when manually changing the checksums to trigger builds though... Regardless I decided to just get rid of the shell/cksum/protoc dependencies altogether, and have a completely rust based build system for the protobuf files. ~When trying to get rid of the shell script I moved the CRCs to the lib.rs file. I'm also now using an existing crate to calculate the checksum as there's no need to be compatible with `cksum`.~ Now this PR removes the checksum system in favour of trusting cargo to run build.rs at the right times, and always re-generating the rust files. However the regenerated files are only written to disc (triggering a proper rebuild) when those resulting files have actually changed (whole-sale string comparison). This avoids all kinds of caching problems, and makes the code simpler IMO. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

kerem

2026-02-27 20:00:14 +03:00

closed this issue
added the
pull-request
label

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#864

No description provided.

Rows
Columns