{"thread":{"id":"4848","subject":"Re : 2 questions on git-send-email usage","startedAt":"2006-07-11T08:46:35Z","lastAt":"2006-07-13T04:40:46Z","messageCount":10,"participants":["moreau francis","Franck Bui-Huu","Junio C Hamano","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"23624","messageId":"20060711084635.81393.qmail@web25809.mail.ukl.yahoo.com","threadId":"4848","inReplyTo":null,"subject":"Re : 2 questions on git-send-email usage","fromName":"moreau francis","fromEmail":"francis_moreau2000@yahoo.fr","sentAt":"2006-07-11T08:46:35Z","receivedAt":"2006-07-11T08:46:35Z","isPatch":false,"sender":{"key":"francis_moreau2000@yahoo.fr","avatar":null},"body":"moreau francis wrote:\n> (please let me CCed when replying)\n> \n> 2006/7/10, Jakub Narebski :\n> > moreau francis wrote:\n> > \n> > > I'm wondering what am I supposed to answer when git-send-email\n> > > is asking me :\n> > >\n> > > Message-ID to be used as In-Reply-To for the first email?\n> > >\n> > > I'm running this command:\n> > >\n> > > $ git-send-email --no-signed-off-by-cc --no-chain-reply-to --to \\\n> > > foo@bar.com --compose /tmp/patch/\n> > >\n> > > to write an introductory message, and all patches are sent as replies to\n> > > this introductory email sent.\n> > \n> > Empty string (i.e. RET) should do if you don't want to attach your series of\n> > patches somewhere in existing thread.\n> \n> ok I'll try\n> \n> --in-reply-to \"\"\n\nok it works. But wouldn't it make more sense to have by default --in-reply-to \"\"\nwhen --compose is set ? That would mean \"by default all patches are sent\nas replies to the email I'm composing\" which is usely what happens, no ?\n\n> \n> > \n> > > I also noticed that git-send-email removes the commit message of each\n> > > patches I sent, I don't think this is the normal behaviour though. What\n> > > am I missing ?\n> > \n> > Are patches formatted using git-format-patch?\n> > \n> \n> yes\n> \n\nI think I have found out a clue. The commit message and Signed-off-by are\nmissing because the header patches are formatted like this:\n\n>From 90df31ca209f85108976d18916f33f352a6ef340 Mon Sep 17 00:00:00 2001\nFrom: Francis \nDate: Thu, 8 Jun 2006 09:51:12 +0200\nSubject: [PATCH 3/4] step #3: interrupt implementation\n(cherry picked from 427778e2e622cdefa2c834edcc19bf102a35bc2d commit)\n(cherry picked from fe4692336801fcbb42bb734bb6b6f9c041d63087 commit)\nSigned-off-by: Francis \n---\n\n2 RETs is missing. One after the Subject line and the other before the \nSigned-off-by line. If I add the first missing RET, all works fine. I guess\nit's missing because of git-cherry-pick command. But I don't understand\nwhy the last RET is missing\n\nCan anybody tell me why ?\n\nThanks\n\nFrancis\n"},{"id":"23628","messageId":"44B37893.5090501@innova-card.com","threadId":"4848","inReplyTo":"20060711084635.81393.qmail@web25809.mail.ukl.yahoo.com","subject":"Re: Re : 2 questions on git-send-email usage","fromName":"Franck Bui-Huu","fromEmail":"vagabon.xyz@gmail.com","sentAt":"2006-07-11T10:08:19Z","receivedAt":"2006-07-11T10:08:19Z","isPatch":false,"sender":{"key":"vagabon.xyz@gmail.com","avatar":null},"body":"moreau francis wrote:\n> 2 RETs is missing. One after the Subject line and the other before the \n> Signed-off-by line. If I add the first missing RET, all works fine. I guess\n> it's missing because of git-cherry-pick command. But I don't understand\n> why the last RET is missing\n> \n> Can anybody tell me why ?\n> \n\nMaybe that patch does what you want.\n\n-- >8 --\n\nSubject: [PATCH] Add a newline before appending \"Signed-off-by:\"\n\nIt looks nicer.\n\nSigned-off-by: Franck Bui-Huu \n---\n log-tree.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex ebb49f2..2551a3f 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -19,7 +19,7 @@ static int append_signoff(char *buf, int\n \tchar *cp = buf;\n \n \t/* Do we have enough space to add it? */\n-\tif (buf_sz - at <= strlen(signed_off_by) + signoff_len + 2)\n+\tif (buf_sz - at <= strlen(signed_off_by) + signoff_len + 3)\n \t\treturn at;\n \n \t/* First see if we already have the sign-off by the signer */\n@@ -34,6 +34,7 @@ static int append_signoff(char *buf, int\n \t\t\treturn at; /* we already have him */\n \t}\n \n+\tbuf[at++] = '\\n';\n \tstrcpy(buf + at, signed_off_by);\n \tat += strlen(signed_off_by);\n \tstrcpy(buf + at, signoff);\n-- \n1.4.1.g35c6-dirty\n"},{"id":"23656","messageId":"7vd5cc3q0o.fsf@assigned-by-dhcp.cox.net","threadId":"4848","inReplyTo":"44B37893.5090501@innova-card.com","subject":"Re: Re : 2 questions on git-send-email usage","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-11T19:22:47Z","receivedAt":"2006-07-11T19:22:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Franck Bui-Huu writes:\n\n> Maybe that patch does what you want.\n>\n> -- >8 --\n>\n> Subject: [PATCH] Add a newline before appending \"Signed-off-by:\"\n>\n> It looks nicer.\n>\n> Signed-off-by: Franck Bui-Huu \n\nHaven't checked the code around the patch yet, but does it work\nwhen the original commit log message ends with a blank line and\nexisting signed-off-by lines by other people? You do not want\nan extra blank lines there.\n"},{"id":"23696","messageId":"44B4A6CA.3020206@innova-card.com","threadId":"4848","inReplyTo":"7vd5cc3q0o.fsf@assigned-by-dhcp.cox.net","subject":"Re: Re : 2 questions on git-send-email usage","fromName":"Franck Bui-Huu","fromEmail":"vagabon.xyz@gmail.com","sentAt":"2006-07-12T07:37:46Z","receivedAt":"2006-07-12T07:37:46Z","isPatch":false,"sender":{"key":"vagabon.xyz@gmail.com","avatar":null},"body":"Junio C Hamano wrote:\n> \n> Haven't checked the code around the patch yet, but does it work\n> when the original commit log message ends with a blank line and\n> existing signed-off-by lines by other people? You do not want\n> an extra blank lines there.\n> \n\nargh, no I just tested the previous case. Here is an update\nwhich fix all cases.\n\n-- >8 --\n\n[PATCH] Add a newline before appending \"Signed-off-by:\"\n\nIt looks nicer.\n\nSigned-off-by: Franck Bui-Huu \n---\n log-tree.c | 6 +++++-\n 1 files changed, 5 insertions(+), 1 deletions(-)\n\ndiff --git a/log-tree.c b/log-tree.c\nindex 9d8d46f..69d5c8a 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -17,9 +17,10 @@ static int append_signoff(char *buf, int\n \tint signoff_len = strlen(signoff);\n \tstatic const char signed_off_by[] = \"Signed-off-by: \";\n \tchar *cp = buf;\n+\tint has_signoff = 0;\n \n \t/* Do we have enough space to add it? */\n-\tif (buf_sz - at <= strlen(signed_off_by) + signoff_len + 2)\n+\tif (buf_sz - at <= strlen(signed_off_by) + signoff_len + 3)\n \t\treturn at;\n \n \t/* First see if we already have the sign-off by the signer */\n@@ -32,8 +33,11 @@ static int append_signoff(char *buf, int\n \t\t !strncmp(cp, signoff, signoff_len) &&\n \t\t isspace(cp[signoff_len]))\n \t\t\treturn at; /* we already have him */\n+\t\thas_signoff = 1;\n \t}\n \n+\tif (!has_signoff)\n+\t\tbuf[at++] = '\\n';\n \tstrcpy(buf + at, signed_off_by);\n \tat += strlen(signed_off_by);\n \tstrcpy(buf + at, signoff);\n-- \n1.4.1.g55b7\n"},{"id":"23712","messageId":"Pine.LNX.4.64.0607120834200.5623@g5.osdl.org","threadId":"4848","inReplyTo":"44B4A6CA.3020206@innova-card.com","subject":"Re: Re : 2 questions on git-send-email usage","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-12T15:43:02Z","receivedAt":"2006-07-12T15:43:02Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 12 Jul 2006, Franck Bui-Huu wrote:\n> \n> [PATCH] Add a newline before appending \"Signed-off-by:\"\n> \n> It looks nicer.\n\nYes. However, I think the sign-off detection is a bit broken (quite \nindependently of your patch).\n\nA number of people end up capitalizing the sign-off differently, so you \nhave lines like \"Signed-Off-By: Xy Zzy \".\n\nAlso, at least for the kernel, we often have alternative formats, like\n\n\tAcked-by: Elliot Xavier Ample \n\nand for that case, adding the extra newline is actually bad.\n\nSo I would suggest a totally different approach: instead of using \n\"strstr(comments, signed_off_by)\", it would probably be much better to \njust look for the last non-empty line, and see if it matches the format\n\n\t\"^[nonspace]*: .*@.*$\"\n\n(yeah, that's not a valid regexp, but you get the idea).\n\nOn a slightly related note, I absolutely _hate_ how cherry-picking adds \n\"(cherry-picked from commit )\" at the end. It's wrong for so many \nreasons, one of them being that it then breaks things like this, but the \nmain one being that will quite often actually end up not even \n_existing_ in the resulting archive (you cherry-picked from your private \nbranch, and even if you keep your branch, you don't necessarily push it \nout).\n\nJunio, can we make the default _not_ to do it, please?\n\n\t\t\tLinus\n"},{"id":"23715","messageId":"7v7j2izthd.fsf@assigned-by-dhcp.cox.net","threadId":"4848","inReplyTo":"Pine.LNX.4.64.0607120834200.5623@g5.osdl.org","subject":"Re: Re : 2 questions on git-send-email usage","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-12T16:19:10Z","receivedAt":"2006-07-12T16:19:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On a slightly related note, I absolutely _hate_ how cherry-picking adds \n> \"(cherry-picked from commit )\" at the end. It's wrong for so many \n> reasons, one of them being that it then breaks things like this, but the \n> main one being that will quite often actually end up not even \n> _existing_ in the resulting archive (you cherry-picked from your private \n> branch, and even if you keep your branch, you don't necessarily push it \n> out).\n>\n> Junio, can we make the default _not_ to do it, please?\n\nI understand that it can be annoying.\n\nI was hoping that however when you do something like this:\n\n\tgit log --filter-backported-commits v2.6.16.9..v2.6.17\n\nan improved log-inspection tool could notice and filter out the\nones that was backported from the mainline to the maintenance\nbranch.\n\nBut I realize that is probably a faulty logic. First of all,\nnot all backports are cherry-picked (the same patch can be\napplied from an e-mail, for example), so if we really want to do\nthe above filtering, we would need to be able to detect the same\npatch anyway without \"cherry-picked from\" information.\n\nTo a certain degree, git-patch-id would help detecting such a\nduplicated patch, but it would not help you detect \"moral\nequivalent\" changes that are textually different. A cherry-pick\nafter a conflict resolution that ends up applying a textually\ndifferent patch would still leave the \"cherry-picked from\"\nmessage in the commit log (or a \"note \" header if we implement\nit to reduce cluttering the log message), which would be the\nonly advantage of recording the information somehow. But that\nis probably not worth it -- it means \"moral equivalent\" changes\nneed to be recorded somehow by hand, which is unnecessarly\ndeveloper burden if it is rare enough to want to filter such\nduplicates when inspecting the log.\n\nDo people find \"cherry-picked from\" information useful? Does\nanybody mind if we change the default not to record it?\n"},{"id":"23716","messageId":"7v1wsqzt8f.fsf@assigned-by-dhcp.cox.net","threadId":"4848","inReplyTo":"Pine.LNX.4.64.0607120834200.5623@g5.osdl.org","subject":"Re: Re : 2 questions on git-send-email usage","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-12T16:24:32Z","receivedAt":"2006-07-12T16:24:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Yes. However, I think the sign-off detection is a bit broken (quite \n> independently of your patch).\n>\n> A number of people end up capitalizing the sign-off differently, so you \n> have lines like \"Signed-Off-By: Xy Zzy \".\n>\n> Also, at least for the kernel, we often have alternative formats, like\n>\n> \tAcked-by: Elliot Xavier Ample \n>\n> and for that case, adding the extra newline is actually bad.\n>\n> So I would suggest a totally different approach: instead of using \n> \"strstr(comments, signed_off_by)\", it would probably be much better to \n> just look for the last non-empty line, and see if it matches the format\n>\n> \t\"^[nonspace]*: .*@.*$\"\n\nDocumentation/SubmittingPatches (the kernel one) does not show\nthe ugly Camel-Case-With-Hyphen spelling, and I've been wondring\nwhy people do that. A hidden agenda by me was to migrate people\naway from that practice, but that is an independent issue ;-).\n\nI like your \"detect lines that looks like a RFC2822 header that\nhas some e-mail address\" approach quite a lot.\n"},{"id":"23718","messageId":"Pine.LNX.4.64.0607120931150.5623@g5.osdl.org","threadId":"4848","inReplyTo":"7v1wsqzt8f.fsf@assigned-by-dhcp.cox.net","subject":"Re: Re : 2 questions on git-send-email usage","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-12T16:37:42Z","receivedAt":"2006-07-12T16:37:42Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 12 Jul 2006, Junio C Hamano wrote:\n\n> Linus Torvalds writes:\n> >\n> > A number of people end up capitalizing the sign-off differently, so you \n> > have lines like \"Signed-Off-By: Xy Zzy \".\n> \n> Documentation/SubmittingPatches (the kernel one) does not show\n> the ugly Camel-Case-With-Hyphen spelling, and I've been wondring\n> why people do that.\n\nYeah, I actually try to edit it to the \"proper\" format when I notice it \n(which is not most of the time, but it's pretty rare to begin with).\n\nMore commonly, people mistype their own email addresses, and \n_occasionally_ just mis-type the whole Signed-off-by: line (we've got a \nfew semi-colons instead of colons in the kernel, for example, and some \nlines that are missing the final '>' in the email etc.\n\nSo being somewhat forgiving might help, but I think another thing that \nmigth help is a flag to \"git-am\" to _not_ apply a patch that lacks a \nprevious sign-off.\n\nI, for example, don't use the --signoff flag, partly because I want to \nmake sure that I sign of only on patches that already have a sign-off from \nthe previous person when it comes as email (or I add the sign-off only \nafter looking at the patch closely). But if there was a \n\"--error-on-no-signoff\" flag, I could use it.\n\n\t\t\tLinus\n"},{"id":"23734","messageId":"7vac7exgvs.fsf@assigned-by-dhcp.cox.net","threadId":"4848","inReplyTo":"Pine.LNX.4.64.0607120931150.5623@g5.osdl.org","subject":"Re: Re : 2 questions on git-send-email usage","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-13T04:34:15Z","receivedAt":"2006-07-13T04:34:15Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> So being somewhat forgiving might help, but I think another thing that \n> migth help is a flag to \"git-am\" to _not_ apply a patch that lacks a \n> previous sign-off.\n\nHow about having this in $GIT_DIR/hooks/applypatch-msg?\n\n\t#!/bin/sh\n\tgrep '^Signed-off-by: ' \"$1\" >/dev/null\n"},{"id":"23737","messageId":"Pine.LNX.4.64.0607122139090.5623@g5.osdl.org","threadId":"4848","inReplyTo":"7vac7exgvs.fsf@assigned-by-dhcp.cox.net","subject":"Re: Re : 2 questions on git-send-email usage","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-07-13T04:40:46Z","receivedAt":"2006-07-13T04:40:46Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 12 Jul 2006, Junio C Hamano wrote:\n> \n> How about having this in $GIT_DIR/hooks/applypatch-msg?\n> \n> \t#!/bin/sh\n> \tgrep '^Signed-off-by: ' \"$1\" >/dev/null\n\nNot good, because then I have no way to select a different behaviour with \na flag. If I decide it was ok to apply (say, it's just a silly typo fix), \nI would want to say so.\n\n\t\tLinus\n"}]}