{"thread":{"id":"17877","subject":"[PATCH 0/2] RUNTIME_PREFIX enhancements","startedAt":"2009-02-18T15:10:54Z","lastAt":"2009-02-20T08:35:56Z","messageCount":20,"participants":["Johannes Schindelin","Johannes Sixt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"105314","messageId":"cover.1234969572u.git.johannes.schindelin@gmx.de","threadId":"17877","inReplyTo":null,"subject":"[PATCH 0/2] RUNTIME_PREFIX enhancements","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-18T15:10:54Z","receivedAt":"2009-02-18T15:10:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When I ran \"make\" in msysGit's /git/, I was greeted by a pretty unfriendly \nmessage about a RUNTIME_PREFIX that could not be determined.\n\nNot understanding the code in exec_cmd.c, I went back to a comment I made \nto one of the iterations of the RUNTIME_PREFIX patch series (back when I \nspent a considerable amount of time to understand the code), and turned it \ninto a patch myself.\n\nI hope you will agree that the code introduced in 1/2 is more \nunderstandable than what is removed in 2/2 in favor of the former.\n\nThe real meat comes in patch 2/2:\n\nThe problem is that Windows will look in the current directory before \nlooking in the PATH when it tries to execute a program.  So it will find \nthe executable C:\\msysgit\\git\\git.exe and be unable to strip the suffices \n\"libexec/git-core\" or \"bin\".\n\nI just added \"git\" (which should not hurt other users, but instead help \nthem if they did not install Git but run it in-place).\n\nJohannes Schindelin (2):\n  Introduce the function strip_path_suffix()\n  system_path(): simplify using strip_path_suffix(), and add suffix\n    \"git\"\n\n cache.h    |    2 ++\n exec_cmd.c |   32 +++-----------------------------\n path.c     |   33 +++++++++++++++++++++++++++++++++\n 3 files changed, 38 insertions(+), 29 deletions(-)\n"},{"id":"105316","messageId":"7b5ee74d497bbb7144b8683725cd83c8bbd33f9a.1234969572u.git.johannes.schindelin@gmx.de","threadId":"17877","inReplyTo":"cover.1234969572u.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/2] Introduce the function strip_path_suffix()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-18T15:11:04Z","receivedAt":"2009-02-18T15:11:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The function strip_path_suffix() will try to split the given path into\nprefix/suffix.  The suffix has to be passed to the function, and if the\npath matches, the prefix is set.\n\nArbitrary runs of directory separators (\"slashes\") are assumed identical.\n\nExample:\n\n\tstrip_path_suffix(\"C:\\\\msysgit/\\\\libexec\\\\git-core\",\n\t\t\"libexec///git-core\", &prefix)\n\nwill set prefix to \"C:\\\\msysgit\" and return 0.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n cache.h |    2 ++\n path.c  |   33 +++++++++++++++++++++++++++++++++\n 2 files changed, 35 insertions(+), 0 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4e28c24..2186143 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -629,6 +629,8 @@ const char *make_nonrelative_path(const char *path);\n const char *make_relative_path(const char *abs, const char *base);\n int normalize_path_copy(char *dst, const char *src);\n int longest_ancestor_length(const char *path, const char *prefix_list);\n+int strip_path_suffix(const char *path, const char *suffix,\n+\t\tconst char **prefix);\n \n /* Read and unpack a sha1 file into memory, write memory to a sha1 file */\n extern int sha1_object_info(const unsigned char *, unsigned long *);\ndiff --git a/path.c b/path.c\nindex 4b9107f..947e5b7 100644\n--- a/path.c\n+++ b/path.c\n@@ -499,3 +499,36 @@ int longest_ancestor_length(const char *path, const char *prefix_list)\n \n \treturn max_len;\n }\n+\n+/* strip arbitrary amount of directory separators at end of path */\n+static inline int chomp_trailing_dir_sep(const char *path, int len)\n+{\n+\twhile (len && is_dir_sep(path[len - 1]))\n+\t\tlen--;\n+\treturn len;\n+}\n+\n+/* sets prefix if the suffix matches */\n+int strip_path_suffix(const char *path, const char *suffix, const char **prefix)\n+{\n+\tint path_len = strlen(path), suffix_len = strlen(suffix);\n+\n+\twhile (suffix_len) {\n+\t\tif (!path_len)\n+\t\t\treturn 1;\n+\n+\t\tif (is_dir_sep(path[path_len - 1])) {\n+\t\t\tif (!is_dir_sep(suffix[suffix_len - 1]))\n+\t\t\t\treturn 1;\n+\t\t\tpath_len = chomp_trailing_dir_sep(path, path_len);\n+\t\t\tsuffix_len = chomp_trailing_dir_sep(suffix, suffix_len);\n+\t\t}\n+\t\telse if (path[--path_len] != suffix[--suffix_len])\n+\t\t\treturn 1;\n+\t}\n+\n+\tif (path_len && !is_dir_sep(path[path_len - 1]))\n+\t\treturn 1;\n+\t*prefix = xstrndup(path, chomp_trailing_dir_sep(path, path_len));\n+\treturn 0;\n+}\n-- \n1.6.1.1.825.g72a9f\n"},{"id":"105315","messageId":"ba10ee3899e1b274c71c91a48a5319d9f18db502.1234969572u.git.johannes.schindelin@gmx.de","threadId":"17877","inReplyTo":"cover.1234969572u.git.johannes.schindelin@gmx.de","subject":"[PATCH 2/2] system_path(): simplify using strip_path_suffix(), and add suffix \"git\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-18T15:11:08Z","receivedAt":"2009-02-18T15:11:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"At least for the author of this patch, the logic in system_path() was\ntoo hard to understand.  Using the function strip_path_suffix() documents\nthe idea of the code better.\n\nThe real change is to add the suffix \"git\", so that a runtime prefix will\nbe computed correctly even when the executable was called in /git/ as is\nthe case in msysGit (Windows insists to search the current directory\nbefore the PATH when looking for an executable).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n exec_cmd.c |   32 +++-----------------------------\n 1 files changed, 3 insertions(+), 29 deletions(-)\n\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex f234066..f89268b 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -23,35 +23,9 @@ const char *system_path(const char *path)\n \tassert(argv0_path);\n \tassert(is_absolute_path(argv0_path));\n \n-\tif (!prefix) {\n-\t\tconst char *strip[] = {\n-\t\t\tGIT_EXEC_PATH,\n-\t\t\tBINDIR,\n-\t\t\t0\n-\t\t};\n-\t\tconst char **s;\n-\n-\t\tfor (s = strip; *s; s++) {\n-\t\t\tconst char *sargv = argv0_path + strlen(argv0_path);\n-\t\t\tconst char *ss = *s + strlen(*s);\n-\t\t\twhile (argv0_path < sargv && *s < ss\n-\t\t\t\t&& (*sargv == *ss ||\n-\t\t\t\t    (is_dir_sep(*sargv) && is_dir_sep(*ss)))) {\n-\t\t\t\tsargv--;\n-\t\t\t\tss--;\n-\t\t\t}\n-\t\t\tif (*s == ss) {\n-\t\t\t\tstruct strbuf d = STRBUF_INIT;\n-\t\t\t\t/* We also skip the trailing directory separator. */\n-\t\t\t\tassert(sargv - argv0_path - 1 >= 0);\n-\t\t\t\tstrbuf_add(&d, argv0_path, sargv - argv0_path - 1);\n-\t\t\t\tprefix = strbuf_detach(&d, NULL);\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t}\n-\t}\n-\n-\tif (!prefix) {\n+\tif (!prefix && strip_path_suffix(argv0_path, GIT_EXEC_PATH, &prefix) &&\n+\t\t\tstrip_path_suffix(argv0_path, BINDIR, &prefix) &&\n+\t\t\tstrip_path_suffix(argv0_path, \"git\", &prefix)) {\n \t\tprefix = PREFIX;\n \t\tfprintf(stderr, \"RUNTIME_PREFIX requested, \"\n \t\t\t\t\"but prefix computation failed.  \"\n-- \n1.6.1.1.825.g72a9f\n"},{"id":"105335","messageId":"499C63C2.3050405@kdbg.org","threadId":"17877","inReplyTo":"cover.1234969572u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 0/2] RUNTIME_PREFIX enhancements","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-18T19:38:42Z","receivedAt":"2009-02-18T19:38:42Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> When I ran \"make\" in msysGit's /git/, I was greeted by a pretty unfriendly \n> message about a RUNTIME_PREFIX that could not be determined.\n\n;)\n\nI have a patch in my private tree that removes the warning.\n\nActually I wouldn't mind the warning because it is only visible for \ndevelopers. Unfortunately, it is poison for gitk (you know, Tcl/Tk \ntreats any output on stderr as program failure...)\n\n> The real meat comes in patch 2/2:\n> \n> The problem is that Windows will look in the current directory before \n> looking in the PATH when it tries to execute a program.  So it will find \n> the executable C:\\msysgit\\git\\git.exe and be unable to strip the suffices \n> \"libexec/git-core\" or \"bin\".\n> \n> I just added \"git\" (which should not hurt other users, but instead help \n> them if they did not install Git but run it in-place).\n\nThis only silences the warning, but there is no guarantee that the \nresulting git suite works because your msysgit developer may not have \ninstalled stuff in C:\\msysgit\\libexec\\git-core, yet. What the patch does \nis exactly the same as if the compiled-in prefix that the warning \nmentions were used, which in the msysgit case on your machine happens to \nbe C:\\msysgit. Or am I missing something?\n\nI think that the better solution is to remove the warning instead of \nintroducing this special case suffix \"git\".\n\n-- Hannes\n"},{"id":"105336","messageId":"499C63E7.5040306@kdbg.org","threadId":"17877","inReplyTo":"7b5ee74d497bbb7144b8683725cd83c8bbd33f9a.1234969572u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 1/2] Introduce the function strip_path_suffix()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-18T19:39:19Z","receivedAt":"2009-02-18T19:39:19Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> The function strip_path_suffix() will try to split the given path into\n> prefix/suffix.  The suffix has to be passed to the function, and if the\n> path matches, the prefix is set.\n> \n> Arbitrary runs of directory separators (\"slashes\") are assumed identical.\n> \n> Example:\n> \n> \tstrip_path_suffix(\"C:\\\\msysgit/\\\\libexec\\\\git-core\",\n> \t\t\"libexec///git-core\", &prefix)\n> \n> will set prefix to \"C:\\\\msysgit\" and return 0.\n\nBut you implemented it so that prefix is actually \"C:\\\\msysgit/\\\\\" \n(unless, of course, I'm reading the code wrong).\n\n> +/* sets prefix if the suffix matches */\n> +int strip_path_suffix(const char *path, const char *suffix, const char **prefix)\n\nFor a general purpose function, the API is very unnatural (and geared \ntowards its only user in patch 2/2). I'd expect that the return value is \nthe result or NULL on failure.\n\nI would not raise this concern if this were a static function near its \nonly call site.\n\n> +{\n> +\tint path_len = strlen(path), suffix_len = strlen(suffix);\n> +\n> +\twhile (suffix_len) {\n> +\t\tif (!path_len)\n> +\t\t\treturn 1;\n> +\n> +\t\tif (is_dir_sep(path[path_len - 1])) {\n> +\t\t\tif (!is_dir_sep(suffix[suffix_len - 1]))\n> +\t\t\t\treturn 1;\n> +\t\t\tpath_len = chomp_trailing_dir_sep(path, path_len);\n> +\t\t\tsuffix_len = chomp_trailing_dir_sep(suffix, suffix_len);\n> +\t\t}\n> +\t\telse if (path[--path_len] != suffix[--suffix_len])\n> +\t\t\treturn 1;\n> +\t}\n> +\n> +\tif (path_len && !is_dir_sep(path[path_len - 1]))\n> +\t\treturn 1;\n\nShould strip_path_suffix(\"foo/bar\", \"foo/bar\", &prefix) succeed and \nprefix be the empty string? This implementation says it should be so. \nThat's just a question...\n\n> +\t*prefix = xstrndup(path, chomp_trailing_dir_sep(path, path_len));\n> +\treturn 0;\n> +}\n\n-- Hannes\n"},{"id":"105341","messageId":"alpine.DEB.1.00.0902182208070.10279@pacific.mpi-cbg.de","threadId":"17877","inReplyTo":"499C63C2.3050405@kdbg.org","subject":"Re: [PATCH 0/2] RUNTIME_PREFIX enhancements","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-18T21:11:48Z","receivedAt":"2009-02-18T21:11:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Feb 2009, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > When I ran \"make\" in msysGit's /git/, I was greeted by a pretty unfriendly\n> > message about a RUNTIME_PREFIX that could not be determined.\n> \n> ;)\n> \n> I have a patch in my private tree that removes the warning.\n\nI briefly considered this.  But my patch actually does something different \nthan hiding an error condition.  It copes with the very peculiar situation \nwe have on Windows.\n\nI still would like to see the warning if something goes wrong -- _outside_ \n/git/.\n\n> This only silences the warning, but there is no guarantee that the \n> resulting git suite works because your msysgit developer may not have \n> installed stuff in C:\\msysgit\\libexec\\git-core, yet.\n\nThe good thing: it still works.  Why?  Because Git actually does _not_ \nsearch the Git binaries in libexec/git-core.  It adds that directory to \nPATH and lets the PATH lookup handle the searching.  So it kind of works, \nbut in a different manner than you think it works.\n\nBut that cannot be helped.\n\nCiao,\nDscho\n"},{"id":"105342","messageId":"alpine.DEB.1.00.0902182211550.10279@pacific.mpi-cbg.de","threadId":"17877","inReplyTo":"499C63E7.5040306@kdbg.org","subject":"Re: [PATCH 1/2] Introduce the function strip_path_suffix()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-18T21:13:20Z","receivedAt":"2009-02-18T21:13:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Feb 2009, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > The function strip_path_suffix() will try to split the given path into \n> > prefix/suffix.  The suffix has to be passed to the function, and if \n> > the path matches, the prefix is set.\n> > \n> > Arbitrary runs of directory separators (\"slashes\") are assumed \n> > identical.\n> > \n> > Example:\n> > \n> >  strip_path_suffix(\"C:\\\\msysgit/\\\\libexec\\\\git-core\",\n> >   \"libexec///git-core\", &prefix)\n> > \n> > will set prefix to \"C:\\\\msysgit\" and return 0.\n> \n> But you implemented it so that prefix is actually \"C:\\\\msysgit/\\\\\" \n> (unless, of course, I'm reading the code wrong).\n\nThis is supposed to handle that case:\n\n\t*prefix = xstrndup(path, chomp_trailing_dir_sep(path, path_len));\n\n> > +/* sets prefix if the suffix matches */\n> > +int strip_path_suffix(const char *path, const char *suffix, const char\n> > **prefix)\n> \n> For a general purpose function, the API is very unnatural (and geared \n> towards its only user in patch 2/2). I'd expect that the return value is \n> the result or NULL on failure.\n\nGood point.  Will change.\n\n> > +{\n> > +\tint path_len = strlen(path), suffix_len = strlen(suffix);\n> > +\n> > +\twhile (suffix_len) {\n> > +\t\tif (!path_len)\n> > +\t\t\treturn 1;\n> > +\n> > +\t\tif (is_dir_sep(path[path_len - 1])) {\n> > +\t\t\tif (!is_dir_sep(suffix[suffix_len - 1]))\n> > +\t\t\t\treturn 1;\n> > +\t\t\tpath_len = chomp_trailing_dir_sep(path, path_len);\n> > +\t\t\tsuffix_len = chomp_trailing_dir_sep(suffix,\n> > suffix_len);\n> > +\t\t}\n> > +\t\telse if (path[--path_len] != suffix[--suffix_len])\n> > +\t\t\treturn 1;\n> > +\t}\n> > +\n> > +\tif (path_len && !is_dir_sep(path[path_len - 1]))\n> > +\t\treturn 1;\n> \n> Should strip_path_suffix(\"foo/bar\", \"foo/bar\", &prefix) succeed and \n> prefix be the empty string? This implementation says it should be so. \n> That's just a question...\n\nYes, I assumed that that makes sense.\n\nCiao,\nDscho\n"},{"id":"105353","messageId":"499C8B44.1020106@kdbg.org","threadId":"17877","inReplyTo":"alpine.DEB.1.00.0902182211550.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH 1/2] Introduce the function strip_path_suffix()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-18T22:27:16Z","receivedAt":"2009-02-18T22:27:16Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> On Wed, 18 Feb 2009, Johannes Sixt wrote:\n>> Johannes Schindelin schrieb:\n>>>  strip_path_suffix(\"C:\\\\msysgit/\\\\libexec\\\\git-core\",\n>>>   \"libexec///git-core\", &prefix)\n>>>\n>>> will set prefix to \"C:\\\\msysgit\" and return 0.\n>> But you implemented it so that prefix is actually \"C:\\\\msysgit/\\\\\" \n>> (unless, of course, I'm reading the code wrong).\n> \n> This is supposed to handle that case:\n> \n> \t*prefix = xstrndup(path, chomp_trailing_dir_sep(path, path_len));\n\nAh, I missed that final call of chomp_trailing_dir_sep.\n\n-- Hannes\n"},{"id":"105354","messageId":"499C9123.2070104@kdbg.org","threadId":"17877","inReplyTo":"alpine.DEB.1.00.0902182208070.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH 0/2] RUNTIME_PREFIX enhancements","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-18T22:52:19Z","receivedAt":"2009-02-18T22:52:19Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> On Wed, 18 Feb 2009, Johannes Sixt wrote:\n>> This only silences the warning, but there is no guarantee that the \n>> resulting git suite works because your msysgit developer may not have \n>> installed stuff in C:\\msysgit\\libexec\\git-core, yet.\n> \n> The good thing: it still works.  Why?  Because Git actually does _not_ \n> search the Git binaries in libexec/git-core.  It adds that directory to \n> PATH and lets the PATH lookup handle the searching.  So it kind of works, \n> but in a different manner than you think it works.\n\nI see why it works: Because we add argv0_path to PATH. I had forgotten \nabout this fact. It comes after git_exec_path(), and so we pick up the \nbinaries and shell scripts from the build directory if there is nothing \nin $prefix/libexec/git-core.\n\nIt's sad that your patch requires the source to live in a directory \nnamed 'git'. But I don't know how to do it better. :-(\n\n-- Hannes\n"},{"id":"105357","messageId":"alpine.DEB.1.00.0902190017460.10279@pacific.mpi-cbg.de","threadId":"17877","inReplyTo":"499C8B44.1020106@kdbg.org","subject":"Re: [PATCH 1/2] Introduce the function strip_path_suffix()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-18T23:18:17Z","receivedAt":"2009-02-18T23:18:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Feb 2009, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > On Wed, 18 Feb 2009, Johannes Sixt wrote:\n> > > Johannes Schindelin schrieb:\n> > > >  strip_path_suffix(\"C:\\\\msysgit/\\\\libexec\\\\git-core\",\n> > > >   \"libexec///git-core\", &prefix)\n> > > >\n> > > > will set prefix to \"C:\\\\msysgit\" and return 0.\n> > > But you implemented it so that prefix is actually \"C:\\\\msysgit/\\\\\"\n> > > (unless, of course, I'm reading the code wrong).\n> > \n> > This is supposed to handle that case:\n> > \n> >  *prefix = xstrndup(path, chomp_trailing_dir_sep(path, path_len));\n> \n> Ah, I missed that final call of chomp_trailing_dir_sep.\n\nI have to admit that I was aware of the fact that it is easy to miss, but \ncould not find a better way to present it.\n\nCiao,\nDscho\n"},{"id":"105358","messageId":"alpine.DEB.1.00.0902190019300.10279@pacific.mpi-cbg.de","threadId":"17877","inReplyTo":"499C9123.2070104@kdbg.org","subject":"Re: [PATCH 0/2] RUNTIME_PREFIX enhancements","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-18T23:19:57Z","receivedAt":"2009-02-18T23:19:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Feb 2009, Johannes Sixt wrote:\n\n> It's sad that your patch requires the source to live in a directory \n> named 'git'. But I don't know how to do it better. :-(\n\nLet's move it to /bin/\n\nDucks,\nDscho\n"},{"id":"105496","messageId":"cover.1235070304u.git.johannes.schindelin@gmx.de","threadId":"17877","inReplyTo":"499C63E7.5040306@kdbg.org","subject":"[PATCH v2 0/2] RUNTIME_PREFIX enhancements","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-19T19:10:44Z","receivedAt":"2009-02-19T19:10:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The idea is to simplify the RUNTIME_PREFIX code, and to make Git handle \nthe case where the binary was found in $ROOT/git/, too.\n\nChanges since v1:\n\n- changed the signature of strip_path_suffix() as suggested by Hannes:\n\n\tchar *strip_path_suffix(const char *path, const char *suffix)\n\n- added a test to test-path-utils\n\nJohannes Schindelin (2):\n  Introduce the function strip_path_suffix()\n  system_path(): simplify using strip_path_suffix(), and add suffix\n    \"git\"\n\n cache.h               |    1 +\n exec_cmd.c            |   33 ++++-----------------------------\n path.c                |   32 ++++++++++++++++++++++++++++++++\n t/t0060-path-utils.sh |    4 ++++\n test-path-utils.c     |    6 ++++++\n 5 files changed, 47 insertions(+), 29 deletions(-)\n"},{"id":"105495","messageId":"f6344cbba22e049806796f4920e229fb2e539d5c.1235070304u.git.johannes.schindelin@gmx.de","threadId":"17877","inReplyTo":"499C63E7.5040306@kdbg.org","subject":"[PATCH v2 1/2] Introduce the function strip_path_suffix()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-19T19:10:49Z","receivedAt":"2009-02-19T19:10:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The function strip_path_suffix() will try to strip a given suffix from \na given path.  The suffix must start at a directory boundary (i.e. \"core\" \nis not a path suffix of \"libexec/git-core\", but \"git-core\" is).\n\nArbitrary runs of directory separators (\"slashes\") are assumed identical.\n\nExample:\n\n\tstrip_path_suffix(\"C:\\\\msysgit/\\\\libexec\\\\git-core\",\n\t\t\"libexec///git-core\", &prefix)\n\nwill set prefix to \"C:\\\\msysgit\" and return 0.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n cache.h               |    1 +\n path.c                |   32 ++++++++++++++++++++++++++++++++\n t/t0060-path-utils.sh |    4 ++++\n test-path-utils.c     |    6 ++++++\n 4 files changed, 43 insertions(+), 0 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex c926bc7..1fa184d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -630,6 +630,7 @@ const char *make_nonrelative_path(const char *path);\n const char *make_relative_path(const char *abs, const char *base);\n int normalize_path_copy(char *dst, const char *src);\n int longest_ancestor_length(const char *path, const char *prefix_list);\n+char *strip_path_suffix(const char *path, const char *suffix);\n \n /* Read and unpack a sha1 file into memory, write memory to a sha1 file */\n extern int sha1_object_info(const unsigned char *, unsigned long *);\ndiff --git a/path.c b/path.c\nindex 4b9107f..2030bf1 100644\n--- a/path.c\n+++ b/path.c\n@@ -499,3 +499,35 @@ int longest_ancestor_length(const char *path, const char *prefix_list)\n \n \treturn max_len;\n }\n+\n+/* strip arbitrary amount of directory separators at end of path */\n+static inline int chomp_trailing_dir_sep(const char *path, int len)\n+{\n+\twhile (len && is_dir_sep(path[len - 1]))\n+\t\tlen--;\n+\treturn len;\n+}\n+\n+/* sets prefix if the suffix matches */\n+char *strip_path_suffix(const char *path, const char *suffix)\n+{\n+\tint path_len = strlen(path), suffix_len = strlen(suffix);\n+\n+\twhile (suffix_len) {\n+\t\tif (!path_len)\n+\t\t\treturn NULL;\n+\n+\t\tif (is_dir_sep(path[path_len - 1])) {\n+\t\t\tif (!is_dir_sep(suffix[suffix_len - 1]))\n+\t\t\t\treturn NULL;\n+\t\t\tpath_len = chomp_trailing_dir_sep(path, path_len);\n+\t\t\tsuffix_len = chomp_trailing_dir_sep(suffix, suffix_len);\n+\t\t}\n+\t\telse if (path[--path_len] != suffix[--suffix_len])\n+\t\t\treturn NULL;\n+\t}\n+\n+\tif (path_len && !is_dir_sep(path[path_len - 1]))\n+\t\treturn NULL;\n+\treturn xstrndup(path, chomp_trailing_dir_sep(path, path_len));\n+}\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 4ed1f0b..8336114 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -85,4 +85,8 @@ ancestor /foo/bar :://foo/.:: 4\n ancestor /foo/bar //foo/./::/bar 4\n ancestor /foo/bar ::/bar -1\n \n+test_expect_success 'strip_path_suffix' '\n+\ttest c:/msysgit = $(test-path-utils strip_path_suffix \\\n+\t\tc:/msysgit/libexec//git-core libexec/git-core)\n+'\n test_done\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex 5168a8e..d261398 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -26,6 +26,12 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n \n+\tif (argc == 4 && !strcmp(argv[1], \"strip_path_suffix\")) {\n+\t\tchar *prefix = strip_path_suffix(argv[2], argv[3]);\n+\t\tprintf(\"%s\\n\", prefix ? prefix : \"(null)\");\n+\t\treturn 0;\n+\t}\n+\n \tfprintf(stderr, \"%s: unknown function name: %s\\n\", argv[0],\n \t\targv[1] ? argv[1] : \"(there was none)\");\n \treturn 1;\n-- \n1.6.2.rc1.380.g0299c\n"},{"id":"105497","messageId":"b91ca090ef4eed9e457b579df2e3fbede3d26299.1235070304u.git.johannes.schindelin@gmx.de","threadId":"17877","inReplyTo":"499C63E7.5040306@kdbg.org","subject":"[PATCH v2 2/2] system_path(): simplify using strip_path_suffix(), and add suffix \"git\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-19T19:10:53Z","receivedAt":"2009-02-19T19:10:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"At least for the author of this patch, the logic in system_path() was\ntoo hard to understand.  Using the function strip_path_suffix() documents\nthe idea of the code better.\n\nThe real change is to add the suffix \"git\", so that a runtime prefix will\nbe computed correctly even when the executable was called in /git/ as is\nthe case in msysGit (Windows insists to search the current directory\nbefore the PATH when looking for an executable).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n exec_cmd.c |   33 ++++-----------------------------\n 1 files changed, 4 insertions(+), 29 deletions(-)\n\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex f234066..217c125 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -23,35 +23,10 @@ const char *system_path(const char *path)\n \tassert(argv0_path);\n \tassert(is_absolute_path(argv0_path));\n \n-\tif (!prefix) {\n-\t\tconst char *strip[] = {\n-\t\t\tGIT_EXEC_PATH,\n-\t\t\tBINDIR,\n-\t\t\t0\n-\t\t};\n-\t\tconst char **s;\n-\n-\t\tfor (s = strip; *s; s++) {\n-\t\t\tconst char *sargv = argv0_path + strlen(argv0_path);\n-\t\t\tconst char *ss = *s + strlen(*s);\n-\t\t\twhile (argv0_path < sargv && *s < ss\n-\t\t\t\t&& (*sargv == *ss ||\n-\t\t\t\t    (is_dir_sep(*sargv) && is_dir_sep(*ss)))) {\n-\t\t\t\tsargv--;\n-\t\t\t\tss--;\n-\t\t\t}\n-\t\t\tif (*s == ss) {\n-\t\t\t\tstruct strbuf d = STRBUF_INIT;\n-\t\t\t\t/* We also skip the trailing directory separator. */\n-\t\t\t\tassert(sargv - argv0_path - 1 >= 0);\n-\t\t\t\tstrbuf_add(&d, argv0_path, sargv - argv0_path - 1);\n-\t\t\t\tprefix = strbuf_detach(&d, NULL);\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t}\n-\t}\n-\n-\tif (!prefix) {\n+\tif (!prefix &&\n+\t    !(prefix = strip_path_suffix(argv0_path, GIT_EXEC_PATH)) &&\n+\t    !(prefix = strip_path_suffix(argv0_path, BINDIR)) &&\n+\t    !(prefix = strip_path_suffix(argv0_path, \"git\"))) {\n \t\tprefix = PREFIX;\n \t\tfprintf(stderr, \"RUNTIME_PREFIX requested, \"\n \t\t\t\t\"but prefix computation failed.  \"\n-- \n1.6.2.rc1.380.g0299c\n"},{"id":"105506","messageId":"alpine.DEB.1.00.0902192110290.6223@intel-tinevez-2-302","threadId":"17877","inReplyTo":"f6344cbba22e049806796f4920e229fb2e539d5c.1235070304u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 1/2] Introduce the function strip_path_suffix()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-19T20:12:07Z","receivedAt":"2009-02-19T20:12:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 19 Feb 2009, Johannes Schindelin wrote:\n\n> The function strip_path_suffix() will try to strip a given suffix from \n> a given path.  The suffix must start at a directory boundary (i.e. \"core\" \n> is not a path suffix of \"libexec/git-core\", but \"git-core\" is).\n> \n> Arbitrary runs of directory separators (\"slashes\") are assumed identical.\n> \n> Example:\n> \n> \tstrip_path_suffix(\"C:\\\\msysgit/\\\\libexec\\\\git-core\",\n> \t\t\"libexec///git-core\", &prefix)\n> \n> will set prefix to \"C:\\\\msysgit\" and return 0.\n\nAargh, of course I managed to forget to adjust the example.  Here it goes:\n\n\tprefix = strip_path_suffix(\"C:\\\\msysgit/\\\\libexec\\\\git-core\",\n\t\t\t\"libexec///git-core\");\n\nwill set prefix to \"C:\\\\msysgit\".\n\nCiao,\nDscho\n"},{"id":"105509","messageId":"499DC08F.3030106@kdbg.org","threadId":"17877","inReplyTo":"f6344cbba22e049806796f4920e229fb2e539d5c.1235070304u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 1/2] Introduce the function strip_path_suffix()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-19T20:26:55Z","receivedAt":"2009-02-19T20:26:55Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> +/* sets prefix if the suffix matches */\n\n/*\n  * If path ends with suffix (complete path components), returns the\n  * part before suffix (sans trailing directory separators).\n  * Otherwise returns NULL.\n  */\n\n> +char *strip_path_suffix(const char *path, const char *suffix)\n\nLooks good otherwise.\n\n-- Hannes\n"},{"id":"105511","messageId":"499DC375.60105@kdbg.org","threadId":"17877","inReplyTo":"b91ca090ef4eed9e457b579df2e3fbede3d26299.1235070304u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 2/2] system_path(): simplify using strip_path_suffix(), and add suffix \"git\"","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-19T20:39:17Z","receivedAt":"2009-02-19T20:39:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> The real change is to add the suffix \"git\", so that a runtime prefix will\n> be computed correctly even when the executable was called in /git/ as is\n> the case in msysGit (Windows insists to search the current directory\n> before the PATH when looking for an executable).\n\nmsysgit is important enough to be supported by upstream git, in \nparticular since it is *the* user of RUNTIME_PREFIX, so that this \nspecial-casing is warranted. Therefore:\n\nAcked-by: Johannes Sixt <j6t@kdbg.org>\n\n-- Hannes\n"},{"id":"105513","messageId":"alpine.DEB.1.00.0902192143320.6223@intel-tinevez-2-302","threadId":"17877","inReplyTo":"499DC08F.3030106@kdbg.org","subject":"Re: [PATCH v2 1/2] Introduce the function strip_path_suffix()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-19T20:44:12Z","receivedAt":"2009-02-19T20:44:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 19 Feb 2009, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > +/* sets prefix if the suffix matches */\n> \n> /*\n>  * If path ends with suffix (complete path components), returns the\n>  * part before suffix (sans trailing directory separators).\n>  * Otherwise returns NULL.\n>  */\n\nThanks.  It shows that I did not really have time to polish this properly \ntoday, does it not?\n\nCiao,\nDscho\n"},{"id":"105512","messageId":"alpine.DEB.1.00.0902192147180.6223@intel-tinevez-2-302","threadId":"17877","inReplyTo":"499DC375.60105@kdbg.org","subject":"Re: [PATCH v2 2/2] system_path(): simplify using strip_path_suffix(), and add suffix \"git\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-19T20:48:10Z","receivedAt":"2009-02-19T20:48:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 19 Feb 2009, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > The real change is to add the suffix \"git\", so that a runtime prefix will\n> > be computed correctly even when the executable was called in /git/ as is\n> > the case in msysGit (Windows insists to search the current directory\n> > before the PATH when looking for an executable).\n> \n> msysgit is important enough to be supported by upstream git, in particular\n> since it is *the* user of RUNTIME_PREFIX, so that this special-casing is\n> warranted. Therefore:\n> \n> Acked-by: Johannes Sixt <j6t@kdbg.org>\n\nThanks.\n\nNote, however, that IMHO RUNTIME_PREFIX would be good for non-Windows, \ntoo, as it would make Git relocatable.\n\nCiao,\nDscho\n"},{"id":"105584","messageId":"7vy6w1ec9f.fsf@gitster.siamese.dyndns.org","threadId":"17877","inReplyTo":"499DC375.60105@kdbg.org","subject":"Re: [PATCH v2 2/2] system_path(): simplify using strip_path_suffix(), and add suffix \"git\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-20T08:35:56Z","receivedAt":"2009-02-20T08:35:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks; both patches applied.\n"}]}