Without reading much of the code, my knee jerk reactions are:
* This probably can (and from the longer term perspective, should) be
done inside a pre-push hook that can decline pushing;
* I do not think it should use two separate push_refs call into transport
(first with dry-run and second with real).
Immediately after match_refs() call in transport_push(), you know if
the push is a non-fast-forward (in which case you do not know what you
will be losing anyway because you haven't seen what you are missing
from the other end) or exactly what your fast-forward push will be
sending, so between that call and the actual transport->push_refs()
would be the ideal place to call the hook, with a list of "ref old
new", without running a dry-run.
for a few reasons.
(1) When push.confirm is set, you do not want to interact with the user
when the standard input is not a terminal. But an automated script
that runs git-push can still use an appropriate pre-push hook to make
the decision to intervene without human presense.
(2) As your --show-subjects patch shows, the likes and dislikes of the
output format for confirmation would be highly personal. A separate
hook that is fed list of <ref, old, new> would make it easier to
customize this to suite people's tastes.
(3) I do not trust the use of the fmt_merge_message() code in this
codepath. That code, like all the major parts of git, relies on
being able to use the object flag bits for its own purpose, and there
is a chance that the way transports (present and future) optimizes
(or may want to optimize in the future) the object transfer by
implementing clever common ancestry discovery, similar to what is
done for the fetch-pack side.
If we force the actual confirmation process out to a separate process
that runs a hook, I do not have to worry about that, which is a huge
relief for maintainability of the system.
(4) The same objects flag bits contamination issue makes me worried about
your approach of running one transport_push() with dry-run and then
another without.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html