Re: Scripted clone generating an incomplete, unusable .git/config
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:50:01
Subsystem:
the rest · Maintainer:
Linus Torvalds
Jeff King wrote:
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 <redacted> Signed-off-by: Jonathan Nieder <redacted> --- 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;