{"thread":{"id":"61765","subject":"[PATCH] git-gui: do not exit upon prepare-commit-msg hook failure","startedAt":"2024-07-11T14:03:44Z","lastAt":"2024-07-13T20:50:12Z","messageCount":4,"participants":["Anthony Loiseau","Eric Sunshine","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"498510","messageId":"20240711132542.9792-1-anthony@loiseau.fr","threadId":"61765","inReplyTo":null,"subject":"[PATCH] git-gui: do not exit upon prepare-commit-msg hook failure","fromName":"Anthony Loiseau","fromEmail":"anthony@loiseau.fr","sentAt":"2024-07-11T13:25:39Z","receivedAt":"2024-07-11T14:03:44Z","isPatch":true,"sender":{"key":"anthony@loiseau.fr","avatar":null},"body":"Targeted issue\n==============\n\ngit-gui is currently unusable when prepare-commit-msg hook fails, since\nit closes as soon as user dismiss modal error popup shown on start.\n\nProposal\n========\n\nNext mail is a proposal to make this failure non-fatal. Popup is still\nshown but not in its fatal variant (which removes the \"you must fix this\nbefore commiting\" unwanted sentence), and git-gui is not terminated upon\npopup dismiss.\n\nWith this proposal, user is clearly aware of a prepare-commit-msg hook\nfailure but is not denied to use git-gui and can even commit.\nPre-filled commit message content is likely empty or not filled in this\ncase, which is not a big issue.\n\nOther hooks are not affected. commit-msg hook and next ones are still\ntriggered upon commit action, telling commit can not be performed (fatal\nvariant of the popup) without terminating git-gui upon dismiss.\n\nHow to test\n===========\n\ncat > .git/hooks/fake_failing_hook <<EOF\n#!/bin/sh\necho 'FAKE FAILING HOOK $0' >&2\nexit 1\nEOF\n\nfor i in .git/hooks/*.sample\ndo\n\tln -svf fake_failing_hook \"${i%%.sample}\"\ndone\n\nchmod u+x .git/hooks/*\nrm .git/GITGUI_MSG\ngit gui\n\n\nAnthony Loiseau (1):\n  git-gui: do not exit upon prepare-commit-msg hook failure\n\n git-gui/git-gui.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\n-- \n2.45.2\n\n"},{"id":"498515","messageId":"20240711132542.9792-2-anthony@loiseau.fr","threadId":"61765","inReplyTo":"20240711132542.9792-1-anthony@loiseau.fr","subject":"[PATCH] git-gui: do not exit upon prepare-commit-msg hook failure","fromName":"Anthony Loiseau","fromEmail":"anthony@loiseau.fr","sentAt":"2024-07-11T13:25:40Z","receivedAt":"2024-07-11T14:43:44Z","isPatch":true,"sender":{"key":"anthony@loiseau.fr","avatar":null},"body":"prepare-commit-msg hook is fired as soon git-gui is started\nand upon F5 in order to pre-fill commit message area.\n\nHaving it fatal, forcibly exiting app when this hook fails\nrendered git-gui unusable in this case. Fix this by not\ntreating this hook as fatal, and not exiting app when failure\npopup is dismissed.\n\nThis way, user can use git-gui when he/she dismisses failure popup\nof a failed prepare-commit-msg hook.\n\nNote that this commit does not deny user from commiting when\nprepare-commit-msg failed. Message is simply not pre-filled.\n\nSigned-off-by: Anthony Loiseau <anthony@loiseau.fr>\n---\n git-gui/git-gui.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex 8fe7538e72..ab6caaf2ae 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -1643,9 +1643,8 @@ proc prepare_commit_msg_hook_wait {fd_ph} {\n \tif {[eof $fd_ph]} {\n \t\tif {[catch {close $fd_ph}]} {\n \t\t\tui_status [mc \"Commit declined by prepare-commit-msg hook.\"]\n-\t\t\thook_failed_popup prepare-commit-msg $pch_error\n+\t\t\thook_failed_popup prepare-commit-msg $pch_error 0\n \t\t\tcatch {file delete [gitdir PREPARE_COMMIT_MSG]}\n-\t\t\texit 1\n \t\t} else {\n \t\t\tload_message PREPARE_COMMIT_MSG\n \t\t}\n-- \n2.45.2\n\n"},{"id":"498523","messageId":"CAPig+cTV6e=D3L3my5igCewnDNyx12o=j+AAqRCry-UFiQoASg@mail.gmail.com","threadId":"61765","inReplyTo":"20240711132542.9792-1-anthony@loiseau.fr","subject":"Re: [PATCH] git-gui: do not exit upon prepare-commit-msg hook failure","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-07-11T17:15:26Z","receivedAt":"2024-07-11T17:15:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Adding the current git-gui maintainer[*] to Cc: list...\n\n[*] https://lore.kernel.org/git/0241021e-0b17-4031-ad9f-8abe8e0c0097@kdbg.org/\n\nOn Thu, Jul 11, 2024 at 10:04 AM Anthony Loiseau <anthony@loiseau.fr> wrote:\n>\n> Targeted issue\n> ==============\n>\n> git-gui is currently unusable when prepare-commit-msg hook fails, since\n> it closes as soon as user dismiss modal error popup shown on start.\n>\n> Proposal\n> ========\n>\n> Next mail is a proposal to make this failure non-fatal. Popup is still\n> shown but not in its fatal variant (which removes the \"you must fix this\n> before commiting\" unwanted sentence), and git-gui is not terminated upon\n> popup dismiss.\n>\n> With this proposal, user is clearly aware of a prepare-commit-msg hook\n> failure but is not denied to use git-gui and can even commit.\n> Pre-filled commit message content is likely empty or not filled in this\n> case, which is not a big issue.\n>\n> Other hooks are not affected. commit-msg hook and next ones are still\n> triggered upon commit action, telling commit can not be performed (fatal\n> variant of the popup) without terminating git-gui upon dismiss.\n>\n> How to test\n> ===========\n>\n> cat > .git/hooks/fake_failing_hook <<EOF\n> #!/bin/sh\n> echo 'FAKE FAILING HOOK $0' >&2\n> exit 1\n> EOF\n>\n> for i in .git/hooks/*.sample\n> do\n>         ln -svf fake_failing_hook \"${i%%.sample}\"\n> done\n>\n> chmod u+x .git/hooks/*\n> rm .git/GITGUI_MSG\n> git gui\n>\n>\n> Anthony Loiseau (1):\n>   git-gui: do not exit upon prepare-commit-msg hook failure\n>\n>  git-gui/git-gui.sh | 3 +--\n>  1 file changed, 1 insertion(+), 2 deletions(-)\n>\n> --\n> 2.45.2\n>\n>\n"},{"id":"498641","messageId":"7789e98f-df21-4390-943c-1c96262bd0d6@kdbg.org","threadId":"61765","inReplyTo":"20240711132542.9792-2-anthony@loiseau.fr","subject":"Re: [PATCH] git-gui: do not exit upon prepare-commit-msg hook failure","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2024-07-13T20:50:01Z","receivedAt":"2024-07-13T20:50:12Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"[j6t: removed Shawn from the Cc list]\n\nAm 11.07.24 um 15:25 schrieb Anthony Loiseau:\n> prepare-commit-msg hook is fired as soon git-gui is started\n> and upon F5 in order to pre-fill commit message area.\n> \n> Having it fatal, forcibly exiting app when this hook fails\n> rendered git-gui unusable in this case. Fix this by not\n> treating this hook as fatal, and not exiting app when failure\n> popup is dismissed.\n\nIt is understandable that a Git GUI that cannot be started looks like a\nproblem that needs solving. However, I am not happy with the proposed\nsolution.\n\nThe first reason is that even if Git GUI can now run, the error message\nstill reappears every time when Rescan (F5) is used and must be\ndismissed each time. (The version of Git GUI that I use does a rescan on\nwindow activation and now runs into rescan loop after the dismission; I\ncan't even exit Git GUI anymore.)\n\n> This way, user can use git-gui when he/she dismisses failure popup\n> of a failed prepare-commit-msg hook.\n> \n> Note that this commit does not deny user from commiting when\n> prepare-commit-msg failed. Message is simply not pre-filled.\n\nThis is the second reason. As a first step, an attempt should be made to\nfill the edit box with the unprocessed commit message.\n\nI am unsure what next steps should be, though.\n\n- Should we not allow to make a commit?\n- Should we allow to edit the message and then permit a commit?\n\nI can't tell, because I do not know for what purposes people use the\nprepare-commit-msg hook.\n\nWithout these two points addressed, I consider it better that Git GUI\ndoes not even start or exits.\n\n> \n> Signed-off-by: Anthony Loiseau <anthony@loiseau.fr>\n> ---\n>  git-gui/git-gui.sh | 3 +--\n>  1 file changed, 1 insertion(+), 2 deletions(-)\n> \n> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n> index 8fe7538e72..ab6caaf2ae 100755\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -1643,9 +1643,8 @@ proc prepare_commit_msg_hook_wait {fd_ph} {\n>  \tif {[eof $fd_ph]} {\n>  \t\tif {[catch {close $fd_ph}]} {\n>  \t\t\tui_status [mc \"Commit declined by prepare-commit-msg hook.\"]\n> -\t\t\thook_failed_popup prepare-commit-msg $pch_error\n> +\t\t\thook_failed_popup prepare-commit-msg $pch_error 0\n>  \t\t\tcatch {file delete [gitdir PREPARE_COMMIT_MSG]}\n> -\t\t\texit 1\n>  \t\t} else {\n>  \t\t\tload_message PREPARE_COMMIT_MSG\n>  \t\t}\n\nBelow this context is the same catch {file delete ...}. Since it is now\nreached, the instance in the if-branch should be removed.\n\n-- Hannes\n\n"}]}