3 ms·

>Though we're also talking about an organization that couldn't merge a PR for a year that fixed a one liner. A mistake that should never have gotten through rev

>Though we're also talking about an organization that couldn't merge a PR for a year that fixed a one liner. A mistake that should never have gotten through review. Seriously, who uses a while loop counter checking for equality?!? I'm still convinced they left the "bug" because it made them money What is this in reference to? I tried to search for it but only found this comment. “Github while loop fix that was in review for a year”?

godelski 7mo ago
It was the safe_sleep function. Here's an issue on it [0]. IIRC there was an early issue, but really this is code that never should have made it in. Here's the conditional in question SECONDS=0 while [[ $SECONDS -lt $1 ]]; do : done Here's the fix... (s/!=/-lt) while [[ $SECONDS -lt $1 ]]; It's a fallback sleep function for if you don't have the sleep command, (or read, or ping) then it'll increment SECONDS (special variable) until the time has passed because : does nothing (it will peg your CPU though). Problem is the loop isn't computed with infinite precision. Doesn't take a genius to figure out < is infinitely better than != here and you'd be right to guess that people did in fact waste thousands of dollars getting stuck in infinite loops that were entirely unavoidable. Here's the actual merge...[1] At least it didn't take them months to merge this line, which should have existed from day 1 too (a very very well known pattern for writing bash scripts)[2] [0] https://github.com/actions/runner/issues/3792 https://github.com/actions/runner/issues/3792 [1] https://github.com/actions/runner/pull/3157/changes https://github.com/actions/runner/pull/3157/changes [2] https://github.com/meshtastic/firmware/pull/7922 https://github.com/meshtastic/firmware/pull/7922
- jmholla 7mo ago
  FYI, in your reproduction, both of the conditionals are the same. But you are right, the initial implementation was `!=` while [[ $SECONDS != $1 ]]; do became while [[ $SECONDS -lt $1 ]]; do
  - godelski 7mo ago
    >.< idk why I copy pasted my line with the correction again. Sorry about that, but glad you got it despite that haha

godelski 7mo ago

It was the safe_sleep function. Here's an issue on it [0]. IIRC there was an early issue, but really this is code that never should have made it in. Here's the conditional in question SECONDS=0 while [[ $SECONDS -lt $1 ]]; do : done Here's the fix... (s/!=/-lt) while [[ $SECONDS -lt $1 ]]; It's a fallback sleep function for if you don't have the sleep command, (or read, or ping) then it'll increment SECONDS (special variable) until the time has passed because : does nothing (it will peg your CPU though). Problem is the loop isn't computed with infinite precision. Doesn't take a genius to figure out < is infinitely better than != here and you'd be right to guess that people did in fact waste thousands of dollars getting stuck in infinite loops that were entirely unavoidable. Here's the actual merge...[1] At least it didn't take them months to merge this line, which should have existed from day 1 too (a very very well known pattern for writing bash scripts)[2] [0] https://github.com/actions/runner/issues/3792 https://github.com/actions/runner/issues/3792 [1] https://github.com/actions/runner/pull/3157/changes https://github.com/actions/runner/pull/3157/changes [2] https://github.com/meshtastic/firmware/pull/7922 https://github.com/meshtastic/firmware/pull/7922

jmholla 7mo ago
FYI, in your reproduction, both of the conditionals are the same. But you are right, the initial implementation was `!=` while [[ $SECONDS != $1 ]]; do became while [[ $SECONDS -lt $1 ]]; do
- godelski 7mo ago
  >.< idk why I copy pasted my line with the correction again. Sorry about that, but glad you got it despite that haha