ci: remove binary size check #710
Conversation
|
And anything which depends on an implementation in YamlScript is right the fuck out. |
|
If the goal is to delete the current implementation and add back something different later, all of .github/actions can get wiped too. +1 to getting rid of this, probably should have been a python script if something more json-capable than bash was needed. It's impossible to repro this job locally as-is. |
Remove the "binary size check" workflow because it is often broken and now and then makes me sit through a discussion about its security needs. These discussions always make me feel very silly. I originally hoped this CI job would be a useful part of assuring that we would not accidentally bloat binaries when backtrace merges into std. However, it feels like that is not achieved by this workflow and at some point it has definitely outweighed the cost of maintaining it. We can readd something if we find a better way to implement things which is notably less jank and gets less complaints.
workingjubilee
force-pushed
the
remove-binary-size-check-workflow
branch
from
55e8a40 to
7718dae
Compare
|
Good catch. Thank you. |
|
If you do wind up reimplementing, I have some logic for finding the exact PR diff that you could probably steal https://github.com/rust-lang/compiler-builtins/blob/a4c748f72a1dce652cc3e41c3a8425731bd1519a/ci/ci-util.py#L124-L179 (used there for running the multi-hour tests only if relevant files changed) and for comparing to a baseline that was saved as an artifact https://github.com/rust-lang/compiler-builtins/blob/a4c748f72a1dce652cc3e41c3a8425731bd1519a/ci/ci-util.py#L271-L343 (the icount benchmarks use the latest run on master as a baseline rather than running twice across the diff). I'm not "very satisfied" with that solution because it seems like those things should be way easier, but I would say "somewhat satisfied" since it's been reasonably problem-free for a while. |
|
Just popping in to say that even though it didn't last and was annoying for you while it did, I really appreciate that you did this in the first place. It was a cool experiment, I learnt a lot about this library and Rust internals from all of the discussions around it, and it was just frankly nice to know that factors that are important to embedded developers continue to be important to people who work on Rust. I just don't want you to think that your initial effort on this was a waste of time or unappreciated because you've had to turf it now. It happens. I honestly think that the best way to make concessions for backtrace's impact on code size is with something as "simple" as an equivalent to ("Simple" is in quotes because I know it's not, I've looked into it, it's extremely non-trivial and there's even an RFC for it somewhere that languishes because of the difficulty. Simple as in, interface simple, not implementation simple.) It's currently achievable by DIY building std, which we're going to have to do anyway since our platform was demoted to tier 3, so it's at least doable. Thanks again. |
Remove the "binary size check" workflow because it is often broken and now and then makes me sit through a discussion about its security needs. These discussions always make me feel very silly. I originally hoped this CI job would be a useful part of assuring that we would not accidentally bloat binaries when backtrace merges into std. However, it feels like that is not achieved by this workflow and at some point it has definitely outweighed the cost of maintaining it. We can readd something if we find a better way to implement things which is notably less jank and gets less complaints.
Pick up the following pull requests: * ci: remove binary size check (not relevant in rust-lang/rust) <rust-lang/backtrace-rs#710> * Upgrade `ruzstd`, `object`, and `addr2line` to the latest versions <rust-lang/backtrace-rs#718>
|
@detly Of course! I wanted this to work out... I just also... well... It was a mercy killing. :^) |
…ngjubilee Update the `backtrace` submodule Pick up the following pull requests: * ci: remove binary size check (not relevant in rust-lang/rust) <rust-lang/backtrace-rs#710> * Upgrade `ruzstd`, `object`, and `addr2line` to the latest versions <rust-lang/backtrace-rs#718>
Rollup merge of #142562 - tgross35:update-backtrace, r=workingjubilee Update the `backtrace` submodule Pick up the following pull requests: * ci: remove binary size check (not relevant in rust-lang/rust) <rust-lang/backtrace-rs#710> * Upgrade `ruzstd`, `object`, and `addr2line` to the latest versions <rust-lang/backtrace-rs#718>
Remove the "binary size check" workflow because it is often broken and now and then makes me sit through a discussion about its security needs. These discussions always make me feel very silly.
I originally hoped this CI job would be a useful part of assuring that we would not accidentally bloat binaries when backtrace merges into std. However, it feels like that is not achieved by this workflow and at some point it has definitely outweighed the cost of maintaining it.
We can readd something if we find a better way to implement things which is notably less jank and gets less complaints.