Re: Scripted clone generating an incomplete, unusable .git/config
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Nov 12, 2010, 04:32 UTC
- Message-ID
- <20101112043229.GB10765@burratino>
- In-Reply-To
- <20101111190508.GA3038@sigill.intra.peff.net>
Jeff King wrote:
Show 5 quoted lines
> I don't think it makes sense for git-clone to do this itself. If we are > going to say "SIGPIPE should default to SIG_DFL on startup" then we > should do it as the very first thing in the git wrapper, not just for > git-clone. That gives each git program a known starting point of > behavior.
Here's what that might look like.
-- 8< -- Subject: SIGPIPE and other fatal signals should default to SIG_DFL
The intuitive behavior when a git command receives a fatal signal is for it to die.
Indeed, that is the default handling. However, POSIX semantics allow the parent of a process to override that default by ignoring a signal, since ignored signals are preserved by fork() and exec(). For example, Python 2.6 and 2.7's os.popen() runs a shell with SIGPIPE ignored (Python issue 1736483).
Worst of all, in such a situation, many git commands use sigchain_push_common() to run cleanup actions (removing temporary files, discarding locks, that kind of thing) before passing control to the original handler; if that handler ignores the signal rather than exiting, then execution will continue in an unplanned-for and unusual state.
So give the signals in question default handling at the start of main(), before sigchain_push_common can be called.
NEEDSWORK:
- imposes an obnoxious maintenance burden. New non-builtins that might call sigchain_push_common() would need to have default handling restored at the start of main().
- needs tests, especially for interaction with "git daemon" client shutdown
Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Currently git daemon uses SIG_IGN state on SIGTERM to protect children with active connections. Why isn't that causing the same sort of problems as os.popen() causes?
check-racy.c | 1 + daemon.c | 1 + fast-import.c | 1 + git.c | 2 ++ http-backend.c | 1 + http-fetch.c | 1 + http-push.c | 1 + imap-send.c | 1 + remote-curl.c | 1 + shell.c | 2 ++ show-index.c | 2 ++ upload-pack.c | 1 + 12 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/check-racy.c b/check-racy.c index 00d92a1..f1ad9d5 100644 --- a/check-racy.c +++ b/check-racy.c @@ -6,6 +6,7 @@ int main(int ac, char **av) int dirty, clean, racy; dirty = clean = racy = 0; + sigchain_push_common(SIG_DFL); read_cache(); for (i = 0; i < active_nr; i++) { struct cache_entry *ce = active_cache[i]; diff --git a/daemon.c b/daemon.c index 7ccd097..7719f33 100644 --- a/daemon.c +++ b/daemon.c @@ -1001,6 +1001,7 @@ int main(int argc, char **argv) gid_t gid = 0; int i; + sigchain_push_common(SIG_DFL); git_extract_argv0_path(argv[0]); for (i = 1; i < argc; i++) { diff --git a/fast-import.c b/fast-import.c index dc48245..ce28759 100644 --- a/fast-import.c +++ b/fast-import.c @@ -3027,6 +3027,7 @@ int main(int argc, const char **argv) { unsigned int i; + sigchain_push_common(SIG_DFL); git_extract_argv0_path(argv[0]); if (argc == 2 && !strcmp(argv[1], "-h")) diff --git a/git.c b/git.c index e08d0f1..a8677fb 100644 --- a/git.c +++ b/git.c @@ -502,6 +502,8 @@ int main(int argc, const char **argv) if (!cmd) cmd = "git-help"; + sigchain_push_common(SIG_DFL); + /* * "git-xxxx" is the same as "git xxxx", but we obviously: * diff --git a/http-backend.c b/http-backend.c index 14c90c2..b03455a 100644 --- a/http-backend.c +++ b/http-backend.c @@ -550,6 +550,7 @@ int main(int argc, char **argv) char *cmd_arg = NULL; int i; + sigchain_push_common(SIG_DFL); git_extract_argv0_path(argv[0]); set_die_routine(die_webcgi); diff --git a/http-fetch.c b/http-fetch.c index 762c750..aca4e8f 100644 --- a/http-fetch.c +++ b/http-fetch.c @@ -24,6 +24,7 @@ int main(int argc, const char **argv) int get_verbosely = 0; int get_recover = 0; + sigchain_push_common(SIG_DFL); git_extract_argv0_path(argv[0]); while (arg < argc && argv[arg][0] == '-') { diff --git a/http-push.c b/http-push.c index c9bcd11..a9d0894 100644 --- a/http-push.c +++ b/http-push.c @@ -1791,6 +1791,7 @@ int main(int argc, char **argv) struct remote *remote; char *rewritten_url = NULL; + sigchain_push_common(SIG_DFL); git_extract_argv0_path(argv[0]); repo = xcalloc(sizeof(*repo), 1); diff --git a/imap-send.c b/imap-send.c index 71506a8..eb13adc 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1538,6 +1538,7 @@ int main(int argc, char **argv) int nongit_ok; + sigchain_push_common(SIG_DFL); git_extract_argv0_path(argv[0]); if (argc != 1) usage(imap_send_usage); diff --git a/remote-curl.c b/remote-curl.c index 04d4813..804492d 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -787,6 +787,7 @@ int main(int argc, const char **argv) struct strbuf buf = STRBUF_INIT; int nongit; + sigchain_push_common(SIG_DFL); git_extract_argv0_path(argv[0]); setup_git_directory_gently(&nongit); if (argc < 2) { diff --git a/shell.c b/shell.c index dea4cfd..9798b99 100644 --- a/shell.c +++ b/shell.c @@ -137,6 +137,8 @@ int main(int argc, char **argv) int devnull_fd; int count; + sigchain_push_common(SIG_DFL); + /* * Always open file descriptors 0/1/2 to avoid clobbering files * in die(). It also avoids not messing up when the pipes are diff --git a/show-index.c b/show-index.c index 4c0ac13..7d48c88 100644 --- a/show-index.c +++ b/show-index.c @@ -11,6 +11,8 @@ int main(int argc, char **argv) unsigned int version; static unsigned int top_index[256]; + sigchain_push_common(SIG_DFL); + if (argc != 1) usage(show_index_usage); if (fread(top_index, 2 * 4, 1, stdin) != 1) diff --git a/upload-pack.c b/upload-pack.c index f05e422..4c764f7 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -682,6 +682,7 @@ int main(int argc, char **argv) int i; int strict = 0; + sigchain_push_common(SIG_DFL); git_extract_argv0_path(argv[0]); read_replace_refs = 0;