terminate Sashiko early when HEAD commit hash not found

[TheSven73]

We expect to be always able to retrieve the HEAD commit hash. Reports from the field suggest that this is not always the case. We do not yet understand the root cause.

Failure to retrieve the commit hash results in deadlocks, which prevent patches from completing review.

Take a leaf out of The Pragmatic Programmer's "Dead Software tells no lies" and terminate early when the unexpected happens.

Pros:

• failing reviews are retried after Sashiko restarts (instead of deadlocked)
• the termination log allows us to learn why the unexpected happened

Cons:

• if not intermittent, failing reviews may be retried forever

See #248

[derekbarbosa]

Hi @TheSven73 !

Do you think that this may be complementary to #228?

Some of the changes there attempt to resolve a proper baseline/head when one cannot be found. I see here the emphasis of this PR is to "fail better" when a commit hash cannot be resolved.

I would welcome a few eyes on that, if you have some spare time :)

[TheSven73]

Hi @derekbarbosa

From what I understand, it's not quite the same thing. The issue here is something that should always work (a git repo always has a HEAD) which fails, the unexpected untested failure path percolates, and eventually threatens the integrity of the running Sashiko instance. It struggles on, wounded :)

Whereas your #228 appear to deal with improvements to DESIGN_BASELINE_DETECTION.md and/or DESIGN_ROBUST_BASELINE.md

Do you think that sounds right? Let me know, thanks.

[derekbarbosa]

Hi @derekbarbosa

From what I understand, it's not quite the same thing. The issue here is something that should always work (a git repo always has a HEAD) which fails, the unexpected untested failure path percolates, and eventually threatens the integrity of the running Sashiko instance. It struggles on, wounded :)
Whereas your #228 appear to deal with improvements to DESIGN_BASELINE_DETECTION.md and/or DESIGN_ROBUST_BASELINE.md

Do you think that sounds right? Let me know, thanks.

Ah, thanks for the clarification. I hadn't had a chance to look at the code when following the parent discussion from Arnaldo -- so sorry for offloading my "wet compute" to you 😄

[rgushchin]

I don't think panic'ing the main binary is justified here. Instead the review should be terminated gracefully with patch(set) getting into the "failed to apply" state. Panicing the main binary will be really bad for the user experience.