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

[PATCHv12 02/10] receive-pack.c: die instead of error in case of possible future bug

From
Stefan Beller <sbeller@google.com>
Date
Jan 8, 2015, 03:23 UTC
Message-ID
<1420687404-13997-3-git-send-email-sbeller@google.com>
In-Reply-To
<1420687404-13997-1-git-send-email-sbeller@google.com>

Discussion on the previous patch revealed we rather want to err on the safe side. To do so we need to stop receive-pack in case of the possible future bug when connectivity is not checked on a shallow push.

Also while touching that code we considered that removing the reported refs may be harmful in some situations. Sound the message more like a "This Cannot Happen, Please Investigate!" instead of giving advice to remove refs.

Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
Notes:
    v12:
    * no advice in case we die.
    
    v11:
    * only die at the end so the loop works out for all refs.
    * Remove the advice to delete refs.
    
    > If the message says
    >        fatal: BUG: connectivity check skipped???
    > then it has exactly the right amount of information to tell me what to
    > do.  Now I have
    > - a short string to grep for in the source code (or on the web) to
    >    find out what happened
    
    And do a git blame to see previous versions?
    
    I am not so sure of this patch any more as it actually stops people
    doing work if they want to do so. (They may deliberately choose to
    ignore the BUG:... message, because of a deadline in 2 hours.)
    
    So I do think this helps on getting people to report the bug in the
    future if it arises faster, but on the other hand if we assume the
    faulty hardware scenario and the deadline we actually stop people
    from getting their desired work done.
    
    This patch doesn't actually relate to the topic of the series
    (atomic pushes), but is a cleanup-as-we-go patch. If we need to
    have further discussion on this, I'd rather want to delay this patch
    and have a follow up on top of the atomic series.
    
    Thanks,
    Stefan
    
    v10:
    * new in v10.
 builtin/receive-pack.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 2ebaf66..3bdb158 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1061,9 +1061,7 @@ static void warn_if_skipped_connectivity_check(struct command *commands,
 		}
 	}
 	if (!checked_connectivity)
-		error("BUG: run 'git fsck' for safety.\n"
-		      "If there are errors, try to remove "
-		      "the reported refs above");
+		die("BUG: connectivity check skipped???");
 }
 
 static void execute_commands(struct command *commands,
-- 
2.2.1.62.g3f15098
Previous: Stefan BellerNext: Stefan Beller
Message 3 of 17 in “[PATCHv12 00/10] atomic pushes”
  1. Stefan BellerJan 8, 2015
  2. 01/10 receive-pack.c: shorten the execute_commands loop over all commandsStefan Beller, Jan 8, 2015
  3. 02/10 receive-pack.c: die instead of error in case of possible future bugStefan Beller, Jan 8, 2015
  4. 03/10 receive-pack.c: move iterating over all commands outside execute_commandsStefan Beller, Jan 8, 2015
  5. 04/10 receive-pack.c: move transaction handling in a central placeStefan Beller, Jan 8, 2015
  6. 05/10 receive-pack.c: add execute_commands_atomic functionStefan Beller, Jan 8, 2015
  7. 06/10 receive-pack.c: negotiate atomic push supportStefan Beller, Jan 8, 2015
  8. Junio C HamanoJan 8, 2015
  9. Eric SunshineJan 12, 2015
  10. Stefan BellerJan 12, 2015
  11. Junio C HamanoJan 13, 2015
  12. 07/10 send-pack: rename ref_update_to_be_sent to check_to_send_updateStefan Beller, Jan 8, 2015
  13. 08/10 send-pack.c: add --atomic command line argumentStefan Beller, Jan 8, 2015
  14. Junio C HamanoJan 12, 2015
  15. 09/10 push.c: add an --atomic argumentStefan Beller, Jan 8, 2015
  16. 10/10 t5543-atomic-push.sh: add basic tests for atomic pushesStefan Beller, Jan 8, 2015
  17. Eric SunshineJan 12, 2015

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.