{"thread":{"id":"42760","subject":"git-credentials-store.exe crash","startedAt":"2016-06-30T21:26:29Z","lastAt":"2016-07-06T15:37:30Z","messageCount":21,"participants":["dmh@ucar.edu","Jeff King","Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"290618","messageId":"7210bce6-093b-9fdf-9a9b-94059ad0d4e3@ucar.edu","threadId":"42760","inReplyTo":null,"subject":"git-credentials-store.exe crash","fromName":"dmh@ucar.edu","fromEmail":"dmh@ucar.edu","sentAt":"2016-06-30T21:24:41Z","receivedAt":"2016-06-30T21:26:29Z","isPatch":false,"sender":{"key":"dmh@ucar.edu","avatar":null},"body":"Since I cannot submit a github issue, I will try here.\n\n\n\n  - Which version of Git for Windows are you using? 32-bit or 64-bit? \nInclude the\n    output of `git version` as well.\ngit version 2.9.0.windows.1\n32-bit\n\n  - Which version of Windows are you running? 32-bit or 64-bit?\n64-bit windows 7\n\n  - What options did you set as part of the installation? Or did you \nchoose the\n    defaults?\nno git credentials manager\n\n  - Any other interesting things about your environment that might be \nrelated\n    to the issue you're seeing?\nUnknown\n\n### Details\n\n  - Which terminal/shell are you running Git from? e.g \nBash/CMD/PowerShell/other\n\n  cygwin bash\n\n  - What commands did you run to trigger this issue? If you can provide a\n    [Minimal, Complete, and Verifiable \nexample](http://stackoverflow.com/help/mcve)\n    this will help us understand the issue.\n\nI kept getting complaints by git about credentials lock existing.\ndeleted it and then git-credentials-store crashed as follows.\n```\nCarson:part2: git push\nfatal: unable to get credential storage lock: File exists\nEverything up-to-date\nCarson:part2: ls ~/.git*\n/cygdrive/c/Users/dmh/.gitconfig* \n/cygdrive/c/Users/dmh/.gitignore*\n/cygdrive/c/Users/dmh/.git-credentials*       /cygdrive/c/Users/dmh/.gitk*\n/cygdrive/c/Users/dmh/.git-credentials.lock*\nCarson:part2: rm ~/.git-credentials.lock\nCarson:part2: git push\nThis application has requested the Runtime to terminate it in an unusual \nway.\nPlease contact the application's support team for more information.\nA s s e r t i o n   f a i l e d !\n\n  P r o g r a m :   C : \\ t o o l s \\ G i t \\ m i n g w 3 2 \\ l i b e x \ne c \\ g i\n  t - c o r e \\ g i t - c r e d e n t i a l - s t o r e . e x e\n  F i l e :   e x e c _ c m d . c ,   L i n e   2 3\n\n  E x p r e s s i o n :   a r g v 0 _ p a t h\n  Everything up-to-date\nCarson:part2:\n\n```\n  - What did you expect to occur after running these commands?\ncompletion with no errors\n\n  - What actually happened instead?\n\n  git-credentials-store failed.\n\n  - If the problem was occurring with a specific repository, can you \nprovide the\n    URL to that repository to help us with testing?\n\turl = https://github.com/Unidata/thredds.git\n\n"},{"id":"290623","messageId":"20160701040715.GB4832@sigill.intra.peff.net","threadId":"42760","inReplyTo":"7210bce6-093b-9fdf-9a9b-94059ad0d4e3@ucar.edu","subject":"Re: git-credentials-store.exe crash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T04:07:15Z","receivedAt":"2016-07-01T04:14:12Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 30, 2016 at 03:24:41PM -0600, dmh@ucar.edu wrote:\n\n> Carson:part2: git push\n> This application has requested the Runtime to terminate it in an unusual\n> way.\n> Please contact the application's support team for more information.\n> A s s e r t i o n   f a i l e d !\n> \n>  P r o g r a m :   C : \\ t o o l s \\ G i t \\ m i n g w 3 2 \\ l i b e x e c \\\n> g i\n>  t - c o r e \\ g i t - c r e d e n t i a l - s t o r e . e x e\n>  F i l e :   e x e c _ c m d . c ,   L i n e   2 3\n> \n>  E x p r e s s i o n :   a r g v 0 _ p a t h\n>  Everything up-to-date\n\nInteresting. It's failing on the assert(argv0_path) in system_path().\n\nThat's part of the RUNTIME_PREFIX code which is built only on Windows,\nso this is a Windows-specific issue.\n\nI can guess the reason that argv0_path is not set is that\ncredential-store has its own main() function and does not call\ngit_extract_argv0_path(). It never mattered before, but as of v2.8.0,\none of the library functions it calls wants to use system_path(), which\nassumes that the argv0 stuff has been set up.\n\nI'm preparing some patches to fix this case (and some other related\nones).\n\n-Peff\n"},{"id":"290625","messageId":"20160701055532.GA4488@sigill.intra.peff.net","threadId":"42760","inReplyTo":"20160701040715.GB4832@sigill.intra.peff.net","subject":"[PATCH 0/5] consistent setup code for external commands","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T05:55:32Z","receivedAt":"2016-07-01T05:55:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 01, 2016 at 12:07:15AM -0400, Jeff King wrote:\n\n> Interesting. It's failing on the assert(argv0_path) in system_path().\n> \n> That's part of the RUNTIME_PREFIX code which is built only on Windows,\n> so this is a Windows-specific issue.\n> \n> I can guess the reason that argv0_path is not set is that\n> credential-store has its own main() function and does not call\n> git_extract_argv0_path(). It never mattered before, but as of v2.8.0,\n> one of the library functions it calls wants to use system_path(), which\n> assumes that the argv0 stuff has been set up.\n> \n> I'm preparing some patches to fix this case (and some other related\n> ones).\n\nHere they are:\n\n  [1/5]: add an extra level of indirection to main()\n  [2/5]: common-main: call git_extract_argv0_path()\n  [3/5]: common-main: call sanitize_stdfds()\n  [4/5]: common-main: call restore_sigpipe_to_default()\n  [5/5]: common-main: call git_setup_gettext()\n\nThe first patch is refactoring so we can fix this problem once, rather\nthan in the dozens of programs that need it.\n\nThe second should fix the problem you saw. It's unfortunate that the\ntests didn't detect it; there's some discussion of that in the commit\nmessage.\n\nPatches 3-5 are other places where we can use the refactoring to be more\nconsistent and remove boilerplate code.\n\nI almost did a patch 6 moving trace_command_performance(), but I'm not\nsure if people would care or not. Running \"git foo\" already covers that,\neven for an external command, because the git wrapper waits until the\nsub-process finishes. Setting it up in each sub-program would mean:\n\n  - you get it for dashed externals you run directly, too\n\n  - for external programs run as \"git foo\", you get _two_ times. One for\n    the time the actual sub-program ran, and one with the overhead of\n    the git wrapper process.\n\n    I'm not sure if people actually care about that distinction, or\n    whether the extra number would simply be annoying.\n\nSo I punted. It's easy to move it over later if anybody cares.\n\n-Peff\n"},{"id":"290626","messageId":"20160701055858.GA4593@sigill.intra.peff.net","threadId":"42760","inReplyTo":"20160701055532.GA4488@sigill.intra.peff.net","subject":"[PATCH 1/5] add an extra level of indirection to main()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T05:58:58Z","receivedAt":"2016-07-01T05:59:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are certain startup tasks that we expect every git\nprocess to do. In some cases this is just to improve the\nquality of the program (e.g., setting up gettext()). In\nothers it is a requirement for using certain functions in\nlibgit.a (e.g., system_path() expects that you have called\ngit_extract_argv0_path()).\n\nMost commands are builtins and are covered by the git.c\nversion of main(). However, there are still a few external\ncommands that use their own main(). Each of these has to\nremember to include the correct startup sequence, and we are\nnot always consistent.\n\nRather than just fix the inconsistencies, let's make this\nharder to get wrong by providing a common main() that can\nrun this standard startup.\n\nWe basically have two options to do this:\n\n - the compat/mingw.h file already does something like this by\n   adding a #define that replaces the definition of main with a\n   wrapper that calls mingw_startup().\n\n   The upside is that the code in each program doesn't need\n   to be changed at all; it's rewritten on the fly by the\n   preprocessor.\n\n   The downside is that it may make debugging of the startup\n   sequence a bit more confusing, as the preprocessor is\n   quietly inserting new code.\n\n - the builtin functions are all of the form cmd_foo(),\n   and git.c's main() calls them.\n\n   This is much more explicit, which may make things more\n   obvious to somebody reading the code. It's also more\n   flexible (because of course we have to figure out _which_\n   cmd_foo() to call).\n\n   The downside is that each of the builtins must define\n   cmd_foo(), instead of just main().\n\nThis patch chooses the latter option, preferring the more\nexplicit approach, even though it is more invasive. We\nintroduce a new file common-main.c, with the \"real\" main. It\nexpects to call cmd_main() from whatever other objects it is\nlinked against.\n\nWe link common-main.o against anything that links against\nlibgit.a, since we know that such programs will need to do\nthis setup. Note that common-main.o can't actually go inside\nlibgit.a, as the linker would not pick up its main()\nfunction automatically (it has no callers).\n\nThe rest of the patch is just adjusting all of the various\nexternal programs (mostly in t/helper) to use cmd_main().\nI've provided a global declaration for cmd_main(), which\nmeans that all of the programs also need to match its\nsignature. In particular, many functions need to switch to\n\"const char **\" instead of \"char **\" for argv. This effect\nripples out to a few other variables and functions, as well.\n\nThis makes the patch even more invasive, but the end result\nis much better. We should be treating argv strings as const\nanyway, and now all programs conform to the same signature\n(which also matches the way builtins are defined).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI waffled between the two mechanisms. Opinions welcome.\n\n Makefile                               | 17 +++++++++++++----\n common-main.c                          | 12 ++++++++++++\n credential-cache--daemon.c             |  2 +-\n credential-cache.c                     |  2 +-\n credential-store.c                     |  2 +-\n daemon.c                               |  8 ++++----\n fast-import.c                          |  4 ++--\n git-compat-util.h                      |  2 ++\n git.c                                  |  3 +--\n http-backend.c                         |  2 +-\n http-fetch.c                           |  2 +-\n http-push.c                            |  6 +++---\n imap-send.c                            |  2 +-\n remote-curl.c                          |  2 +-\n remote-testsvn.c                       |  2 +-\n sh-i18n--envsubst.c                    |  2 +-\n shell.c                                |  2 +-\n show-index.c                           |  2 +-\n t/helper/test-chmtime.c                |  2 +-\n t/helper/test-config.c                 |  2 +-\n t/helper/test-ctype.c                  |  2 +-\n t/helper/test-date.c                   |  8 ++++----\n t/helper/test-delta.c                  |  2 +-\n t/helper/test-dump-cache-tree.c        |  2 +-\n t/helper/test-dump-split-index.c       |  2 +-\n t/helper/test-dump-untracked-cache.c   |  2 +-\n t/helper/test-fake-ssh.c               |  2 +-\n t/helper/test-genrandom.c              |  2 +-\n t/helper/test-hashmap.c                |  2 +-\n t/helper/test-index-version.c          |  2 +-\n t/helper/test-line-buffer.c            |  2 +-\n t/helper/test-match-trees.c            |  2 +-\n t/helper/test-mergesort.c              |  2 +-\n t/helper/test-mktemp.c                 |  2 +-\n t/helper/test-parse-options.c          |  2 +-\n t/helper/test-path-utils.c             |  4 ++--\n t/helper/test-prio-queue.c             |  2 +-\n t/helper/test-read-cache.c             |  2 +-\n t/helper/test-regex.c                  |  2 +-\n t/helper/test-revision-walking.c       |  2 +-\n t/helper/test-run-command.c            |  2 +-\n t/helper/test-scrap-cache-tree.c       |  2 +-\n t/helper/test-sha1-array.c             |  2 +-\n t/helper/test-sha1.c                   |  2 +-\n t/helper/test-sigchain.c               |  2 +-\n t/helper/test-string-list.c            |  2 +-\n t/helper/test-submodule-config.c       |  6 +++---\n t/helper/test-subprocess.c             |  2 +-\n t/helper/test-svn-fe.c                 |  4 ++--\n t/helper/test-urlmatch-normalization.c |  2 +-\n t/helper/test-wildmatch.c              |  2 +-\n upload-pack.c                          |  2 +-\n 52 files changed, 89 insertions(+), 67 deletions(-)\n create mode 100644 common-main.c\n\ndiff --git a/Makefile b/Makefile\nindex de5a030..717dc34 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -939,7 +939,7 @@ BUILTIN_OBJS += builtin/verify-tag.o\n BUILTIN_OBJS += builtin/worktree.o\n BUILTIN_OBJS += builtin/write-tree.o\n \n-GITLIBS = $(LIB_FILE) $(XDIFF_LIB)\n+GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB)\n EXTLIBS =\n \n GIT_USER_AGENT = git/$(GIT_VERSION)\n@@ -1572,7 +1572,15 @@ TCLTK_PATH_SQ = $(subst ','\\'',$(TCLTK_PATH))\n DIFF_SQ = $(subst ','\\'',$(DIFF))\n PERLLIB_EXTRA_SQ = $(subst ','\\'',$(PERLLIB_EXTRA))\n \n-LIBS = $(GITLIBS) $(EXTLIBS)\n+# We must filter out any object files from $(GITLIBS),\n+# as it is typically used like:\n+#\n+#   foo: foo.o $(GITLIBS)\n+#\t$(CC) $(filter %.o,$^) $(LIBS)\n+#\n+# where we use it as a dependency. Since we also pull object files\n+# from the dependency list, that would make each entry appear twice.\n+LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)\n \n BASIC_CFLAGS += -DSHA1_HEADER='$(SHA1_HEADER_SQ)' \\\n \t$(COMPAT_CFLAGS)\n@@ -1708,8 +1716,8 @@ git.sp git.s git.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_INFO_PATH=\"$(infodir_relative_SQ)\"'\n \n git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n-\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) git.o \\\n-\t\t$(BUILTIN_OBJS) $(LIBS)\n+\t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n+\t\t$(filter %.o,$^) $(LIBS)\n \n help.sp help.s help.o: common-cmds.h\n \n@@ -1902,6 +1910,7 @@ TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS))\n OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \\\n \t$(XDIFF_OBJS) \\\n \t$(VCSSVN_OBJS) \\\n+\tcommon-main.o \\\n \tgit.o\n ifndef NO_CURL\n \tOBJECTS += http.o http-walker.o remote-curl.o\ndiff --git a/common-main.c b/common-main.c\nnew file mode 100644\nindex 0000000..2b96bbf\n--- /dev/null\n+++ b/common-main.c\n@@ -0,0 +1,12 @@\n+#include \"git-compat-util.h\"\n+\n+int main(int argc, char **av)\n+{\n+\t/*\n+\t * This const trickery is explained in\n+\t * 84d32bf7678259c08406571cd6ce4b7a6724dcba\n+\t */\n+\tconst char **argv = (const char **)av;\n+\n+\treturn cmd_main(argc, argv);\n+}\ndiff --git a/credential-cache--daemon.c b/credential-cache--daemon.c\nindex 1f14d56..1e5f16a 100644\n--- a/credential-cache--daemon.c\n+++ b/credential-cache--daemon.c\n@@ -257,7 +257,7 @@ static void init_socket_directory(const char *path)\n \tfree(path_copy);\n }\n \n-int main(int argc, const char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tconst char *socket_path;\n \tint ignore_sighup = 0;\ndiff --git a/credential-cache.c b/credential-cache.c\nindex 86e21de..cc8a6ee 100644\n--- a/credential-cache.c\n+++ b/credential-cache.c\n@@ -83,7 +83,7 @@ static void do_cache(const char *socket, const char *action, int timeout,\n \tstrbuf_release(&buf);\n }\n \n-int main(int argc, const char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tchar *socket_path = NULL;\n \tint timeout = 900;\ndiff --git a/credential-store.c b/credential-store.c\nindex 5714167..55ca1b1 100644\n--- a/credential-store.c\n+++ b/credential-store.c\n@@ -142,7 +142,7 @@ static void lookup_credential(const struct string_list *fns, struct credential *\n \t\t\treturn; /* Found credential */\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tconst char * const usage[] = {\n \t\t\"git credential-store [<options>] <action>\",\ndiff --git a/daemon.c b/daemon.c\nindex 46dddac..4326cd0 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -32,7 +32,7 @@ static const char daemon_usage[] =\n \"           [<directory>...]\";\n \n /* List of acceptable pathname prefixes */\n-static char **ok_paths;\n+static const char **ok_paths;\n static int strict_paths;\n \n /* If this is set, git-daemon-export-ok is not required */\n@@ -240,7 +240,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi)\n \t}\n \n \tif ( ok_paths && *ok_paths ) {\n-\t\tchar **pp;\n+\t\tconst char **pp;\n \t\tint pathlen = strlen(path);\n \n \t\t/* The validation is done on the paths after enter_repo\n@@ -1192,7 +1192,7 @@ static int serve(struct string_list *listen_addr, int listen_port,\n \treturn service_loop(&socklist);\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tint listen_port = 0;\n \tstruct string_list listen_addr = STRING_LIST_INIT_NODUP;\n@@ -1207,7 +1207,7 @@ int main(int argc, char **argv)\n \tgit_extract_argv0_path(argv[0]);\n \n \tfor (i = 1; i < argc; i++) {\n-\t\tchar *arg = argv[i];\n+\t\tconst char *arg = argv[i];\n \t\tconst char *v;\n \n \t\tif (skip_prefix(arg, \"--listen=\", &v)) {\ndiff --git a/fast-import.c b/fast-import.c\nindex 59630ce..7cb7434 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -302,7 +302,7 @@ static int failure;\n static FILE *pack_edges;\n static unsigned int show_stats = 1;\n static int global_argc;\n-static char **global_argv;\n+static const char **global_argv;\n \n /* Memory pools */\n static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);\n@@ -3445,7 +3445,7 @@ static void parse_argv(void)\n \t\tread_marks();\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tunsigned int i;\n \ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 49d4029..1930444 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -1045,3 +1045,5 @@ struct tm *git_gmtime_r(const time_t *, struct tm *);\n #endif\n \n #endif\n+\n+extern int cmd_main(int, const char **);\ndiff --git a/git.c b/git.c\nindex 968a8a4..e244404 100644\n--- a/git.c\n+++ b/git.c\n@@ -630,9 +630,8 @@ static void restore_sigpipe_to_default(void)\n \tsignal(SIGPIPE, SIG_DFL);\n }\n \n-int main(int argc, char **av)\n+int cmd_main(int argc, const char **argv)\n {\n-\tconst char **argv = (const char **) av;\n \tconst char *cmd;\n \tint done_help = 0;\n \ndiff --git a/http-backend.c b/http-backend.c\nindex 2148814..411c835 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -632,7 +632,7 @@ static struct service_cmd {\n \t{\"POST\", \"/git-receive-pack$\", service_rpc}\n };\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tchar *method = getenv(\"REQUEST_METHOD\");\n \tchar *dir;\ndiff --git a/http-fetch.c b/http-fetch.c\nindex ba3ea10..eb559eb 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -6,7 +6,7 @@\n static const char http_fetch_usage[] = \"git http-fetch \"\n \"[-c] [-t] [-a] [-v] [--recover] [-w ref] [--stdin] commit-id url\";\n \n-int main(int argc, const char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct walker *walker;\n \tint commits_on_stdin = 0;\ndiff --git a/http-push.c b/http-push.c\nindex a092f02..df20e44 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1692,12 +1692,12 @@ static void run_request_queue(void)\n #endif\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct transfer_request *request;\n \tstruct transfer_request *next_request;\n \tint nr_refspec = 0;\n-\tchar **refspec = NULL;\n+\tconst char **refspec = NULL;\n \tstruct remote_lock *ref_lock = NULL;\n \tstruct remote_lock *info_ref_lock = NULL;\n \tstruct rev_info revs;\n@@ -1717,7 +1717,7 @@ int main(int argc, char **argv)\n \n \targv++;\n \tfor (i = 1; i < argc; i++, argv++) {\n-\t\tchar *arg = *argv;\n+\t\tconst char *arg = *argv;\n \n \t\tif (*arg == '-') {\n \t\t\tif (!strcmp(arg, \"--all\")) {\ndiff --git a/imap-send.c b/imap-send.c\nindex 938c691..890e1cb 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -1494,7 +1494,7 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server,\n }\n #endif\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct strbuf all_msgs = STRBUF_INIT;\n \tint total;\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 672b382..a068340 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -984,7 +984,7 @@ static void parse_push(struct strbuf *buf)\n \tfree(specs);\n }\n \n-int main(int argc, const char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tint nongit;\ndiff --git a/remote-testsvn.c b/remote-testsvn.c\nindex f05ff45..32631eb 100644\n--- a/remote-testsvn.c\n+++ b/remote-testsvn.c\n@@ -284,7 +284,7 @@ static int do_command(struct strbuf *line)\n \treturn 0;\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct strbuf buf = STRBUF_INIT, url_sb = STRBUF_INIT,\n \t\t\tprivate_ref_sb = STRBUF_INIT, marksfilename_sb = STRBUF_INIT,\ndiff --git a/sh-i18n--envsubst.c b/sh-i18n--envsubst.c\nindex 2842a22..e06b2c1 100644\n--- a/sh-i18n--envsubst.c\n+++ b/sh-i18n--envsubst.c\n@@ -64,7 +64,7 @@ static void note_variables (const char *string);\n static void subst_from_stdin (void);\n \n int\n-main (int argc, char *argv[])\n+cmd_main (int argc, const char *argv[])\n {\n   /* Default values for command line options.  */\n   /* unsigned short int show_variables = 0; */\ndiff --git a/shell.c b/shell.c\nindex c5439a6..3dd7fdc 100644\n--- a/shell.c\n+++ b/shell.c\n@@ -138,7 +138,7 @@ static struct commands {\n \t{ NULL },\n };\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tchar *prog;\n \tconst char **user_argv;\ndiff --git a/show-index.c b/show-index.c\nindex acf8d54..575f9c5 100644\n--- a/show-index.c\n+++ b/show-index.c\n@@ -4,7 +4,7 @@\n static const char show_index_usage[] =\n \"git show-index\";\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tint i;\n \tunsigned nr;\ndiff --git a/t/helper/test-chmtime.c b/t/helper/test-chmtime.c\nindex dfe8a83..e760256 100644\n--- a/t/helper/test-chmtime.c\n+++ b/t/helper/test-chmtime.c\n@@ -56,7 +56,7 @@ static int timespec_arg(const char *arg, long int *set_time, int *set_eq)\n \treturn 1;\n }\n \n-int main(int argc, char *argv[])\n+int cmd_main(int argc, const char **argv)\n {\n \tstatic int verbose;\n \ndiff --git a/t/helper/test-config.c b/t/helper/test-config.c\nindex 6a77552..d143cd7 100644\n--- a/t/helper/test-config.c\n+++ b/t/helper/test-config.c\n@@ -33,7 +33,7 @@\n  */\n \n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tint i, val;\n \tconst char *v;\ndiff --git a/t/helper/test-ctype.c b/t/helper/test-ctype.c\nindex 707a821..bb72c47 100644\n--- a/t/helper/test-ctype.c\n+++ b/t/helper/test-ctype.c\n@@ -28,7 +28,7 @@ static int is_in(const char *s, int ch)\n #define LOWER \"abcdefghijklmnopqrstuvwxyz\"\n #define UPPER \"ABCDEFGHIJKLMNOPQRSTUVWXYZ\"\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tTEST_CLASS(isdigit, DIGIT);\n \tTEST_CLASS(isspace, \" \\n\\r\\t\");\ndiff --git a/t/helper/test-date.c b/t/helper/test-date.c\nindex 63f3735..0f3cfb1 100644\n--- a/t/helper/test-date.c\n+++ b/t/helper/test-date.c\n@@ -5,7 +5,7 @@ static const char *usage_msg = \"\\n\"\n \"  test-date parse [date]...\\n\"\n \"  test-date approxidate [date]...\\n\";\n \n-static void show_dates(char **argv, struct timeval *now)\n+static void show_dates(const char **argv, struct timeval *now)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \n@@ -17,7 +17,7 @@ static void show_dates(char **argv, struct timeval *now)\n \tstrbuf_release(&buf);\n }\n \n-static void parse_dates(char **argv, struct timeval *now)\n+static void parse_dates(const char **argv, struct timeval *now)\n {\n \tstruct strbuf result = STRBUF_INIT;\n \n@@ -36,7 +36,7 @@ static void parse_dates(char **argv, struct timeval *now)\n \tstrbuf_release(&result);\n }\n \n-static void parse_approxidate(char **argv, struct timeval *now)\n+static void parse_approxidate(const char **argv, struct timeval *now)\n {\n \tfor (; *argv; argv++) {\n \t\ttime_t t;\n@@ -45,7 +45,7 @@ static void parse_approxidate(char **argv, struct timeval *now)\n \t}\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct timeval now;\n \tconst char *x;\ndiff --git a/t/helper/test-delta.c b/t/helper/test-delta.c\nindex 4595cd6..59937dc 100644\n--- a/t/helper/test-delta.c\n+++ b/t/helper/test-delta.c\n@@ -15,7 +15,7 @@\n static const char usage_str[] =\n \t\"test-delta (-d|-p) <from_file> <data_file> <out_file>\";\n \n-int main(int argc, char *argv[])\n+int cmd_main(int argc, const char **argv)\n {\n \tint fd;\n \tstruct stat st;\ndiff --git a/t/helper/test-dump-cache-tree.c b/t/helper/test-dump-cache-tree.c\nindex bb53c0a..44f3290 100644\n--- a/t/helper/test-dump-cache-tree.c\n+++ b/t/helper/test-dump-cache-tree.c\n@@ -54,7 +54,7 @@ static int dump_cache_tree(struct cache_tree *it,\n \treturn errs;\n }\n \n-int main(int ac, char **av)\n+int cmd_main(int ac, const char **av)\n {\n \tstruct index_state istate;\n \tstruct cache_tree *another = cache_tree();\ndiff --git a/t/helper/test-dump-split-index.c b/t/helper/test-dump-split-index.c\nindex 861d28c..d168924 100644\n--- a/t/helper/test-dump-split-index.c\n+++ b/t/helper/test-dump-split-index.c\n@@ -7,7 +7,7 @@ static void show_bit(size_t pos, void *data)\n \tprintf(\" %d\", (int)pos);\n }\n \n-int main(int ac, char **av)\n+int cmd_main(int ac, const char **av)\n {\n \tstruct split_index *si;\n \tint i;\ndiff --git a/t/helper/test-dump-untracked-cache.c b/t/helper/test-dump-untracked-cache.c\nindex 0a1c285..50112cc 100644\n--- a/t/helper/test-dump-untracked-cache.c\n+++ b/t/helper/test-dump-untracked-cache.c\n@@ -40,7 +40,7 @@ static void dump(struct untracked_cache_dir *ucd, struct strbuf *base)\n \tstrbuf_setlen(base, len);\n }\n \n-int main(int ac, char **av)\n+int cmd_main(int ac, const char **av)\n {\n \tstruct untracked_cache *uc;\n \tstruct strbuf base = STRBUF_INIT;\ndiff --git a/t/helper/test-fake-ssh.c b/t/helper/test-fake-ssh.c\nindex 980de21..12beee9 100644\n--- a/t/helper/test-fake-ssh.c\n+++ b/t/helper/test-fake-ssh.c\n@@ -2,7 +2,7 @@\n #include \"run-command.h\"\n #include \"strbuf.h\"\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tconst char *trash_directory = getenv(\"TRASH_DIRECTORY\");\n \tstruct strbuf buf = STRBUF_INIT;\ndiff --git a/t/helper/test-genrandom.c b/t/helper/test-genrandom.c\nindex 54824d0..8d11d22 100644\n--- a/t/helper/test-genrandom.c\n+++ b/t/helper/test-genrandom.c\n@@ -6,7 +6,7 @@\n \n #include \"git-compat-util.h\"\n \n-int main(int argc, char *argv[])\n+int cmd_main(int argc, const char **argv)\n {\n \tunsigned long count, next = 0;\n \tunsigned char *c;\ndiff --git a/t/helper/test-hashmap.c b/t/helper/test-hashmap.c\nindex cc2891d..7aa9440 100644\n--- a/t/helper/test-hashmap.c\n+++ b/t/helper/test-hashmap.c\n@@ -138,7 +138,7 @@ static void perf_hashmap(unsigned int method, unsigned int rounds)\n  *\n  * perfhashmap method rounds -> test hashmap.[ch] performance\n  */\n-int main(int argc, char *argv[])\n+int cmd_main(int argc, const char **argv)\n {\n \tchar line[1024];\n \tstruct hashmap map;\ndiff --git a/t/helper/test-index-version.c b/t/helper/test-index-version.c\nindex 05d4699..f569f6b 100644\n--- a/t/helper/test-index-version.c\n+++ b/t/helper/test-index-version.c\n@@ -1,6 +1,6 @@\n #include \"cache.h\"\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct cache_header hdr;\n \tint version;\ndiff --git a/t/helper/test-line-buffer.c b/t/helper/test-line-buffer.c\nindex 1e58f04..81575fe 100644\n--- a/t/helper/test-line-buffer.c\n+++ b/t/helper/test-line-buffer.c\n@@ -50,7 +50,7 @@ static void handle_line(const char *line, struct line_buffer *stdin_buf)\n \thandle_command(line, arg + 1, stdin_buf);\n }\n \n-int main(int argc, char *argv[])\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct line_buffer stdin_buf = LINE_BUFFER_INIT;\n \tstruct line_buffer file_buf = LINE_BUFFER_INIT;\ndiff --git a/t/helper/test-match-trees.c b/t/helper/test-match-trees.c\nindex d446b8e..e939502 100644\n--- a/t/helper/test-match-trees.c\n+++ b/t/helper/test-match-trees.c\n@@ -1,7 +1,7 @@\n #include \"cache.h\"\n #include \"tree.h\"\n \n-int main(int ac, char **av)\n+int cmd_main(int ac, const char **av)\n {\n \tstruct object_id hash1, hash2, shifted;\n \tstruct tree *one, *two;\ndiff --git a/t/helper/test-mergesort.c b/t/helper/test-mergesort.c\nindex ea3b959..335cf6b 100644\n--- a/t/helper/test-mergesort.c\n+++ b/t/helper/test-mergesort.c\n@@ -22,7 +22,7 @@ static int compare_strings(const void *a, const void *b)\n \treturn strcmp(x->text, y->text);\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct line *line, *p = NULL, *lines = NULL;\n \tstruct strbuf sb = STRBUF_INIT;\ndiff --git a/t/helper/test-mktemp.c b/t/helper/test-mktemp.c\nindex c8c5421..89d9b2f 100644\n--- a/t/helper/test-mktemp.c\n+++ b/t/helper/test-mktemp.c\n@@ -3,7 +3,7 @@\n  */\n #include \"git-compat-util.h\"\n \n-int main(int argc, char *argv[])\n+int cmd_main(int argc, const char **argv)\n {\n \tif (argc != 2)\n \t\tusage(\"Expected 1 parameter defining the temporary file template\");\ndiff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c\nindex 8a1235d..d51d292 100644\n--- a/t/helper/test-parse-options.c\n+++ b/t/helper/test-parse-options.c\n@@ -94,7 +94,7 @@ static void show(struct string_list *expect, int *status, const char *fmt, ...)\n \tstrbuf_release(&buf);\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tconst char *prefix = \"prefix/\";\n \tconst char *usage[] = {\ndiff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c\nindex ba805b3..1ebe0f7 100644\n--- a/t/helper/test-path-utils.c\n+++ b/t/helper/test-path-utils.c\n@@ -156,7 +156,7 @@ static struct test_data dirname_data[] = {\n \t{ NULL,              NULL     }\n };\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tif (argc == 3 && !strcmp(argv[1], \"normalize_path_copy\")) {\n \t\tchar *buf = xmallocz(strlen(argv[2]));\n@@ -213,7 +213,7 @@ int main(int argc, char **argv)\n \t}\n \n \tif (argc >= 4 && !strcmp(argv[1], \"prefix_path\")) {\n-\t\tchar *prefix = argv[2];\n+\t\tconst char *prefix = argv[2];\n \t\tint prefix_len = strlen(prefix);\n \t\tint nongit_ok;\n \t\tsetup_git_directory_gently(&nongit_ok);\ndiff --git a/t/helper/test-prio-queue.c b/t/helper/test-prio-queue.c\nindex 7be72f0..ae58fff 100644\n--- a/t/helper/test-prio-queue.c\n+++ b/t/helper/test-prio-queue.c\n@@ -16,7 +16,7 @@ static void show(int *v)\n \tfree(v);\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct prio_queue pq = { intcmp };\n \ndiff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c\nindex b25bcf1..2a7990e 100644\n--- a/t/helper/test-read-cache.c\n+++ b/t/helper/test-read-cache.c\n@@ -1,6 +1,6 @@\n #include \"cache.h\"\n \n-int main (int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tint i, cnt = 1;\n \tif (argc == 2)\ndiff --git a/t/helper/test-regex.c b/t/helper/test-regex.c\nindex 0dc598e..37b7f06 100644\n--- a/t/helper/test-regex.c\n+++ b/t/helper/test-regex.c\n@@ -1,6 +1,6 @@\n #include \"git-compat-util.h\"\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tchar *pat = \"[^={} \\t]+\";\n \tchar *str = \"={}\\nfred\";\ndiff --git a/t/helper/test-revision-walking.c b/t/helper/test-revision-walking.c\nindex 3d03133..b8e6fe1 100644\n--- a/t/helper/test-revision-walking.c\n+++ b/t/helper/test-revision-walking.c\n@@ -45,7 +45,7 @@ static int run_revision_walk(void)\n \treturn got_revision;\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tif (argc < 2)\n \t\treturn 1;\ndiff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c\nindex 30a64a9..c71ea4f 100644\n--- a/t/helper/test-run-command.c\n+++ b/t/helper/test-run-command.c\n@@ -49,7 +49,7 @@ static int task_finished(int result,\n \treturn 1;\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct child_process proc = CHILD_PROCESS_INIT;\n \tint jobs;\ndiff --git a/t/helper/test-scrap-cache-tree.c b/t/helper/test-scrap-cache-tree.c\nindex 6efee31..5b2fd09 100644\n--- a/t/helper/test-scrap-cache-tree.c\n+++ b/t/helper/test-scrap-cache-tree.c\n@@ -5,7 +5,7 @@\n \n static struct lock_file index_lock;\n \n-int main(int ac, char **av)\n+int cmd_main(int ac, const char **av)\n {\n \thold_locked_index(&index_lock, 1);\n \tif (read_cache() < 0)\ndiff --git a/t/helper/test-sha1-array.c b/t/helper/test-sha1-array.c\nindex 60ea1d5..09f7790 100644\n--- a/t/helper/test-sha1-array.c\n+++ b/t/helper/test-sha1-array.c\n@@ -6,7 +6,7 @@ static void print_sha1(const unsigned char sha1[20], void *data)\n \tputs(sha1_to_hex(sha1));\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct sha1_array array = SHA1_ARRAY_INIT;\n \tstruct strbuf line = STRBUF_INIT;\ndiff --git a/t/helper/test-sha1.c b/t/helper/test-sha1.c\nindex e57eae1..a1c13f5 100644\n--- a/t/helper/test-sha1.c\n+++ b/t/helper/test-sha1.c\n@@ -1,6 +1,6 @@\n #include \"cache.h\"\n \n-int main(int ac, char **av)\n+int cmd_main(int ac, const char **av)\n {\n \tgit_SHA_CTX ctx;\n \tunsigned char sha1[20];\ndiff --git a/t/helper/test-sigchain.c b/t/helper/test-sigchain.c\nindex e499fce..b71edbd 100644\n--- a/t/helper/test-sigchain.c\n+++ b/t/helper/test-sigchain.c\n@@ -13,7 +13,7 @@ X(two)\n X(three)\n #undef X\n \n-int main(int argc, char **argv) {\n+int cmd_main(int argc, const char **argv) {\n \tsigchain_push(SIGTERM, one);\n \tsigchain_push(SIGTERM, two);\n \tsigchain_push(SIGTERM, three);\ndiff --git a/t/helper/test-string-list.c b/t/helper/test-string-list.c\nindex 14bdf9d..4a68967 100644\n--- a/t/helper/test-string-list.c\n+++ b/t/helper/test-string-list.c\n@@ -41,7 +41,7 @@ static int prefix_cb(struct string_list_item *item, void *cb_data)\n \treturn starts_with(item->string, prefix);\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tif (argc == 5 && !strcmp(argv[1], \"split\")) {\n \t\tstruct string_list list = STRING_LIST_INIT_DUP;\ndiff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c\nindex dab8c27..2cffded 100644\n--- a/t/helper/test-submodule-config.c\n+++ b/t/helper/test-submodule-config.c\n@@ -2,7 +2,7 @@\n #include \"submodule-config.h\"\n #include \"submodule.h\"\n \n-static void die_usage(int argc, char **argv, const char *msg)\n+static void die_usage(int argc, const char **argv, const char *msg)\n {\n \tfprintf(stderr, \"%s\\n\", msg);\n \tfprintf(stderr, \"Usage: %s [<commit> <submodulepath>] ...\\n\", argv[0]);\n@@ -14,9 +14,9 @@ static int git_test_config(const char *var, const char *value, void *cb)\n \treturn parse_submodule_config_option(var, value);\n }\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n-\tchar **arg = argv;\n+\tconst char **arg = argv;\n \tint my_argc = argc;\n \tint output_url = 0;\n \tint lookup_name = 0;\ndiff --git a/t/helper/test-subprocess.c b/t/helper/test-subprocess.c\nindex 56881a0..30c5765 100644\n--- a/t/helper/test-subprocess.c\n+++ b/t/helper/test-subprocess.c\n@@ -1,7 +1,7 @@\n #include \"cache.h\"\n #include \"run-command.h\"\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tint nogit = 0;\ndiff --git a/t/helper/test-svn-fe.c b/t/helper/test-svn-fe.c\nindex 120ec96..7667c08 100644\n--- a/t/helper/test-svn-fe.c\n+++ b/t/helper/test-svn-fe.c\n@@ -11,7 +11,7 @@\n static const char test_svnfe_usage[] =\n \t\"test-svn-fe (<dumpfile> | [-d] <preimage> <delta> <len>)\";\n \n-static int apply_delta(int argc, char *argv[])\n+static int apply_delta(int argc, const char **argv)\n {\n \tstruct line_buffer preimage = LINE_BUFFER_INIT;\n \tstruct line_buffer delta = LINE_BUFFER_INIT;\n@@ -35,7 +35,7 @@ static int apply_delta(int argc, char *argv[])\n \treturn 0;\n }\n \n-int main(int argc, char *argv[])\n+int cmd_main(int argc, const char **argv)\n {\n \tif (argc == 2) {\n \t\tif (svndump_init(argv[1]))\ndiff --git a/t/helper/test-urlmatch-normalization.c b/t/helper/test-urlmatch-normalization.c\nindex 090bf21..49b6e83 100644\n--- a/t/helper/test-urlmatch-normalization.c\n+++ b/t/helper/test-urlmatch-normalization.c\n@@ -1,7 +1,7 @@\n #include \"git-compat-util.h\"\n #include \"urlmatch.h\"\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tconst char usage[] = \"test-urlmatch-normalization [-p | -l] <url1> | <url1> <url2>\";\n \tchar *url1, *url2;\ndiff --git a/t/helper/test-wildmatch.c b/t/helper/test-wildmatch.c\nindex 578b164..52be876 100644\n--- a/t/helper/test-wildmatch.c\n+++ b/t/helper/test-wildmatch.c\n@@ -1,6 +1,6 @@\n #include \"cache.h\"\n \n-int main(int argc, char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tint i;\n \tfor (i = 2; i < argc; i++) {\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 9e03c27..80dda75 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -820,7 +820,7 @@ static int upload_pack_config(const char *var, const char *value, void *unused)\n \treturn parse_hide_refs_config(var, value, \"uploadpack\");\n }\n \n-int main(int argc, const char **argv)\n+int cmd_main(int argc, const char **argv)\n {\n \tconst char *dir;\n \tint strict = 0;\n-- \n2.9.0.317.g65b4e7c\n\n"},{"id":"290627","messageId":"20160701060404.GB4593@sigill.intra.peff.net","threadId":"42760","inReplyTo":"20160701055532.GA4488@sigill.intra.peff.net","subject":"[PATCH 2/5] common-main: call git_extract_argv0_path()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T06:04:04Z","receivedAt":"2016-07-01T06:05:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Every program which links against libgit.a must call this\nfunction, or risk hitting an assert() in system_path() that\nchecks whether we have configured argv0_path (though only\nwhen RUNTIME_PREFIX is defined, so essentially only on\nWindows).\n\nLooking at the diff, you can see that putting it into the\ncommon main() saves us having to do it individually in each\nof the external commands. But what you can't see are the\ncases where we _should_ have been doing so, but weren't\n(e.g., git-credential-store, and all of the t/helper test\nprograms).\n\nThis has been an accident-waiting-to-happen for a long time,\nbut wasn't triggered until recently because it involves one\nof those programs actually calling system_path(). That\nhappened with git-credential-store in v2.8.0 with ae5f677\n(lazily load core.sharedrepository, 2016-03-11). The\nprogram:\n\n  - takes a lock file, which...\n\n  - opens a tempfile, which...\n\n  - calls adjust_shared_perm to fix permissions, which...\n\n  - lazy-loads the config (as of ae5f677), which...\n\n  - calls system_path() to find the location of\n    /etc/gitconfig\n\nOn systems with RUNTIME_PREFIX, this means credential-store\nreliably hits that assert() and cannot be used.\n\nWe never noticed in the test suite, because we set\nGIT_CONFIG_NOSYSTEM there, which skips the system_path()\nlookup entirely.  But if we were to tweak git_config() to\nfind /etc/gitconfig even when we aren't going to open it,\nthen the test suite shows multiple failures (for\ncredential-store, and for some other test helpers). I didn't\ninclude that tweak here because it's way too specific to\nthis particular call to be worth carrying around what is\nessentially dead code.\n\nThe implementation is fairly straightforward, with one\nexception: there is exactly one caller (git.c) that actually\ncares about the result of the function, and not the\nside-effect of setting up argv0_path. We can accommodate\nthat by simply replacing the value of argv[0] in the array\nwe hand down to cmd_main().\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n common-main.c    | 3 +++\n daemon.c         | 3 ---\n fast-import.c    | 3 ---\n git.c            | 2 +-\n http-backend.c   | 1 -\n http-fetch.c     | 2 --\n http-push.c      | 2 --\n imap-send.c      | 2 --\n remote-curl.c    | 1 -\n remote-testsvn.c | 1 -\n shell.c          | 2 --\n upload-pack.c    | 1 -\n 12 files changed, 4 insertions(+), 19 deletions(-)\n\ndiff --git a/common-main.c b/common-main.c\nindex 2b96bbf..57c912a 100644\n--- a/common-main.c\n+++ b/common-main.c\n@@ -1,4 +1,5 @@\n #include \"git-compat-util.h\"\n+#include \"exec_cmd.h\"\n \n int main(int argc, char **av)\n {\n@@ -8,5 +9,7 @@ int main(int argc, char **av)\n \t */\n \tconst char **argv = (const char **)av;\n \n+\targv[0] = git_extract_argv0_path(argv[0]);\n+\n \treturn cmd_main(argc, argv);\n }\ndiff --git a/daemon.c b/daemon.c\nindex 4326cd0..d33ee5a 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1,6 +1,5 @@\n #include \"cache.h\"\n #include \"pkt-line.h\"\n-#include \"exec_cmd.h\"\n #include \"run-command.h\"\n #include \"strbuf.h\"\n #include \"string-list.h\"\n@@ -1204,8 +1203,6 @@ int cmd_main(int argc, const char **argv)\n \n \tgit_setup_gettext();\n \n-\tgit_extract_argv0_path(argv[0]);\n-\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tconst char *v;\ndiff --git a/fast-import.c b/fast-import.c\nindex 7cb7434..f12cd00 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -164,7 +164,6 @@ Format of STDIN stream:\n #include \"refs.h\"\n #include \"csum-file.h\"\n #include \"quote.h\"\n-#include \"exec_cmd.h\"\n #include \"dir.h\"\n #include \"run-command.h\"\n \n@@ -3449,8 +3448,6 @@ int cmd_main(int argc, const char **argv)\n {\n \tunsigned int i;\n \n-\tgit_extract_argv0_path(argv[0]);\n-\n \tgit_setup_gettext();\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\ndiff --git a/git.c b/git.c\nindex e244404..3b4e12d 100644\n--- a/git.c\n+++ b/git.c\n@@ -635,7 +635,7 @@ int cmd_main(int argc, const char **argv)\n \tconst char *cmd;\n \tint done_help = 0;\n \n-\tcmd = git_extract_argv0_path(argv[0]);\n+\tcmd = argv[0];\n \tif (!cmd)\n \t\tcmd = \"git-help\";\n \ndiff --git a/http-backend.c b/http-backend.c\nindex 411c835..5375cbc 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -642,7 +642,6 @@ int cmd_main(int argc, const char **argv)\n \n \tgit_setup_gettext();\n \n-\tgit_extract_argv0_path(argv[0]);\n \tset_die_routine(die_webcgi);\n \tset_die_is_recursing_routine(die_webcgi_recursing);\n \ndiff --git a/http-fetch.c b/http-fetch.c\nindex eb559eb..244cd0d 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -24,8 +24,6 @@ int cmd_main(int argc, const char **argv)\n \n \tgit_setup_gettext();\n \n-\tgit_extract_argv0_path(argv[0]);\n-\n \twhile (arg < argc && argv[arg][0] == '-') {\n \t\tif (argv[arg][1] == 't') {\n \t\t\tget_tree = 1;\ndiff --git a/http-push.c b/http-push.c\nindex df20e44..3a5fecf 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1711,8 +1711,6 @@ int cmd_main(int argc, const char **argv)\n \n \tgit_setup_gettext();\n \n-\tgit_extract_argv0_path(argv[0]);\n-\n \trepo = xcalloc(1, sizeof(*repo));\n \n \targv++;\ndiff --git a/imap-send.c b/imap-send.c\nindex 890e1cb..125b218 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -1500,8 +1500,6 @@ int cmd_main(int argc, const char **argv)\n \tint total;\n \tint nongit_ok;\n \n-\tgit_extract_argv0_path(argv[0]);\n-\n \tgit_setup_gettext();\n \n \tsetup_git_directory_gently(&nongit_ok);\ndiff --git a/remote-curl.c b/remote-curl.c\nindex a068340..d39f4cf 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -991,7 +991,6 @@ int cmd_main(int argc, const char **argv)\n \n \tgit_setup_gettext();\n \n-\tgit_extract_argv0_path(argv[0]);\n \tsetup_git_directory_gently(&nongit);\n \tif (argc < 2) {\n \t\terror(\"remote-curl: usage: git remote-curl <remote> [<url>]\");\ndiff --git a/remote-testsvn.c b/remote-testsvn.c\nindex 32631eb..f87bf85 100644\n--- a/remote-testsvn.c\n+++ b/remote-testsvn.c\n@@ -292,7 +292,6 @@ int cmd_main(int argc, const char **argv)\n \tstatic struct remote *remote;\n \tconst char *url_in;\n \n-\tgit_extract_argv0_path(argv[0]);\n \tsetup_git_directory();\n \tif (argc < 2 || argc > 3) {\n \t\tusage(\"git-remote-svn <remote-name> [<url>]\");\ndiff --git a/shell.c b/shell.c\nindex 3dd7fdc..ca00807 100644\n--- a/shell.c\n+++ b/shell.c\n@@ -147,8 +147,6 @@ int cmd_main(int argc, const char **argv)\n \n \tgit_setup_gettext();\n \n-\tgit_extract_argv0_path(argv[0]);\n-\n \t/*\n \t * Always open file descriptors 0/1/2 to avoid clobbering files\n \t * in die().  It also avoids messing up when the pipes are dup'ed\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 80dda75..681fd2f 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -839,7 +839,6 @@ int cmd_main(int argc, const char **argv)\n \tgit_setup_gettext();\n \n \tpacket_trace_identity(\"upload-pack\");\n-\tgit_extract_argv0_path(argv[0]);\n \tcheck_replace_refs = 0;\n \n \targc = parse_options(argc, argv, NULL, options, upload_pack_usage, 0);\n-- \n2.9.0.317.g65b4e7c\n\n"},{"id":"290628","messageId":"20160701060602.GC4593@sigill.intra.peff.net","threadId":"42760","inReplyTo":"20160701055532.GA4488@sigill.intra.peff.net","subject":"[PATCH 3/5] common-main: call sanitize_stdfds()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T06:06:02Z","receivedAt":"2016-07-01T06:06:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is setup that should be done in every program for\nsafety, but we never got around to adding it everywhere (so\nbuiltins benefited from the call in git.c, but any external\ncommands did not). Putting it in the common main() gives us\nthis safety everywhere.\n\nNote that the case in daemon.c is a little funny. We wait\nuntil we know whether we want to daemonize, and then either:\n\n - call daemonize(), which will close stdio and reopen it to\n   /dev/null under the hood\n\n - sanitize_stdfds(), to fix up any odd cases\n\nBut that is way too late; the point of sanitizing is to give\nus reliable descriptors on 0/1/2, and we will already have\nexecuted code, possibly called die(), etc. The sanitizing\nshould be the very first thing that happens.\n\nWith this patch, git-daemon will sanitize first, and can\nremove the call in the non-daemonize case. It does mean that\ndaemonize() may just end up closing the descriptors we\nopened, but that's not a big deal (it's not wrong to do so,\nnor is it really less optimal than the case where our parent\nprocess redirected us from /dev/null ahead of time).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n common-main.c | 9 ++++++++-\n daemon.c      | 3 +--\n git.c         | 7 -------\n shell.c       | 7 -------\n 4 files changed, 9 insertions(+), 17 deletions(-)\n\ndiff --git a/common-main.c b/common-main.c\nindex 57c912a..353c6ea 100644\n--- a/common-main.c\n+++ b/common-main.c\n@@ -1,4 +1,4 @@\n-#include \"git-compat-util.h\"\n+#include \"cache.h\"\n #include \"exec_cmd.h\"\n \n int main(int argc, char **av)\n@@ -9,6 +9,13 @@ int main(int argc, char **av)\n \t */\n \tconst char **argv = (const char **)av;\n \n+\t/*\n+\t * Always open file descriptors 0/1/2 to avoid clobbering files\n+\t * in die().  It also avoids messing up when the pipes are dup'ed\n+\t * onto stdin/stdout/stderr in the child processes we spawn.\n+\t */\n+\tsanitize_stdfds();\n+\n \targv[0] = git_extract_argv0_path(argv[0]);\n \n \treturn cmd_main(argc, argv);\ndiff --git a/daemon.c b/daemon.c\nindex d33ee5a..8646f33 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1378,8 +1378,7 @@ int cmd_main(int argc, const char **argv)\n \tif (detach) {\n \t\tif (daemonize())\n \t\t\tdie(\"--detach not supported on this platform\");\n-\t} else\n-\t\tsanitize_stdfds();\n+\t}\n \n \tif (pid_file)\n \t\twrite_file(pid_file, \"%\"PRIuMAX, (uintmax_t) getpid());\ndiff --git a/git.c b/git.c\nindex 3b4e12d..b65083c 100644\n--- a/git.c\n+++ b/git.c\n@@ -639,13 +639,6 @@ int cmd_main(int argc, const char **argv)\n \tif (!cmd)\n \t\tcmd = \"git-help\";\n \n-\t/*\n-\t * Always open file descriptors 0/1/2 to avoid clobbering files\n-\t * in die().  It also avoids messing up when the pipes are dup'ed\n-\t * onto stdin/stdout/stderr in the child processes we spawn.\n-\t */\n-\tsanitize_stdfds();\n-\n \trestore_sigpipe_to_default();\n \n \tgit_setup_gettext();\ndiff --git a/shell.c b/shell.c\nindex ca00807..5e70acb 100644\n--- a/shell.c\n+++ b/shell.c\n@@ -147,13 +147,6 @@ int cmd_main(int argc, const char **argv)\n \n \tgit_setup_gettext();\n \n-\t/*\n-\t * Always open file descriptors 0/1/2 to avoid clobbering files\n-\t * in die().  It also avoids messing up when the pipes are dup'ed\n-\t * onto stdin/stdout/stderr in the child processes we spawn.\n-\t */\n-\tsanitize_stdfds();\n-\n \t/*\n \t * Special hack to pretend to be a CVS server\n \t */\n-- \n2.9.0.317.g65b4e7c\n\n"},{"id":"290629","messageId":"20160701060701.GE4593@sigill.intra.peff.net","threadId":"42760","inReplyTo":"20160701055532.GA4488@sigill.intra.peff.net","subject":"[PATCH 5/5] common-main: call git_setup_gettext()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T06:07:01Z","receivedAt":"2016-07-01T06:07:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This should be part of every program, as otherwise users do\nnot get translated error messages. However, some external\ncommands forgot to do so (e.g., git-credential-store). This\nfixes them, and eliminates the repeated code in programs\nthat did remember to use it.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n common-main.c  | 2 ++\n daemon.c       | 2 --\n fast-import.c  | 2 --\n git.c          | 2 --\n http-backend.c | 2 --\n http-fetch.c   | 2 --\n http-push.c    | 2 --\n imap-send.c    | 2 --\n remote-curl.c  | 2 --\n shell.c        | 2 --\n show-index.c   | 2 --\n upload-pack.c  | 2 --\n 12 files changed, 2 insertions(+), 22 deletions(-)\n\ndiff --git a/common-main.c b/common-main.c\nindex 20e55ef..3be5ad1 100644\n--- a/common-main.c\n+++ b/common-main.c\n@@ -37,6 +37,8 @@ int main(int argc, char **av)\n \t */\n \tsanitize_stdfds();\n \n+\tgit_setup_gettext();\n+\n \targv[0] = git_extract_argv0_path(argv[0]);\n \n \trestore_sigpipe_to_default();\ndiff --git a/daemon.c b/daemon.c\nindex 8646f33..e647254 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1201,8 +1201,6 @@ int cmd_main(int argc, const char **argv)\n \tstruct credentials *cred = NULL;\n \tint i;\n \n-\tgit_setup_gettext();\n-\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tconst char *v;\ndiff --git a/fast-import.c b/fast-import.c\nindex f12cd00..bf53ac9 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -3448,8 +3448,6 @@ int cmd_main(int argc, const char **argv)\n {\n \tunsigned int i;\n \n-\tgit_setup_gettext();\n-\n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage(fast_import_usage);\n \ndiff --git a/git.c b/git.c\nindex ccb24fd..0f1937f 100644\n--- a/git.c\n+++ b/git.c\n@@ -618,8 +618,6 @@ int cmd_main(int argc, const char **argv)\n \tif (!cmd)\n \t\tcmd = \"git-help\";\n \n-\tgit_setup_gettext();\n-\n \ttrace_command_performance(argv);\n \n \t/*\ndiff --git a/http-backend.c b/http-backend.c\nindex 5375cbc..0d59499 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -640,8 +640,6 @@ int cmd_main(int argc, const char **argv)\n \tchar *cmd_arg = NULL;\n \tint i;\n \n-\tgit_setup_gettext();\n-\n \tset_die_routine(die_webcgi);\n \tset_die_is_recursing_routine(die_webcgi_recursing);\n \ndiff --git a/http-fetch.c b/http-fetch.c\nindex 244cd0d..3b556d6 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -22,8 +22,6 @@ int cmd_main(int argc, const char **argv)\n \tint get_verbosely = 0;\n \tint get_recover = 0;\n \n-\tgit_setup_gettext();\n-\n \twhile (arg < argc && argv[arg][0] == '-') {\n \t\tif (argv[arg][1] == 't') {\n \t\t\tget_tree = 1;\ndiff --git a/http-push.c b/http-push.c\nindex 3a5fecf..dacada9 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1709,8 +1709,6 @@ int cmd_main(int argc, const char **argv)\n \tint new_refs;\n \tstruct ref *ref, *local_refs;\n \n-\tgit_setup_gettext();\n-\n \trepo = xcalloc(1, sizeof(*repo));\n \n \targv++;\ndiff --git a/imap-send.c b/imap-send.c\nindex 125b218..9cbe27f 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -1500,8 +1500,6 @@ int cmd_main(int argc, const char **argv)\n \tint total;\n \tint nongit_ok;\n \n-\tgit_setup_gettext();\n-\n \tsetup_git_directory_gently(&nongit_ok);\n \tgit_imap_config();\n \ndiff --git a/remote-curl.c b/remote-curl.c\nindex d39f4cf..6b83b77 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -989,8 +989,6 @@ int cmd_main(int argc, const char **argv)\n \tstruct strbuf buf = STRBUF_INIT;\n \tint nongit;\n \n-\tgit_setup_gettext();\n-\n \tsetup_git_directory_gently(&nongit);\n \tif (argc < 2) {\n \t\terror(\"remote-curl: usage: git remote-curl <remote> [<url>]\");\ndiff --git a/shell.c b/shell.c\nindex 5e70acb..464ee1a 100644\n--- a/shell.c\n+++ b/shell.c\n@@ -145,8 +145,6 @@ int cmd_main(int argc, const char **argv)\n \tstruct commands *cmd;\n \tint count;\n \n-\tgit_setup_gettext();\n-\n \t/*\n \t * Special hack to pretend to be a CVS server\n \t */\ndiff --git a/show-index.c b/show-index.c\nindex 575f9c5..1ead41e 100644\n--- a/show-index.c\n+++ b/show-index.c\n@@ -11,8 +11,6 @@ int cmd_main(int argc, const char **argv)\n \tunsigned int version;\n \tstatic unsigned int top_index[256];\n \n-\tgit_setup_gettext();\n-\n \tif (argc != 1)\n \t\tusage(show_index_usage);\n \tif (fread(top_index, 2 * 4, 1, stdin) != 1)\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 681fd2f..8769b1b 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -836,8 +836,6 @@ int cmd_main(int argc, const char **argv)\n \t\tOPT_END()\n \t};\n \n-\tgit_setup_gettext();\n-\n \tpacket_trace_identity(\"upload-pack\");\n \tcheck_replace_refs = 0;\n \n-- \n2.9.0.317.g65b4e7c\n"},{"id":"290630","messageId":"20160701060635.GD4593@sigill.intra.peff.net","threadId":"42760","inReplyTo":"20160701055532.GA4488@sigill.intra.peff.net","subject":"[PATCH 4/5] common-main: call restore_sigpipe_to_default()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T06:06:35Z","receivedAt":"2016-07-01T06:07:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is another safety/sanity setup that should be in force\neverywhere, but which we only applied in git.c. This did\ncatch most cases, since even external commands are typically\nrun via \"git ...\" (and the restoration applies to\nsub-processes, too). But there were cases we missed, such as\nsomebody calling git-upload-pack directly via ssh, or\nscripts which use dashed external commands directly.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n common-main.c | 23 +++++++++++++++++++++++\n git.c         | 23 -----------------------\n 2 files changed, 23 insertions(+), 23 deletions(-)\n\ndiff --git a/common-main.c b/common-main.c\nindex 353c6ea..20e55ef 100644\n--- a/common-main.c\n+++ b/common-main.c\n@@ -1,6 +1,27 @@\n #include \"cache.h\"\n #include \"exec_cmd.h\"\n \n+/*\n+ * Many parts of Git have subprograms communicate via pipe, expect the\n+ * upstream of a pipe to die with SIGPIPE when the downstream of a\n+ * pipe does not need to read all that is written.  Some third-party\n+ * programs that ignore or block SIGPIPE for their own reason forget\n+ * to restore SIGPIPE handling to the default before spawning Git and\n+ * break this carefully orchestrated machinery.\n+ *\n+ * Restore the way SIGPIPE is handled to default, which is what we\n+ * expect.\n+ */\n+static void restore_sigpipe_to_default(void)\n+{\n+\tsigset_t unblock;\n+\n+\tsigemptyset(&unblock);\n+\tsigaddset(&unblock, SIGPIPE);\n+\tsigprocmask(SIG_UNBLOCK, &unblock, NULL);\n+\tsignal(SIGPIPE, SIG_DFL);\n+}\n+\n int main(int argc, char **av)\n {\n \t/*\n@@ -18,5 +39,7 @@ int main(int argc, char **av)\n \n \targv[0] = git_extract_argv0_path(argv[0]);\n \n+\trestore_sigpipe_to_default();\n+\n \treturn cmd_main(argc, argv);\n }\ndiff --git a/git.c b/git.c\nindex b65083c..ccb24fd 100644\n--- a/git.c\n+++ b/git.c\n@@ -609,27 +609,6 @@ static int run_argv(int *argcp, const char ***argv)\n \treturn done_alias;\n }\n \n-/*\n- * Many parts of Git have subprograms communicate via pipe, expect the\n- * upstream of a pipe to die with SIGPIPE when the downstream of a\n- * pipe does not need to read all that is written.  Some third-party\n- * programs that ignore or block SIGPIPE for their own reason forget\n- * to restore SIGPIPE handling to the default before spawning Git and\n- * break this carefully orchestrated machinery.\n- *\n- * Restore the way SIGPIPE is handled to default, which is what we\n- * expect.\n- */\n-static void restore_sigpipe_to_default(void)\n-{\n-\tsigset_t unblock;\n-\n-\tsigemptyset(&unblock);\n-\tsigaddset(&unblock, SIGPIPE);\n-\tsigprocmask(SIG_UNBLOCK, &unblock, NULL);\n-\tsignal(SIGPIPE, SIG_DFL);\n-}\n-\n int cmd_main(int argc, const char **argv)\n {\n \tconst char *cmd;\n@@ -639,8 +618,6 @@ int cmd_main(int argc, const char **argv)\n \tif (!cmd)\n \t\tcmd = \"git-help\";\n \n-\trestore_sigpipe_to_default();\n-\n \tgit_setup_gettext();\n \n \ttrace_command_performance(argv);\n-- \n2.9.0.317.g65b4e7c\n\n"},{"id":"290639","messageId":"alpine.DEB.2.20.1607010938140.12947@virtualbox","threadId":"42760","inReplyTo":"20160701040715.GB4832@sigill.intra.peff.net","subject":"Re: git-credentials-store.exe crash","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-01T07:38:33Z","receivedAt":"2016-07-01T07:39:03Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nit is sad when bug reporters simply delete the part of our bug reporting\ntemplate that says: \"I searched for existing bug reports but did not find\nany\".\n\nIt saves the reporters a few minutes... and spends the time of you and me\nin the hour range.\n\nIn this particular case, the bug report is an obvious duplicate of\nhttps://github.com/git-for-windows/git/issues/766\n\nThe saddest part is that I already spent time to investigate this and to\ncome up with a work-around, and I already committed and pushed it.\n\nSo those hours are now thoroughly wasted.\n\nDisappointed,\nDscho\n"},{"id":"290640","messageId":"20160701074354.GA12006@sigill.intra.peff.net","threadId":"42760","inReplyTo":"alpine.DEB.2.20.1607010938140.12947@virtualbox","subject":"Re: git-credentials-store.exe crash","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T07:43:54Z","receivedAt":"2016-07-01T07:44:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 01, 2016 at 09:38:33AM +0200, Johannes Schindelin wrote:\n\n> it is sad when bug reporters simply delete the part of our bug reporting\n> template that says: \"I searched for existing bug reports but did not find\n> any\".\n> \n> It saves the reporters a few minutes... and spends the time of you and me\n> in the hour range.\n> \n> In this particular case, the bug report is an obvious duplicate of\n> https://github.com/git-for-windows/git/issues/766\n> \n> The saddest part is that I already spent time to investigate this and to\n> come up with a work-around, and I already committed and pushed it.\n> \n> So those hours are now thoroughly wasted.\n\nYeah, you're right. Same bug, and your patch obviously is the right\nthing to do as an immediate fix.\n\nIf it's any consolation, I diagnosed the bug quite quickly, and the\nextra hours went to solving the greater problem that you mentioned in\nhttps://github.com/git-for-windows/git/issues/766#issuecomment-226985175.\n\nSo hopefully there is still some value in my series.\n\n-Peff\n"},{"id":"290641","messageId":"alpine.DEB.2.20.1607010939350.12947@virtualbox","threadId":"42760","inReplyTo":"20160701055532.GA4488@sigill.intra.peff.net","subject":"Re: [PATCH 0/5] consistent setup code for external commands","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-01T07:45:58Z","receivedAt":"2016-07-01T07:47:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Fri, 1 Jul 2016, Jeff King wrote:\n\n>   - for external programs run as \"git foo\", you get _two_ times.\n\nI considered doing this in my work-around, but decided against it because\nI deemed it too inelegant to call it twice.\n\nBut your patch series has more benefits than just fixing the assertion in\nthe credential-store.\n\nFor one, it makes test programs such as test-dump-untracked-cache.exe\nusable outside Git's test suite without any major envvar jujitsu.\n\nSo once the dust settles, I will revert my commit in git-for-windows.git\nand merge in your patches.\n\nThanks,\nDscho\n"},{"id":"290643","messageId":"alpine.DEB.2.20.1607011000320.12947@virtualbox","threadId":"42760","inReplyTo":"20160701060404.GB4593@sigill.intra.peff.net","subject":"Re: [PATCH 2/5] common-main: call git_extract_argv0_path()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-01T08:05:19Z","receivedAt":"2016-07-01T08:05:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Fri, 1 Jul 2016, Jeff King wrote:\n\n> This has been an accident-waiting-to-happen for a long time,\n> but wasn't triggered until recently because it involves one\n> of those programs actually calling system_path(). That\n> happened with git-credential-store in v2.8.0 with ae5f677\n> (lazily load core.sharedrepository, 2016-03-11). The\n> program:\n> \n>   - takes a lock file, which...\n> \n>   - opens a tempfile, which...\n> \n>   - calls adjust_shared_perm to fix permissions, which...\n> \n>   - lazy-loads the config (as of ae5f677), which...\n> \n>   - calls system_path() to find the location of\n>     /etc/gitconfig\n> \n> On systems with RUNTIME_PREFIX, this means credential-store\n> reliably hits that assert() and cannot be used.\n\nThank you for that thorough write-up. I am now even more upset that we had\nto go through the same steps (it took me an hour to figure out what was\ngoing wrong, mostly due to abort() *not* spitting out a stack trace, so I\nhad to wield some gdb magic).\n\nI am partly to blame here, of course, because I did not report what I did\nto this mailing list. But then: 1) I considered this a Windows-only\nproblem, and 2) I was really already swamped, as it were.\n\nThe patch is good, of course, as are the rest of the patches (I did not\nreally look at them very thoroughly, but then, they are pretty obvious\nimprovements).\n\nCiao,\nDscho\n"},{"id":"290645","messageId":"alpine.DEB.2.20.1607010953440.12947@virtualbox","threadId":"42760","inReplyTo":"20160701055858.GA4593@sigill.intra.peff.net","subject":"Re: [PATCH 1/5] add an extra level of indirection to main()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-01T08:04:44Z","receivedAt":"2016-07-01T08:10:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Fri, 1 Jul 2016, Jeff King wrote:\n\n> I waffled between the two mechanisms. Opinions welcome.\n\nI am happy you took the cmd_main() approach: we do have to play some\ntricks on Windows, in particular in some upcoming changes that support\nbuilding with MS Visual C++ (we want to ensure that `argv` is in UTF-8,\nwhich means that we actually have to use the UTF-16 versions and convert\nthem manually lest argv has the current Windows encoding of strings).\nWhich means that we still have to use that mingw_startup() trick you\nmentioned, and which would have interfered had you chosen a similar\nmethod.\n\n> diff --git a/common-main.c b/common-main.c\n> new file mode 100644\n> index 0000000..2b96bbf\n> --- /dev/null\n> +++ b/common-main.c\n> @@ -0,0 +1,12 @@\n> +#include \"git-compat-util.h\"\n> +\n> +int main(int argc, char **av)\n> +{\n> +\t/*\n> +\t * This const trickery is explained in\n> +\t * 84d32bf7678259c08406571cd6ce4b7a6724dcba\n\nThis commit message says that mingw_main() is declared with char **argv,\nand that is the reason why we have to convert. Maybe spell that out here,\nand then in a subsequent commit, we can fix the mingw_main() declaration?\n\n> +\t */\n> +\tconst char **argv = (const char **)av;\n> +\n> +\treturn cmd_main(argc, argv);\n> +}\n\nCiao,\nDscho\n"},{"id":"290646","messageId":"20160701081919.GA21076@sigill.intra.peff.net","threadId":"42760","inReplyTo":"alpine.DEB.2.20.1607010953440.12947@virtualbox","subject":"Re: [PATCH 1/5] add an extra level of indirection to main()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T08:19:20Z","receivedAt":"2016-07-01T08:26:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 01, 2016 at 10:04:44AM +0200, Johannes Schindelin wrote:\n\n> Hi Peff,\n> \n> On Fri, 1 Jul 2016, Jeff King wrote:\n> \n> > I waffled between the two mechanisms. Opinions welcome.\n> \n> I am happy you took the cmd_main() approach: we do have to play some\n> tricks on Windows, in particular in some upcoming changes that support\n> building with MS Visual C++ (we want to ensure that `argv` is in UTF-8,\n> which means that we actually have to use the UTF-16 versions and convert\n> them manually lest argv has the current Windows encoding of strings).\n> Which means that we still have to use that mingw_startup() trick you\n> mentioned, and which would have interfered had you chosen a similar\n> method.\n\nI actually wondered if it would make sense to get rid of the\nmingw_main() macro, and do it here as just:\n\n    #ifdef MINGW\n    mingw_startup();\n    #endif\n\nor something. But I didn't look deeply at it, and anyway I am afraid to\ntouch anything in that area because I can't even compile-test it.\n\n> > diff --git a/common-main.c b/common-main.c\n> > new file mode 100644\n> > index 0000000..2b96bbf\n> > --- /dev/null\n> > +++ b/common-main.c\n> > @@ -0,0 +1,12 @@\n> > +#include \"git-compat-util.h\"\n> > +\n> > +int main(int argc, char **av)\n> > +{\n> > +\t/*\n> > +\t * This const trickery is explained in\n> > +\t * 84d32bf7678259c08406571cd6ce4b7a6724dcba\n> \n> This commit message says that mingw_main() is declared with char **argv,\n> and that is the reason why we have to convert. Maybe spell that out here,\n> and then in a subsequent commit, we can fix the mingw_main() declaration?\n\nThe description was sufficiently long that I didn't want to try\nrepeating it, and opted for a reference instead. If you're planning to\nfix mingw_main(), I'd be happy to do that as a preparatory patch, and\nthen just skip this trickery entirely. :)\n\n-Peff\n"},{"id":"290659","messageId":"alpine.DEB.2.20.1607011449380.12947@virtualbox","threadId":"42760","inReplyTo":"20160701081919.GA21076@sigill.intra.peff.net","subject":"Re: [PATCH 1/5] add an extra level of indirection to main()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-01T13:39:03Z","receivedAt":"2016-07-01T13:39:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Fri, 1 Jul 2016, Jeff King wrote:\n\n> On Fri, Jul 01, 2016 at 10:04:44AM +0200, Johannes Schindelin wrote:\n> \n> > On Fri, 1 Jul 2016, Jeff King wrote:\n> > \n> > > I waffled between the two mechanisms. Opinions welcome.\n> > \n> > I am happy you took the cmd_main() approach: we do have to play some\n> > tricks on Windows, in particular in some upcoming changes that support\n> > building with MS Visual C++ (we want to ensure that `argv` is in UTF-8,\n> > which means that we actually have to use the UTF-16 versions and convert\n> > them manually lest argv has the current Windows encoding of strings).\n> > Which means that we still have to use that mingw_startup() trick you\n> > mentioned, and which would have interfered had you chosen a similar\n> > method.\n> \n> I actually wondered if it would make sense to get rid of the\n> mingw_main() macro, and do it here as just:\n> \n>     #ifdef MINGW\n>     mingw_startup();\n>     #endif\n> \n> or something. But I didn't look deeply at it, and anyway I am afraid to\n> touch anything in that area because I can't even compile-test it.\n\nSure. There are a couple of patches in flight to support MSVC better, and\none part is a duplication of mingw_startup(). I would like to fix that\nbefore merging, of course, and hope that it will naturally be helped by\nyour patch series.\n\n> > > diff --git a/common-main.c b/common-main.c\n> > > new file mode 100644\n> > > index 0000000..2b96bbf\n> > > --- /dev/null\n> > > +++ b/common-main.c\n> > > @@ -0,0 +1,12 @@\n> > > +#include \"git-compat-util.h\"\n> > > +\n> > > +int main(int argc, char **av)\n> > > +{\n> > > +\t/*\n> > > +\t * This const trickery is explained in\n> > > +\t * 84d32bf7678259c08406571cd6ce4b7a6724dcba\n> > \n> > This commit message says that mingw_main() is declared with char **argv,\n> > and that is the reason why we have to convert. Maybe spell that out here,\n> > and then in a subsequent commit, we can fix the mingw_main() declaration?\n> \n> The description was sufficiently long that I didn't want to try\n> repeating it, and opted for a reference instead. If you're planning to\n> fix mingw_main(), I'd be happy to do that as a preparatory patch, and\n> then just skip this trickery entirely. :)\n\nDeal:\n\n½/5 is in 5c54dff5c54e68a1101d8fe37aefc6158fddd7f2 and the fixup for 1/5\nis in 7b74f7aabb56b428c74f5983c066dc9ea8fe5116 in the 'common-main' branch\non https://github.com/dscho/git.\n\n(I had to resolve merge conflicts in a couple of the later patches, so\nfeel free to just use the branch, but please note that I cherry-picked a\npatch to let me compile with DEVELOPER=1 on Windows, so you might want to\ndrop 7b74f7aabb56b428c74f5983c066dc9ea8fe5116.)\n\nCiao,\nDscho"},{"id":"290716","messageId":"xmqq4m89vw5i.fsf@gitster.mtv.corp.google.com","threadId":"42760","inReplyTo":"20160701055532.GA4488@sigill.intra.peff.net","subject":"Re: [PATCH 0/5] consistent setup code for external commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-01T22:19:37Z","receivedAt":"2016-07-01T22:19:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Jul 01, 2016 at 12:07:15AM -0400, Jeff King wrote:\n>\n>> Interesting. It's failing on the assert(argv0_path) in system_path().\n>> \n>> That's part of the RUNTIME_PREFIX code which is built only on Windows,\n>> so this is a Windows-specific issue.\n>> \n>> I can guess the reason that argv0_path is not set is that\n>> credential-store has its own main() function and does not call\n>> git_extract_argv0_path(). It never mattered before, but as of v2.8.0,\n>> one of the library functions it calls wants to use system_path(), which\n>> assumes that the argv0 stuff has been set up.\n>> \n>> I'm preparing some patches to fix this case (and some other related\n>> ones).\n>\n> Here they are:\n>\n>   [1/5]: add an extra level of indirection to main()\n>   [2/5]: common-main: call git_extract_argv0_path()\n>   [3/5]: common-main: call sanitize_stdfds()\n>   [4/5]: common-main: call restore_sigpipe_to_default()\n>   [5/5]: common-main: call git_setup_gettext()\n\nAs this is also a fix to maint-2.8 track, I tweaked them to ensure\nthat they apply there, and queued the result as jk/common-main.  I\ndouble checked the result by comparing the result of applying these\nfive patches directly on top of master, and the result of merging\nthat jk/common-main (based on maint-2.8) into master, and they seem\nto match.\n\nThanks.\n"},{"id":"290719","messageId":"20160701223809.GA32545@sigill.intra.peff.net","threadId":"42760","inReplyTo":"alpine.DEB.2.20.1607011449380.12947@virtualbox","subject":"Re: [PATCH 1/5] add an extra level of indirection to main()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T22:38:10Z","receivedAt":"2016-07-01T22:38:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 01, 2016 at 03:39:03PM +0200, Johannes Schindelin wrote:\n\n> > The description was sufficiently long that I didn't want to try\n> > repeating it, and opted for a reference instead. If you're planning to\n> > fix mingw_main(), I'd be happy to do that as a preparatory patch, and\n> > then just skip this trickery entirely. :)\n> \n> Deal:\n> \n> ½/5 is in 5c54dff5c54e68a1101d8fe37aefc6158fddd7f2 and the fixup for 1/5\n> is in 7b74f7aabb56b428c74f5983c066dc9ea8fe5116 in the 'common-main' branch\n> on https://github.com/dscho/git.\n\nHrm. I didn't find 5c54dff5c when I fetched from dscho/git. However,\nhaving seen 7b74f7aa, the one that comes on top, I actually think that's\nwhat we really want anyway. It's really my first patch that makes your\ncleanup safe to do.\n\n-Peff\n"},{"id":"290720","messageId":"20160701223912.GB32545@sigill.intra.peff.net","threadId":"42760","inReplyTo":"xmqq4m89vw5i.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/5] consistent setup code for external commands","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-01T22:39:12Z","receivedAt":"2016-07-01T22:39:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 01, 2016 at 03:19:37PM -0700, Junio C Hamano wrote:\n\n> > Here they are:\n> >\n> >   [1/5]: add an extra level of indirection to main()\n> >   [2/5]: common-main: call git_extract_argv0_path()\n> >   [3/5]: common-main: call sanitize_stdfds()\n> >   [4/5]: common-main: call restore_sigpipe_to_default()\n> >   [5/5]: common-main: call git_setup_gettext()\n> \n> As this is also a fix to maint-2.8 track, I tweaked them to ensure\n> that they apply there, and queued the result as jk/common-main.  I\n> double checked the result by comparing the result of applying these\n> five patches directly on top of master, and the result of merging\n> that jk/common-main (based on maint-2.8) into master, and they seem\n> to match.\n\nThanks, this obviously is a regression in v2.8, but I didn't even think\nabout that and just built it on top of master.\n\nI think the cleanup that Dscho suggested elsewhere in the thread is a\ngood idea on top, but that can also just wait and come separately.\n\n-Peff\n"},{"id":"290734","messageId":"alpine.DEB.2.20.1607020851060.12947@virtualbox","threadId":"42760","inReplyTo":"20160701223809.GA32545@sigill.intra.peff.net","subject":"Re: [PATCH 1/5] add an extra level of indirection to main()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-02T06:52:34Z","receivedAt":"2016-07-02T06:52:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Fri, 1 Jul 2016, Jeff King wrote:\n\n> On Fri, Jul 01, 2016 at 03:39:03PM +0200, Johannes Schindelin wrote:\n> \n> > > The description was sufficiently long that I didn't want to try\n> > > repeating it, and opted for a reference instead. If you're planning to\n> > > fix mingw_main(), I'd be happy to do that as a preparatory patch, and\n> > > then just skip this trickery entirely. :)\n> > \n> > Deal:\n> > \n> > ½/5 is in 5c54dff5c54e68a1101d8fe37aefc6158fddd7f2 and the fixup for 1/5\n> > is in 7b74f7aabb56b428c74f5983c066dc9ea8fe5116 in the 'common-main' branch\n> > on https://github.com/dscho/git.\n> \n> Hrm. I didn't find 5c54dff5c when I fetched from dscho/git. However,\n> having seen 7b74f7aa, the one that comes on top, I actually think that's\n> what we really want anyway. It's really my first patch that makes your\n> cleanup safe to do.\n\nThe reason is obvious: I forgot to push :-( Hopefully you did not\nduplicate all my work to move my patch in front of the branch, to make it\na preparatory patch.\n\nIt is pushed now.\n\nSorry!\nDscho"},{"id":"290878","messageId":"xmqq60sivlrt.fsf@gitster.mtv.corp.google.com","threadId":"42760","inReplyTo":"20160701223912.GB32545@sigill.intra.peff.net","subject":"Re: [PATCH 0/5] consistent setup code for external commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-06T15:17:26Z","receivedAt":"2016-07-06T15:17:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Jul 01, 2016 at 03:19:37PM -0700, Junio C Hamano wrote:\n>\n>> > Here they are:\n>> >\n>> >   [1/5]: add an extra level of indirection to main()\n>> >   [2/5]: common-main: call git_extract_argv0_path()\n>> >   [3/5]: common-main: call sanitize_stdfds()\n>> >   [4/5]: common-main: call restore_sigpipe_to_default()\n>> >   [5/5]: common-main: call git_setup_gettext()\n>> \n>> As this is also a fix to maint-2.8 track, I tweaked them to ensure\n>> that they apply there, and queued the result as jk/common-main.  I\n>> double checked the result by comparing the result of applying these\n>> five patches directly on top of master, and the result of merging\n>> that jk/common-main (based on maint-2.8) into master, and they seem\n>> to match.\n>\n> Thanks, this obviously is a regression in v2.8, but I didn't even think\n> about that and just built it on top of master.\n>\n> I think the cleanup that Dscho suggested elsewhere in the thread is a\n> good idea on top, but that can also just wait and come separately.\n\nOK.\n\nI think that amounts to the following single patch, which I cherry\npicked from the topic in Dscho's repository he mentioned earlier.\n\nWith this applied on top of jk/common-main-2.8, when I merge it to\nhis b764cdf, the result matches his common-main topic, so we three\nare on the same page, I'd think, and the result can be fed to the\n2.8.x maintenance track.\n\n-- >8 --\nFrom: Johannes Schindelin <johannes.schindelin@gmx.de>\nDate: Fri, 1 Jul 2016 15:01:28 +0200\nSubject: [PATCH] mingw: declare main()'s argv as const\n\nIn 84d32bf (sparse: Fix mingw_main() argument number/type errors,\n2013-04-27), we addressed problems identified by the 'sparse' tool where\nargv was declared inconsistently. The way we addressed it was by casting\nfrom the non-const version to the const-version.\n\nThis patch is long overdue, fixing compat/mingw.h's declaration to\nmake the \"argv\" parameter const.  This also allows us to lose the\n\"const\" trickery introduced earlier to common-main.c:main().\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n common-main.c  | 8 +-------\n compat/mingw.h | 2 +-\n 2 files changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/common-main.c b/common-main.c\nindex 3be5ad1..44a29e8 100644\n--- a/common-main.c\n+++ b/common-main.c\n@@ -22,15 +22,9 @@ static void restore_sigpipe_to_default(void)\n \tsignal(SIGPIPE, SIG_DFL);\n }\n \n-int main(int argc, char **av)\n+int main(int argc, const char **argv)\n {\n \t/*\n-\t * This const trickery is explained in\n-\t * 84d32bf7678259c08406571cd6ce4b7a6724dcba\n-\t */\n-\tconst char **argv = (const char **)av;\n-\n-\t/*\n \t * Always open file descriptors 0/1/2 to avoid clobbering files\n \t * in die().  It also avoids messing up when the pipes are dup'ed\n \t * onto stdin/stdout/stderr in the child processes we spawn.\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 69bb43d..1ac9086 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -535,7 +535,7 @@ extern CRITICAL_SECTION pinfo_cs;\n void mingw_startup();\n #define main(c,v) dummy_decl_mingw_main(); \\\n static int mingw_main(c,v); \\\n-int main(int argc, char **argv) \\\n+int main(int argc, const char **argv) \\\n { \\\n \tmingw_startup(); \\\n \treturn mingw_main(__argc, (void *)__argv); \\\n-- \n2.9.0-457-ge524329\n\n"},{"id":"290885","messageId":"alpine.DEB.2.20.1607061736310.6426@virtualbox","threadId":"42760","inReplyTo":"xmqq60sivlrt.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/5] consistent setup code for external commands","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-06T15:36:54Z","receivedAt":"2016-07-06T15:37:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 6 Jul 2016, Junio C Hamano wrote:\n\n> I think that amounts to the following single patch, which I cherry\n> picked from the topic in Dscho's repository he mentioned earlier.\n\nThat patch would be fine by me, too.\n\nThanks,\nDscho\n"}]}