-
Notifications
You must be signed in to change notification settings - Fork 261
ci: remove binary size check #710
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
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.
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.
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.