Re: [PATCH] Uses git-credential for git-imap-send
On Sat, Apr 26, 2014 at 11:30:11AM -0700, Dan Albert wrote: I had resent a less broken patch after I noticed the tabs, but it seems to have gotten lost. Better formatted patch at the bottom of this message. Your emails (including this one) are multipart/alternatives with an html part, which will cause the mailing list software to reject them. This email also still seems whitespace-damaged to me (the leading tabs are collapsed to a single space). It looks like you're using gmail to send; you might try using git send-email (the example at the end of git help send-email can walk you through it). About imap vs. imaps: I actually had your exact line in before, but decided that as long as its for the same host the user probably wants to use the same credentials for both imap and imaps (if they for some reason had both configured). Hard coding imap allows them to use either protocol with only one keychain entry. The use case is a stretch, but it doesn't do any harm to implement it this way. My concerns with conflating the two are: 1. The system helper might care about the distinction and prefer imaps (e.g., it might already have the credential stored for your regular mail client, which uses imaps). But osxkeychain is the only helper that makes the distinction, and I don't really know how OS X's keychain code handles the distinction. 2. With http and https, we are careful to make the distinction, because we would not want to accidentally share a credential over http that was stored via https. But it's pretty easy to use an http URL rather than an https one. It's probably pretty rare to accidentally turn off imap SSL. So I'd be OK with leaving it as imap for now, and waiting for somebody to actually come up with a real case where the distinction matters. --- Uses git-credential for git-imap-send git-imap-send was directly prompting for a password rather than using git-credential. git-send-email, on the other hand, supports git-credential. This is a necessary improvement for users that use two factor authentication, as they should not be expected to remember all of their app specific passwords. Signed-off-by: Dan Albert danalb...@google.com --- imap-send.c | 29 +++-- 1 file changed, 15 insertions(+), 14 deletions(-) A side note on formatting your commit message: The maintainer picks up patches from the list with git am, which will take everything up to the first --- as the commit message, and discard everything after up to the start of the diff. So in this case it would take your cover-letter material as the commit message, and drop your actual commit message. The usual formats are either to put the cover letter material after the ---, like: $COMMIT_MESSAGE Signed-off-by: You --- $COVER_LETTER $DIFFSTAT $DIFF or to use a scissors-line -- 8 -- instead of three-dash: $COVER_LETTER -- 8 -- $COMMIT_MESSAGE --- $DIFFSTAT $DIFF -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert Stop starting pager recursively
On Sun, Apr 27, 2014 at 09:12:39AM +0700, Duy Nguyen wrote: The intent of the commit was that is a stupid thing to do, but it's not so obvious from the first glance, do not freeze my system for my mistake. But if it stops an actual use case, then I agree it should be reverted. Thanks for the explanation. I think we should just go with Jörn's patch as-is, then. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re:Used Digital Duplicator
Hello! We are a chinese company, which specialized in manufacturing office supply: 1. Used duplicator machines 2. Spare parts for duplicators 3. Colorblack toner cartridges 4.barcode printer 5.printhead If you would like to know, Pls contact us in following way or reply this email: printh...@foxmail.com Best Regards Used copierDigital duplicator, Spare partsToner cartridge, Barcode productsOffice equipment supplies Guangzhou,China Skype:luckylily211
Re: Recording the current branch on each commit?
- Ursprungligt meddelande - Från: Jeremy Morton ad...@game-point.net Till: git@vger.kernel.org Skickat: söndag, 27 apr 2014 1:56:47 Ämne: Recording the current branch on each commit? Currently, git records a checksum, author, commit date/time, and commit message with every commit (as get be seen from 'git log'). I think it would be useful if, along with the Author and Date, git recorded the name of the current branch on each commit. The branch name can provide useful contextual information. For instance, let's say I'm developing a suite of games. If the commit message says Added basic options dialog, it might be useful to see that the branch name is pacman-minigame indicating that the commit pertains to the options dialog in the Pacman minigame. Basically, I'm saying that well-named branches can and do carry useful contextual information that oughtn't to be thrown away. Currently, when you delete that branch, you lose the branch name altogether. So what do you think? Would it be good to have a patch to add this feature? Branch names are usually poorly named, so often you don't lose much. One way some people to is to always merge with --no-ff, that way you see the branch name in the merge commit. A very popular way of tracking context is to add some id, such as a bugzilla issue number, to the header or footer of the commit message. Often a branch contains many issues, but the branch itself isn't very interesting. Tools like gitblit, gitweb, gerrit etc can easily be configured to link to the issue using a regular expression. -- robin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Recording the current branch on each commit?
On Sun, Apr 27, 2014 at 1:56 AM, Jeremy Morton ad...@game-point.net wrote: Currently, git records a checksum, author, commit date/time, and commit message with every commit (as get be seen from 'git log'). I think it would be useful if, along with the Author and Date, git recorded the name of the current branch on each commit. This has been discussed multiple times in the past. One example here: http://thread.gmane.org/gmane.comp.version-control.git/229422 I believe the current conclusion (if any) is that encoding such information as a _structural_ part of the commit object is not useful. See the old thread(s) for the actual pro/con arguments. That said, you are of course free to add this information to your own commit messages, by appending something like Made-on-branch: frotz. In a company setting, you can even create a commit message template or (prepare-)commit-msg hook to have this line created automatically for you and your co-workers. You could even append such information retroactively to existing commits with git notes. There is also the current interpret-trailers effort by Christian Couder [1] that should be useful in creating and managing such lines. [1]: http://thread.gmane.org/gmane.comp.version-control.git/245874 The branch name can provide useful contextual information. For instance, let's say I'm developing a suite of games. If the commit message says Added basic options dialog, it might be useful to see that the branch name is pacman-minigame indicating that the commit pertains to the options dialog in the Pacman minigame. In that partcular case, ISTM that the context (pacman-minigame) would actually be better preserved elsewhere. E.g. the commits touch files in a particular minigames/pacman subdir, or you prefix the context in the commit message (pacman-minigame: Added basic options dialog). Also, such a topic branch is often tied to a specific issue in some bug/issue tracker, and it would in any case be natural to mention the bug/issue ID in the commit message, at which point the tracker can provide more context and discussion. Basically, I'm saying that well-named branches can and do carry useful contextual information that oughtn't to be thrown away. Currently, when you delete that branch, you lose the branch name altogether. Some would argue that branches are not always well-named... But anyway, if the branch ends up getting merged to the mainline, the merge commit defaults to a message like Merge branch 'pacman-minigame'. So what do you think? Would it be good to have a patch to add this feature? One is free to try, of course, but I wouldn't get my hopes up for a patch that changes the fundamental format of the commit object to include something that many users/workflows would consider to be pure cruft. If you still believe that this is useful enough to warrant a change to the commit object format, it is probably better to start off putting the information in the commit message (as described above), and provide some tools that demonstrate the added value of this information. If that is successful and gains momentum, the git community can certainly reconsider whether it makes sense to fold it into a more formalized part of the commit object. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] blame: large-scale performance rewrite
Shawn Pearce spea...@spearce.org writes: On Sat, Apr 26, 2014 at 10:30 AM, David Kastrup d...@gnu.org wrote: David Kastrup d...@gnu.org writes: Here's some example: dak@lola:/usr/local/tmp/wortliste$ time git blame -n -s wortliste /tmp/wl1 real15m47.118s user14m39.928s sys 1m1.872s Hah, this is quite the torture test. git before your patch is taking 22m11s on my laptop to compute this. (This was with default options, I noticed you passed -s to suppress the author formatting.) dak@lola:/usr/local/tmp/wortliste$ time ../git/git blame -n -s wortliste /tmp/wl2 real3m40.947s user2m40.296s sys 0m59.440s Meanwhile JGit computed in 4m30s on the same hardware. So I guess we are fine. At least the stuff I fixed with regard to performance would seem to be done right in JGit to start with. Its still not as fast as I want it to be. :-) Most of the diff data/CRC is computed over and over because of the blackbox use of xdiff. And then the delta-chain storage is packing stuff based on CRCs as well (not sure whether it keeps them around for unpacking). So there is a lot that could likely be improved while keeping the same basic algorithms, by cracking open the black boxes of the xdiff engine and the delta-chain coding. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Updated C# userdiff patterns.
On 26 Apr 2014, at 21:50, Johannes Sixt j...@kdbg.org wrote: Am 26.04.2014 20:33, schrieb Marius Ungureanu: ... add as many unit tests I can. Great! Keep in mind that quantity is secondary. Quality counts. Obviously. By ‘as many’, I meant to cover all the cases here. I’ll start a new thread with the new patch as soon as I’m done with it. If possible, do not start a new thread, but post your new patch as a reply in this thread. Okay, understood. Thanks, -- Hannes Thanks, Marius -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Updated C# userdiff patterns.
Heya, so I’ll add the patches in the next 2 emails. I’ve changed a bit the main body of the methods/constructors regex. Basically, I’ve made the first item after the modifiers optional. That’s the return type and it’s not used in any case by operator overloads or constructors/destructors. I also added lots of symbols to the name of the function. Those are the symbols of the operators that the language allows you to overload. Thanks in advance, Marius.-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] update C# userdiff patterns
Remove all keywords since they don't cause conflicts. Add method modifiers: abstract, async, explicit, extern, implicit, partial, operator. Add properties modifiers: abstract, readonly. Add type modifiers: abstract. Parse C# operator functions. Fix constructor parsing. Signed-off-by: Marius Ungureanu marius.ungure...@xamarin.com --- userdiff.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/userdiff.c b/userdiff.c index fad52d6..e7ba5e3 100644 --- a/userdiff.c +++ b/userdiff.c @@ -133,14 +133,12 @@ PATTERNS(cpp, |[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lLuU]* |[-+*/%^|=!]=|--|\\+\\+|=?|=?||\\|\\||::|-\\*?|\\.\\*), PATTERNS(csharp, -/* Keywords */ -!^[ \t]*(do|while|for|if|else|instanceof|new|return|switch|case|throw|catch|using)\n /* Methods and constructors */ -^[ \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n +^[ \t]*(((abstract|async|explicit|extern|implicit|internal|new|operator|override|partial|private|protected|public|sealed|static|unsafe|virtual)[ \t]+)*[][@._[:alnum:]]*[ \t]*[-@.~=+!*%|^_[:alnum:]]+[ \t]*\\(.*\\))[ \t]*$\n /* Properties */ -^[ \t]*(((static|public|internal|private|protected|new|virtual|sealed|override|unsafe)[ \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n +^[ \t]*(((abstract|internal|new|override|private|protected|public|readonly|sealed|static|unsafe|virtual)[ \t]+)*[][@.~_[:alnum:]]+[ \t]+[@._[:alnum:]]+)[ \t]*$\n /* Type definitions */ -^[ \t]*(((static|public|internal|private|protected|new|unsafe|sealed|abstract|partial)[ \t]+)*(class|enum|interface|struct)[ \t]+.*)$\n +^[ \t]*(((abstract|internal|new|override|partial|private|protected|public|sealed|static|unsafe)[ \t]+)*(class|enum|interface|struct)[ \t]+.*)$\n /* Namespace */ ^[ \t]*(namespace[ \t]+.*)$, /* -- */ -- 1.8.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] add csharp userdiff tests
--- t/t4018/csharp-constructor | 4 t/t4018/csharp-destructor | 4 t/t4018/csharp-function| 4 t/t4018/csharp-member | 4 t/t4018/csharp-namespace | 4 t/t4018/csharp-operator| 4 t/t4018/csharp-property| 4 t/t4018/csharp-skip-call | 5 + 8 files changed, 33 insertions(+) create mode 100644 t/t4018/csharp-constructor create mode 100644 t/t4018/csharp-destructor create mode 100644 t/t4018/csharp-function create mode 100644 t/t4018/csharp-member create mode 100644 t/t4018/csharp-namespace create mode 100644 t/t4018/csharp-operator create mode 100644 t/t4018/csharp-property create mode 100644 t/t4018/csharp-skip-call diff --git a/t/t4018/csharp-constructor b/t/t4018/csharp-constructor new file mode 100644 index 000..a39cffb --- /dev/null +++ b/t/t4018/csharp-constructor @@ -0,0 +1,4 @@ +MyClass(RIGHT) +{ + ChangeMe; +} diff --git a/t/t4018/csharp-destructor b/t/t4018/csharp-destructor new file mode 100644 index 000..55106d9 --- /dev/null +++ b/t/t4018/csharp-destructor @@ -0,0 +1,4 @@ +~MyClass(RIGHT) +{ + ChangeMe; +} diff --git a/t/t4018/csharp-function b/t/t4018/csharp-function new file mode 100644 index 000..a5d59a3 --- /dev/null +++ b/t/t4018/csharp-function @@ -0,0 +1,4 @@ +virtual DoStuff(RIGHT) +{ + ChangeMe; +} diff --git a/t/t4018/csharp-member b/t/t4018/csharp-member new file mode 100644 index 000..4939d53 --- /dev/null +++ b/t/t4018/csharp-member @@ -0,0 +1,4 @@ +unsafe class RIGHT +{ + int ChangeMe; +} diff --git a/t/t4018/csharp-namespace b/t/t4018/csharp-namespace new file mode 100644 index 000..6749980 --- /dev/null +++ b/t/t4018/csharp-namespace @@ -0,0 +1,4 @@ +namespace RIGHT +{ + ChangeMe; +} diff --git a/t/t4018/csharp-operator b/t/t4018/csharp-operator new file mode 100644 index 000..5b00263 --- /dev/null +++ b/t/t4018/csharp-operator @@ -0,0 +1,4 @@ +A operator+(RIGHT) +{ + ChangeMe; +} diff --git a/t/t4018/csharp-property b/t/t4018/csharp-property new file mode 100644 index 000..82d67fc --- /dev/null +++ b/t/t4018/csharp-property @@ -0,0 +1,4 @@ +public virtual int RIGHT +{ + get { ChangeMe; } +} diff --git a/t/t4018/csharp-skip-call b/t/t4018/csharp-skip-call new file mode 100644 index 000..e89d083 --- /dev/null +++ b/t/t4018/csharp-skip-call @@ -0,0 +1,5 @@ +virtual void RIGHT() +{ + call(); + ChangeMe; +} -- 1.8.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] add csharp userdiff tests
Ugh, there’s an empty line at the beginning of the 2nd patch that shouldn’t be there. Thanks, Marius-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert Stop starting pager recursively
- Original Message - On Sun, Apr 27, 2014 at 09:12:39AM +0700, Duy Nguyen wrote: The intent of the commit was that is a stupid thing to do, but it's not so obvious from the first glance, do not freeze my system for my mistake. But if it stops an actual use case, then I agree it should be reverted. Thanks for the explanation. I think we should just go with Jörn's patch as-is, then. Agreed. At best, the commit message could be improved to explain the situation, but the patch itself is OK. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] add csharp userdiff tests
Am 27.04.2014 15:47, schrieb Marius Ungureanu: --- Thanks. Please signed off your patch. When you re-send, please place [PATCH v3 n/m] in the subject (and drop the Re:) and note what you changed compared to the previous (or all earlier) rounds. The place for this note is here, after the --- marker. Have a look at Documentation/SubmittingPatches. t/t4018/csharp-constructor | 4 t/t4018/csharp-destructor | 4 t/t4018/csharp-function| 4 t/t4018/csharp-member | 4 t/t4018/csharp-namespace | 4 t/t4018/csharp-operator| 4 t/t4018/csharp-property| 4 t/t4018/csharp-skip-call | 5 + 8 files changed, 33 insertions(+) create mode 100644 t/t4018/csharp-constructor create mode 100644 t/t4018/csharp-destructor create mode 100644 t/t4018/csharp-function create mode 100644 t/t4018/csharp-member create mode 100644 t/t4018/csharp-namespace create mode 100644 t/t4018/csharp-operator create mode 100644 t/t4018/csharp-property create mode 100644 t/t4018/csharp-skip-call Unfortunately, I think you have reduced the test cases too far. One of the main properties of C# code is that usually member and property definitions are indented and there is a class definition around them: class Foo { Foo(X) {} virtual void DoStuff() {} public int X; }; In your examples, you omitted the surrounding class definition and did not indent the member definitions. By doing so, the test cases do not demonstrate that the csharp userdiff patterns are significantly different from the default userdiff pattern: in the examples you present, the default pattern would have picked the same hunk headers as the csharp patterns! For a reviewer who is not (or only marginally) familiar with C# (like myself), it would have been very instructive to present patches with test cases that demonstrate weaknesses of the old patterns before patches that fix them. For example, you say that you fix the constructor pattern. But I am unable to judge what is wrong and how you fix it. The commit message is the right place to add text that helps reviewers. You can mark a userdiff test case that demonstrates a breakage by including the work broken somewhere in the file. See http://www.repo.or.cz/w/alt-git.git/commitdiff/9cc444f0570b196f1c51664ce2de1d8e1dee6046 diff --git a/t/t4018/csharp-constructor b/t/t4018/csharp-constructor new file mode 100644 index 000..a39cffb --- /dev/null +++ b/t/t4018/csharp-constructor @@ -0,0 +1,4 @@ +MyClass(RIGHT) +{ + ChangeMe; +} diff --git a/t/t4018/csharp-destructor b/t/t4018/csharp-destructor new file mode 100644 index 000..55106d9 --- /dev/null +++ b/t/t4018/csharp-destructor @@ -0,0 +1,4 @@ +~MyClass(RIGHT) +{ + ChangeMe; +} diff --git a/t/t4018/csharp-function b/t/t4018/csharp-function new file mode 100644 index 000..a5d59a3 --- /dev/null +++ b/t/t4018/csharp-function @@ -0,0 +1,4 @@ +virtual DoStuff(RIGHT) +{ + ChangeMe; +} diff --git a/t/t4018/csharp-member b/t/t4018/csharp-member new file mode 100644 index 000..4939d53 --- /dev/null +++ b/t/t4018/csharp-member @@ -0,0 +1,4 @@ +unsafe class RIGHT +{ + int ChangeMe; +} diff --git a/t/t4018/csharp-namespace b/t/t4018/csharp-namespace new file mode 100644 index 000..6749980 --- /dev/null +++ b/t/t4018/csharp-namespace @@ -0,0 +1,4 @@ +namespace RIGHT +{ + ChangeMe; +} diff --git a/t/t4018/csharp-operator b/t/t4018/csharp-operator new file mode 100644 index 000..5b00263 --- /dev/null +++ b/t/t4018/csharp-operator csharp-user-defined-operator would more precisely describe the case. I wouldn't mind seening other file names being a bit more elaborate, but I find this one particularly ambiguous. @@ -0,0 +1,4 @@ +A operator+(RIGHT) +{ + ChangeMe; +} diff --git a/t/t4018/csharp-property b/t/t4018/csharp-property new file mode 100644 index 000..82d67fc --- /dev/null +++ b/t/t4018/csharp-property @@ -0,0 +1,4 @@ +public virtual int RIGHT +{ + get { ChangeMe; } +} diff --git a/t/t4018/csharp-skip-call b/t/t4018/csharp-skip-call new file mode 100644 index 000..e89d083 --- /dev/null +++ b/t/t4018/csharp-skip-call @@ -0,0 +1,5 @@ +virtual void RIGHT() +{ + call(); + ChangeMe; +} In this last test case, you want to demonstrate that the line call() is not picked as hunk header. As written, the line would never be picked as hunk header, even if it would match some pattern, because it is too close to ChangeMe. You must have at least one more line between call() and ChangeMe. BTW, what is the expected hunk header in a diff like the following where class Foo is at line 1 in the file (just above the hunk)? @@ -2,3 +2,3 @@ { - // old comment + // new comment public whatever() That is, when the class definition is undecorated (no unsafe etc.) -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of
Re: [PATCH 2/2] add csharp userdiff tests
On 27 Apr 2014, at 19:19, Johannes Sixt j...@kdbg.org wrote: Am 27.04.2014 15:47, schrieb Marius Ungureanu: --- Thanks. Please signed off your patch. Ah, yes, forgot to do that. When you re-send, please place [PATCH v3 n/m] in the subject (and drop the Re:) and note what you changed compared to the previous (or all earlier) rounds. The place for this note is here, after the --- marker. Have a look at Documentation/SubmittingPatches. Will do so. t/t4018/csharp-constructor | 4 t/t4018/csharp-destructor | 4 t/t4018/csharp-function| 4 t/t4018/csharp-member | 4 t/t4018/csharp-namespace | 4 t/t4018/csharp-operator| 4 t/t4018/csharp-property| 4 t/t4018/csharp-skip-call | 5 + 8 files changed, 33 insertions(+) create mode 100644 t/t4018/csharp-constructor create mode 100644 t/t4018/csharp-destructor create mode 100644 t/t4018/csharp-function create mode 100644 t/t4018/csharp-member create mode 100644 t/t4018/csharp-namespace create mode 100644 t/t4018/csharp-operator create mode 100644 t/t4018/csharp-property create mode 100644 t/t4018/csharp-skip-call Unfortunately, I think you have reduced the test cases too far. One of the main properties of C# code is that usually member and property definitions are indented and there is a class definition around them: class Foo { Foo(X) {} virtual void DoStuff() {} public int X; }; In your examples, you omitted the surrounding class definition and did not indent the member definitions. By doing so, the test cases do not demonstrate that the csharp userdiff patterns are significantly different from the default userdiff pattern: in the examples you present, the default pattern would have picked the same hunk headers as the csharp patterns! Ah, I think I over judged minimal sample here. I’ll do so. Is it okay though if I add a few tests to show what is broken? I think this can’t be solved at a regex level. For a reviewer who is not (or only marginally) familiar with C# (like myself), it would have been very instructive to present patches with test cases that demonstrate weaknesses of the old patterns before patches that fix them. For example, you say that you fix the constructor pattern. But I am unable to judge what is wrong and how you fix it. The commit message is the right place to add text that helps reviewers. Well, the previous pattern didn’t match constructors as they should be at a logical level. That means, it considered the constructor name as being the return type. It’s just a logical change that helped with writing operator function parsing. You can mark a userdiff test case that demonstrates a breakage by including the work broken somewhere in the file. See http://www.repo.or.cz/w/alt-git.git/commitdiff/9cc444f0570b196f1c51664ce2de1d8e1dee6046 diff --git a/t/t4018/csharp-constructor b/t/t4018/csharp-constructor new file mode 100644 index 000..a39cffb --- /dev/null +++ b/t/t4018/csharp-constructor @@ -0,0 +1,4 @@ +MyClass(RIGHT) +{ +ChangeMe; +} diff --git a/t/t4018/csharp-destructor b/t/t4018/csharp-destructor new file mode 100644 index 000..55106d9 --- /dev/null +++ b/t/t4018/csharp-destructor @@ -0,0 +1,4 @@ +~MyClass(RIGHT) +{ +ChangeMe; +} diff --git a/t/t4018/csharp-function b/t/t4018/csharp-function new file mode 100644 index 000..a5d59a3 --- /dev/null +++ b/t/t4018/csharp-function @@ -0,0 +1,4 @@ +virtual DoStuff(RIGHT) +{ +ChangeMe; +} diff --git a/t/t4018/csharp-member b/t/t4018/csharp-member new file mode 100644 index 000..4939d53 --- /dev/null +++ b/t/t4018/csharp-member @@ -0,0 +1,4 @@ +unsafe class RIGHT +{ +int ChangeMe; +} diff --git a/t/t4018/csharp-namespace b/t/t4018/csharp-namespace new file mode 100644 index 000..6749980 --- /dev/null +++ b/t/t4018/csharp-namespace @@ -0,0 +1,4 @@ +namespace RIGHT +{ +ChangeMe; +} diff --git a/t/t4018/csharp-operator b/t/t4018/csharp-operator new file mode 100644 index 000..5b00263 --- /dev/null +++ b/t/t4018/csharp-operator csharp-user-defined-operator would more precisely describe the case. I wouldn't mind seening other file names being a bit more elaborate, but I find this one particularly ambiguous. Okay, I’ll name them better. Also improve them. @@ -0,0 +1,4 @@ +A operator+(RIGHT) +{ +ChangeMe; +} diff --git a/t/t4018/csharp-property b/t/t4018/csharp-property new file mode 100644 index 000..82d67fc --- /dev/null +++ b/t/t4018/csharp-property @@ -0,0 +1,4 @@ +public virtual int RIGHT +{ +get { ChangeMe; } +} diff --git a/t/t4018/csharp-skip-call b/t/t4018/csharp-skip-call new file mode 100644 index 000..e89d083 --- /dev/null +++ b/t/t4018/csharp-skip-call @@ -0,0 +1,5 @@ +virtual void RIGHT() +{ +call(); +ChangeMe; +} In this last test case, you want to demonstrate that the line call() is not
Re: [PATCH 2/2] add csharp userdiff tests
You can mark a userdiff test case that demonstrates a breakage by including the work broken somewhere in the file. See http://www.repo.or.cz/w/alt-git.git/commitdiff/9cc444f0570b196f1c51664ce2de1d8e1dee6046 -- Hannes Would you like me to add tests first, then fix them? Or just like this?-- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Recording the current branch on each commit?
On 27/04/2014 09:51, Robin Rosenberg wrote: Currently, git records a checksum, author, commit date/time, and commit message with every commit (as get be seen from 'git log'). I think it would be useful if, along with the Author and Date, git recorded the name of the current branch on each commit. The branch name can provide useful contextual information. For instance, let's say I'm developing a suite of games. If the commit message says Added basic options dialog, it might be useful to see that the branch name is pacman-minigame indicating that the commit pertains to the options dialog in the Pacman minigame. Basically, I'm saying that well-named branches can and do carry useful contextual information that oughtn't to be thrown away. Currently, when you delete that branch, you lose the branch name altogether. So what do you think? Would it be good to have a patch to add this feature? Branch names are usually poorly named, so often you don't lose much. One way Speak for yourself - I give my branches useful names. :-) I definitely feel that I am often losing useful contextual information by throwing away the branch name. some people to is to always merge with --no-ff, that way you see the branch name in the merge commit. But surely, it's recommended with Git that you try to avoid doing --no-ff merges to avoid commit noise? Also, it is a lot more hassle (and no doubt, CPU cycles) to track down where a branch was merged to try and figure out which branch name a commit pertained to, not to mention the fact that the commit could've been moved since. Nothing short of tagging the commit with the branch name when the commit is made will definitely record the branch name at the time of committing. A very popular way of tracking context is to add some id, such as a bugzilla issue number, to the header or footer of the commit message. Often a branch contains many issues, but the branch itself isn't very interesting. Tools like gitblit, gitweb, gerrit etc can easily be configured to link to the issue using a regular expression. Yes, and I have done this kind of thing in the past. However you really don't want to put the bug# on every single commit pertaining to that bug; you have to go to the effort of looking the bug# up every time, you'll sometimes forget, and besides it takes up space that could be used for a commit message. As short commit messages are valued in Git, it's particularly bad to waste space this way. Much better would be to include the bug# as part of the branch name, and then if you record the branch name upon checkin you always get a reference to the bug#. Also, you don't always have something you can link a commit to in an issue tracker. You may just be implementing a feature that has been agreed upon, independently of any such tracker. In that case, there's no bug# to link to. -- Best regards, Jeremy Morton (Jez) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] blame: large-scale performance rewrite
On Sat, Apr 26, 2014 at 2:39 PM, David Kastrup d...@gnu.org wrote: At least the stuff I fixed with regard to performance would seem to be done right in JGit to start with. Hah! Its Java. We have to do things right, otherwise its too slow. :-) Its still not as fast as I want it to be. :-) Most of the diff data/CRC is computed over and over because of the blackbox use of xdiff. Yes, I have been thinking about this all week. JGit blame uses HistogramDiff by default instead of MyersDiff. The first stage after we trim common header/trailer from both files is to compute a hash of each line and store those hashes. Those hashes are discarded as the blame algorithm moves to the next commit. Clearly for a commit chain of A - B - C, the hashes computed at B for the A-B compare can be reused for the B-C compare. This is not the case in either git-core or JGit, because the diff algorithm is a block box to the blame algorithm. I think this is what you mean by the CRC being computed again. For any given compare blame has a list of regions it is interested in learning about from the diff algorithm. Anything outside of those regions is useless noise that will be discarded. I have been pondering pushing that region list down into the diff algorithm so it can avoid executing on sections that are not relevant to the caller. At least for HistogramDiff this makes some sense, the algorithm is recursively applied after it finds a longest common subsequence. If one side of the LCS is outside of the region of interest from blame, there is no value in recursing on that portion. If the blame region list covers a small enough portion, it may even make sense to avoid the common header/trailer elimination preprocessing step. Unfortunately that sounds hard as you could be working with a file like a ChangeLog which grows on one of those sides. And then the delta-chain storage is packing stuff based on CRCs as well (not sure whether it keeps them around for unpacking). There are CRCs validated by libz during inflation, but these aren't rechecked once the inflated bytes are cached in that silly undersized 16M delta base cache. So there is a lot that could likely be improved while keeping the same basic algorithms, by cracking open the black boxes of the xdiff engine and the delta-chain coding. The delta chain coding has no relationship to the source file. Currently even plain text files are delta chain coded on a byte basis, not a line basis. Just matching up the delta coding against a source text file to determine lines 0-N are common is costly, since you have a byte range in the delta coding and you want a line range out the end. To make things more challenging, the delta chain coding can be against completely different blobs. In a compare of A-B from the commit graph being walked by blame there is no requirement the delta coding uses this pairing, and it almost certainly never uses this direction (its usually B-A, if its even this pair!). Given your comments in the other patch, I understand why you probably won't be working on blame more. But the above may help someone else that has available time to continue. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Uses git-credential for git-imap-send
git-imap-send was directly prompting for a password rather than using git-credential. git-send-email, on the other hand, supports git-credential. This is a necessary improvement for users that use two factor authentication, as they should not be expected to remember all of their app specific passwords. Signed-off-by: Dan Albert danalb...@google.com --- About imap vs. imaps: I actually had your exact line in before, but decided that as long as its for the same host the user probably wants to use the same credentials for both imap and imaps (if they for some reason had both configured). Hard coding imap allows them to use either protocol with only one keychain entry. The use case is a stretch, but it doesn't do any harm to implement it this way. My concerns with conflating the two are: 1. The system helper might care about the distinction and prefer imaps (e.g., it might already have the credential stored for your regular mail client, which uses imaps). But osxkeychain is the only helper that makes the distinction, and I don't really know how OS X's keychain code handles the distinction. 2. With http and https, we are careful to make the distinction, because we would not want to accidentally share a credential over http that was stored via https. But it's pretty easy to use an http URL rather than an https one. It's probably pretty rare to accidentally turn off imap SSL. So I'd be OK with leaving it as imap for now, and waiting for somebody to actually come up with a real case where the distinction matters. These are good points. I've made the change. imap-send.c | 29 +++-- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/imap-send.c b/imap-send.c index 0bc6f7f..112fc83 100644 --- a/imap-send.c +++ b/imap-send.c @@ -23,9 +23,9 @@ */ #include cache.h +#include credential.h #include exec_cmd.h #include run-command.h -#include prompt.h #ifdef NO_OPENSSL typedef void *SSL; #endif @@ -946,6 +946,7 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha static struct imap_store *imap_open_store(struct imap_server_conf *srvc) { + struct credential cred = CREDENTIAL_INIT; struct imap_store *ctx; struct imap *imap; char *arg, *rsp; @@ -1101,19 +1102,11 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc) goto bail; } if (!srvc-pass) { - struct strbuf prompt = STRBUF_INIT; - strbuf_addf(prompt, Password (%s@%s): , srvc-user, srvc-host); - arg = git_getpass(prompt.buf); - strbuf_release(prompt); - if (!*arg) { - fprintf(stderr, Skipping account %s@%s, no password\n, srvc-user, srvc-host); - goto bail; - } - /* -* getpass() returns a pointer to a static buffer. make a copy -* for long term storage. -*/ - srvc-pass = xstrdup(arg); + cred.username = xstrdup(srvc-user); + cred.protocol = xstrdup(srvc-use_ssl ? imaps : imap); + cred.host = xstrdup(srvc-host); + credential_fill(cred); + srvc-pass = xstrdup(cred.password); } if (CAP(NOLOGIN)) { fprintf(stderr, Skipping account %s@%s, server forbids LOGIN\n, srvc-user, srvc-host); @@ -1153,10 +1146,18 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc) } } /* !preauth */ + if (cred.username) + credential_approve(cred); + credential_clear(cred); + ctx-prefix = ; return ctx; bail: + if (cred.username) + credential_reject(cred); + credential_clear(cred); + imap_close_store(ctx); return NULL; } -- 2.0.0.rc1.1.gce060f5 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 2/5] test: add test_write_lines helper
API and implementation as suggested by Junio. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- t/README| 22 ++ t/test-lib-functions.sh | 5 + 2 files changed, 27 insertions(+) diff --git a/t/README b/t/README index caeeb9d..2d6232f 100644 --- a/t/README +++ b/t/README @@ -596,6 +596,28 @@ library for your script to use. ... ' + - test_write_lines text + + Split text to white-space separated words and write it out on standard + output, one word per line. + Useful to prepare multi-line files in a compact form. + + Example: + + test_write_lines a b c d e f g foo + + Is a more compact equivalent of: + cat foo -EOF + a + b + c + d + e + f + g + EOF + + - test_pause This command is useful for writing and debugging tests and must be diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 158e10a..f581535 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -717,6 +717,11 @@ test_ln_s_add () { fi } +# This function writes out its parameters, one per line +test_write_lines () { + printf %s\n $@ +} + perl () { command $PERL_PATH $@ } -- MST -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/5] patch-id: make it stable against hunk reordering
Patch id changes if users reorder file diffs that make up a patch. As the result is functionally equivalent, a different patch id is surprising to many users. In particular, reordering files using diff -O is helpful to make patches more readable (e.g. API header diff before implementation diff). Add an option to change patch-id behaviour making it stable against these kinds of patch change: calculate SHA1 hash for each hunk separately and sum all hashes (using a symmetrical sum) to get patch id We use a 20byte sum and not xor - since xor would give 0 output for patches that have two identical diffs, which isn't all that unlikely (e.g. append the same line in two places). The new behaviour is enabled - when patchid.stable is true - when --stable flag is present Using a new flag --unstable or setting patchid.stable to false force the historical behaviour. In the documentation, clarify that patch ID can now be a sum of hashes, not a hash. Document how command line and config options affect the behaviour. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- builtin/patch-id.c | 74 +- Documentation/git-patch-id.txt | 37 ++--- 2 files changed, 91 insertions(+), 20 deletions(-) diff --git a/builtin/patch-id.c b/builtin/patch-id.c index 3cfe02d..77db873 100644 --- a/builtin/patch-id.c +++ b/builtin/patch-id.c @@ -1,17 +1,14 @@ #include builtin.h -static void flush_current_id(int patchlen, unsigned char *id, git_SHA_CTX *c) +static void flush_current_id(int patchlen, unsigned char *id, unsigned char *result) { - unsigned char result[20]; char name[50]; if (!patchlen) return; - git_SHA1_Final(result, c); memcpy(name, sha1_to_hex(id), 41); printf(%s %s\n, sha1_to_hex(result), name); - git_SHA1_Init(c); } static int remove_space(char *line) @@ -56,10 +53,31 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after) return 1; } -static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct strbuf *line_buf) +static void flush_one_hunk(unsigned char *result, git_SHA_CTX *ctx) +{ + unsigned char hash[20]; + unsigned short carry = 0; + int i; + + git_SHA1_Final(hash, ctx); + git_SHA1_Init(ctx); + /* 20-byte sum, with carry */ + for (i = 0; i 20; ++i) { + carry += result[i] + hash[i]; + result[i] = carry; + carry = 8; + } +} + +static int get_one_patchid(unsigned char *next_sha1, unsigned char *result, + struct strbuf *line_buf, int stable) { int patchlen = 0, found_next = 0; int before = -1, after = -1; + git_SHA_CTX ctx; + + git_SHA1_Init(ctx); + hashclr(result); while (strbuf_getwholeline(line_buf, stdin, '\n') != EOF) { char *line = line_buf-buf; @@ -107,6 +125,8 @@ static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct st break; /* Else we're parsing another header. */ + if (stable) + flush_one_hunk(result, ctx); before = after = -1; } @@ -119,39 +139,63 @@ static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct st /* Compute the sha without whitespace */ len = remove_space(line); patchlen += len; - git_SHA1_Update(ctx, line, len); + git_SHA1_Update(ctx, line, len); } if (!found_next) hashclr(next_sha1); + flush_one_hunk(result, ctx); + return patchlen; } -static void generate_id_list(void) +static void generate_id_list(int stable) { - unsigned char sha1[20], n[20]; - git_SHA_CTX ctx; + unsigned char sha1[20], n[20], result[20]; int patchlen; struct strbuf line_buf = STRBUF_INIT; - git_SHA1_Init(ctx); hashclr(sha1); while (!feof(stdin)) { - patchlen = get_one_patchid(n, ctx, line_buf); - flush_current_id(patchlen, sha1, ctx); + patchlen = get_one_patchid(n, result, line_buf, stable); + flush_current_id(patchlen, sha1, result); hashcpy(sha1, n); } strbuf_release(line_buf); } -static const char patch_id_usage[] = git patch-id patch; +static const char patch_id_usage[] = git patch-id [--stable | --unstable] patch; + +static int git_patch_id_config(const char *var, const char *value, void *cb) +{ + int *stable = cb; + + if (!strcmp(var, patchid.stable)) { + *stable = git_config_bool(var, value); + return 0; + } + + return git_default_config(var, value, cb); +} int cmd_patch_id(int argc, const char **argv, const char
[PATCH v6 3/5] patch-id-test: test stable and unstable behaviour
Verify that patch ID supports an algorithm that is stable against diff split and reordering. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- t/t4204-patch-id.sh | 102 ++-- 1 file changed, 91 insertions(+), 11 deletions(-) diff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh index d2c930d..7732370 100755 --- a/t/t4204-patch-id.sh +++ b/t/t4204-patch-id.sh @@ -5,27 +5,44 @@ test_description='git patch-id' . ./test-lib.sh test_expect_success 'setup' ' - test_commit initial foo a - test_commit first foo b - git checkout -b same HEAD^ - test_commit same-msg foo b - git checkout -b notsame HEAD^ - test_commit notsame-msg foo c + as=a a a a a a a a # eight a + test_write_lines $as foo + test_write_lines $as bar + git add foo bar + git commit -a -m initial + test_write_lines $as b foo + test_write_lines $as b bar + git commit -a -m first + git checkout -b same master + git commit --amend -m same-msg + git checkout -b notsame master + echo c foo + echo c bar + git commit --amend -a -m notsame-msg + test_write_lines bar foo bar-then-foo + test_write_lines foo bar foo-then-bar ' test_expect_success 'patch-id output is well-formed' ' - git log -p -1 | git patch-id output + git log -p -1 | git patch-id output grep ^[a-f0-9]\{40\} $(git rev-parse HEAD)$ output ' +#calculate patch id. Make sure output is not empty. calc_patch_id () { - git patch-id | - sed s# .*## patch-id_$1 + name=$1 + shift + git patch-id $@ | + sed s/ .*// patch-id_$name + test_line_count -gt 0 patch-id_$name +} + +get_top_diff () { + git log -p -1 $@ -O bar-then-foo -- } get_patch_id () { - git log -p -1 $1 | git patch-id | - sed s# .*## patch-id_$1 + get_top_diff $1 | calc_patch_id $@ } test_expect_success 'patch-id detects equality' ' @@ -56,6 +73,69 @@ test_expect_success 'whitespace is irrelevant in footer' ' test_cmp patch-id_master patch-id_same ' +cmp_patch_id () { + if + test $1 = relevant + then + ! test_cmp patch-id_$2 patch-id_$3 + else + test_cmp patch-id_$2 patch-id_$3 + fi +} + +test_patch_id_file_order () { + relevant=$1 + shift + name=order-${1}-$relevant + shift + get_top_diff master | calc_patch_id $name $@ + git checkout same + git format-patch -1 --stdout -O foo-then-bar | + calc_patch_id ordered-$name $@ + cmp_patch_id $relevant $name ordered-$name + +} + +# combined test for options: add more tests here to make them +# run with all options +test_patch_id () { + test_patch_id_file_order $@ +} + +# small tests with detailed diagnostic for basic options. +test_expect_success 'file order is irrelevant with --stable' ' + test_patch_id_file_order irrelevant --stable --stable +' + +test_expect_success 'file order is relevant with --unstable' ' + test_patch_id_file_order relevant --unstable --unstable +' + +#Now test various option combinations. +test_expect_success 'default is unstable' ' + test_patch_id relevant default +' + +test_expect_success 'patchid.stable = true is stable' ' + test_config patchid.stable true + test_patch_id irrelevant patchid.stable=true +' + +test_expect_success 'patchid.stable = false is unstable' ' + test_config patchid.stable false + test_patch_id relevant patchid.stable=false +' + +test_expect_success '--unstable overrides patchid.stable = true' ' + test_config patchid.stable true + test_patch_id relevant patchid.stable=true--unstable --unstable +' + +test_expect_success '--stable overrides patchid.stable = false' ' + test_config patchid.stable false + test_patch_id irrelevant patchid.stable=false--stable --stable +' + test_expect_success 'patch-id supports git-format-patch MIME output' ' get_patch_id master git checkout same -- MST -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 4/5] patch-id: change default to stable
--stable has been the default in 'next' for a few weeks with no ill effects. Change the default to that so that users don't have to remember to enable it. Update documentation to match behaviour change. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- builtin/patch-id.c | 4 ++-- Documentation/git-patch-id.txt | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/patch-id.c b/builtin/patch-id.c index 77db873..e11a6a7 100644 --- a/builtin/patch-id.c +++ b/builtin/patch-id.c @@ -185,9 +185,9 @@ int cmd_patch_id(int argc, const char **argv, const char *prefix) git_config(git_patch_id_config, stable); - /* If nothing is set, default to unstable. */ + /* If nothing is set, default to stable. */ if (stable 0) - stable = 0; + stable = 1; if (argc == 2 !strcmp(argv[1], --stable)) stable = 1; diff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt index fa562d3..1e2ca75 100644 --- a/Documentation/git-patch-id.txt +++ b/Documentation/git-patch-id.txt @@ -43,7 +43,7 @@ OPTIONS of -Oorderfile, thereby making existing databases storing such unstable or historical patch-ids unusable. - This is the default if patchid.stable is set to true. + This is the default. --unstable:: Use an unstable hash as the patch ID. With this option, @@ -52,7 +52,7 @@ OPTIONS patch-ids produced by git 1.9 and older (who do not deal with reordered patches) may want to use this option. - This is the default. + This is the default if patchid.stable is set to false. patch:: The diff to create the ID of. -- MST -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 5/9] patch-id: document new behaviour
On Thu, Apr 24, 2014 at 03:12:14PM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: +--unstable:: +Use a non-symmetrical sum of hashes, such that reordering What is a non-symmetrical sum? Non-symmetrical combination function is better? I do not think either is very good X-. The primary points to convey for --stable are: - Two patches produced by comparing the same two trees with two different settings for -Oorderfile will result in the same patchc signature, thereby allowing the computed result to be used as a key to index some metainformation about the change between the two trees; - It will produce a result different from the plain vanilla patch-id has always produced even when used on a diff output taken without any use of -Oorderfile, thereby making existing databases keyed by patch-ids unusable. The fact that we happened to use a patch-id that catches that somebody reordered the same patch into different file order and declares that they are two different changes is a more historical accident than a designed goal. I would even say that we would have used the stable version from the beginning if we thought that -Oorderfile would be widely used when these two features both appeared. Even though I was the guilty one who introduced it, I'd admit that -Oorderfile has merely been a curiosity from its inception and has been a failed experiment, not in the sense that the feature does not work as adverertised (it does), but in the sense that it is not widely used (evidenced by the lack of complaints on missing diff.orderfile for a long time) at all. With -Oorderfile being a failed experiment, the unstability did not matter, so it has stuck. The only two things worth mentioning about --unstable, if our future direction is to see diff.orderfile and --stable a lot more widely used, are: (1) it keeps producing the same patch-id as existing versions of Git, so users with existing databases (who do not deal with reordered patches) may want to use it; and perhaps (2) it will not consider a patch taken with -Oorderfile and another without it from the same source the same patches. Mathmatically speaking, mentioning non-symmetrial might be one way of expressing the latter point (2), but stressing on that alone without mentioning (1) misses the point. (2) is _not_ a designed feature, so it is not very interesting. Unless you have an existing database, there is no reason to use --unstable. On the other hand (1) is a very relevant thing to mention, as we are talking about a feature that, if unused, may break existing users' data. OK I did just that, pls take a look. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] test/send-email: to-cover, cc-cover tests
Add tests for the new feature. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- t/t9001-send-email.sh | 45 + 1 file changed, 45 insertions(+) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 1ecdacb..97cc094 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1334,6 +1334,51 @@ test_expect_success $PREREQ '--force sends cover letter template anyway' ' test -n $(ls msgtxt*) ' +test_cover_addresses () { + header=$1 + shift + clean_fake_sendmail + rm -fr outdir + git format-patch --cover-letter -2 -o outdir + cover=`echo outdir/-*.patch` + mv $cover cover-to-edit.patch + sed s/^From:/$header: ex...@address.com\nFrom:/ cover-to-edit.patch $cover + git send-email \ + --force \ + --from=Example nob...@example.com \ + --no-to --no-cc \ + $@ \ + --smtp-server=$(pwd)/fake.sendmail \ + outdir/-*.patch \ + outdir/0001-*.patch \ + outdir/0002-*.patch \ + 2errors out + grep ^$header: ex...@address.com msgtxt1 to1 + grep ^$header: ex...@address.com msgtxt2 to2 + grep ^$header: ex...@address.com msgtxt3 to3 + test_line_count = 1 to1 + test_line_count = 1 to2 + test_line_count = 1 to3 +} + +test_expect_success $PREREQ 'to-cover adds To to all mail' ' + test_cover_addresses To --to-cover +' + +test_expect_success $PREREQ 'cc-cover adds Cc to all mail' ' + test_cover_addresses Cc --cc-cover +' + +test_expect_success $PREREQ 'tocover adds To to all mail' ' + test_config sendemail.tocover true + test_cover_addresses To +' + +test_expect_success $PREREQ 'cccover adds Cc to all mail' ' + test_config sendemail.cccover true + test_cover_addresses Cc +' + test_expect_success $PREREQ 'sendemail.aliasfiletype=mailrc' ' clean_fake_sendmail echo alias sbd someb...@example.org .mailrc -- MST -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] git-send-email: two new options: to-cover, cc-cover
Allow extracting To/Cc addresses from cover letter. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Documentation/git-send-email.txt | 12 git-send-email.perl | 16 2 files changed, 28 insertions(+) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index f0e57a5..1733664 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -248,6 +248,18 @@ Automating cc list. Default is the value of 'sendemail.signedoffbycc' configuration value; if that is unspecified, default to --signed-off-by-cc. +--[no-]cc-cover:: + If this is set, emails found in Cc: headers in the cover letter are + added to the cc list for each email set. Default is the value of + 'sendemail.cccover' configuration value; if that is unspecified, + default to --no-cc-cover. + +--[no-]to-cover:: + If this is set, emails found in To: headers in the cover letter are + added to the to list for each email set. Default is the value of + 'sendemail.tocover' configuration value; if that is unspecified, + default to --no-to-cover. + --suppress-cc=category:: Specify an additional category of recipients to suppress the auto-cc of: diff --git a/git-send-email.perl b/git-send-email.perl index 4c138a2..0084cf4 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -80,6 +80,8 @@ git send-email [options] file | directory | rev-list options --to-cmdstr * Email To: via `str \$patch_path` --cc-cmdstr * Email Cc: via `str \$patch_path` --suppress-cc str * author, self, sob, cc, cccmd, body, bodycc, all. +--[no-]cc-cover* Email Cc: addresses in the cover letter. +--[no-]to-cover* Email To: addresses in the cover letter. --[no-]signed-off-by-cc* Send to Signed-off-by: addresses. Default on. --[no-]suppress-from * Send to self. Default off. --[no-]chain-reply-to * Chain In-Reply-To: fields. Default off. @@ -195,6 +197,7 @@ sub do_edit { # Variables with corresponding config settings my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc); +my ($cover_cc, $cover_to); my ($to_cmd, $cc_cmd); my ($smtp_server, $smtp_server_port, @smtp_server_options); my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path); @@ -211,6 +214,8 @@ my %config_bool_settings = ( chainreplyto = [\$chain_reply_to, 0], suppressfrom = [\$suppress_from, undef], signedoffbycc = [\$signed_off_by_cc, undef], +cccover = [\$cover_cc, undef], +tocover = [\$cover_to, undef], signedoffcc = [\$signed_off_by_cc, undef], # Deprecated validate = [\$validate, 1], multiedit = [\$multiedit, undef], @@ -302,6 +307,8 @@ my $rc = GetOptions(h = \$help, suppress-from! = \$suppress_from, suppress-cc=s = \@suppress_cc, signed-off-cc|signed-off-by-cc! = \$signed_off_by_cc, + cc-cover|cc-cover! = \$cover_cc, + to-cover|to-cover! = \$cover_to, confirm=s = \$confirm, dry-run = \$dry_run, envelope-sender=s = \$envelope_sender, @@ -1469,6 +1476,15 @@ foreach my $t (@files) { @to = (@initial_to, @to); @cc = (@initial_cc, @cc); + if ($message_num == 1) { + if (defined $cover_cc and $cover_cc) { + @initial_cc = @cc; + } + if (defined $cover_to and $cover_to) { + @initial_to = @to; + } + } + my $message_was_sent = send_message(); # set up for the next message -- MST -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] git-send-email: two new options: to-cover, cc-cover
On Thu, Apr 03, 2014 at 11:31:51AM -0700, Junio C Hamano wrote: Michael S. Tsirkin m...@redhat.com writes: Allow extracting To/Cc addresses from cover letter. Please say what you are doing with what you extract, which is the more important part of the objective. Extracting is merely a step to achieve that. s/.$/, to be used as To/Cc addresses of the remainder of the series./ or something. thanks, I did that in the new version. I think this will be a very handy feature. If you have a series *and* you bothered to add To/Cc to the cover letter, it is likely that you want all the messages read by these people [*1*]. @@ -1468,6 +1475,15 @@ foreach my $t (@files) { @to = (@initial_to, @to); @cc = (@initial_cc, @cc); + if ($message_num == 1) { + if (defined $cover_cc and $cover_cc) { + @initial_cc = @cc; + } + if (defined $cover_to and $cover_to) { + @initial_to = @to; + } + } + What is stored away with this code to @initial_cc/to includes: - what was given to @initial_cc/to before ll.1468-1469 - what was in @cc/to before ll.1468-1469 when we see the first message [*2*]. The former come from the command line --to/--cc, and the latter comes from the header lines of the first message. Am I reading the code correctly? Exactly. If that is the case, I think the updated code makes sense. Thanks. [Footnote] *1* Allowing this to be disabled is also a good thing this patch does. A 100 patch series that does a tree-wide clean-up may have different set of people on To/Cc of individual patches, and you may want the union of them on To/Cc on the cover letter, so that a person may get the cover letter and a single patch that relates to his area of expertise without having to see the remainder. *2* The first message may not necessarily be the cover letter. Is there a reliable way to detect that? The user may want to send out a series with only a few patches without any cover, and taking To/Cc from the [PATCH 1/3] and propagating them to the rest does not match what the documentation and the option name claim to do. Two things that come to mind: - check that subject has / Needs some manual parsing, I don't like this much - check that there's no patch We could try running git mailinfo but it might give false negatives if cover letter happens to have --- diff a/foo b/bar within it. Worth worrying about? For now I simply updated the documentation. -- MST -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] git-svn: only look at the root path for svn:mergeinfo
On Apr 22, 2014, at 11:54 AM, Eric Wong normalper...@yhbt.net wrote: Jakob Stoklund Olesen stokl...@2pi.dk wrote: Subversion can put mergeinfo on any sub-directory to track cherry-picks. Since cherry-picks are not represented explicitly in git, git-svn should just ignore it. Hi, was git-svn trying to track cherry-picks as merge before? It would try and fail. I didn't explain that properly in the commit message. Suppose I have a standard svn layout with $url/trunk and $url/branches/topic1. My topic1 branch has a change in subdir1 that I want to cherry-pick into trunk: % svn switch $url/trunk % cd subdir1 % svn merge $url/branches/topic1/subdir1 % cd .. % svn commit This operation will set svn:mergeinfo on $url/trunk/subdir1 where a normal full merge would set it on $url/trunk: % svn pg svn:mergeinfo subdir1 /branches/topic1/subdir1:3-4 When git-svn fetches these changes, it currently does examine the svn:mergeinfo change on the subdirectory as if it were a full merge. It then fails to find a revmap for /branches/topic1/subdir1: Couldn't find revmap for file:///tmp/sdb/branches/topic1/subdir1 r5 = 5ce1f687c30495deca40730fb7be3baa0e145479 (refs/remotes/trunk) It is looking for refs/remotes/topic1/subdir1, but we only have the refs/remotes/topic1 branch in git. This patch makes git-svn stop trying to reconstruct those subdirectory merges that we know will fail anyway. This changes behavior a bit, so two independent users of git-svn may not have identical histories as a result, correct? For normal subdirectory cherry-picks as described above, the behavior doesn't change. This is just a performance optimization. For weirder cases where a whole branch has been merged onto a subdirectory of trunk, behavior does change. Currently, git-svn will mark that as a full merge in git. With this change it won't. Can you add a test to ensure this behavior is preserved? Thanks. I'll add a test for the subdirectory merge described above. Sorry, I've never looked at mergeinfo myself, mainly relying on Sam + tests for this. [1] - Historically, git-svn (using defaults) has always tried to preserve identical histories for independent users across different git-svn versions. However, mergeinfo may be enough of a corner-case where we can make an exception. I agree. It doesn't seem worthwhile to try to preserve git-svn's historical behavior in weird corner cases. BTW, this performance optimization matters not because of sporadic manual cherry-picks, but because certain older svn releases would replicate svn:mergeinfo on every subdirectory in a standard merge. With hundreds of subdirectories and thousands of merged branches, git-svn gets completely stuck processing all those mergeinfo lines. Thanks, /jakob -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Recording the current branch on each commit?
On Sun, Apr 27, 2014 at 7:38 PM, Jeremy Morton ad...@game-point.net wrote: On 27/04/2014 10:09, Johan Herland wrote: On Sun, Apr 27, 2014 at 1:56 AM, Jeremy Mortonad...@game-point.net wrote: Currently, git records a checksum, author, commit date/time, and commit message with every commit (as get be seen from 'git log'). I think it would be useful if, along with the Author and Date, git recorded the name of the current branch on each commit. This has been discussed multiple times in the past. One example here: http://thread.gmane.org/gmane.comp.version-control.git/229422 I believe the current conclusion (if any) is that encoding such information as a _structural_ part of the commit object is not useful. See the old thread(s) for the actual pro/con arguments. As far as I can tell from that discussion, the general opposition to encoding the branch name as a structural part of the commit object is that, for some people's workflows, it would be unhelpful and/or misleading. Well fair enough then - why don't we make it a setting that is off by default, and can easily be switched on? That way the people for whom tagging the branch name would be useful have a very easy way to switch it on. Obviously, the feature would necessarily have to be optional, simply because Git would have to keep understanding the old commit object format for a LONG time (probably indefinitely), and there's nothing you can do to prevent others from creating old-style commit objects. Which brings us to another big con at this point: The cost of changing the commit object format. One can argue for or against a new commit object format, but the simple truth at this point is that changing the structure of the commit object is expensive. Even if we were all in agreement about the change (and so far we are not), there are multiple Git implementations (libgit2, jgit, dulwich, etc.) that would all have to learn the new commit object, not to mention that bumping core.repositoryformatversion would probably make your git repo incompatible with a huge number of existing deployments for the foreseeable future. Therefore, the most pragmatic and constructive thing to do at this point, is IMHO to work within the confines of the existing commit object structure. I actually believe using commit message trailers like Made-on-branch: frotz in addition to some helpful infrastructure (hooks, templates, git-interpret-trailers, etc.) should get you pretty much exactly what you want. And if this feature turns out to be extremely useful for a lot of users, we can certainly consider changing the commit object format in the future. I know that for the workflows I personally have used in the past, such tagging would be very useful. Quite often I have been looking through the Git log and wondered what feature a commit was part of, because I have feature branches. Just knowing that branch name would be really useful, but the branch has since been deleted... and in the case of a ff-merge (which I thought was recommended in Git if possible), the branch name is completely gone. True. The branch name is - for better or worse - simply not considered very important by Git, and a Git commit is simply not considered (by Git at least) to be part of or otherwise belong to any branch. Instead the commit history/graph is what Git considers important, and the branch names are really just more-or-less ephemeral pointers into that graph. AFAIK, recording the current branch name in commits was not considered to the worth including in Linus' original design, and since then it seems to only have come up a few times on the mailing list. This is quite central to Git's design, and changing it at this point should not be done lightly. IINM, Mercurial does this differently, so that may be a better fit for the workflows where keeping track of branch names is very important. That said, you are of course free to add this information to your own commit messages, by appending something like Made-on-branch: frotz. In a company setting, you can even create a commit message template or (prepare-)commit-msg hook to have this line created automatically for you and your co-workers. You could even append such information retroactively to existing commits with git notes. There is also the current interpret-trailers effort by Christian Couder [1] that should be useful in creating and managing such lines. [1]: http://thread.gmane.org/gmane.comp.version-control.git/245874 Well I guess that's another way of doing it. So, why aren't Author and Date trailers? They don't seem any more fundamental to me than branch name. I mean the only checkin information you really *need* is the checksum, and commit's parents. The Author and Date are just extra pieces of information you might find useful sometimes, right? A bit like some people might find branch checkin name useful sometimes...? Yeah, sure. Author and Date (and Committer, for that matter) is just
Re: [PATCH 2/2] add csharp userdiff tests
Am 27.04.2014 18:46, schrieb Marius Ungureanu: Is it okay though if I add a few tests to show what is broken? I think this can’t be solved at a regex level. It's OK to add tests that show breakages even if there is no immediate solution. You can mark a userdiff test case that demonstrates a breakage by including the work broken somewhere in the file. See http://www.repo.or.cz/w/alt-git.git/commitdiff/9cc444f0570b196f1c51664ce2de1d8e1dee6046 You add tests including broken cases first, and then in the follow-up patch that fixes the broken ones, you also mark the tests as fixed, like I did in the follow-up patch of the above example: http://www.repo.or.cz/w/alt-git.git/commitdiff/8a2e8da367f7175465118510b474ad365161d6b1 -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 1/2] trailer: fix to ignore any line starting with '#'
It looks like the commit-msg hook is passed a commit message that can contain lines starting with a '#'. Those comment lines will be removed from the commit message after the hook is run. If we want git interpret-trailers to be able to process commit messages correctly in the commit-msg hook we need to ignore those lines. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t7513-interpret-trailers.sh | 26 ++ trailer.c | 29 ++--- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh index 9aae721..aa63b1b 100755 --- a/t/t7513-interpret-trailers.sh +++ b/t/t7513-interpret-trailers.sh @@ -150,6 +150,32 @@ test_expect_success 'with 2 files arguments' ' test_cmp expected actual ' +test_expect_success 'with message that has comments' ' + cat basic_message message_with_comments + sed -e s/ Z\$/ / message_with_comments -\EOF + # comment + + # other comment + Cc: Z + # yet another comment + Reviewed-by: Johan + Reviewed-by: Z + # last comment + + EOF + cat basic_patch message_with_comments + cat basic_message expected + cat expected -\EOF + # comment + + Cc: Peff + Reviewed-by: Johan + EOF + cat basic_patch expected + git interpret-trailers --trim-empty --trailer Cc: Peff message_with_comments actual + test_cmp expected actual +' + test_expect_success 'with commit complex message and trailer args' ' cat complex_message_body expected sed -e s/ Z\$/ / expected -\EOF diff --git a/trailer.c b/trailer.c index 4d32b42..81b2c5c 100644 --- a/trailer.c +++ b/trailer.c @@ -644,10 +644,9 @@ static int find_patch_start(struct strbuf **lines, int count) /* * Return the (0 based) index of the first trailer line or count if * there are no trailers. Trailers are searched only in the lines from - * index (count - 1) down to index 0. The has_blank_line parameter - * tells if there is a blank line before the trailers. + * index (count - 1) down to index 0. */ -static int find_trailer_start(struct strbuf **lines, int count, int *has_blank_line) +static int find_trailer_start(struct strbuf **lines, int count) { int start, only_spaces = 1; @@ -656,10 +655,11 @@ static int find_trailer_start(struct strbuf **lines, int count, int *has_blank_l * for a line with only spaces before lines with one ':'. */ for (start = count - 1; start = 0; start--) { + if (lines[start]-buf[0] == '#') + continue; if (contains_only_spaces(lines[start]-buf)) { if (only_spaces) continue; - *has_blank_line = 1; return start + 1; } if (strchr(lines[start]-buf, ':')) { @@ -667,13 +667,20 @@ static int find_trailer_start(struct strbuf **lines, int count, int *has_blank_l only_spaces = 0; continue; } - *has_blank_line = start == count - 1 ? - 0 : contains_only_spaces(lines[start + 1]-buf); return count; } - *has_blank_line = only_spaces ? count 0 : 0; - return only_spaces ? count : start + 1; + return only_spaces ? count : 0; +} + +static int has_blank_line_before(struct strbuf **lines, int start) +{ + for (;start = 0; start--) { + if (lines[start]-buf[0] == '#') + continue; + return contains_only_spaces(lines[start]-buf); + } + return 0; } static void print_lines(struct strbuf **lines, int start, int end) @@ -688,19 +695,19 @@ static int process_input_file(struct strbuf **lines, struct trailer_item **in_tok_last) { int count = 0; - int patch_start, trailer_start, has_blank_line, i; + int patch_start, trailer_start, i; /* Get the line count */ while (lines[count]) count++; patch_start = find_patch_start(lines, count); - trailer_start = find_trailer_start(lines, patch_start, has_blank_line); + trailer_start = find_trailer_start(lines, patch_start); /* Print lines before the trailers as is */ print_lines(lines, 0, trailer_start); - if (!has_blank_line) + if (!has_blank_line_before(lines, trailer_start - 1)) printf(\n); /* Parse trailer lines */ -- 1.9.rc0.17.g651113e -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 0/2] fix and examples for git interpret-trailers
As I sent version 11 of the Add interpret-trailers builtin only a few days ago, I am sending now a short series on top instead of another full version. Christian Couder (2): trailer: fix to ignore any line starting with '#' trailer: add examples to the documentation Documentation/git-interpret-trailers.txt | 98 +++- t/t7513-interpret-trailers.sh| 26 + trailer.c| 29 ++ 3 files changed, 141 insertions(+), 12 deletions(-) -- 1.9.rc0.17.g651113e -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 2/2] trailer: add examples to the documentation
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-interpret-trailers.txt | 98 +++- 1 file changed, 97 insertions(+), 1 deletion(-) diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt index 450ec54..42c2f71 100644 --- a/Documentation/git-interpret-trailers.txt +++ b/Documentation/git-interpret-trailers.txt @@ -134,9 +134,105 @@ If the command contains the `$ARG` string, this string will be replaced with the value part of an existing trailer with the same token, if any, before the command is launched. +EXAMPLES + + +* Configure a 'sign' trailer with a 'Signed-off-by: ' key, and then + add two of these trailers to a message: ++ + +$ git config trailer.sign.key Signed-off-by: +$ cat msg.txt +subject + +message +$ cat msg.txt | git interpret-trailers --trailer 'sign: Alice al...@example.com' --trailer 'sign: Bob b...@example.com' +subject + +message + +Signed-off-by: Alice al...@example.com +Signed-off-by: Bob b...@example.com + + +* Extract the last commit as a patch, and add a 'Cc' and a + 'Reviewed-by' trailer to it: ++ + +$ git format-patch -1 +0001-foo.patch +$ git interpret-trailers --trailer 'Cc: Alice al...@example.com' --trailer 'Reviewed-by: Bob b...@example.com' 0001-foo.patch 0001-bar.patch + + +* Configure a 'sign' trailer with a command to automatically add a + 'Signed-off-by: ' with the author information only if there is no + 'Signed-off-by: ' already, and show how it works: ++ + +$ git config trailer.sign.key Signed-off-by: +$ git config trailer.sign.ifmissing add +$ git config trailer.sign.ifexists doNothing +$ git config trailer.sign.command 'echo $(git config user.name) $(git config user.email)' +$ git interpret-trailers EOF + EOF + +Signed-off-by: Bob b...@example.com +$ git interpret-trailers EOF + Signed-off-by: Alice al...@example.com + EOF + +Signed-off-by: Alice al...@example.com + + +* Configure a 'fix' trailer with a command to show the subject of a + commit that is fixed, and show how it works: ++ + +$ git config trailer.fix.key Fixes # +$ git config trailer.fix.ifExists overwrite +$ git config trailer.fix.command git log -1 --oneline --format=\%h (%s)\ --abbrev-commit --abbrev=14 \$ARG +$ git interpret-trailers EOF + subject + + message + + fix: HEAD~2 + EOF +subject + +message + +Fixes #fe3187489d69c4 (subject of fixed commit) + + +* Configure a commit template with some trailers with empty values, + then configure a commit-msg hook that uses git interpret-trailers to + remove trailers with empty values and to add a 'git-version' + trailer: ++ + +$ cat commit_template.txt EOF + ***subject*** + + ***message*** + + Fixes: + Cc: + Reviewed-by: + Signed-off-by: + EOF +$ git config commit.template commit_template.txt +$ cat .git/hooks/commit-msg EOF +#!/bin/sh +git interpret-trailers --trim-empty --trailer git-version: \$(git describe) \$1 \$1.new +mv \$1.new \$1 +EOF +$ chmod +x .git/hooks/commit-msg + + SEE ALSO -linkgit:git-commit[1] +linkgit:git-commit[1], linkgit:git-format-patch[1], linkgit:git-config[1] GIT --- -- 1.9.rc0.17.g651113e -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Recording the current branch on each commit?
On 27/04/2014 20:33, Johan Herland wrote: On Sun, Apr 27, 2014 at 7:38 PM, Jeremy Mortonad...@game-point.net wrote: On 27/04/2014 10:09, Johan Herland wrote: As far as I can tell from that discussion, the general opposition to encoding the branch name as a structural part of the commit object is that, for some people's workflows, it would be unhelpful and/or misleading. Well fair enough then - why don't we make it a setting that is off by default, and can easily be switched on? That way the people for whom tagging the branch name would be useful have a very easy way to switch it on. Therefore, the most pragmatic and constructive thing to do at this point, is IMHO to work within the confines of the existing commit object structure. I actually believe using commit message trailers like Made-on-branch: frotz in addition to some helpful infrastructure (hooks, templates, git-interpret-trailers, etc.) should get you pretty much exactly what you want. And if this feature turns out to be extremely useful for a lot of users, we can certainly consider changing the commit object format in the future. OK, fair enough. So I guess what I'd like to see, then, is good built-in functionality in Git for these commit message trailers, so that they are very easy to turn on. I'd like to be able to tell co-developers to add a one-liner to their git config file rather than some post-commit script. I know that for the workflows I personally have used in the past, such tagging would be very useful. Quite often I have been looking through the Git log and wondered what feature a commit was part of, because I have feature branches. Just knowing that branch name would be really useful, but the branch has since been deleted... and in the case of a ff-merge (which I thought was recommended in Git if possible), the branch name is completely gone. True. The branch name is - for better or worse - simply not considered very important by Git, and a Git commit is simply not considered (by Git at least) to be part of or otherwise belong to any branch. Please understand that I know this full well. :-) I'm saying that the 'ephemeral' pointers' names are, in themselves, useful - if, like me, you give them meaningful names. What I'm proposing is pretty much an automatic tagging (somehow...) of each commit with the current branch name (if one is available); information that carries roughly the same weight as the commit message. It could be crap, but equally it could be very useful, in some workflows. I think most of us can agree on that. seems to only have come up a few times on the mailing list. This is But it has come up more than once, which would seem to indicate that I'm not the only one with this request. ;-) IINM, Mercurial does this differently, so that may be a better fit for If I'm Not Mistaken - I had to look that one up. the workflows where keeping track of branch names is very important. Nah, I had a look at Mercurial and I think I prefer Git - this branch name thing is just my one bugbear. I definitely prefer Git's concept of a staging area rather than just committing all changes. To do that in Mercurial you have to use mq and all the different (IMHO unintuative) commands that entails, and if you accidentally mq commit then you screw everything up. :-) Mercurial also discourages history rewriting (ie. cleaning up of messy commits), which Git doesn't. I prefer Git's approach here too. Yeah, sure. Author and Date (and Committer, for that matter) is just metadata, and the current branch name is simply just another kind of metadata. All of them are more-or-less free-form text fields, and off the top of my head, I can't really say that if we were to design Git from scratch today, they wouldn't all become optional trailers (or headers, or what-have-you). However, we're not designing Git from scratch, and we have to work with what is already there... Fair point. The branch name can provide useful contextual information. For instance, let's say I'm developing a suite of games. If the commit message says Added basic options dialog, it might be useful to see that the branch name is pacman-minigame indicating that the commit pertains to the options dialog in the Pacman minigame. In that partcular case, ISTM that the context (pacman-minigame) would actually be better preserved elsewhere. E.g. the commits touch files in a particular minigames/pacman subdir, or you prefix the context in the commit message (pacman-minigame: Added basic options dialog). Also, such a topic branch is often tied to a specific Again, this is a pain because you have to remember to manually tag every commit message with pacman-minigame, and it takes up precious space in the (already short) commit message. Yes, which is why I advise you to look at commit message templates, hooks, and interpret-trailers to see if you can find a way to automate this for you and your co-workers. What I'd like to
Re: Recording the current branch on each commit?
I'm skipping a lot of the discussion here, sorry about that, but on one particular note: Jeremy Morton ad...@game-point.net wrote: (...) and besides it takes up space that could be used for a commit message. As short commit messages are valued in Git, it's particularly bad to waste space this way. Not really. While different groups will have different values, the greater git community seems to prefer short _first lines_, of fifty chars or less, while the _body_ should be as verbose as it needs to be (but not more than). Ergo, while the first line shouldn't contain a swath of metadata, the body can easily. A particularly good example of this is almost every commit to the git project itself - there areSigned-of-by lines and such everywhere in the logs. Also, you don't always have something you can link a commit to in an issue tracker. You may just be implementing a feature that has been agreed upon, independently of any such tracker. In that case, there's no bug# to link to. In which case, refer to whatever system you use. If you aren't using a ticketing system, have the line Relates-to: Water cooler conversation with Bob on July 28th or whatever the patches relate to. (Arguably, though, the better solution is to use a ticketing system, or anything that allows discussion to be easily referenced.) Regards, James Denholm. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Recording the current branch on each commit?
On 27/04/2014 22:40, James Denholm wrote: Also, you don't always have something you can link a commit to in an issue tracker. You may just be implementing a feature that has been agreed upon, independently of any such tracker. In that case, there's no bug# to link to. In which case, refer to whatever system you use. If you aren't using a ticketing system, have the line Relates-to: Water cooler conversation with Bob on July 28th or whatever the patches relate to. (Arguably, though, the better solution is to use a ticketing system, or anything that allows discussion to be easily referenced.) Well, as I said elsewhere in this discussion, Git should provide that functionality built-in, IMHO. It would be good to be able to set a one-liner in my .gitconfig to tag each commit with a branch checked into trailer. -- Best regards, Jeremy Morton (Jez) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Recording the current branch on each commit?
Jeremy Morton ad...@game-point.net wrote: On 27/04/2014 22:40, James Denholm wrote: Also, you don't always have something you can link a commit to in an issue tracker. You may just be implementing a feature that has been agreed upon, independently of any such tracker. In that case, there's no bug# to link to. In which case, refer to whatever system you use. If you aren't using a ticketing system, have the line Relates-to: Water cooler conversation with Bob on July 28th or whatever the patches relate to. (Arguably, though, the better solution is to use a ticketing system, or anything that allows discussion to be easily referenced.) Well, as I said elsewhere in this discussion, Git should provide that functionality built-in, IMHO. It would be good to be able to set a one-liner in my .gitconfig to tag each commit with a branch checked into trailer. In that case, write something onto your post-commit hook and the functionality would be achieved. A relates-to line doesn't need a change to the structure of git commits. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Recording the current branch on each commit?
On Sun, Apr 27, 2014 at 10:55 PM, Jeremy Morton ad...@game-point.net wrote: On 27/04/2014 20:33, Johan Herland wrote: On Sun, Apr 27, 2014 at 7:38 PM, Jeremy Mortonad...@game-point.net wrote: On 27/04/2014 10:09, Johan Herland wrote: As far as I can tell from that discussion, the general opposition to encoding the branch name as a structural part of the commit object is that, for some people's workflows, it would be unhelpful and/or misleading. Well fair enough then - why don't we make it a setting that is off by default, and can easily be switched on? That way the people for whom tagging the branch name would be useful have a very easy way to switch it on. Therefore, the most pragmatic and constructive thing to do at this point, is IMHO to work within the confines of the existing commit object structure. I actually believe using commit message trailers like Made-on-branch: frotz in addition to some helpful infrastructure (hooks, templates, git-interpret-trailers, etc.) should get you pretty much exactly what you want. And if this feature turns out to be extremely useful for a lot of users, we can certainly consider changing the commit object format in the future. OK, fair enough. So I guess what I'd like to see, then, is good built-in functionality in Git for these commit message trailers, so that they are very easy to turn on. I'd like to be able to tell co-developers to add a one-liner to their git config file rather than some post-commit script. I think this is what the interpret-trailers effort is about. Unfortunately I have not followed it closely enough to say if your use case is already covered by Christian's (CCed) work. Christian: With your current patch series, is it possible for Jeremy to configure interpret-trailers to automatically append a Made-on-branch: current_branch trailer whenever he creates a commit? [...] What I'd like to see, then, is this trailer functionality built in to Git so that a very minimal amount of setup is needed to get everybody using it. We're basically talking about hijacking the commit messages and tacking on information that they weren't really intended to hold (ie. stuff the developer hasn't manually typed in as a commit message), because of the limitation of the Git commit format. In hindsight, I guess it would've been better to have the Git commit format be more flexible in terms of what headers it allows, so that new headers could easily be added and some headers could be optional. Which - if you squint at it a little - is sort of what the interpret-trailers effort does. AFAIK, it (combined with hooks) allows you to configure a set of optional s/headers/trailers/ and the policies surrounding those. Only if it's a non-ff merge, which results in less tidy commit trees, and hence is often recommended against. Not at all. If you're developing a series of commits with a common purpose (a.k.a. a topic branch) I would very much argue for non-ff-merging this, _exactly_ because the merge commit allows you to introduce the entire topic as a single entity. The merge commit message (in addition to containing the branch name) is also the natural place to describe more general things about the topic as a whole - sort of like the cover letter to a patch series. Would you recommend that every single commit be made in a branch that gets merged into master, then? So, no direct commits to master? There are a lot of variables here, and it really comes down to your (team's) preferred workflow. Different teams/people prefer different workflows, and git's toolbox allows a wide (probably the widest among any VCS) variety of workflows to be expressed. So I really cannot make any sweeping/simple recommendations that will apply to all cases. Although I prefer collecting related commits on a topic branch that are then (non-ff) merged into master, I also see the value of performing a quick single-commit bugfix directly on master. In the latter case, the commit should obviously be self-sufficient and self-explanatory. However, once your work start spanning more than a few commits, you should really think about putting it on a separate branch, where it can be (re)organized in a way that is logical and reviewable (interactive rebase FTW). When I think about it, this might only apply to centralized workflows where team members (typically co-workers) push their own work to a shared repository/branch. As a counterexample: in git.git, pretty much every change (whether it consists of a single patch, or a series of patches) gets its own $who/$what branch in Junio's tree, and are then merged to 'pu'. When deemed worthy, they are merged to 'next' and - later - to 'master'. So here, even single-commit changes get their own branch and subsequent merge commit. Likewise, with GitHub's pull-request workflow, you make changes on a branch in one repo, and then request that branch to be pulled (i.e. merged) into another repo, regardless of whether
Re: Recording the current branch on each commit?
On 04/28/2014 01:03 AM, Johan Herland wrote: On Sun, Apr 27, 2014 at 7:38 PM, Jeremy Morton ad...@game-point.net wrote: On 27/04/2014 10:09, Johan Herland wrote: On Sun, Apr 27, 2014 at 1:56 AM, Jeremy Mortonad...@game-point.net wrote: Currently, git records a checksum, author, commit date/time, and commit message with every commit (as get be seen from 'git log'). I think it would be useful if, along with the Author and Date, git recorded the name of the current branch on each commit. This has been discussed multiple times in the past. One example here: http://thread.gmane.org/gmane.comp.version-control.git/229422 I believe the current conclusion (if any) is that encoding such information as a _structural_ part of the commit object is not useful. See the old thread(s) for the actual pro/con arguments. As far as I can tell from that discussion, the general opposition to encoding the branch name as a structural part of the commit object is that, for some people's workflows, it would be unhelpful and/or misleading. Well fair enough then - why don't we make it a setting that is off by default, and can easily be switched on? That way the people for whom tagging the branch name would be useful have a very easy way to switch it on. Obviously, the feature would necessarily have to be optional, simply because Git would have to keep understanding the old commit object format for a LONG time (probably indefinitely), and there's nothing you can do to prevent others from creating old-style commit objects. Which brings us to another big con at this point: The cost of changing the commit object format. One can argue for or against a new commit object format, but the simple truth at this point is that changing the structure of the commit object is expensive. Even if we were all in agreement about the change (and so far we are not), there are multiple Git implementations (libgit2, jgit, dulwich, etc.) that would all have to learn the new commit object, not to mention that bumping core.repositoryformatversion would probably make your git repo incompatible with a huge number of existing deployments for the foreseeable future. Therefore, the most pragmatic and constructive thing to do at this point, is IMHO to work within the confines of the existing commit object structure. I actually believe using commit message trailers like Made-on-branch: frotz in addition to some helpful infrastructure (hooks, templates, git-interpret-trailers, etc.) should get you pretty much exactly what you want. And if this feature turns out to be extremely useful for a lot of users, we can certainly consider changing the commit object format in the future. I know that for the workflows I personally have used in the past, such tagging would be very useful. Quite often I have been looking through the Git log and wondered what feature a commit was part of, because I have feature branches. Just knowing that branch name would be really useful, but the branch has since been deleted... and in the case of a ff-merge (which I thought was recommended in Git if possible), the branch name is completely gone. True. The branch name is - for better or worse - simply not considered very important by Git, and a Git commit is simply not considered (by Git at least) to be part of or otherwise belong to any branch. Instead the commit history/graph is what Git considers important, and the branch names are really just more-or-less ephemeral pointers into that graph. AFAIK, recording the current branch name in commits was not considered to the worth including in Linus' original design, and since then it seems to only have come up a few times on the mailing list. This is quite central to Git's design, and changing it at this point should not be done lightly. IINM, Mercurial does this differently, so that may be a better fit for the workflows where keeping track of branch names is very important. That said, you are of course free to add this information to your own commit messages, by appending something like Made-on-branch: frotz. In a company setting, you can even create a commit message template or (prepare-)commit-msg hook to have this line created automatically for you and your co-workers. You could even append such information retroactively to existing commits with git notes. There is also the current interpret-trailers effort by Christian Couder [1] that should be useful in creating and managing such lines. [1]: http://thread.gmane.org/gmane.comp.version-control.git/245874 Well I guess that's another way of doing it. So, why aren't Author and Date trailers? They don't seem any more fundamental to me than branch name. I mean the only checkin information you really *need* is the checksum, and commit's parents. The Author and Date are just extra pieces of information you might find useful sometimes, right? A bit like some people might find branch checkin name useful sometimes...? Yeah, sure. Author and Date (and Committer, for that
[PATCH v2] l10n: de.po: translate 45 new messages
Translate 45 new messages came from git.pot update in 5e078fc (l10n: git.pot: v2.0.0 round 1 (45 new, 28 removed)). Signed-off-by: Ralf Thielow ralf.thie...@gmail.com Acked-by: Thomas Rast t...@thomasrast.ch --- po/de.po | 127 +-- 1 file changed, 50 insertions(+), 77 deletions(-) diff --git a/po/de.po b/po/de.po index 59acf20..e8cae80 100644 --- a/po/de.po +++ b/po/de.po @@ -436,13 +436,13 @@ msgstr[1] vor %lu Jahren #, c-format msgid failed to read orderfile '%s' msgstr Fehler beim Lesen der Reihenfolgedatei '%s'. #: diffcore-rename.c:517 msgid Performing inexact rename detection -msgstr +msgstr Führe Erkennung für ungenaue Umbenennung aus #: diff.c:113 #, c-format msgid Failed to parse dirstat cut-off percentage '%s'\n msgstr Fehler beim Parsen des abgeschnittenen \dirstat\ Prozentsatzes '%s'\n @@ -964,29 +964,32 @@ msgid Perhaps you forgot to add either ':/' or '.' ? msgstr :(exclude) Muster, aber keine anderen Pfadspezifikationen angegeben.\n Vielleicht haben Sie vergessen entweder ':/' oder '.' hinzuzufügen? #: progress.c:224 -#, fuzzy msgid done -msgstr Fertig\n +msgstr Fertig #: read-cache.c:1238 #, c-format msgid index.version set, but the value is invalid.\n Using version %i msgstr +index.version gesetzt, aber Wert ungültig.\n +Verwende Version %i #: read-cache.c:1248 #, c-format msgid GIT_INDEX_VERSION set, but the value is invalid.\n Using version %i msgstr +GIT_INDEX_VERSION gesetzt, aber Wert ungültig.\n +Verwende Version %i #: remote.c:758 #, c-format msgid Cannot fetch both %s and %s to %s msgstr Kann 'fetch' nicht für sowohl %s als auch %s nach %s ausführen. @@ -1400,15 +1403,14 @@ msgstr Konnte git-Verweis %s nicht erstellen #: submodule.c:1132 #, c-format msgid Could not set core.worktree in %s msgstr Konnte core.worktree in '%s' nicht setzen. #: unpack-trees.c:206 -#, fuzzy msgid Checking out files -msgstr Prüfe Patch %s... +msgstr Checke Dateien aus #: urlmatch.c:120 msgid invalid URL scheme name or missing '://' suffix msgstr Ungültiges URL-Schema oder Suffix '://' fehlt #: urlmatch.c:144 urlmatch.c:297 urlmatch.c:356 @@ -1554,55 +1556,47 @@ msgstr von beiden hinzugefügt: #: wt-status.c:264 msgid both modified: msgstr von beiden geändert: #: wt-status.c:266 -#, fuzzy, c-format +#, c-format msgid bug: unhandled unmerged status %x -msgstr Fehler: unbehandelter Differenz-Status %c +msgstr Bug: unbehandelter Unmerged-Status %x #: wt-status.c:274 -#, fuzzy msgid new file: -msgstr neue Datei +msgstr neue Datei: #: wt-status.c:276 -#, fuzzy msgid copied: -msgstr kopiert +msgstr kopiert: #: wt-status.c:278 -#, fuzzy msgid deleted: -msgstr gelöscht +msgstr gelöscht: #: wt-status.c:280 -#, fuzzy msgid modified: -msgstr geändert +msgstr geändert: #: wt-status.c:282 -#, fuzzy msgid renamed: -msgstr umbenannt +msgstr umbenannt: #: wt-status.c:284 -#, fuzzy msgid typechange: -msgstr Typänderung +msgstr Typänderung: #: wt-status.c:286 -#, fuzzy msgid unknown: -msgstr unbekannt +msgstr unbekannt: #: wt-status.c:288 -#, fuzzy msgid unmerged: -msgstr nicht zusammengeführt +msgstr nicht gemerged: #: wt-status.c:370 msgid new commits, msgstr neue Commits, #: wt-status.c:372 @@ -3543,13 +3537,12 @@ msgstr setzt HEAD zu benanntem Commit #: builtin/checkout.c:1096 msgid set upstream info for new branch msgstr setzt Informationen zum Upstream-Branch für den neuen Branch #: builtin/checkout.c:1098 -#, fuzzy msgid new-branch msgstr neuer Branch #: builtin/checkout.c:1098 msgid new unparented branch msgstr neuer Branch ohne Eltern-Commit @@ -3791,22 +3784,20 @@ msgstr löscht nur ignorierte Dateien #: builtin/clean.c:903 msgid -x and -X cannot be used together msgstr Die Optionen -x und -X können nicht gemeinsam verwendet werden. #: builtin/clean.c:907 -#, fuzzy msgid clean.requireForce set to true and neither -i, -n, nor -f given; refusing to clean msgstr clean.requireForce auf \true\ gesetzt und weder -i, -n noch -f gegeben; \clean\ verweigert #: builtin/clean.c:910 -#, fuzzy msgid clean.requireForce defaults to true and neither -i, -n, nor -f given; refusing to clean msgstr clean.requireForce standardmäßig auf \true\ gesetzt und weder -i, -n noch - f gegeben; \clean\ verweigert @@ -4419,17 +4410,14 @@ msgstr #: builtin/commit.c:1129 msgid Clever... amending the last one with dirty index. msgstr Klug... den letzten Commit mit einer geänderten Staging-Area nachbessern. #: builtin/commit.c:1131 -#, fuzzy msgid Explicit paths specified without -i or -o; assuming --only paths... -msgstr -Explizite Pfade ohne -i oder -o angegeben; unter der Annahme von --only -Pfaden... +msgstr Explizite Pfade ohne -i oder -o angegeben; nehme --only an #: builtin/commit.c:1143 builtin/tag.c:639 #, c-format msgid Invalid cleanup mode %s msgstr Ungültiger \cleanup\ Modus %s @@
Re: [RFC/PATCH 1/2] trailer: fix to ignore any line starting with '#'
On 04/27/2014 10:12 PM, Christian Couder wrote: It looks like the commit-msg hook is passed a commit message that can contain lines starting with a '#'. Those comment lines will be removed from the commit message after the hook is run. If we want git interpret-trailers to be able to process commit messages correctly in the commit-msg hook we need to ignore those lines. Shouldn't this take into account the config setting core.commentchar? Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html