{"thread":{"id":"34985","subject":"[PATCH 0/2] fetch: trivial fixes","startedAt":"2013-09-21T14:09:21Z","lastAt":"2013-10-15T21:50:45Z","messageCount":15,"participants":["Felipe Contreras","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"227987","messageId":"1379772563-11000-1-git-send-email-felipe.contreras@gmail.com","threadId":"34985","inReplyTo":null,"subject":"[PATCH 0/2] fetch: trivial fixes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-21T14:09:21Z","receivedAt":"2013-09-21T14:09:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Felipe Contreras (2):\n fetch: add missing documentation\n remote: fix trivial memory leak\n\n Documentation/git-fetch.txt | 3 +++\n remote.c | 3 ++-\n 2 files changed, 5 insertions(+), 1 deletion(-)\n\n-- \n1.8.4.2.gac946cf.dirty\n"},{"id":"227988","messageId":"1379772563-11000-2-git-send-email-felipe.contreras@gmail.com","threadId":"34985","inReplyTo":"1379772563-11000-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 1/2] fetch: add missing documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-21T14:09:22Z","receivedAt":"2013-09-21T14:09:22Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no mention of the 'origin' default, or the fact that the\nupstream tracking branch remote is used.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/git-fetch.txt | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt\nindex e08a028..7e75dc4 100644\n--- a/Documentation/git-fetch.txt\n+++ b/Documentation/git-fetch.txt\n@@ -37,6 +37,9 @@ or from several repositories at once if is given and\n there is a remotes. entry in the configuration file.\n (See linkgit:git-config[1]).\n \n+When no remote is specified, by the default the `origin` remote will be used,\n+unless there's an upstream branch configured for the current branch.\n+\n OPTIONS\n -------\n include::fetch-options.txt[]\n-- \n1.8.4.2.gac946cf.dirty\n"},{"id":"227989","messageId":"1379772563-11000-3-git-send-email-felipe.contreras@gmail.com","threadId":"34985","inReplyTo":"1379772563-11000-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH 2/2] remote: fix trivial memory leak","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-21T14:09:23Z","receivedAt":"2013-09-21T14:09:23Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no need to set the default remote name beforehand, only to be\noverridden later on, and causing a memory leak, we can do it after the\nconfiguration has been handled.\n\nSigned-off-by: Felipe Contreras \n---\n remote.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/remote.c b/remote.c\nindex efcba93..654e7f5 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -480,7 +480,6 @@ static void read_config(void)\n \tint flag;\n \tif (default_remote_name) /* did this already */\n \t\treturn;\n-\tdefault_remote_name = xstrdup(\"origin\");\n \tcurrent_branch = NULL;\n \thead_ref = resolve_ref_unsafe(\"HEAD\", sha1, 0, &flag);\n \tif (head_ref && (flag & REF_ISSYMREF) &&\n@@ -489,6 +488,8 @@ static void read_config(void)\n \t\t\tmake_branch(head_ref + strlen(\"refs/heads/\"), 0);\n \t}\n \tgit_config(handle_config, NULL);\n+\tif (!default_remote_name)\n+\t\tdefault_remote_name = xstrdup(\"origin\");\n \talias_all_urls();\n }\n \n-- \n1.8.4.2.gac946cf.dirty\n"},{"id":"228120","messageId":"20130924050343.GF2766@sigill.intra.peff.net","threadId":"34985","inReplyTo":"1379772563-11000-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 1/2] fetch: add missing documentation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-24T05:03:43Z","receivedAt":"2013-09-24T05:03:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 21, 2013 at 09:09:22AM -0500, Felipe Contreras wrote:\n\n> There's no mention of the 'origin' default, or the fact that the\n> upstream tracking branch remote is used.\n\nSounds like a good thing to mention.\n\n> diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt\n> index e08a028..7e75dc4 100644\n> --- a/Documentation/git-fetch.txt\n> +++ b/Documentation/git-fetch.txt\n> @@ -37,6 +37,9 @@ or from several repositories at once if is given and\n> there is a remotes. entry in the configuration file.\n> (See linkgit:git-config[1]).\n> \n> +When no remote is specified, by the default the `origin` remote will be used,\n\ns/the (default the)/\\1/\n\n> +unless there's an upstream branch configured for the current branch.\n\nShould this be \"upstream remote\" rather than \"upstream branch\"? I don't\nthink we should be looking at branch.*.merge at all for git-fetch.\n\n-Peff\n"},{"id":"228121","messageId":"20130924051908.GG2766@sigill.intra.peff.net","threadId":"34985","inReplyTo":"1379772563-11000-3-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH 2/2] remote: fix trivial memory leak","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-24T05:19:08Z","receivedAt":"2013-09-24T05:19:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 21, 2013 at 09:09:23AM -0500, Felipe Contreras wrote:\n\n> diff --git a/remote.c b/remote.c\n> index efcba93..654e7f5 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -480,7 +480,6 @@ static void read_config(void)\n> \tint flag;\n> \tif (default_remote_name) /* did this already */\n> \t\treturn;\n> -\tdefault_remote_name = xstrdup(\"origin\");\n> \tcurrent_branch = NULL;\n> \thead_ref = resolve_ref_unsafe(\"HEAD\", sha1, 0, &flag);\n> \tif (head_ref && (flag & REF_ISSYMREF) &&\n> @@ -489,6 +488,8 @@ static void read_config(void)\n> \t\t\tmake_branch(head_ref + strlen(\"refs/heads/\"), 0);\n> \t}\n> \tgit_config(handle_config, NULL);\n> +\tif (!default_remote_name)\n> +\t\tdefault_remote_name = xstrdup(\"origin\");\n> \talias_all_urls();\n\nI wondered if we might also leak when seeing duplicate config options\n(i.e., leaking the old one when replacing it with the new). But we don't\nactually strdup() the configured remote names, but instead just point\ninto the \"struct branch\", which owns the data.\n\nSo I think an even better fix would be:\n\n-- >8 --\nSubject: remote: do not copy \"origin\" string literal\n\nOur default_remote_name starts at \"origin\", but may be\noverridden by the config file. In the former case, we\nallocate a new string, but in the latter case, we point to\nthe remote name in an existing \"struct branch\".\n\nThis gives the variable inconsistent free() semantics (we\nare sometimes responsible for freeing the string and\nsometimes pointing to somebody else's storage), and causes a\nsmall leak when the allocated string is overridden by\nconfig.\n\nWe can fix both by simply dropping the extra copy and\npointing to the string literal.\n\nNoticed-by: Felipe Contreras \nSigned-off-by: Jeff King \n---\n remote.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/remote.c b/remote.c\nindex e9fedfa..9f1a8aa 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -483,7 +483,7 @@ static void read_config(void)\n \tint flag;\n \tif (default_remote_name) /* did this already */\n \t\treturn;\n-\tdefault_remote_name = xstrdup(\"origin\");\n+\tdefault_remote_name = \"origin\";\n \tcurrent_branch = NULL;\n \thead_ref = resolve_ref_unsafe(\"HEAD\", sha1, 0, &flag);\n \tif (head_ref && (flag & REF_ISSYMREF) &&\n-- \n1.8.4.rc3.19.g9da5bf6\n"},{"id":"228122","messageId":"CAMP44s1FFTS=wOcWBfqg3zt3TO-aM83oXHcGwBcwvt_HnXrn8g@mail.gmail.com","threadId":"34985","inReplyTo":"20130924050343.GF2766@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] fetch: add missing documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-24T05:23:21Z","receivedAt":"2013-09-24T05:23:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Sep 24, 2013 at 12:03 AM, Jeff King wrote:\n> On Sat, Sep 21, 2013 at 09:09:22AM -0500, Felipe Contreras wrote:\n>\n>> There's no mention of the 'origin' default, or the fact that the\n>> upstream tracking branch remote is used.\n>\n> Sounds like a good thing to mention.\n>\n>> diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt\n>> index e08a028..7e75dc4 100644\n>> --- a/Documentation/git-fetch.txt\n>> +++ b/Documentation/git-fetch.txt\n>> @@ -37,6 +37,9 @@ or from several repositories at once if is given and\n>> there is a remotes. entry in the configuration file.\n>> (See linkgit:git-config[1]).\n>>\n>> +When no remote is specified, by the default the `origin` remote will be used,\n>\n> s/the (default the)/\\1/\n\nRight.\n\n>> +unless there's an upstream branch configured for the current branch.\n>\n> Should this be \"upstream remote\" rather than \"upstream branch\"? I don't\n> think we should be looking at branch.*.merge at all for git-fetch.\n\nAs a general user, how do I configure the \"upstream remote\"?\n\n-- \nFelipe Contreras\n"},{"id":"228123","messageId":"20130924053023.GA5875@sigill.intra.peff.net","threadId":"34985","inReplyTo":"CAMP44s1FFTS=wOcWBfqg3zt3TO-aM83oXHcGwBcwvt_HnXrn8g@mail.gmail.com","subject":"Re: [PATCH 1/2] fetch: add missing documentation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-24T05:30:23Z","receivedAt":"2013-09-24T05:30:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 24, 2013 at 12:23:21AM -0500, Felipe Contreras wrote:\n\n> > Should this be \"upstream remote\" rather than \"upstream branch\"? I don't\n> > think we should be looking at branch.*.merge at all for git-fetch.\n> \n> As a general user, how do I configure the \"upstream remote\"?\n\nYeah, it's not a term we use elsewhere, so it's not great. Probably\n\"default remote\" would be better, or even just say \"branch.*.remote for\nthe current branch\" or something.\n\nI dunno. I don't particularly like any of those, but I really dislike\nthe imprecision of \"upstream branch\" in this case.\n\n-Peff\n"},{"id":"228126","messageId":"CAMP44s1ezYMSuQRMp_SY1HqLiuyuf0tjuf3Fn6fXF2drODZ_iw@mail.gmail.com","threadId":"34985","inReplyTo":"20130924053023.GA5875@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] fetch: add missing documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-24T05:36:38Z","receivedAt":"2013-09-24T05:36:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Sep 24, 2013 at 12:30 AM, Jeff King wrote:\n> On Tue, Sep 24, 2013 at 12:23:21AM -0500, Felipe Contreras wrote:\n>\n>> > Should this be \"upstream remote\" rather than \"upstream branch\"? I don't\n>> > think we should be looking at branch.*.merge at all for git-fetch.\n>>\n>> As a general user, how do I configure the \"upstream remote\"?\n>\n> Yeah, it's not a term we use elsewhere, so it's not great. Probably\n> \"default remote\" would be better, or even just say \"branch.*.remote for\n> the current branch\" or something.\n\nYeah, general users don't know what you are talking about when you say that.\n\n> I dunno. I don't particularly like any of those, but I really dislike\n> the imprecision of \"upstream branch\" in this case.\n\nFor most users it's the remote configured by:\n\n% git branch --set-upstream-to foo\n% git checkout -b foo origin/foo\n% git checkout -t -b foo bar\n\nSo when they read \"upstream branch\" they know from where it got configured.\n\n-- \nFelipe Contreras\n"},{"id":"228128","messageId":"20130924054053.GA6192@sigill.intra.peff.net","threadId":"34985","inReplyTo":"CAMP44s1ezYMSuQRMp_SY1HqLiuyuf0tjuf3Fn6fXF2drODZ_iw@mail.gmail.com","subject":"Re: [PATCH 1/2] fetch: add missing documentation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-24T05:40:53Z","receivedAt":"2013-09-24T05:40:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 24, 2013 at 12:36:38AM -0500, Felipe Contreras wrote:\n\n> > Yeah, it's not a term we use elsewhere, so it's not great. Probably\n> > \"default remote\" would be better, or even just say \"branch.*.remote for\n> > the current branch\" or something.\n> \n> Yeah, general users don't know what you are talking about when you say that.\n\nRight, I understand your complaint and agree that those terms are\npotentially confusing.\n\n> > I dunno. I don't particularly like any of those, but I really dislike\n> > the imprecision of \"upstream branch\" in this case.\n> \n> For most users it's the remote configured by:\n> \n> % git branch --set-upstream-to foo\n> % git checkout -b foo origin/foo\n> % git checkout -t -b foo bar\n> \n> So when they read \"upstream branch\" they know from where it got configured.\n\nYes, but it is also wrong, in the sense that the upstream branch is\nunrelated. You are giving breadcrumbs to users who know \"upstream\nbranch\" as a concept and nothing else, but you are misleading users who\nknow that branch.*.remote exists.\n\nI was hoping you might suggest something that can help both users by\nbeing both precise and giving the appropriate breadcrumbs.\n\n-Peff\n"},{"id":"228132","messageId":"CAMP44s3QfkvXjgmhWPXN7qonbEPpvJFyVm82EBOMSjX7ng3OQg@mail.gmail.com","threadId":"34985","inReplyTo":"20130924054053.GA6192@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] fetch: add missing documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-24T05:57:36Z","receivedAt":"2013-09-24T05:57:36Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Sep 24, 2013 at 12:40 AM, Jeff King wrote:\n> On Tue, Sep 24, 2013 at 12:36:38AM -0500, Felipe Contreras wrote:\n>\n>> > Yeah, it's not a term we use elsewhere, so it's not great. Probably\n>> > \"default remote\" would be better, or even just say \"branch.*.remote for\n>> > the current branch\" or something.\n>>\n>> Yeah, general users don't know what you are talking about when you say that.\n>\n> Right, I understand your complaint and agree that those terms are\n> potentially confusing.\n>\n>> > I dunno. I don't particularly like any of those, but I really dislike\n>> > the imprecision of \"upstream branch\" in this case.\n>>\n>> For most users it's the remote configured by:\n>>\n>> % git branch --set-upstream-to foo\n>> % git checkout -b foo origin/foo\n>> % git checkout -t -b foo bar\n>>\n>> So when they read \"upstream branch\" they know from where it got configured.\n>\n> Yes, but it is also wrong, in the sense that the upstream branch is\n> unrelated. You are giving breadcrumbs to users who know \"upstream\n> branch\" as a concept and nothing else, but you are misleading users who\n> know that branch.*.remote exists.\n\nNo, I'm not. The users that know branch.*.remote exists know why it\nexists. The part where it is explained, 'git config --help', is\nperfectly clear: \"When on branch , it tells 'git fetch' and 'git\npush' which remote to fetch from/push to.\". So what does\nbranch..remote does, if not precisely what the documentation\nsays?\n\nThis is not a rhetorical question, I'm actually expecting you to\nanswer, if a user knows that branch..remote exists, how would\nthe above confuse him?\n\n> I was hoping you might suggest something that can help both users by\n> being both precise and giving the appropriate breadcrumbs.\n\nThis is documentation for a Git porcelain command,\nbranch..remote is an implementation detail, and it's irrelevant\nin the documentation at this level.\n\n-- \nFelipe Contreras\n"},{"id":"228133","messageId":"20130924061043.GA6678@sigill.intra.peff.net","threadId":"34985","inReplyTo":"CAMP44s3QfkvXjgmhWPXN7qonbEPpvJFyVm82EBOMSjX7ng3OQg@mail.gmail.com","subject":"Re: [PATCH 1/2] fetch: add missing documentation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-24T06:10:43Z","receivedAt":"2013-09-24T06:10:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 24, 2013 at 12:57:36AM -0500, Felipe Contreras wrote:\n\n> No, I'm not. The users that know branch.*.remote exists know why it\n> exists. The part where it is explained, 'git config --help', is\n> perfectly clear: \"When on branch , it tells 'git fetch' and 'git\n> push' which remote to fetch from/push to.\". So what does\n> branch..remote does, if not precisely what the documentation\n> says?\n> \n> This is not a rhetorical question, I'm actually expecting you to\n> answer, if a user knows that branch..remote exists, how would\n> the above confuse him?\n\nI do not know if by \"above\" you mean the part of git-config.1 you quoted\nabove, or the text you are proposing to put into git-fetch.1.\n\nIf the former, then I do not think it is confusing at all. The existing\ntext explains exactly what is going on.\n\nIf the latter, then my concern is that the term \"upstream branch\"\nimplies implies that \"git fetch\" depends on branch.*.merge, but it does\nnot.\n\n> > I was hoping you might suggest something that can help both users by\n> > being both precise and giving the appropriate breadcrumbs.\n> \n> This is documentation for a Git porcelain command,\n> branch..remote is an implementation detail, and it's irrelevant\n> in the documentation at this level.\n\nI don't think it is the end of the world if we say \"upstream branch\". I\nwas hoping to find a term that could be both friendly and accurate.\n\nAnd failing that, I hoped you might say \"I see what you are saying, but\nI cannot think of a term that is more precise that does not sacrifice\nfriendliness\". But as I seem incapable of even communicating the issue\nto you, I'm giving up. It is not worth wasting more time on it.\n\n-Peff\n"},{"id":"228135","messageId":"CAMP44s23FMeZP=xum1X9bvHuKVo=j8O=8_8DVNgXq9F7Cpsr7A@mail.gmail.com","threadId":"34985","inReplyTo":"20130924061043.GA6678@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] fetch: add missing documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-24T06:31:48Z","receivedAt":"2013-09-24T06:31:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Sep 24, 2013 at 1:10 AM, Jeff King wrote:\n> On Tue, Sep 24, 2013 at 12:57:36AM -0500, Felipe Contreras wrote:\n>\n>> No, I'm not. The users that know branch.*.remote exists know why it\n>> exists. The part where it is explained, 'git config --help', is\n>> perfectly clear: \"When on branch , it tells 'git fetch' and 'git\n>> push' which remote to fetch from/push to.\". So what does\n>> branch..remote does, if not precisely what the documentation\n>> says?\n>>\n>> This is not a rhetorical question, I'm actually expecting you to\n>> answer, if a user knows that branch..remote exists, how would\n>> the above confuse him?\n>\n> I do not know if by \"above\" you mean the part of git-config.1 you quoted\n> above, or the text you are proposing to put into git-fetch.1.\n>\n> If the former, then I do not think it is confusing at all. The existing\n> text explains exactly what is going on.\n>\n> If the latter, then my concern is that the term \"upstream branch\"\n> implies implies that \"git fetch\" depends on branch.*.merge, but it does\n> not.\n\nThat does not answer the question. The user knows branch..remote\nexists, the user reads \"When no remote is specified, by the default\nthe upstream branch configured for the current branch will be used\",\nhow would that confuse him?\n\nSince you are not able to answer, I will answer for you; the user\nwould not be confused, because the user knows the upstream tracking\nbranch is composed by branch..remote+merge, if the user doesn't\nknow that (very highly unlikely), the user can ignore the whole text,\nand the concept of \"upstream branch\" (which any user familiar with\nbranch..remote would know anyway).\n\nSo, to this hypothetical non-existent user, such text would read \"When\nno remote is specified, by the default whaa-whaa-whaa will be used\".\nDoesn't matter what whaa-whaa-whaa is, the user knows\nbranch..remote will be used, because that's what the\ndocumentation for branch..remote says.\n\nBut let's imagine an even more hypothetical user who does actually\nwonder what would take priority, whaa-whaa-whaa, or\nbranch..remote. Well, clearly, if he hasn't specifically set\nwhaa-whaa-whaa, then his beloved branch..remote would be used,\nbut wait, there's more, he wonders if whaa-whaa-whaa is actually set\nsomehow, without him knowing, well then there are three options: 1)\nwhaa-whaa-whaa takes priority, 2) branch..remote takes priority\n3) they are the same. Of these only 1) is a problem.\n\nBut this extremely extremely hypothetical user can just go ahead and\ngoogle \"git upstream branch\", and quickly find it indeed is\nbranch..remote+merge.\n\nSo... Where is the problem?\n\n>> > I was hoping you might suggest something that can help both users by\n>> > being both precise and giving the appropriate breadcrumbs.\n>>\n>> This is documentation for a Git porcelain command,\n>> branch..remote is an implementation detail, and it's irrelevant\n>> in the documentation at this level.\n>\n> I don't think it is the end of the world if we say \"upstream branch\". I\n> was hoping to find a term that could be both friendly and accurate.\n>\n> And failing that, I hoped you might say \"I see what you are saying, but\n> I cannot think of a term that is more precise that does not sacrifice\n> friendliness\". But as I seem incapable of even communicating the issue\n> to you, I'm giving up. It is not worth wasting more time on it.\n\nAnd I was hoping you wouldn't use rhetorical warfare and label things\nas \"inaccurate\", \"imprecise\", \"breadcrumbs\".\n\nAt this porcelain level, branch..remote does not exist, so\n\"upstream branch\" is accurate. Period.\n\n-- \nFelipe Contreras\n"},{"id":"228138","messageId":"20130924065454.GB7257@sigill.intra.peff.net","threadId":"34985","inReplyTo":"CAMP44s23FMeZP=xum1X9bvHuKVo=j8O=8_8DVNgXq9F7Cpsr7A@mail.gmail.com","subject":"Re: [PATCH 1/2] fetch: add missing documentation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-09-24T06:54:54Z","receivedAt":"2013-09-24T06:54:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 24, 2013 at 01:31:48AM -0500, Felipe Contreras wrote:\n\n> > I don't think it is the end of the world if we say \"upstream branch\". I\n> > was hoping to find a term that could be both friendly and accurate.\n> >\n> > And failing that, I hoped you might say \"I see what you are saying, but\n> > I cannot think of a term that is more precise that does not sacrifice\n> > friendliness\". But as I seem incapable of even communicating the issue\n> > to you, I'm giving up. It is not worth wasting more time on it.\n> \n> And I was hoping you wouldn't use rhetorical warfare and label things\n> as \"inaccurate\", \"imprecise\", \"breadcrumbs\".\n\nFWIW, the term \"breadcrumbs\" was meant as a _good_ thing. I meant that\nyou are using a term that will link the user to other concepts that use\nthe same term (like \"branch --set-upstream-to\"), and that is something\nwe would like to keep.\n\nAs for the others, I find your accusation of rhetorical warfare\nridiculous. Insulting your patch with non-constructive insults would be\nrhetorical. Saying \"I think it has a flaw, here are my reasons, and I\nhope we can come up with a solution that does not have that flaw without\nweakening the other properties\" is collaboration. Or an attempt at it\nanyway.\n\nI do not know why you and I have so much trouble communicating on even\nbasic things. I am willing to accept that it is entirely my fault. But\nthat does not change the fact that I often find it a waste of time, and\nI plan to do less of it by ending my involvement in threads that seem to\nbe unproductive.\n\n> At this porcelain level, branch..remote does not exist, so\n> \"upstream branch\" is accurate. Period.\n\nI do not agree with your first sentence at all. And your second one is\npurely rhetorical.\n\nI can elaborate if you really care, but I have a feeling you do not.\n\n-Peff\n"},{"id":"228142","messageId":"CAMP44s3potrCHyj8QYVfqBjsOqxh0xnuSfrK92enVWKND11CeA@mail.gmail.com","threadId":"34985","inReplyTo":"20130924065454.GB7257@sigill.intra.peff.net","subject":"Re: [PATCH 1/2] fetch: add missing documentation","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-09-24T07:41:37Z","receivedAt":"2013-09-24T07:41:37Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Sep 24, 2013 at 1:54 AM, Jeff King wrote:\n> On Tue, Sep 24, 2013 at 01:31:48AM -0500, Felipe Contreras wrote:\n>\n>> > I don't think it is the end of the world if we say \"upstream branch\". I\n>> > was hoping to find a term that could be both friendly and accurate.\n>> >\n>> > And failing that, I hoped you might say \"I see what you are saying, but\n>> > I cannot think of a term that is more precise that does not sacrifice\n>> > friendliness\". But as I seem incapable of even communicating the issue\n>> > to you, I'm giving up. It is not worth wasting more time on it.\n>>\n>> And I was hoping you wouldn't use rhetorical warfare and label things\n>> as \"inaccurate\", \"imprecise\", \"breadcrumbs\".\n>\n> FWIW, the term \"breadcrumbs\" was meant as a _good_ thing. I meant that\n> you are using a term that will link the user to other concepts that use\n> the same term (like \"branch --set-upstream-to\"), and that is something\n> we would like to keep.\n\n\"breadcrumbs\" implies it's not the full thing the user expects. While\nin some cases it might indeed lead the user to further concepts, it's\nalso likely the user already knows what \"upstream branch\" means, in\nwhich case it's not a \"breadcrumb\", it's the full meal.\n\nIt's as misleading as saying Atlas Shrugged is a light read, sure, it\nmight be to some people, but it's certainly not the case for\neverybody, so saying so is not an actual fact.\n\n> As for the others, I find your accusation of rhetorical warfare\n> ridiculous. Insulting your patch with non-constructive insults would be\n> rhetorical.\n\nThat actually wouldn't be rhetorical. If you say \"I think this is\ncrap\", that would simply be stating an opinion, and everyone would\nunderstand that, saying \"This is imprecise\" is rhetorical, because you\nconvey an opinion as a fact, and it's actually much more damaging than\nthe former, because people wouldn't read it as an opinion, which is\nprecisely what rhetoric is about: persuation, rightly or not.\n\nEven worst, you might believe it yourself.\n\n> Saying \"I think it has a flaw, here are my reasons, and I\n> hope we can come up with a solution that does not have that flaw without\n> weakening the other properties\" is collaboration.\n\nBut you didn't say you *thought* there was a flaw, you said there was\na flaw, specifically, you said it was imprecise.\n\nUSA didn't say \"Well, uhm, we think Iraq has weapons of mass\ndestruction\", they said \"Iraq has weapons of mass destruction, and\nit's not a question of _if_ they are going to use them, but _when_\".\nThese two statements cause a very different reaction from the\nlistener, thanks to the art of rhetoric.\n\n> I do not know why you and I have so much trouble communicating on even\n> basic things. I am willing to accept that it is entirely my fault. But\n> that does not change the fact that I often find it a waste of time, and\n> I plan to do less of it by ending my involvement in threads that seem to\n> be unproductive.\n\nI think it's very clear, you think your opinions are worth a hundred\ntimes more than my opinions. Actually, it's even worst, you think your\nopinions are facts. What kind of person challenges facts? only stupid\nlittle brats on the Internet, or morons, right? But when two equals\nhave a difference of opinion, it's just that.\n\nIf you accepted that my opinion weighs as much has yours, and that\nwhen we differ it might be you the one that is wrong, as much as it\nmight be me, things would go very differently.\n\n>> At this porcelain level, branch..remote does not exist, so\n>> \"upstream branch\" is accurate. Period.\n>\n> I do not agree with your first sentence at all.\n\n> And your second one is purely rhetorical.\n\nExactly. You used rhetoric to convey your opinions as facts, so I used\nit to the same effect.\n\n> I can elaborate if you really care, but I have a feeling you do not.\n\nIf you are going to present your opinion as facts, then no, I do not care.\n\nIf you are going to explain why you *personally* *think* *in your\nopinion* that branch..remote is a porcelain level concept, then\nsure, I would be interested in hearing why. But first I would like for\nyou do an exercise and find out where exactly does this high level\nconcept lands in the ProGit book.\n\n-- \nFelipe Contreras\n"},{"id":"229025","messageId":"xmqqob6qcife.fsf@gitster.dls.corp.google.com","threadId":"34985","inReplyTo":"20130924051908.GG2766@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] remote: fix trivial memory leak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-15T21:50:45Z","receivedAt":"2013-10-15T21:50:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I wondered if we might also leak when seeing duplicate config options\n> (i.e., leaking the old one when replacing it with the new). But we don't\n> actually strdup() the configured remote names, but instead just point\n> into the \"struct branch\", which owns the data.\n\nIn addition, we do not copy this string to remote->name in make_remote(),\nso even if we start allowing destruction of existing remote[], the\nresulting code will stay safe.\n\n> So I think an even better fix would be:\n>\n> -- >8 --\n> Subject: remote: do not copy \"origin\" string literal\n>\n> Our default_remote_name starts at \"origin\", but may be\n> overridden by the config file. In the former case, we\n> allocate a new string, but in the latter case, we point to\n> the remote name in an existing \"struct branch\".\n>\n> This gives the variable inconsistent free() semantics (we\n> are sometimes responsible for freeing the string and\n> sometimes pointing to somebody else's storage), and causes a\n> small leak when the allocated string is overridden by\n> config.\n>\n> We can fix both by simply dropping the extra copy and\n> pointing to the string literal.\n>\n> Noticed-by: Felipe Contreras \n> Signed-off-by: Jeff King \n\nSounds sensible. Thanks.\n\n> ---\n> remote.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/remote.c b/remote.c\n> index e9fedfa..9f1a8aa 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -483,7 +483,7 @@ static void read_config(void)\n> \tint flag;\n> \tif (default_remote_name) /* did this already */\n> \t\treturn;\n> -\tdefault_remote_name = xstrdup(\"origin\");\n> +\tdefault_remote_name = \"origin\";\n> \tcurrent_branch = NULL;\n> \thead_ref = resolve_ref_unsafe(\"HEAD\", sha1, 0, &flag);\n> \tif (head_ref && (flag & REF_ISSYMREF) &&\n"}]}