git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: jrnieder@gmail.com, pclouds@gmail.com
Cc: gitster@pobox.com, git@vger.kernel.org, sunshine@sunshineco.com,
	mhagger@alum.mit.edu, ronniesahlberg@gmail.com,
	Stefan Beller <sbeller@google.com>
Subject: [PATCHv11 02/11] receive-pack.c: die instead of error in case of possible future bug
Date: Tue,  6 Jan 2015 11:40:47 -0800	[thread overview]
Message-ID: <1420573247-10250-1-git-send-email-sbeller@google.com> (raw)
In-Reply-To: <20150105212523.GN29365@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. Running 'git fsck' should not cause any problems though.

Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---

Notes:
    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 | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 2ebaf66..7b0d0f4 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1061,9 +1061,8 @@ 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: run 'git fsck' for safety.\n"
+		    "BUG: connectivity check skipped???");
 }
 
 static void execute_commands(struct command *commands,
-- 
2.2.1.62.g3f15098

  reply	other threads:[~2015-01-06 19:41 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-30 23:41 [PATCH 0/9] atomic pushes Stefan Beller
2014-12-30 23:41 ` [PATCHv9 1/9] receive-pack.c: shorten the execute_commands loop over all commands Stefan Beller
2015-01-03  2:20   ` Jonathan Nieder
2015-01-03  9:53     ` Duy Nguyen
2015-01-05 18:02       ` Stefan Beller
2015-01-05 18:25         ` [PATCHv10 01/10] " Stefan Beller
2015-01-05 18:25           ` [PATCHv10 02/10] receive-pack.c: die instead of error in assure_connectivity_checked Stefan Beller
2015-01-05 20:17             ` Jonathan Nieder
2015-01-05 21:15               ` Stefan Beller
2015-01-05 21:25                 ` Jonathan Nieder
2015-01-06 19:40                   ` Stefan Beller [this message]
2015-01-06 19:46                     ` [PATCHv11 02/11] receive-pack.c: die instead of error in case of possible future bug Jonathan Nieder
2015-01-05 20:22           ` [PATCHv10 01/10] receive-pack.c: shorten the execute_commands loop over all commands Jonathan Nieder
2015-01-05 21:07             ` Stefan Beller
2015-01-05 21:18               ` Jonathan Nieder
2015-01-06 19:34                 ` [PATCHv11 01/11] " Stefan Beller
2014-12-30 23:41 ` [PATCHv9 2/9] receive-pack.c: move iterating over all commands outside execute_commands Stefan Beller
2014-12-30 23:41 ` [PATCHv9 3/9] receive-pack.c: move transaction handling in a central place Stefan Beller
2014-12-30 23:41 ` [PATCHv9 4/9] receive-pack.c: add execute_commands_atomic function Stefan Beller
2014-12-30 23:41 ` [PATCHv9 5/9] receive-pack.c: negotiate atomic push support Stefan Beller
2014-12-30 23:41 ` [PATCHv9 6/9] send-pack: rename ref_update_to_be_sent to check_to_send_update Stefan Beller
2014-12-30 23:41 ` [PATCHv9 7/9] send-pack.c: add --atomic command line argument Stefan Beller
2014-12-30 23:41 ` [PATCHv9 8/9] push.c: add an --atomic argument Stefan Beller
2014-12-30 23:41 ` [PATCHv9 9/9] t5543-atomic-push.sh: add basic tests for atomic pushes Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1420573247-10250-1-git-send-email-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=mhagger@alum.mit.edu \
    --cc=pclouds@gmail.com \
    --cc=ronniesahlberg@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).