{"thread":{"id":"14522","subject":"[PATCH] builtin-clone: Use is_dir_sep() instead of '/'","startedAt":"2008-07-18T07:34:41Z","lastAt":"2008-07-21T13:43:17Z","messageCount":16,"participants":["Johannes Sixt","Junio C Hamano","Johannes Schindelin","Steffen Prohaska","Daniel Barkalow","Kristian Høgsberg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"83836","messageId":"1216366485-12201-1-git-send-email-johannes.sixt@telecom.at","threadId":"14522","inReplyTo":null,"subject":"[PATCH] Windows related patches","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-07-18T07:34:41Z","receivedAt":"2008-07-18T07:34:41Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Eric Raible (1):\n Teach lookup_prog not to select directories\n\nJohannes Sixt (2):\n builtin-clone: Use is_dir_sep() instead of '/'\n Windows: set gitexecdir = $(bindir)\n\nPeter Harris (1):\n Add ANSI control code emulation for the Windows console\n"},{"id":"83837","messageId":"1216366485-12201-2-git-send-email-johannes.sixt@telecom.at","threadId":"14522","inReplyTo":"1216366485-12201-1-git-send-email-johannes.sixt@telecom.at","subject":"[PATCH] Teach lookup_prog not to select directories","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-07-18T07:34:42Z","receivedAt":"2008-07-18T07:34:42Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Eric Raible \n\nWithout this simple fix \"git gui\" in the git source directory\nfinds the git-gui directory instead of the tcl script in /usr/bin.\n\nSigned-off-by: Eric Raible \nSigned-off-by: Johannes Sixt \n---\n compat/mingw.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/compat/mingw.c b/compat/mingw.c\nindex 3a05fe7..bb33c8d 100644\n--- a/compat/mingw.c\n+++ b/compat/mingw.c\n@@ -536,7 +536,8 @@ static char *lookup_prog(const char *dir, const char *cmd, int isexe, int exe_on\n \t\treturn xstrdup(path);\n \tpath[strlen(path)-4] = '\\0';\n \tif ((!exe_only || isexe) && access(path, F_OK) == 0)\n-\t\treturn xstrdup(path);\n+\t\tif (!(GetFileAttributes(path) & FILE_ATTRIBUTE_DIRECTORY))\n+\t\t\treturn xstrdup(path);\n \treturn NULL;\n }\n \n-- \n1.5.6.1.275.g0a3e0f\n"},{"id":"83834","messageId":"1216366485-12201-3-git-send-email-johannes.sixt@telecom.at","threadId":"14522","inReplyTo":"1216366485-12201-2-git-send-email-johannes.sixt@telecom.at","subject":"[PATCH] builtin-clone: Use is_dir_sep() instead of '/'","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-07-18T07:34:43Z","receivedAt":"2008-07-18T07:34:43Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Signed-off-by: Johannes Sixt \n---\n builtin-clone.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 643c7d4..fddf47f 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -115,7 +115,7 @@ static char *guess_dir_name(const char *repo, int is_bundle)\n \t\t\tif (!after_slash_or_colon)\n \t\t\t\tend = p;\n \t\t\tp += 7;\n-\t\t} else if (*p == '/' || *p == ':') {\n+\t\t} else if (is_dir_sep(*p) || *p == ':') {\n \t\t\tif (end == limit)\n \t\t\t\tend = p;\n \t\t\tafter_slash_or_colon = 1;\n-- \n1.5.6.1.275.g0a3e0f\n"},{"id":"83838","messageId":"1216366485-12201-4-git-send-email-johannes.sixt@telecom.at","threadId":"14522","inReplyTo":"1216366485-12201-3-git-send-email-johannes.sixt@telecom.at","subject":"[PATCH] Add ANSI control code emulation for the Windows console","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-07-18T07:34:44Z","receivedAt":"2008-07-18T07:34:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Peter Harris \n\nThis adds only the minimum necessary to keep git pull/merge's diffstat from\nwrapping. Notably absent is support for the K (erase) operation, and support\nfor POSIX write.\n\nSigned-off-by: Peter Harris \nSigned-off-by: Johannes Sixt \n---\n Makefile | 2 +-\n compat/mingw.h | 11 ++\n compat/winansi.c | 345 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 357 insertions(+), 1 deletions(-)\n create mode 100644 compat/winansi.c\n\ndiff --git a/Makefile b/Makefile\nindex 78e08d3..2c6cc9d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -740,7 +740,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n \tCOMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat\n \tCOMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1\n \tCOMPAT_CFLAGS += -DSTRIP_EXTENSION=\\\".exe\\\"\n-\tCOMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o\n+\tCOMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o compat/winansi.o\n \tEXTLIBS += -lws2_32\n \tX = .exe\n \ttemplate_dir = ../share/git-core/templates/\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex 6bc049a..5f11114 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -194,6 +194,17 @@ sig_handler_t mingw_signal(int sig, sig_handler_t handler);\n #define signal mingw_signal\n \n /*\n+ * ANSI emulation wrappers\n+ */\n+\n+int winansi_fputs(const char *str, FILE *stream);\n+int winansi_printf(const char *format, ...) __attribute__((format (printf, 1, 2)));\n+int winansi_fprintf(FILE *stream, const char *format, ...) __attribute__((format (printf, 2, 3)));\n+#define fputs winansi_fputs\n+#define printf(...) winansi_printf(__VA_ARGS__)\n+#define fprintf(...) winansi_fprintf(__VA_ARGS__)\n+\n+/*\n * git specific compatibility\n */\n \ndiff --git a/compat/winansi.c b/compat/winansi.c\nnew file mode 100644\nindex 0000000..e2d96df\n--- /dev/null\n+++ b/compat/winansi.c\n@@ -0,0 +1,345 @@\n+/*\n+ * Copyright 2008 Peter Harris \n+ */\n+\n+#include \n+#include \"../git-compat-util.h\"\n+\n+/*\n+ Functions to be wrapped:\n+*/\n+#undef printf\n+#undef fprintf\n+#undef fputs\n+/* TODO: write */\n+\n+/*\n+ ANSI codes used by git: m, K\n+\n+ This file is git-specific. Therefore, this file does not attempt\n+ to implement any codes that are not used by git.\n+\n+ TODO: K\n+*/\n+\n+static HANDLE console;\n+static WORD plain_attr;\n+static WORD attr;\n+static int negative;\n+\n+static void init(void)\n+{\n+\tCONSOLE_SCREEN_BUFFER_INFO sbi;\n+\n+\tstatic int initialized = 0;\n+\tif (initialized)\n+\t\treturn;\n+\n+\tconsole = GetStdHandle(STD_OUTPUT_HANDLE);\n+\tif (console == INVALID_HANDLE_VALUE)\n+\t\tconsole = NULL;\n+\n+\tif (!console)\n+\t\treturn;\n+\n+\tGetConsoleScreenBufferInfo(console, &sbi);\n+\tattr = plain_attr = sbi.wAttributes;\n+\tnegative = 0;\n+\n+\tinitialized = 1;\n+}\n+\n+\n+#define FOREGROUND_ALL (FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE)\n+#define BACKGROUND_ALL (BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_BLUE)\n+\n+static void set_console_attr(void)\n+{\n+\tWORD attributes = attr;\n+\tif (negative) {\n+\t\tattributes &= ~FOREGROUND_ALL;\n+\t\tattributes &= ~BACKGROUND_ALL;\n+\n+\t\t/* This could probably use a bitmask\n+\t\t instead of a series of ifs */\n+\t\tif (attr & FOREGROUND_RED)\n+\t\t\tattributes |= BACKGROUND_RED;\n+\t\tif (attr & FOREGROUND_GREEN)\n+\t\t\tattributes |= BACKGROUND_GREEN;\n+\t\tif (attr & FOREGROUND_BLUE)\n+\t\t\tattributes |= BACKGROUND_BLUE;\n+\n+\t\tif (attr & BACKGROUND_RED)\n+\t\t\tattributes |= FOREGROUND_RED;\n+\t\tif (attr & BACKGROUND_GREEN)\n+\t\t\tattributes |= FOREGROUND_GREEN;\n+\t\tif (attr & BACKGROUND_BLUE)\n+\t\t\tattributes |= FOREGROUND_BLUE;\n+\t}\n+\tSetConsoleTextAttribute(console, attributes);\n+}\n+\n+static const char *set_attr(const char *str)\n+{\n+\tconst char *func;\n+\tsize_t len = strspn(str, \"0123456789;\");\n+\tfunc = str + len;\n+\n+\tswitch (*func) {\n+\tcase 'm':\n+\t\tdo {\n+\t\t\tlong val = strtol(str, (char **)&str, 10);\n+\t\t\tswitch (val) {\n+\t\t\tcase 0: /* reset */\n+\t\t\t\tattr = plain_attr;\n+\t\t\t\tnegative = 0;\n+\t\t\t\tbreak;\n+\t\t\tcase 1: /* bold */\n+\t\t\t\tattr |= FOREGROUND_INTENSITY;\n+\t\t\t\tbreak;\n+\t\t\tcase 2: /* faint */\n+\t\t\tcase 22: /* normal */\n+\t\t\t\tattr &= ~FOREGROUND_INTENSITY;\n+\t\t\t\tbreak;\n+\t\t\tcase 3: /* italic */\n+\t\t\t\t/* Unsupported */\n+\t\t\t\tbreak;\n+\t\t\tcase 4: /* underline */\n+\t\t\tcase 21: /* double underline */\n+\t\t\t\t/* Wikipedia says this flag does nothing */\n+\t\t\t\t/* Furthermore, mingw doesn't define this flag\n+\t\t\t\tattr |= COMMON_LVB_UNDERSCORE; */\n+\t\t\t\tbreak;\n+\t\t\tcase 24: /* no underline */\n+\t\t\t\t/* attr &= ~COMMON_LVB_UNDERSCORE; */\n+\t\t\t\tbreak;\n+\t\t\tcase 5: /* slow blink */\n+\t\t\tcase 6: /* fast blink */\n+\t\t\t\t/* We don't have blink, but we do have\n+\t\t\t\t background intensity */\n+\t\t\t\tattr |= BACKGROUND_INTENSITY;\n+\t\t\t\tbreak;\n+\t\t\tcase 25: /* no blink */\n+\t\t\t\tattr &= ~BACKGROUND_INTENSITY;\n+\t\t\t\tbreak;\n+\t\t\tcase 7: /* negative */\n+\t\t\t\tnegative = 1;\n+\t\t\t\tbreak;\n+\t\t\tcase 27: /* positive */\n+\t\t\t\tnegative = 0;\n+\t\t\t\tbreak;\n+\t\t\tcase 8: /* conceal */\n+\t\t\tcase 28: /* reveal */\n+\t\t\t\t/* Unsupported */\n+\t\t\t\tbreak;\n+\t\t\tcase 30: /* Black */\n+\t\t\t\tattr &= ~FOREGROUND_ALL;\n+\t\t\t\tbreak;\n+\t\t\tcase 31: /* Red */\n+\t\t\t\tattr &= ~FOREGROUND_ALL;\n+\t\t\t\tattr |= FOREGROUND_RED;\n+\t\t\t\tbreak;\n+\t\t\tcase 32: /* Green */\n+\t\t\t\tattr &= ~FOREGROUND_ALL;\n+\t\t\t\tattr |= FOREGROUND_GREEN;\n+\t\t\t\tbreak;\n+\t\t\tcase 33: /* Yellow */\n+\t\t\t\tattr &= ~FOREGROUND_ALL;\n+\t\t\t\tattr |= FOREGROUND_RED | FOREGROUND_GREEN;\n+\t\t\t\tbreak;\n+\t\t\tcase 34: /* Blue */\n+\t\t\t\tattr &= ~FOREGROUND_ALL;\n+\t\t\t\tattr |= FOREGROUND_BLUE;\n+\t\t\t\tbreak;\n+\t\t\tcase 35: /* Magenta */\n+\t\t\t\tattr &= ~FOREGROUND_ALL;\n+\t\t\t\tattr |= FOREGROUND_RED | FOREGROUND_BLUE;\n+\t\t\t\tbreak;\n+\t\t\tcase 36: /* Cyan */\n+\t\t\t\tattr &= ~FOREGROUND_ALL;\n+\t\t\t\tattr |= FOREGROUND_GREEN | FOREGROUND_BLUE;\n+\t\t\t\tbreak;\n+\t\t\tcase 37: /* White */\n+\t\t\t\tattr |= FOREGROUND_RED |\n+\t\t\t\t\tFOREGROUND_GREEN |\n+\t\t\t\t\tFOREGROUND_BLUE;\n+\t\t\t\tbreak;\n+\t\t\tcase 38: /* Unknown */\n+\t\t\t\tbreak;\n+\t\t\tcase 39: /* reset */\n+\t\t\t\tattr &= ~FOREGROUND_ALL;\n+\t\t\t\tattr |= (plain_attr & FOREGROUND_ALL);\n+\t\t\t\tbreak;\n+\t\t\tcase 40: /* Black */\n+\t\t\t\tattr &= ~BACKGROUND_ALL;\n+\t\t\t\tbreak;\n+\t\t\tcase 41: /* Red */\n+\t\t\t\tattr &= ~BACKGROUND_ALL;\n+\t\t\t\tattr |= BACKGROUND_RED;\n+\t\t\t\tbreak;\n+\t\t\tcase 42: /* Green */\n+\t\t\t\tattr &= ~BACKGROUND_ALL;\n+\t\t\t\tattr |= BACKGROUND_GREEN;\n+\t\t\t\tbreak;\n+\t\t\tcase 43: /* Yellow */\n+\t\t\t\tattr &= ~BACKGROUND_ALL;\n+\t\t\t\tattr |= BACKGROUND_RED | BACKGROUND_GREEN;\n+\t\t\t\tbreak;\n+\t\t\tcase 44: /* Blue */\n+\t\t\t\tattr &= ~BACKGROUND_ALL;\n+\t\t\t\tattr |= BACKGROUND_BLUE;\n+\t\t\t\tbreak;\n+\t\t\tcase 45: /* Magenta */\n+\t\t\t\tattr &= ~BACKGROUND_ALL;\n+\t\t\t\tattr |= BACKGROUND_RED | BACKGROUND_BLUE;\n+\t\t\t\tbreak;\n+\t\t\tcase 46: /* Cyan */\n+\t\t\t\tattr &= ~BACKGROUND_ALL;\n+\t\t\t\tattr |= BACKGROUND_GREEN | BACKGROUND_BLUE;\n+\t\t\t\tbreak;\n+\t\t\tcase 47: /* White */\n+\t\t\t\tattr |= BACKGROUND_RED |\n+\t\t\t\t\tBACKGROUND_GREEN |\n+\t\t\t\t\tBACKGROUND_BLUE;\n+\t\t\t\tbreak;\n+\t\t\tcase 48: /* Unknown */\n+\t\t\t\tbreak;\n+\t\t\tcase 49: /* reset */\n+\t\t\t\tattr &= ~BACKGROUND_ALL;\n+\t\t\t\tattr |= (plain_attr & BACKGROUND_ALL);\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\t/* Unsupported code */\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tstr++;\n+\t\t} while (*(str-1) == ';');\n+\n+\t\tset_console_attr();\n+\t\tbreak;\n+\tcase 'K':\n+\t\t/* TODO */\n+\t\tbreak;\n+\tdefault:\n+\t\t/* Unsupported code */\n+\t\tbreak;\n+\t}\n+\n+\treturn func + 1;\n+}\n+\n+static int ansi_emulate(const char *str, FILE *stream)\n+{\n+\tint rv = 0;\n+\tconst char *pos = str;\n+\n+\twhile (*pos) {\n+\t\tpos = strstr(str, \"\\033[\");\n+\t\tif (pos) {\n+\t\t\tsize_t len = pos - str;\n+\n+\t\t\tif (len) {\n+\t\t\t\tsize_t out_len = fwrite(str, 1, len, stream);\n+\t\t\t\trv += out_len;\n+\t\t\t\tif (out_len < len)\n+\t\t\t\t\treturn rv;\n+\t\t\t}\n+\n+\t\t\tstr = pos + 2;\n+\t\t\trv += 2;\n+\n+\t\t\tfflush(stream);\n+\n+\t\t\tpos = set_attr(str);\n+\t\t\trv += pos - str;\n+\t\t\tstr = pos;\n+\t\t} else {\n+\t\t\trv += strlen(str);\n+\t\t\tfputs(str, stream);\n+\t\t\treturn rv;\n+\t\t}\n+\t}\n+\treturn rv;\n+}\n+\n+int winansi_fputs(const char *str, FILE *stream)\n+{\n+\tint rv;\n+\n+\tif (!isatty(fileno(stream)))\n+\t\treturn fputs(str, stream);\n+\n+\tinit();\n+\n+\tif (!console)\n+\t\treturn fputs(str, stream);\n+\n+\trv = ansi_emulate(str, stream);\n+\n+\tif (rv >= 0)\n+\t\treturn 0;\n+\telse\n+\t\treturn EOF;\n+}\n+\n+static int winansi_vfprintf(FILE *stream, const char *format, va_list list)\n+{\n+\tint len, rv;\n+\tchar small_buf[256];\n+\tchar *buf = small_buf;\n+\tva_list cp;\n+\n+\tif (!isatty(fileno(stream)))\n+\t\tgoto abort;\n+\n+\tinit();\n+\n+\tif (!console)\n+\t\tgoto abort;\n+\n+\tva_copy(cp, list);\n+\tlen = vsnprintf(small_buf, sizeof(small_buf), format, cp);\n+\tva_end(cp);\n+\n+\tif (len > sizeof(small_buf) - 1) {\n+\t\tbuf = malloc(len + 1);\n+\t\tif (!buf)\n+\t\t\tgoto abort;\n+\n+\t\tlen = vsnprintf(buf, len + 1, format, list);\n+\t}\n+\n+\trv = ansi_emulate(buf, stream);\n+\n+\tif (buf != small_buf)\n+\t\tfree(buf);\n+\treturn rv;\n+\n+abort:\n+\trv = vfprintf(stream, format, list);\n+\treturn rv;\n+}\n+\n+int winansi_fprintf(FILE *stream, const char *format, ...)\n+{\n+\tva_list list;\n+\tint rv;\n+\n+\tva_start(list, format);\n+\trv = winansi_vfprintf(stream, format, list);\n+\tva_end(list);\n+\n+\treturn rv;\n+}\n+\n+int winansi_printf(const char *format, ...)\n+{\n+\tva_list list;\n+\tint rv;\n+\n+\tva_start(list, format);\n+\trv = winansi_vfprintf(stdout, format, list);\n+\tva_end(list);\n+\n+\treturn rv;\n+}\n-- \n1.5.6.1.275.g0a3e0f\n"},{"id":"83835","messageId":"1216366485-12201-5-git-send-email-johannes.sixt@telecom.at","threadId":"14522","inReplyTo":"1216366485-12201-4-git-send-email-johannes.sixt@telecom.at","subject":"[PATCH] Windows: set gitexecdir = $(bindir)","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-07-18T07:34:45Z","receivedAt":"2008-07-18T07:34:45Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"The \"dash-less\" change aims to remove git commands from $PATH. It does so\nby defining a GIT_EXEC_PATH that is different from $(bindir). On Windows\nwe want a relocatable installation of the git tool, so we cannot use an\nabsolute GIT_EXEC_PATH. Therefore, the implementation of\nbuiltin_exec_path() on Windows derives the exec-path from the command\ninvocation, and disregards GIT_EXEC_PATH. But this broke when\n$(gitexecdir) became different from $(bindir), so we restore the earlier\nbehavior here.\n\nThis counteracts the aims of the \"dash-less\" change on Windows, but better\nthis way than having no working git at all.\n\nSigned-off-by: Johannes Sixt \n---\n Makefile | 1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 36339d3..7d466b3 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -742,6 +742,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))\n \tCOMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o compat/winansi.o\n \tEXTLIBS += -lws2_32\n \tX = .exe\n+\tgitexecdir = $(bindir)\n \ttemplate_dir = ../share/git-core/templates/\n \tETC_GITCONFIG = ../etc/gitconfig\n endif\n-- \n1.5.6.1.275.g0a3e0f\n"},{"id":"83927","messageId":"7vk5fi67dx.fsf@gitster.siamese.dyndns.org","threadId":"14522","inReplyTo":"1216366485-12201-3-git-send-email-johannes.sixt@telecom.at","subject":"Re: [PATCH] builtin-clone: Use is_dir_sep() instead of '/'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-19T00:32:26Z","receivedAt":"2008-07-19T00:32:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Signed-off-by: Johannes Sixt \n> ---\n> builtin-clone.c | 2 +-\n> 1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/builtin-clone.c b/builtin-clone.c\n> index 643c7d4..fddf47f 100644\n> --- a/builtin-clone.c\n> +++ b/builtin-clone.c\n> @@ -115,7 +115,7 @@ static char *guess_dir_name(const char *repo, int is_bundle)\n> \t\t\tif (!after_slash_or_colon)\n> \t\t\t\tend = p;\n> \t\t\tp += 7;\n> -\t\t} else if (*p == '/' || *p == ':') {\n> +\t\t} else if (is_dir_sep(*p) || *p == ':') {\n> \t\t\tif (end == limit)\n> \t\t\t\tend = p;\n> \t\t\tafter_slash_or_colon = 1;\n\nOk, but the surrounding code in this function look very suspicious.\n\n * The variable \"prefix\" is actually about suffix.\n\n * \"else if\" for explicit \".bundle\" case look redundant; it would never\n trigger, I suspect.\n\n * Values used to increment p for \"prefix\" case and \".bundle\" case are\n inconsistent. I think the redundant one increments by the right value\n (i.e. strlen(prefix)-1 is bogus).\n"},{"id":"83926","messageId":"7vej5q67dq.fsf@gitster.siamese.dyndns.org","threadId":"14522","inReplyTo":"1216366485-12201-5-git-send-email-johannes.sixt@telecom.at","subject":"Re: [PATCH] Windows: set gitexecdir = $(bindir)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-19T00:32:33Z","receivedAt":"2008-07-19T00:32:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> The \"dash-less\" change aims to remove git commands from $PATH. It does so\n> by defining a GIT_EXEC_PATH that is different from $(bindir). On Windows\n> we want a relocatable installation of the git tool, so we cannot use an\n> absolute GIT_EXEC_PATH. Therefore, the implementation of\n> builtin_exec_path() on Windows derives the exec-path from the command\n> invocation,...\n\nSorry, I am not sure if I understand what you are trying to solve. If you\nhave ../libexec/git-core/ in GIT_EXEC_PATH (or have builtin_exec_path()\nuse it), then your installation would look like this:\n\n\t[[some random place]]\n \tbin/git\n libexec/git-core/git-add\n libexec/git-core/git-del\n libexec/git-core/git-dir\n ...\n\nand if \"git\" can figure out it is \"[[some random place]]/bin/git\",\nit can find its subcommands from neighbouring directory, that is still\ninside the random place the user told the installer to use, can't it?\n\n> This counteracts the aims of the \"dash-less\" change on Windows, but better\n> this way than having no working git at all.\n\nI'd agree to the extent that anything is better than having no working\ngit, but this somewhat feels backwards.\n"},{"id":"83940","messageId":"200807191052.20057.johannes.sixt@telecom.at","threadId":"14522","inReplyTo":"7vej5q67dq.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Windows: set gitexecdir = $(bindir)","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-07-19T08:52:19Z","receivedAt":"2008-07-19T08:52:19Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 19. Juli 2008, Junio C Hamano wrote:\n> Sorry, I am not sure if I understand what you are trying to solve. If you\n> have ../libexec/git-core/ in GIT_EXEC_PATH (or have builtin_exec_path()\n> use it), then your installation would look like this:\n>\n> \t[[some random place]]\n> \tbin/git\n> libexec/git-core/git-add\n> libexec/git-core/git-del\n> libexec/git-core/git-dir\n> ...\n>\n> and if \"git\" can figure out it is \"[[some random place]]/bin/git\",\n> it can find its subcommands from neighbouring directory, that is still\n> inside the random place the user told the installer to use, can't it?\n\nYes, but...\n\nTake as an example 'git pull'.\n\n- The first call to git will derive the exec-path \n$prefix/bin/../libexec/git-core and prepend it to $PATH.\n\n- Calls to builtin git commands from inside 'git pull' will then derive the \nexec-path $prefix/bin/../libexec/git-core/../libexec/git-core, that is \n$prefix/libexec/libexec/git-core, and prepend it to $PATH as well. That \ndirectory does not exist - usually - and it does not hurt. But it feels dirty \nand potentially dangerous.\n\n> > This counteracts the aims of the \"dash-less\" change on Windows, but\n> > better this way than having no working git at all.\n>\n> I'd agree to the extent that anything is better than having no working\n> git, but this somewhat feels backwards.\n\nIt certainly does.\n\nI'm hoping that the msysgit crew has an opinion on this. CMD users like me do \nnot care how cluttered $PATH is because there is no command completion that \nwould reveal the 100+ git commands. But msysgit users who are working from a \nbash may want to have them hidden outside $PATH. Or maybe they do not care.\n\n-- Hannes\n"},{"id":"83941","messageId":"200807191132.45648.johannes.sixt@telecom.at","threadId":"14522","inReplyTo":"7vk5fi67dx.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-clone: Use is_dir_sep() instead of '/'","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-07-19T09:32:45Z","receivedAt":"2008-07-19T09:32:45Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 19. Juli 2008, Junio C Hamano wrote:\n> Ok, but the surrounding code in this function look very suspicious.\n\nHow about this then?\n\n-- snip --\nbuiltin-clone: Rewrite guess_dir_name()\n\nThe function has to do three small and independent tasks, but all of them\nwere crammed into a single loop. This rewrites the function entirely by\nunrolling these tasks.\n\nWe also now use is_dir_sep(c) instead of c == '/' to increase portability,\nwhich actually was the primary reason to touch this code.\n\nSigned-off-by: Johannes Sixt \n---\n builtin-clone.c | 55 ++++++++++++++++++++++++++-----------------------------\n 1 files changed, 26 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 8112716..91667d5 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -95,35 +95,32 @@ static char *get_repo_path(const char *repo, int *is_bundle)\n \n static char *guess_dir_name(const char *repo, int is_bundle)\n {\n-\tconst char *p, *start, *end, *limit;\n-\tint after_slash_or_colon;\n-\n-\t/* Guess dir name from repository: strip trailing '/',\n-\t * strip trailing '[:/]*.{git,bundle}', strip leading '.*[/:]'. */\n-\n-\tafter_slash_or_colon = 1;\n-\tlimit = repo + strlen(repo);\n-\tstart = repo;\n-\tend = limit;\n-\tfor (p = repo; p < limit; p++) {\n-\t\tconst char *prefix = is_bundle ? \".bundle\" : \".git\";\n-\t\tif (!prefixcmp(p, prefix)) {\n-\t\t\tif (!after_slash_or_colon)\n-\t\t\t\tend = p;\n-\t\t\tp += strlen(prefix) - 1;\n-\t\t} else if (!prefixcmp(p, \".bundle\")) {\n-\t\t\tif (!after_slash_or_colon)\n-\t\t\t\tend = p;\n-\t\t\tp += 7;\n-\t\t} else if (*p == '/' || *p == ':') {\n-\t\t\tif (end == limit)\n-\t\t\t\tend = p;\n-\t\t\tafter_slash_or_colon = 1;\n-\t\t} else if (after_slash_or_colon) {\n-\t\t\tstart = p;\n-\t\t\tend = limit;\n-\t\t\tafter_slash_or_colon = 0;\n-\t\t}\n+\tconst char *end = repo + strlen(repo), *start, *dot;\n+\n+\t/*\n+\t * Strip trailing slashes\n+\t */\n+\twhile (repo < end && is_dir_sep(end[-1]))\n+\t\tend--;\n+\n+\t/*\n+\t * Find last component, but be prepared that repo could have\n+\t * the form \"remote.example.com:foo.git\", i.e. no slash\n+\t * in the directory part.\n+\t */\n+\tstart = end;\n+\twhile (repo < start && !is_dir_sep(start[-1]) && start[-1] != ':')\n+\t\tstart--;\n+\n+\t/*\n+\t * Strip .{bundle,git}.\n+\t */\n+\tif (is_bundle) {\n+\t\tif (end - start > 7 && !strcmp(end - 7, \".bundle\"))\n+\t\t\tend -= 7;\n+\t} else {\n+\t\tif (end - start > 4 && !strcmp(end - 4, \".git\"))\n+\t\t\tend -= 4;\n \t}\n \n \treturn xstrndup(start, end - start);\n-- \n1.5.6.3.443.g5f70e\n"},{"id":"83943","messageId":"alpine.DEB.1.00.0807191334560.3305@eeepc-johanness","threadId":"14522","inReplyTo":"200807191132.45648.johannes.sixt@telecom.at","subject":"Re: [PATCH] builtin-clone: Use is_dir_sep() instead of '/'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-19T11:35:58Z","receivedAt":"2008-07-19T11:35:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 19 Jul 2008, Johannes Sixt wrote:\n\n> On Samstag, 19. Juli 2008, Junio C Hamano wrote:\n> > Ok, but the surrounding code in this function look very suspicious.\n> \n> How about this then?\n\nI like it. Very clear, very nice. Shorter code (if you look at the \ndiffstat modulo documentation).\n\nCiao,\nDscho\n"},{"id":"83945","messageId":"200807191549.56402.johannes.sixt@telecom.at","threadId":"14522","inReplyTo":"200807191132.45648.johannes.sixt@telecom.at","subject":"Re: [PATCH] builtin-clone: Use is_dir_sep() instead of '/'","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-07-19T13:49:56Z","receivedAt":"2008-07-19T13:49:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 19. Juli 2008, Johannes Sixt wrote:\n> On Samstag, 19. Juli 2008, Junio C Hamano wrote:\n> > Ok, but the surrounding code in this function look very suspicious.\n>\n> How about this then?\n>\n> -- snip --\n> builtin-clone: Rewrite guess_dir_name()\n>\n> The function has to do three small and independent tasks, but all of them\n> were crammed into a single loop. This rewrites the function entirely by\n> unrolling these tasks.\n\nSigh. I knew it, I knew it. If it had been that trivial, then Daniel had done\nit this way in the first place. :-(\n\nThis needs to be squashed in. It makes sure that we handle 'foo/.git';\nand .git was not stripped if we cloned from 'foo.git/'.\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 91667d5..88616b3 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -98,10 +98,16 @@ static char *guess_dir_name(const char *repo, int is_bundle)\n \tconst char *end = repo + strlen(repo), *start, *dot;\n \n \t/*\n-\t * Strip trailing slashes\n+\t * Strip trailing slashes and /.git\n \t */\n \twhile (repo < end && is_dir_sep(end[-1]))\n \t\tend--;\n+\tif (end - repo > 5 && is_dir_sep(end[-5]) &&\n+\t !strncmp(end - 4, \".git\", 4)) {\n+\t\tend -= 5;\n+\t\twhile (repo < end && is_dir_sep(end[-1]))\n+\t\t\tend--;\n+\t}\n \n \t/*\n \t * Find last component, but be prepared that repo could have\n@@ -116,10 +122,10 @@ static char *guess_dir_name(const char *repo, int is_bundle)\n \t * Strip .{bundle,git}.\n \t */\n \tif (is_bundle) {\n-\t\tif (end - start > 7 && !strcmp(end - 7, \".bundle\"))\n+\t\tif (end - start > 7 && !strncmp(end - 7, \".bundle\", 7))\n \t\t\tend -= 7;\n \t} else {\n-\t\tif (end - start > 4 && !strcmp(end - 4, \".git\"))\n+\t\tif (end - start > 4 && !strncmp(end - 4, \".git\", 4))\n \t\t\tend -= 4;\n \t}\n \n"},{"id":"83950","messageId":"9B346ED9-DAD8-4CC7-A1B5-0230ACA358BA@zib.de","threadId":"14522","inReplyTo":"200807191052.20057.johannes.sixt@telecom.at","subject":"Re: [PATCH] Windows: set gitexecdir = $(bindir)","fromName":"Steffen Prohaska","fromEmail":"prohaska@zib.de","sentAt":"2008-07-19T17:10:40Z","receivedAt":"2008-07-19T17:10:40Z","isPatch":true,"sender":{"key":"prohaska@zib.de","avatar":"https://avatars.githubusercontent.com/u/217580?v=4"},"body":"\nOn Jul 19, 2008, at 10:52 AM, Johannes Sixt wrote:\n\n> On Samstag, 19. Juli 2008, Junio C Hamano wrote:\n>> Sorry, I am not sure if I understand what you are trying to solve. \n>> If you\n>> have ../libexec/git-core/ in GIT_EXEC_PATH (or have \n>> builtin_exec_path()\n>> use it), then your installation would look like this:\n>>\n>> \t[[some random place]]\n>> \tbin/git\n>> libexec/git-core/git-add\n>> libexec/git-core/git-del\n>> libexec/git-core/git-dir\n>> ...\n>>\n>> and if \"git\" can figure out it is \"[[some random place]]/bin/git\",\n>> it can find its subcommands from neighbouring directory, that is \n>> still\n>> inside the random place the user told the installer to use, can't it?\n>\n> Yes, but...\n>\n> Take as an example 'git pull'.\n>\n> - The first call to git will derive the exec-path\n> $prefix/bin/../libexec/git-core and prepend it to $PATH.\n>\n> - Calls to builtin git commands from inside 'git pull' will then \n> derive the\n> exec-path $prefix/bin/../libexec/git-core/../libexec/git-core, that is\n> $prefix/libexec/libexec/git-core, and prepend it to $PATH as well. \n> That\n> directory does not exist - usually - and it does not hurt. But it \n> feels dirty\n> and potentially dangerous.\n\nMaybe we can avoid this based on the name of the executable?\nWe would add libexec only if the executable is named \"git\",\nbut not if it is one of the dashed forms \"git-*\".\n\n\tSteffen\n"},{"id":"83951","messageId":"7vmykd238a.fsf@gitster.siamese.dyndns.org","threadId":"14522","inReplyTo":"200807191052.20057.johannes.sixt@telecom.at","subject":"Re: [PATCH] Windows: set gitexecdir = $(bindir)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-19T17:28:05Z","receivedAt":"2008-07-19T17:28:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> On Samstag, 19. Juli 2008, Junio C Hamano wrote:\n>> Sorry, I am not sure if I understand what you are trying to solve. If you\n>> have ../libexec/git-core/ in GIT_EXEC_PATH (or have builtin_exec_path()\n>> use it), then your installation would look like this:\n>>\n>> \t[[some random place]]\n>> \tbin/git\n>> libexec/git-core/git-add\n>> libexec/git-core/git-del\n>> libexec/git-core/git-dir\n>> ...\n>>\n>> and if \"git\" can figure out it is \"[[some random place]]/bin/git\",\n>> it can find its subcommands from neighbouring directory, that is still\n>> inside the random place the user told the installer to use, can't it?\n>\n> Yes, but...\n>\n> Take as an example 'git pull'.\n>\n> - The first call to git will derive the exec-path \n> $prefix/bin/../libexec/git-core and prepend it to $PATH.\n>\n> - Calls to builtin git commands from inside 'git pull' will then derive the \n> exec-path $prefix/bin/../libexec/git-core/../libexec/git-core, that is \n> $prefix/libexec/libexec/git-core, and prepend it to $PATH as well. That \n> directory does not exist - usually - and it does not hurt. But it feels dirty \n> and potentially dangerous.\n\nYou run \"git\" with an argument \"pull\". It needs to figure out where\n\"git-pull\" is, it checks where it came from and adds ../libexec/git-core/.\nThen it runs \"git-pull\" script.\n\nThen the script may have a call to \"git ls-files -u\" or \"git-merge\".\n\n - The former case, \"git\" again needs to find out where \"git-ls-files\"\n is.\n\n If \"git\" is found as bin/git and not as libexec/git-core/git, this\n should be perfectly fine, isn't it? Perhaps we install a duplicate\n copy there by mistake, which is what we need to fix?\n\n - The latter case (our scripts source git-sh-setup so they have libexec\n one in the PATH when they are started) would find \"git-merge\" directly\n and runs it.\n\nIn either case, the programs \"git-ls-files\" and \"git-merge\" do not need to\ndo the same discovery -- are we giving them enough clues when we run them\nto let them avoid that?\n"},{"id":"83953","messageId":"alpine.LNX.1.00.0807191333550.19665@iabervon.org","threadId":"14522","inReplyTo":"200807191549.56402.johannes.sixt@telecom.at","subject":"Re: [PATCH] builtin-clone: Use is_dir_sep() instead of '/'","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-07-19T17:44:11Z","receivedAt":"2008-07-19T17:44:11Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sat, 19 Jul 2008, Johannes Sixt wrote:\n\n> On Samstag, 19. Juli 2008, Johannes Sixt wrote:\n> > On Samstag, 19. Juli 2008, Junio C Hamano wrote:\n> > > Ok, but the surrounding code in this function look very suspicious.\n> >\n> > How about this then?\n> >\n> > -- snip --\n> > builtin-clone: Rewrite guess_dir_name()\n> >\n> > The function has to do three small and independent tasks, but all of them\n> > were crammed into a single loop. This rewrites the function entirely by\n> > unrolling these tasks.\n> \n> Sigh. I knew it, I knew it. If it had been that trivial, then Daniel had done\n> it this way in the first place. :-(\n> \n> This needs to be squashed in. It makes sure that we handle 'foo/.git';\n> and .git was not stripped if we cloned from 'foo.git/'.\n\nI actually got that from Johannes Schindelin, who added bundle support to \nmy clone patch. I remember looking at his change, thinking it looked \noverly complicated, but finding that anything I tried to do to simplify it \nfailed tests. If this gets through the test suite (lots of the tests other \nthan the clone test try to do a wider variety of odd things than I expect \nusers do in practice most of the time), then it's probably a better \nimplementation.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"83964","messageId":"200807192131.44441.johannes.sixt@telecom.at","threadId":"14522","inReplyTo":"7vmykd238a.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Windows: set gitexecdir = $(bindir)","fromName":"Johannes Sixt","fromEmail":"johannes.sixt@telecom.at","sentAt":"2008-07-19T19:31:44Z","receivedAt":"2008-07-19T19:31:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Samstag, 19. Juli 2008, Junio C Hamano wrote:\n> Johannes Sixt writes:\n> > Take as an example 'git pull'.\n> >\n> > - The first call to git will derive the exec-path\n> > $prefix/bin/../libexec/git-core and prepend it to $PATH.\n> >\n> > - Calls to builtin git commands from inside 'git pull' will then derive\n> > the exec-path $prefix/bin/../libexec/git-core/../libexec/git-core, that\n> > is $prefix/libexec/libexec/git-core, and prepend it to $PATH as well.\n> > That directory does not exist - usually - and it does not hurt. But it\n> > feels dirty and potentially dangerous.\n>\n> You run \"git\" with an argument \"pull\". It needs to figure out where\n> \"git-pull\" is, it checks where it came from and adds ../libexec/git-core/.\n> Then it runs \"git-pull\" script.\n>\n> Then the script may have a call to \"git ls-files -u\" or \"git-merge\".\n>\n> - The former case, \"git\" again needs to find out where \"git-ls-files\"\n> is.\n>\n> If \"git\" is found as bin/git and not as libexec/git-core/git, this\n> should be perfectly fine, isn't it? Perhaps we install a duplicate\n> copy there by mistake, which is what we need to fix?\n\nYes, there's libexec/git-core/git. There reason might be that the install \ntarget is simpler to write (to create the hardlinks) just in case $(bindir) \nand $(gitexecdir) are not on the same mount.\n\n> - The latter case (our scripts source git-sh-setup so they have libexec\n> one in the PATH when they are started) would find \"git-merge\" directly\n> and runs it.\n>\n> In either case, the programs \"git-ls-files\" and \"git-merge\" do not need to\n> do the same discovery -- are we giving them enough clues when we run them\n> to let them avoid that?\n\nProbably the only clue is the name itself, like Steffen proposed.\n\nI'll see how I can improve my earlier exec-path patch series.\n\n-- Hannes\n"},{"id":"84194","messageId":"1216647797.3997.6.camel@gaara.bos.redhat.com","threadId":"14522","inReplyTo":"alpine.LNX.1.00.0807191333550.19665@iabervon.org","subject":"Re: [PATCH] builtin-clone: Use is_dir_sep() instead of '/'","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2008-07-21T13:43:17Z","receivedAt":"2008-07-21T13:43:17Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Sat, 2008-07-19 at 13:44 -0400, Daniel Barkalow wrote:\n> On Sat, 19 Jul 2008, Johannes Sixt wrote:\n> \n> > On Samstag, 19. Juli 2008, Johannes Sixt wrote:\n> > > On Samstag, 19. Juli 2008, Junio C Hamano wrote:\n> > > > Ok, but the surrounding code in this function look very suspicious.\n> > >\n> > > How about this then?\n> > >\n> > > -- snip --\n> > > builtin-clone: Rewrite guess_dir_name()\n> > >\n> > > The function has to do three small and independent tasks, but all of them\n> > > were crammed into a single loop. This rewrites the function entirely by\n> > > unrolling these tasks.\n> > \n> > Sigh. I knew it, I knew it. If it had been that trivial, then Daniel had done\n> > it this way in the first place. :-(\n> > \n> > This needs to be squashed in. It makes sure that we handle 'foo/.git';\n> > and .git was not stripped if we cloned from 'foo.git/'.\n> \n> I actually got that from Johannes Schindelin, who added bundle support to \n> my clone patch. I remember looking at his change, thinking it looked \n> overly complicated, but finding that anything I tried to do to simplify it \n> failed tests. If this gets through the test suite (lots of the tests other \n> than the clone test try to do a wider variety of odd things than I expect \n> users do in practice most of the time), then it's probably a better \n> implementation.\n\nI wrote the original guess_git_dir(), and it *is* pretty subtle, sorry.\nI would not rewrite it unless you really have to, since git clone now is\npretty stable and the dir sep separator change doesn't need this kind of\nrewrite to fix the issue. That said, with the last couple of changes\nfrom Johannes Sixt, I believe it handles the same case that I had in\nmind when I first wrote the loop.\n\ncheers,\nKristian\n"}]}