{"thread":{"id":"38943","subject":"[PATCH] enter_repo(): fix suffix precedence documentation","startedAt":"2015-03-30T10:30:33Z","lastAt":"2015-03-31T18:10:20Z","messageCount":8,"participants":["Paul Tan","Matthieu Moy","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"258669","messageId":"1427711433-3104-1-git-send-email-pyokagan@gmail.com","threadId":"38943","inReplyTo":null,"subject":"[PATCH] enter_repo(): fix suffix precedence documentation","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-03-30T10:30:33Z","receivedAt":"2015-03-30T10:30:33Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"The ordering of the list of suffixes tested in enter_repo() is\ndocumented as \"%s.git/.git\", \"%s/.git\", \"%s.git\", \"%s\". This does not\nmatch the ordering of the list of suffixes tested in the code which is\n\"%s/.git\", \"%s\", \"%s.git/.git\", \"%s.git\". Fix this.\n\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n path.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex e608993..6479352 100644\n--- a/path.c\n+++ b/path.c\n@@ -303,8 +303,8 @@ return_null:\n  * (3) \"relative/path\" to mean cwd relative directory; or\n  * (4) \"/absolute/path\" to mean absolute directory.\n  *\n- * Unless \"strict\" is given, we try access() for existence of \"%s.git/.git\",\n- * \"%s/.git\", \"%s.git\", \"%s\" in this order.  The first one that exists is\n+ * Unless \"strict\" is given, we try access() for existence of \"%s/.git\",\n+ * \"%s\", \"%s.git/.git\", \"%s.git\" in this order. The first one that exists is\n  * what we try.\n  *\n  * Second, we try chdir() to that.  Upon failure, we return NULL.\n-- \n2.1.4\n"},{"id":"258670","messageId":"vpqk2xykaiu.fsf@anie.imag.fr","threadId":"38943","inReplyTo":"1427711433-3104-1-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH] enter_repo(): fix suffix precedence documentation","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-03-30T11:08:41Z","receivedAt":"2015-03-30T11:08:41Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> The ordering of the list of suffixes tested in enter_repo() is\n> documented as \"%s.git/.git\", \"%s/.git\", \"%s.git\", \"%s\". This does not\n> match the ordering of the list of suffixes tested in the code which is\n> \"%s/.git\", \"%s\", \"%s.git/.git\", \"%s.git\". Fix this.\n\nIndeed:\n\n\t\tstatic const char *suffix[] = {\n\t\t\t\"/.git\", \"\", \".git/.git\", \".git\", NULL,\n\t\t};\n\nSo the patch is clearly good. Thanks,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"258671","messageId":"20150330114407.GA2936@peff.net","threadId":"38943","inReplyTo":"1427711433-3104-1-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH] enter_repo(): fix suffix precedence documentation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-30T11:44:08Z","receivedAt":"2015-03-30T11:44:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 30, 2015 at 06:30:33PM +0800, Paul Tan wrote:\n\n> The ordering of the list of suffixes tested in enter_repo() is\n> documented as \"%s.git/.git\", \"%s/.git\", \"%s.git\", \"%s\". This does not\n> match the ordering of the list of suffixes tested in the code which is\n> \"%s/.git\", \"%s\", \"%s.git/.git\", \"%s.git\". Fix this.\n\nYes, this was adjusted in b3256eb (standardize and improve lookup rules\nfor external local repos, 2012-02-02), but I failed to update the\ncomment.\n\nYour patch is certainly an improvement, but I think there are more\ninaccuracies in the comment caused by that commit. Maybe squash this on\ntop:\n\ndiff --git a/path.c b/path.c\nindex 56f50bd..586f2c9 100644\n--- a/path.c\n+++ b/path.c\n@@ -383,14 +383,9 @@ return_null:\n  * (3) \"relative/path\" to mean cwd relative directory; or\n  * (4) \"/absolute/path\" to mean absolute directory.\n  *\n- * Unless \"strict\" is given, we try access() for existence of \"%s/.git\",\n- * \"%s\", \"%s.git/.git\", \"%s.git\" in this order. The first one that exists is\n- * what we try.\n- *\n- * Second, we try chdir() to that.  Upon failure, we return NULL.\n- *\n- * Then, we try if the current directory is a valid git repository.\n- * Upon failure, we return NULL.\n+ * Unless \"strict\" is given, we check \"%s/.git\", \"%s\", \"%s.git/.git\", \"%s.git\"\n+ * in this order. We select the first one that is a valid git repository, and\n+ * chdir() to it. If none match, or we fail to chdir, we return NULL.\n  *\n  * If all goes well, we return the directory we used to chdir() (but\n  * before ~user is expanded), avoiding getcwd() resolving symbolic\n"},{"id":"258743","messageId":"1427809167-21711-1-git-send-email-pyokagan@gmail.com","threadId":"38943","inReplyTo":"vpqk2xykaiu.fsf@anie.imag.fr","subject":"[PATCH v2] enter_repo(): fix docs to match code","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-03-31T13:39:27Z","receivedAt":"2015-03-31T13:39:27Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"On Mon, Mar 30, 2015 at 07:44:08AM -0400, Jeff King wrote:\n> Yes, this was adjusted in b3256eb (standardize and improve lookup rules\n> for external local repos, 2012-02-02), but I failed to update the\n> comment.\n> \n> Your patch is certainly an improvement, but I think there are more\n> inaccuracies in the comment caused by that commit. Maybe squash this on\n> top:\n\nThanks for catching.\n\nI've squashed the patches and rewrote the commit message.\n\n-->8--\n\nIn b3256eb (standardize and improve lookup rules for external local\nrepos), enter_repo() was modified to use a different precedence ordering\nof suffixes for DWIM of the repository path, and to ensure that the\nrepository path is actually valid instead of just testing for existence.\nHowever, the documentation was not modified to reflect these changes.\nAs such, fix the documentation to match the code.\n\nDocumentation contributed by Jeff King.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n path.c | 11 +++--------\n 1 file changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex e608993..595da81 100644\n--- a/path.c\n+++ b/path.c\n@@ -303,14 +303,9 @@ return_null:\n  * (3) \"relative/path\" to mean cwd relative directory; or\n  * (4) \"/absolute/path\" to mean absolute directory.\n  *\n- * Unless \"strict\" is given, we try access() for existence of \"%s.git/.git\",\n- * \"%s/.git\", \"%s.git\", \"%s\" in this order.  The first one that exists is\n- * what we try.\n- *\n- * Second, we try chdir() to that.  Upon failure, we return NULL.\n- *\n- * Then, we try if the current directory is a valid git repository.\n- * Upon failure, we return NULL.\n+ * Unless \"strict\" is given, we check \"%s/.git\", \"%s\", \"%s.git/.git\", \"%s.git\"\n+ * in this order. We select the first one that is a valid git repository, and\n+ * chdir() to it. If none match, or we fail to chdir, we return NULL.\n  *\n  * If all goes well, we return the directory we used to chdir() (but\n  * before ~user is expanded), avoiding getcwd() resolving symbolic\n-- \n2.1.4\n"},{"id":"258753","messageId":"xmqq4mp1rrwj.fsf@gitster.dls.corp.google.com","threadId":"38943","inReplyTo":"1427809167-21711-1-git-send-email-pyokagan@gmail.com","subject":"Re: [PATCH v2] enter_repo(): fix docs to match code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-31T17:35:56Z","receivedAt":"2015-03-31T17:35:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Tan <pyokagan@gmail.com> writes:\n\n> On Mon, Mar 30, 2015 at 07:44:08AM -0400, Jeff King wrote:\n>> Yes, this was adjusted in b3256eb (standardize and improve lookup rules\n>> for external local repos, 2012-02-02), but I failed to update the\n>> comment.\n>> \n>> Your patch is certainly an improvement, but I think there are more\n>> inaccuracies in the comment caused by that commit. Maybe squash this on\n>> top:\n>\n> Thanks for catching.\n>\n> I've squashed the patches and rewrote the commit message.\n>\n> -->8--\n>\n> In b3256eb (standardize and improve lookup rules for external local\n> repos), enter_repo() was modified to use a different precedence ordering\n> of suffixes for DWIM of the repository path, and to ensure that the\n> repository path is actually valid instead of just testing for existence.\n> However, the documentation was not modified to reflect these changes.\n> As such, fix the documentation to match the code.\n>\n> Documentation contributed by Jeff King.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> ---\n\nIt appears to me that Peff's \"squash\" is nullifying everything you\ndid, so a more truthful attribution might be\n\n\tFrom: Jeff King <peff@peff.net>\n\n\tcommit log message\n\n\tS-o-b: Jeff\n        S-o-b: Paul Tan\n\nThanks.\n\n>  path.c | 11 +++--------\n>  1 file changed, 3 insertions(+), 8 deletions(-)\n>\n> diff --git a/path.c b/path.c\n> index e608993..595da81 100644\n> --- a/path.c\n> +++ b/path.c\n> @@ -303,14 +303,9 @@ return_null:\n>   * (3) \"relative/path\" to mean cwd relative directory; or\n>   * (4) \"/absolute/path\" to mean absolute directory.\n>   *\n> - * Unless \"strict\" is given, we try access() for existence of \"%s.git/.git\",\n> - * \"%s/.git\", \"%s.git\", \"%s\" in this order.  The first one that exists is\n> - * what we try.\n> - *\n> - * Second, we try chdir() to that.  Upon failure, we return NULL.\n> - *\n> - * Then, we try if the current directory is a valid git repository.\n> - * Upon failure, we return NULL.\n> + * Unless \"strict\" is given, we check \"%s/.git\", \"%s\", \"%s.git/.git\", \"%s.git\"\n> + * in this order. We select the first one that is a valid git repository, and\n> + * chdir() to it. If none match, or we fail to chdir, we return NULL.\n>   *\n>   * If all goes well, we return the directory we used to chdir() (but\n>   * before ~user is expanded), avoiding getcwd() resolving symbolic\n"},{"id":"258758","messageId":"20150331174141.GG18912@peff.net","threadId":"38943","inReplyTo":"xmqq4mp1rrwj.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2] enter_repo(): fix docs to match code","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-03-31T17:41:41Z","receivedAt":"2015-03-31T17:41:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 31, 2015 at 10:35:56AM -0700, Junio C Hamano wrote:\n\n> > In b3256eb (standardize and improve lookup rules for external local\n> > repos), enter_repo() was modified to use a different precedence ordering\n> > of suffixes for DWIM of the repository path, and to ensure that the\n> > repository path is actually valid instead of just testing for existence.\n> > However, the documentation was not modified to reflect these changes.\n> > As such, fix the documentation to match the code.\n> >\n> > Documentation contributed by Jeff King.\n> >\n> > Signed-off-by: Jeff King <peff@peff.net>\n> > Signed-off-by: Paul Tan <pyokagan@gmail.com>\n> > ---\n> \n> It appears to me that Peff's \"squash\" is nullifying everything you\n> did, so a more truthful attribution might be\n\nI'm not sure that's true. The precedence changed, and the \"is it valid\"\ncheck changed. The fix for the first is from Paul, but the latter is\nfrom me. My squash (hopefully) retained his half. My half just took more\nwords to change.\n\nI'm OK with the attribution either way (I do not need the credit, but am\nhappy to take the blame).\n\n-Peff\n"},{"id":"258759","messageId":"xmqqzj6tqcci.fsf@gitster.dls.corp.google.com","threadId":"38943","inReplyTo":"20150331174141.GG18912@peff.net","subject":"Re: [PATCH v2] enter_repo(): fix docs to match code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-31T17:57:17Z","receivedAt":"2015-03-31T17:57:17Z","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> I'm not sure that's true. The precedence changed, and the \"is it valid\"\n> check changed.\n\nYeah, I must have misread the review thread.\n\nI only noticed that the postimage does not have any lines that v1\nchanged from the original, but at the word level there indeed are\nthese changes you listed above.\n\nThanks.\n"},{"id":"258761","messageId":"1427825420-406-1-git-send-email-pyokagan@gmail.com","threadId":"38943","inReplyTo":"xmqq4mp1rrwj.fsf@gitster.dls.corp.google.com","subject":"[PATCH v3] enter_repo(): fix docs to match code","fromName":"Paul Tan","fromEmail":"pyokagan@gmail.com","sentAt":"2015-03-31T18:10:20Z","receivedAt":"2015-03-31T18:10:20Z","isPatch":true,"sender":{"key":"pyokagan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/109479?v=4"},"body":"Hi,\n\nOn Wed, Apr 1, 2015 at 1:35 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> It appears to me that Peff's \"squash\" is nullifying everything you\n> did, so a more truthful attribution might be\n>\n>         From: Jeff King <peff@peff.net>\n>\n>         commit log message\n>\n>         S-o-b: Jeff\n>         S-o-b: Paul Tan\n\nThanks, didn't know that could be done. Fixed patch (and fixed scissors line) below,\n\n-- >8 --\nFrom: Jeff King <peff@peff.net>\n\nIn b3256eb (standardize and improve lookup rules for external local\nrepos), enter_repo() was modified to use a different precedence ordering\nof suffixes for DWIM of the repository path, and to ensure that the\nrepository path is actually valid instead of just testing for existence.\nHowever, the documentation was not modified to reflect these changes.\nAs such, fix the documentation to match the code.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Paul Tan <pyokagan@gmail.com>\n---\n path.c | 11 +++--------\n 1 file changed, 3 insertions(+), 8 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex e608993..595da81 100644\n--- a/path.c\n+++ b/path.c\n@@ -303,14 +303,9 @@ return_null:\n  * (3) \"relative/path\" to mean cwd relative directory; or\n  * (4) \"/absolute/path\" to mean absolute directory.\n  *\n- * Unless \"strict\" is given, we try access() for existence of \"%s.git/.git\",\n- * \"%s/.git\", \"%s.git\", \"%s\" in this order.  The first one that exists is\n- * what we try.\n- *\n- * Second, we try chdir() to that.  Upon failure, we return NULL.\n- *\n- * Then, we try if the current directory is a valid git repository.\n- * Upon failure, we return NULL.\n+ * Unless \"strict\" is given, we check \"%s/.git\", \"%s\", \"%s.git/.git\", \"%s.git\"\n+ * in this order. We select the first one that is a valid git repository, and\n+ * chdir() to it. If none match, or we fail to chdir, we return NULL.\n  *\n  * If all goes well, we return the directory we used to chdir() (but\n  * before ~user is expanded), avoiding getcwd() resolving symbolic\n-- \n2.1.4\n"}]}