refactor(tools): rewrite rk_unpacker in Go for parallel extraction#87
refactor(tools): rewrite rk_unpacker in Go for parallel extraction#87Ri4ards2006 wants to merge 1 commit into
Conversation
|
Hmm, please help me out here. While the first commit looks potentially meaningful, I don’t believe rewriting existing working Python code in Go adds meaningful benefit for the user. It’s a rarely used piece of software, so performance is not critical, and the need to compile a binary before use makes it more of a hassle than just running a Python script. Do you envisage any other benefits from the switch? Is there anything the current Python version doesn’t do which you believe would be helpful? Or are there visible inefficiencies which are annoying enough to warrant a refactor? In either of these cases I believe the better approach is to improve the existing code, not write a completely new thing next to it. The second commit escapes me altogether. It looks like a 3000-LoC code dump to address an unknown use case, and not even explained at all. Needs a separate pull request in any case, as it’s completely unrelated to the first commit. I don’t know if anyone needs a daemon and a bunch of clients to talk to what is standard kernel Type-C machinery though: if you believe it’s needed, please explain in more detail |
b680ce0 to
97e1246
Compare
|
Hi Alexey, Thank you so much for the detailed and critical review. This is my first major open-source contribution, and I highly appreciate your guidance on architecture, maintenance overhead, and PR scoping. To be completely honest, I actually realized yesterday evening that I should have split these two features into separate branches, and I planned to clean it up. I apologize for the confusing 3100-LoC code dump inside a build-tooling PR—that was entirely due to a mistake in my local branching workflow. I have just force-pushed to this branch to completely remove the daemon code and integration from this PR. Let's keep this PR strictly focused on the Regarding the Type-C/UCSI daemon ( Regarding the Go port of
To be completely transparent, there is definitely a bit of personal bias here as well: I am heavily focusing on mastering low-level Go system programming right now, and I used this task as a challenge to see where Go's concurrency could systematically improve host-side build components. That being said, if you feel that maintaining a new language boundary for a tool that isn't a tight bottleneck introduces more hassle than it's worth for the project's long-term lifecycle, I completely respect that. If you prefer to close this PR, I’d still love to help out by porting these robustness fixes (like explicit endianness validation and malformed image checks) back into the legacy Python script instead! Let me know what you think of the cleaned-up PR state! |
|
I'm not a huge promoter of either Python or Go, but if an existing tool works and does its job then I'm sceptical about rewriting it in another language. This tool is not used in image builds in any way, it's just a convenience script for disassembling firmware images packed in Rockchip's format which some vendors publish - to extract separate binaries and filesystem images out of them. It only needs to be done sporadically, on the order of ~once per device type, and never in any concurrent situation or on a CI worker. Definitely not a place where I'd worry about IO or resource efficiency |
Motivation & Personal Note
Hi everyone! First of all, I want to say how incredibly excited I am to submit this. I've been a huge fan of the Flipper ecosystem and have been tinkering with the Flipper Zero for a while. This is my first open-source contribution to a project of this scale!
I'm currently focused on deepening my low-level Linux systems skills and putting my Go expertise to the test. I couldn't think of a better place to apply this than right here in the Flipper build scripts.
Description
This PR introduces two major independent Go-based upgrades to the repository, aimed at optimizing host-side build performance and modernizing target-OS hardware monitoring.
Part 1:
rk_unpackerPerformance Rewrite (Commit 1)io.SectionReaderand async.Poolfor 1 MiB recycled buffers to maintain an execution footprint bounded only by allocation size.rk_unpacker_test.gocontaining table-driven contract tests for ASCII validation and byte-size formatting.Part 2: Native Target Daemon
flipd&flipctlCLI (Commit 2)map[string]*ucsi.PortStatusinside the daemon loop to completely eliminate journal log flooding. Logs are emitted strictly on real role transitions or unplug events.syscall.Ucred) to enforce strict caller authentication.ProtectSystem=strict,RestrictAddressFamilies=AF_UNIX AF_NETLINK).systemctl --root=/hooks instead of flaky chroot operations.Testing Performed
CGO_ENABLED=0).go test -v ./...executes and passes all 40+ generated subtests flawlessly.