git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Patches for git-push --confirm and --show-subjects

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 14, 2009, 00:47 UTC
Message-ID
<7vpr9ugxn5.fsf@alter.siamese.dyndns.org>
In-Reply-To
<1252884685-9169-1-git-send-email-otaylor@redhat.com>
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.
Previous: Owen TaylorNext: Junio C Hamano
Message 6 of 15 in “Patches for git-push --confirm and --show-subjects”
  1. Owen TaylorSep 13, 2009
  2. 1/4 push: add --confirm option to ask before sending updatesOwen Taylor, Sep 13, 2009
  3. 2/4 push: allow configuring default for --confirmOwen Taylor, Sep 13, 2009
  4. 3/4 push: add --show-subjects option to show commit synopsisOwen Taylor, Sep 13, 2009
  5. 4/4 push: allow configuring default for --show-subjectsOwen Taylor, Sep 13, 2009
  6. Junio C HamanoSep 14, 2009
  7. Junio C HamanoSep 14, 2009
  8. Owen TaylorSep 14, 2009
  9. Daniel BarkalowSep 14, 2009
  10. Owen TaylorSep 14, 2009
  11. Junio C HamanoSep 15, 2009
  12. Owen TaylorSep 15, 2009
  13. Junio C HamanoSep 15, 2009
  14. Owen TaylorSep 15, 2009
  15. Daniel BarkalowSep 15, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.