* [PATCH] gitk tag delete/rename support
From: Leon KUKOVEC @ 2012-11-25 19:05 UTC (permalink / raw)
To: git; +Cc: Paul Mackerras, Leon KUKOVEC
In-Reply-To: <CAOrOd9woDs16as+t-EReQ8NtXfYm9mpd0XaFFs-XL=pg+JK1Lw@mail.gmail.com>
Right clicking on a tag pops up a menu, which allows
tag to be renamed or deleted.
Signed-off-by: Leon KUKOVEC <leon.kukovec@gmail.com>
---
gitk-git/gitk | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 154 insertions(+)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index d93bd99..38cc233 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2032,6 +2032,7 @@ proc makewindow {} {
global have_tk85 use_ttk NS
global git_version
global worddiff
+ global tagctxmenu
# The "mc" arguments here are purely so that xgettext
# sees the following string as needing to be translated
@@ -2581,6 +2582,13 @@ proc makewindow {} {
{mc "Run git gui blame on this line" command {external_blame_diff}}
}
$diff_menu configure -tearoff 0
+
+ set tagctxmenu .tagctxmenu
+ makemenu $tagctxmenu {
+ {mc "Rename this tag" command mvtag}
+ {mc "Delete this tag" command rmtag}
+ }
+ $tagctxmenu configure -tearoff 0
}
# Windows sends all mouse wheel events to the current focused window, not
@@ -6400,6 +6408,7 @@ proc drawtags {id x xt y1} {
-font $font -tags [list tag.$id text]]
if {$ntags >= 0} {
$canv bind $t <1> [list showtag $tag_quoted 1]
+ $canv bind $t $ctxbut [list showtagmenu %X %Y $id $tag_quoted]
} elseif {$nheads >= 0} {
$canv bind $t $ctxbut [list headmenu %X %Y $id $tag_quoted]
}
@@ -8931,6 +8940,113 @@ proc domktag {} {
return 1
}
+proc mvtag {} {
+ global mvtagtop
+ global tagmenuid tagmenutag tagctxmenu maintag NS
+ global mvtagtag
+
+ set mvtagtag $tagmenutag
+ set top .movetag
+ set mvtagtop $top
+ catch {destroy $top}
+ ttk_toplevel $top
+ make_transient $top .
+
+ ${NS}::label $top.msg -text [mc "Enter a new tag name:"]
+ ${NS}::entry $top.tag -width 60 -textvariable mvtagtag
+
+ grid $top.msg -sticky w -row 0 -column 0
+ grid $top.tag -sticky w -row 0 -column 1
+
+ ${NS}::frame $top.buts
+ ${NS}::button $top.buts.gen -text [mc "Rename"] -command mvtaggo
+ ${NS}::button $top.buts.can -text [mc "Cancel"] -command mvtagcan
+ bind $top <Key-Return> mvtaggo
+ bind $top <Key-Escape> mvtagcan
+ grid $top.buts.gen $top.buts.can
+ grid columnconfigure $top.buts 0 -weight 1 -uniform a
+ grid columnconfigure $top.buts 1 -weight 1 -uniform a
+ grid $top.buts - -pady 10 -sticky ew
+}
+
+proc domvtag {} {
+ global mvtagtop env tagids idtags tagmenutag tagmenuid mvtagtag
+
+ set tag $mvtagtag
+ set id $tagmenuid
+
+ # add tag
+ # XXX: reuse domktag including keeping comment from the original tag.
+ if {[catch {
+ exec git tag $tag $id
+ } err]} {
+ error_popup "[mc "Error renaming tag:"] $err" $mvtagtop
+ return 0
+ }
+
+ # delete old tag, content stored in $tagmenutag and $tagmenuid
+ dormtag
+
+ set tagids($tag) $id
+ lappend idtags($id) $tag
+ redrawtags $id
+ addedtag $id
+ dispneartags 0
+ run refill_reflist
+ return 1
+}
+
+proc rmtag {} {
+ global rmtagtop
+ global tagmenuid tagmenutag tagctxmenu maintag NS
+
+ set top .maketag
+ set rmtagtop $top
+ catch {destroy $top}
+ ttk_toplevel $top
+ make_transient $top .
+ ${NS}::label $top.title -text [mc "Delete tag"]
+ grid $top.title - -pady 10
+
+ ${NS}::label $top.msg -text [mc "You are about to delete a tag"]
+ ${NS}::label $top.tagname -foreground Red -text [mc "$tagmenutag"]
+ grid $top.msg -sticky w -row 0 -column 0
+ grid $top.tagname -sticky w -row 0 -column 1
+
+ ${NS}::frame $top.buts
+ ${NS}::button $top.buts.gen -text [mc "Delete"] -command rmtaggo
+ ${NS}::button $top.buts.can -text [mc "Cancel"] -command rmtagcan
+ bind $top <Key-Return> rmtaggo
+ bind $top <Key-Escape> rmtagcan
+ grid $top.buts.gen $top.buts.can
+ grid columnconfigure $top.buts 0 -weight 1 -uniform a
+ grid columnconfigure $top.buts 1 -weight 1 -uniform a
+ grid $top.buts - -pady 10 -sticky ew
+}
+
+proc dormtag {} {
+ global rmtagtop env tagids idtags tagmenutag tagmenuid
+
+ set tag $tagmenutag
+ set id $tagmenuid
+
+ if {[catch {
+ exec git tag -d $tag
+ } err]} {
+ error_popup "[mc "Error deleting tag:"] $err" $rmtagtop
+ return 0
+ }
+
+ unset tagids($tag)
+ set idx [lsearch $idtags($id) $tag]
+ set idtags($id) [lreplace $idtags($id) $idx $idx]
+
+ redrawtags $id
+ dispneartags 0
+ run refill_reflist
+ return 1
+}
+
proc redrawtags {id} {
global canv linehtag idpos currentid curview cmitlisted markedid
global canvxmax iddrawn circleitem mainheadid circlecolors
@@ -8974,6 +9090,30 @@ proc mktaggo {} {
mktagcan
}
+proc rmtagcan {} {
+ global rmtagtop
+
+ catch {destroy $rmtagtop}
+ unset rmtagtop
+}
+
+proc rmtaggo {} {
+ if {![dormtag]} return
+ rmtagcan
+}
+
+proc mvtagcan {} {
+ global mvtagtop
+
+ catch {destroy $mvtagtop}
+ unset mvtagtop
+}
+
+proc mvtaggo {} {
+ if {![domvtag]} return
+ mvtagcan
+}
+
proc writecommit {} {
global rowmenuid wrcomtop commitinfo wrcomcmd NS
@@ -9288,6 +9428,20 @@ proc headmenu {x y id head} {
tk_popup $headctxmenu $x $y
}
+# context menu for a tag
+proc showtagmenu {x y id tag} {
+ global tagmenuid tagmenutag tagctxmenu maintag
+
+ stopfinding
+ set tagmenuid $id
+ set tagmenutag $tag
+ set state normal
+
+ $tagctxmenu entryconfigure 0 -state normal
+ $tagctxmenu entryconfigure 1 -state normal
+ tk_popup $tagctxmenu $x $y
+}
+
proc cobranch {} {
global headmenuid headmenuhead headids
global showlocalchanges
--
1.7.9.5
^ permalink raw reply related
* Re: Python extension commands in git - request for policy change
From: Felipe Contreras @ 2012-11-25 21:22 UTC (permalink / raw)
To: esr; +Cc: Nguyen Thai Ngoc Duy, git, msysGit
In-Reply-To: <20121125175051.GD32394@thyrsus.com>
On Sun, Nov 25, 2012 at 6:50 PM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com>:
>> Not according to ohloh:
>>
>> 1) shell 33%
>> 2) tcl 9%
>> 3) perl 9.7%
>>
>> 4) python 1.8%
>
> Look in the Makefile - all that tcl code is buried in gitk. We're
> very, very lucky the author did such a good job, because it's a
> potentially serious headache; who can maintain it?
And gitk is an integral part of git. But if you have different
numbers, what are they?
--
Felipe Contreras
^ permalink raw reply
* Re: Python extension commands in git - request for policy change
From: Felipe Contreras @ 2012-11-25 21:25 UTC (permalink / raw)
To: esr; +Cc: Michael Haggerty, git
In-Reply-To: <20121125173607.GB32394@thyrsus.com>
On Sun, Nov 25, 2012 at 6:36 PM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com>:
>> Of course, but there are experts in C and shell around, not so many
>> python experts. So if somebody sneaks in a python program that makes
>> use of features specific to python 2.7, I doubt anybody would notice.
>
> I would.
And are you going to be around to spot them? It seems my patches for
git-remote-hg slipped by your watch, because it seems they use stuff
specific to python 2.7.
>> And if they did, I doubt that would be reason enough for rejection,
>> supposing that porting to 2.6 would be difficult enough.
>
> In cases like that, backporting is usually pretty easy. Been there, done that.
Exactly. Why would you reject something you can fix easily?
--
Felipe Contreras
^ permalink raw reply
* [PATCH] Document the integration requirements for extension commands.
From: Eric S. Raymond @ 2012-11-25 21:35 UTC (permalink / raw)
To: git
This contains no policy changes or proposals, it simply attempts
to document the interfaces and conventions already in place.
---
Documentation/technical/api-command.txt | 81 +++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
create mode 100644 Documentation/technical/api-command.txt
diff --git a/Documentation/technical/api-command.txt b/Documentation/technical/api-command.txt
new file mode 100644
index 0000000..de76614
--- /dev/null
+++ b/Documentation/technical/api-command.txt
@@ -0,0 +1,81 @@
+= Integrating new subcommands =
+
+This is how-to documentation for people who want to add extension
+commands to git. It should be read alongside api-builtin.txt.
+
+== Runtime environment ==
+
+git subcommands are standalone executables that live in the git
+execution directory, normally /usr/lib/git-core. The git executable itself
+is a thin wrapper that sets GIT_DIR and passes command-line arguments
+to the subcommand.
+
+(If "git foo" is not found in the git execution directory, the wrapper
+will look in the rest of your $PATH for it. Thus, it's possible
+to write local git extensions that don't live in system space.)
+
+== Implementation languages ==
+
+Most subcommands are written in C or shell. A few are written in
+Perl. A tiny minority are written in Python.
+
+While we strongly encourage coding in portable C for portability, these
+specific scripting languages are also acceptable. We won't accept more
+without a very strong technical case, as we don't want to broaden the
+git suite's required dependencies.
+
+C commands are normally written as single modules, named after the
+command, that link a core library called libgit. Thus, your command
+'git-foo' would normally be implemented as a single "git-foo.c"; this
+organization makes it easy for people reading the code to find things.
+
+See the CodingGuidelines document for other guidance on what we consider
+good practice in C and shell, and api-builtin.txt for the support
+functions available to built-in commands written in C.
+
+== What every extension command needs ==
+
+You must have a man page, written in asciidoc (this is what git help
+followed by your subcommand name will display). Be aware that there is
+a local asciidoc configuration and macros which you should use. It's
+often helpful to start by cloning an existing page and replacing the
+text content.
+
+You must have a test, written to report in TAP (Test Anything Protocol).
+Tests are executables (usually shell scripts) that live in the 't'
+subdirectory of the tree. Each test name begins with 't' and a sequence
+number that controls where in the test sequence it will be executed;
+conventionally the rest of the name stem is that of the command
+being tested.
+
+Read the file t/README to learn more about the conventions to be used
+in writing tests, and the test support library.
+
+== Integrating a command ==
+
+Here are the things you need to do when you want to merge a new
+subcommand into the git tree.
+
+1. Append your command name to one of the variables BUILTIN_OBJS,
+EXTRA_PROGRAMS, SCRIPT_SH, SCRIPT_PERL or SCRIPT_PYTHON.
+
+2. Drop its test in the t directory.
+
+3. If your command is implemented in an interpreted language with a
+p-code intermediate form, make sure .gitignore in the main directory
+includes a pattern entry that ignores such files. Python .pyc and
+.pyo files will already be covered.
+
+4. If your command has any dependency on a a particular version of
+your language, document it in the INSTALL file.
+
+5. There is a file command-list.txt in the distribution main directory
+that categorizes commands by type, so they can be listed in appropriate
+subsections in the documentation's summary command list. Add an entry
+for yours. To understand the categories, look at git-cmmands.txt
+in the main directory.
+
+6. When your patch is merged, remind the maintainer to add something
+about it in the RelNotes file.
+
+That's all there is to it.
--
1.7.9.5
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
"Rightful liberty is unobstructed action, according to our will, within limits
drawn around us by the equal rights of others."
-- Thomas Jefferson
^ permalink raw reply related
* Re: Python extension commands in git - request for policy change
From: Krzysztof Mazur @ 2012-11-25 21:41 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20121125024451.1ADD14065F@snark.thyrsus.com>
On Sat, Nov 24, 2012 at 09:44:51PM -0500, Eric S. Raymond wrote:
>
> We're behind the best-practices curve here. The major Linux
> distributions, which have to deal with almost the same set of
> tradeoffs we do, went to Python for pretty much all glue and
> administration scripts outside /etc a decade ago, and the decision has
> served them well.
>
> That, among other things, means up-to-date versions of Python are
> ubiquitous unless we're looking at Windows - in which case Perl and
> shell actually become much bigger portability problems. Mac OS X
> has kept up to date, too; Lion shipped 2.7.1 and that was a major
> release back at this point.
>
What about embedded systems? git is also useful there. C and shell is
everywhere, python is not. Adding additional dependency if it's not
really needed it's not a good idea.
Also not everyone uses up-to-date systems and sometimes you just
care about some critical parts and do not touch everything else and
there is probably quote large number of systems with python < 2.6.
And even when you keep your system up-to-date, there are some GNU/Linux
distros that are still supported, but does not provide recent python - for
instance PLD Ac, which I still use on some systems and will use
until the hardware dies, provides only python 2.4.6 (by the way,
important packages like git are of course quite recent there - 1.7.11.1).
Krzysiek
^ permalink raw reply
* Re: Python extension commands in git - request for policy change
From: Felipe Contreras @ 2012-11-25 21:43 UTC (permalink / raw)
To: esr; +Cc: git
In-Reply-To: <20121125173229.GA32394@thyrsus.com>
On Sun, Nov 25, 2012 at 6:32 PM, Eric S. Raymond <esr@thyrsus.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com>:
>> Seems sensible, but I don't know what "rejection" would actually mean.
>
> Why is this mysterious? We reject a patch when we don't choose to merge it.
Why would you reject it? If, according to you, it's very simple to fix
the portability, then presumably it would take you less time to fix
it, than to reject it (and everything that implies).
>> Too late.
>
> I'd be happy to help you out by auditing them for version dependencies.
Be my guest:
http://git.kernel.org/?p=git/git.git;a=tree;f=contrib/remote-helpers;h=adfdcc164e634c74024c8f69bb0cdb9f3b4a9f18;hb=7b4a70c62f3a83fbd8b44bf712141754a5f64205
Some patches might be missing, so:
https://github.com/felipec/git/tree/fc/remote/hg
>> I don't see what this means in practical terms. People are going to
>> write code in whatever language they want to write code in. How
>> exactly are "we" going to "encourage" them not to do that is not
>> entirely clear to me.
>
> One way is by having clear guidelines for good practice that *include*
> Python, and tell people exactly what the requirements are.
The key word being guideline, which is different from a strict rule.
>> Subcommands are also probably more efficient in c. And lets remember
>> that most people use git through the *official* subcommands.
>
> See my remarks on the 80-20 rule elsewhere in the thread. Execessive
> worship of "efficiency" is a great way to waste effort and pile up
> hidden costs in maintainance problems.
According to the results of the last survey, our users do care about
performance, so I don't think there's anything excessive about it. Are
there any hidden costs in maintenance problems? I don't think so.
The people that like to improve the performance of git, would keep
doing so, and the people that want to use fancy scripts to do fancy
stuff, will keep doing so. It just happens that the former have
actually managed to do it, and go all the way into the mainline.
It would be great if we had a finished libgit2 with all the essential
stuff, and good bindings for python (and other languages), and it
would be great if python really was this touted language, that is easy
to read, and would make things more maintainable. Unfortunately,
that's not the case.
I could write an endless list of what things in the python language
don't make any sense, and how in ruby, for example, they do.
Fortunately, I don't have to.
Git does have problems, but they have nothing to do with maintenance,
or C; they have to do with the user interface, and the documentation
(again, according to our users (and me)). So, I don't see why worry
about moving code from C to python when barely any code in git is
python, specially if it doesn't fix any real issue.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: Python extension commands in git - request for policy change
From: Eric S. Raymond @ 2012-11-25 21:56 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Nguyen Thai Ngoc Duy, git, msysGit
In-Reply-To: <CAMP44s3QNG-sxcZsWmL3RYjXkzOwerj2774t7Abh04A7QR6TCA@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com>:
> And gitk is an integral part of git. But if you have different
> numbers, what are they?
I looked at the Makefile. I saw that there are shell variables that collect
C commands, shell command, Perl commands, and Python commands. There are no
collections of other commands. That makes them the top languages in the
universe we are concerned about
Please don't waste further time on quibbling. We all know that gitk is
an uncomfortable special case and that the project would be far better
off, maintainability-wise, if it were successfully ported to one if these
other languages. Trying to catch me out by triumphantly pointing at gitk
is...juvenile.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
^ permalink raw reply
* Re: Python extension commands in git - request for policy change
From: Eric S. Raymond @ 2012-11-25 22:11 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Michael Haggerty, git
In-Reply-To: <CAMP44s2fSpL13kDAm9W2ti-MERpKukNzNZ_Yt0oOOWMYOQPr2Q@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com>:
> And are you going to be around to spot them? It seems my patches for
> git-remote-hg slipped by your watch, because it seems they use stuff
> specific to python 2.7.
The dev group hasn't decided (in whatever way it decides these
things) to require 2.6 yet. When and if it does, I will volunteer my
services as a Python expert to audit the in-tree Python code for 2.6
conformance and assist the developers in backporting if required.
I will also make myself available to audit future submissions.
I think you know who I am. Junio and the other senior devs certainly
know where to find me. I've been making promises like this, and
*keeping* them, for decades. Please stop wasting our time with
petulant display.
> Exactly. Why would you reject something you can fix easily?
I wouldn't. The point of a policy like this is not to kick incoming
submissions over the horizon as though that were some sort of
accomplishment, it's to let submitters know what is required of
them so they can code up to a standard that supports maintainability.
It would be no different than any of our other portability requirements.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: Python extension commands in git - request for policy change
From: Eric S. Raymond @ 2012-11-25 22:44 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s2ft7vvaGqHUa2CytpAsX8vOF3YQo24PLPsD6y1Dk3GZQ@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com>:
> > I'd be happy to help you out by auditing them for version dependencies.
>
> Be my guest:
> http://git.kernel.org/?p=git/git.git;a=tree;f=contrib/remote-helpers;h=adfdcc164e634c74024c8f69bb0cdb9f3b4a9f18;hb=7b4a70c62f3a83fbd8b44bf712141754a5f64205
>
> Some patches might be missing, so:
> https://github.com/felipec/git/tree/fc/remote/hg
OK, here's what I look for: use of argparse, use of unittest, use
of Collections.counters, use or ordered dictionaries, use of set literals,
use of multiple context managers in one "with", use of memoryview, use of
the comma format specifier. I'm not worried about the changes in repr()
for floating point; I'd be astonished if they mattered in code like this.
Likewise for PyCapsule and importlib.
I don't see obvious problems in that code. Looks pretty vanilla, actually;
the latest version-related blocker I can see is the import of json,
which would have been a problem before 2.5.
You wrote the code. Do you *know* of 2.7-specific constructions in
there that I've missed? If you do, and think of this as a way to
catch me in a mistake and dance triumphantly, you lose - our goal
should be to cooperate to improve the auditing process, not score
silly points.
> > One way is by having clear guidelines for good practice that *include*
> > Python, and tell people exactly what the requirements are.
>
> The key word being guideline, which is different from a strict rule.
Agreed. It's a matter for the dev group to decide when we need rules
and when we need guidelines. I think we need a rule about Python version
conformance that protects older systems, but other things can be guidelines.
> According to the results of the last survey, our users do care about
> performance, so I don't think there's anything excessive about it. Are
> there any hidden costs in maintenance problems? I don't think so.
Then you're either pretending or very naive. Three decades of
experience as a C programmer tells me that C code at any volume is a
*serious* maintainance problem relative to almost any language with
GC. Prudent architects confine it is much as possible.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: Python extension commands in git - request for policy change
From: Eric S. Raymond @ 2012-11-25 22:47 UTC (permalink / raw)
To: Krzysztof Mazur; +Cc: git
In-Reply-To: <20121125214139.GA29465@shrek.podlesie.net>
Krzysztof Mazur <krzysiek@podlesie.net>:
> What about embedded systems? git is also useful there. C and shell is
> everywhere, python is not.
Supposing this is true (and I question it with regard to shell) if you
tell me how you live without gitk and the Perl pieces I'll play that
right back at you as your answer.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* [PATCH 00/11] alternative unify appending of sob
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey
From: Brandon Casey <bcasey@nvidia.com>
I hate to have the battle of the patches, but I kinda prefer the
append_signoff code in sequencer.c over the code in log-tree.c as a
foundation to build on.
So, this series is similar to Duy's "unification" series, but it goes in the
opposite direction and builds on top of sequencer.c and additionally adds the
elements from my original series to treat the "(cherry picked from..." line
added by 'cherry-pick -x' in the same way that other footer elements are
treated (after addressing Junio's comments about rfc2822 continuation lines
and duplicate s-o-b's).
I've integrated Duy's series with a few minor tweaks. I added a couple
of additional tests to t4014 and corrected one of the tests which had
incorrect behavior. I think his sign-off's should still be valid, so I
kept them in. Sorry that I've been slow, and now the two of us are stepping
on each other's toes, but Duy please take a look and let me know if there's
anything you dislike.
The main difference between this series and Duy's, aside from the change in
behavior with respect to a "(cherry picked from...)" line, is that the
detection of a s-o-b footer is done in a somewhat more generic way. The
log-tree.c code expects to find something that looks like "[-A-Za-z]+:" on
the left-hand side, and something that looks like an email address on the
right-hand side. The sequencer.c code relaxes this definition so that an
email address is not required on the right hand side. This allows lines like
"Change-id: IXXXX" or "Bug: XXX" to be considered valid footer elements.
So, this series should result in s-o-b and "(cherry picked from...)" being
appended to commit messages in a more consistent and deterministic way. For
example, the decision about whether to insert a blank line before appending
a s-o-b. As discussed, cherry-pick and commit will only refrain from
appending a s-o-b if the committer's s-o-b appears as the last one in a
conforming footer, while format-patch will refrain from appending it if it
appears anywhere within the footer.
-Brandon
Brandon Casey (8):
sequencer.c: remove broken support for rfc2822 continuation in footer
t/test-lib-functions.sh: allow to specify the tag name to test_commit
t/t3511: add some tests of 'cherry-pick -s' functionality
sequencer.c: recognize "(cherry picked from ..." as part of s-o-b
footer
sequencer.c: always separate "(cherry picked from" from commit body
sequencer.c: teach append_signoff how to detect duplicate s-o-b
sequencer.c: teach append_signoff to avoid adding a duplicate newline
Unify appending signoff in format-patch, commit and sequencer
Nguyễn Thái Ngọc Duy (3):
t4014: more tests about appending s-o-b lines
format-patch: stricter S-o-b detection
format-patch: update append_signoff prototype
builtin/commit.c | 2 +-
builtin/log.c | 13 +--
log-tree.c | 92 ++---------------
revision.h | 2 +-
sequencer.c | 133 +++++++++++++++---------
sequencer.h | 2 +-
t/t3511-cherry-pick-x.sh | 219 +++++++++++++++++++++++++++++++++++++++
t/t4014-format-patch.sh | 262 +++++++++++++++++++++++++++++++++++++++++++++++
t/test-lib-functions.sh | 9 +-
9 files changed, 580 insertions(+), 154 deletions(-)
create mode 100755 t/t3511-cherry-pick-x.sh
--
1.8.0.284.g959048a
^ permalink raw reply
* [PATCH 01/11] sequencer.c: remove broken support for rfc2822 continuation in footer
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>
Commit c1e01b0c generalized the detection of the last paragraph
signed-off-by footer and used rfc2822 as a guideline. Support for rfc2822
style continuation lines was also implemented, but not correctly, so it has
never detected a line beginning with space or tab as a continuation of the
previous line.
Since a commit message is not governed by the line length limits imposed
by rfc2822 for email messages, and it does not seem like this functionality
would produce "better" commit messages anyway, let's remove this broken
functionality.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
sequencer.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 2260490..656df6b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1023,7 +1023,6 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
int hit = 0;
int i, j, k;
int len = sb->len - ignore_footer;
- int first = 1;
const char *buf = sb->buf;
for (i = len - 1; i > 0; i--) {
@@ -1040,11 +1039,6 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
; /* do nothing */
k++;
- if ((buf[k] == ' ' || buf[k] == '\t') && !first)
- continue;
-
- first = 0;
-
for (j = 0; i + j < len; j++) {
ch = buf[i + j];
if (ch == ':')
--
1.8.0.284.g959048a
^ permalink raw reply related
* [PATCH 02/11] t/test-lib-functions.sh: allow to specify the tag name to test_commit
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>
The <message> part of test_commit() may not be appropriate for a tag name.
So let's allow test_commit to accept a fourth argument to specify the tag
name.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
t/test-lib-functions.sh | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8889ba5..9e2b8b8 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -135,12 +135,13 @@ test_pause () {
fi
}
-# Call test_commit with the arguments "<message> [<file> [<contents>]]"
+# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
#
# This will commit a file with the given contents and the given commit
-# message. It will also add a tag with <message> as name.
+# message. It will also add a tag with <message> as name unless <tag> is
+# given.
#
-# Both <file> and <contents> default to <message>.
+# <file>, <contents>, and <tag> all default to <message>.
test_commit () {
notick= &&
@@ -168,7 +169,7 @@ test_commit () {
test_tick
fi &&
git commit $signoff -m "$1" &&
- git tag "$1"
+ git tag "${4:-$1}"
}
# Call test_merge with the arguments "<message> <commit>", where <commit>
--
1.8.0.284.g959048a
^ permalink raw reply related
* [PATCH 03/11] t/t3511: add some tests of 'cherry-pick -s' functionality
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>
Add some tests to ensure that 'cherry-pick -s' operates in the following
manner:
* Inserts a blank line before appending a s-o-b to a commit message that
does not contain a s-o-b footer
* Does not mistake first line "subject: description" as a s-o-b footer
* Does not mistake single word message body as conforming to rfc2822
* Appends a s-o-b when last s-o-b in footer does not match committer
s-o-b, even when committer's s-o-b exists elsewhere in footer.
* Does not append a s-o-b when last s-o-b matches committer s-o-b
* Correctly detects a non-conforming footer containing a mix of s-o-b
like elements and s-o-b elements. (marked "expect failure")
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
t/t3511-cherry-pick-x.sh | 111 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 111 insertions(+)
create mode 100755 t/t3511-cherry-pick-x.sh
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
new file mode 100755
index 0000000..a6c4168
--- /dev/null
+++ b/t/t3511-cherry-pick-x.sh
@@ -0,0 +1,111 @@
+#!/bin/sh
+
+test_description='Test cherry-pick -x and -s'
+
+. ./test-lib.sh
+
+pristine_detach () {
+ git cherry-pick --quit &&
+ git checkout -f "$1^0" &&
+ git read-tree -u --reset HEAD &&
+ git clean -d -f -f -q -x
+}
+
+mesg_one_line='base: commit message'
+
+mesg_no_footer="$mesg_one_line
+
+OneWordBodyThatsNotA-S-o-B"
+
+mesg_with_footer="$mesg_no_footer
+
+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+Signed-off-by: A.U. Thor <author@example.com>
+Signed-off-by: B.U. Thor <buthor@example.com>"
+
+mesg_broken_footer="$mesg_no_footer
+
+Signed-off-by: B.U. Thor <buthor@example.com>
+Not a valid footer element
+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+
+mesg_with_footer_sob="$mesg_with_footer
+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+
+
+test_expect_success setup '
+ git config advice.detachedhead false &&
+ echo unrelated >unrelated &&
+ git add unrelated &&
+ test_commit initial foo a &&
+ test_commit "$mesg_one_line" foo b mesg-one-line &&
+ git reset --hard initial &&
+ test_commit "$mesg_no_footer" foo b mesg-no-footer &&
+ git reset --hard initial &&
+ test_commit "$mesg_broken_footer" foo b mesg-broken-footer &&
+ git reset --hard initial &&
+ test_commit "$mesg_with_footer" foo b mesg-with-footer &&
+ git reset --hard initial &&
+ test_commit "$mesg_with_footer_sob" foo b mesg-with-footer-sob &&
+ pristine_detach initial &&
+ test_commit conflicting unrelated
+'
+
+test_expect_success 'cherry-pick -s inserts blank line after one line subject' '
+ pristine_detach initial &&
+ git cherry-pick -s mesg-one-line &&
+ cat <<-EOF >expect &&
+ $mesg_one_line
+
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_failure 'cherry-pick -s inserts blank line after non-conforming footer' '
+ pristine_detach initial &&
+ git cherry-pick -s mesg-broken-footer &&
+ cat <<-EOF >expect &&
+ $mesg_broken_footer
+
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s inserts blank line when conforming footer not found' '
+ pristine_detach initial &&
+ git cherry-pick -s mesg-no-footer &&
+ cat <<-EOF >expect &&
+ $mesg_no_footer
+
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s adds sob when last sob doesnt match committer' '
+ pristine_detach initial &&
+ git cherry-pick -s mesg-with-footer &&
+ cat <<-EOF >expect &&
+ $mesg_with_footer
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob' '
+ pristine_detach initial &&
+ git cherry-pick -s mesg-with-footer-sob &&
+ cat <<-EOF >expect &&
+ $mesg_with_footer_sob
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_done
--
1.8.0.284.g959048a
^ permalink raw reply related
* [PATCH 04/11] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>
When 'cherry-pick -s' is used to append a signed-off-by line to a cherry
picked commit, it does not currently detect the "(cherry picked from..."
that may have been appended by a previous 'cherry-pick -x' as part of the
s-o-b footer and it will insert a blank line before appending a new s-o-b.
Let's detect "(cherry picked from...)" as part of the footer so that we
will produce this:
Signed-off-by: A U Thor <author@example.com>
(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
Signed-off-by: C O Mitter <committer@example.com>
instead of this:
Signed-off-by: A U Thor <author@example.com>
(cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)
Signed-off-by: C O Mitter <committer@example.com>
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
sequencer.c | 42 ++++++++++++++++++++++++------------
t/t3511-cherry-pick-x.sh | 55 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 84 insertions(+), 13 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 656df6b..19eaf11 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -18,6 +18,7 @@
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
const char sign_off_header[] = "Signed-off-by: ";
+static const char cherry_picked_prefix[] = "(cherry picked from commit ";
static void remove_sequencer_state(void)
{
@@ -492,7 +493,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
if (opts->record_origin) {
- strbuf_addstr(&msgbuf, "(cherry picked from commit ");
+ strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
strbuf_addstr(&msgbuf, ")\n");
}
@@ -1017,11 +1018,32 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
}
-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
+static int is_rfc2822_line(const char *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++) {
+ int ch = buf[i];
+ if (ch == ':')
+ break;
+ if (isalnum(ch) || (ch == '-'))
+ continue;
+ return 0;
+ }
+
+ return 1;
+}
+
+static int is_cherry_pick_from_line(const char *buf, int len)
+{
+ return (strlen(cherry_picked_prefix) + 41) <= len &&
+ !prefixcmp(buf, cherry_picked_prefix);
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
{
- int ch;
int hit = 0;
- int i, j, k;
+ int i, k;
int len = sb->len - ignore_footer;
const char *buf = sb->buf;
@@ -1039,15 +1061,9 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)
; /* do nothing */
k++;
- for (j = 0; i + j < len; j++) {
- ch = buf[i + j];
- if (ch == ':')
- break;
- if (isalnum(ch) ||
- (ch == '-'))
- continue;
+ if (!(is_rfc2822_line(buf+i, k-i) ||
+ is_cherry_pick_from_line(buf+i, k-i)))
return 0;
- }
}
return 1;
}
@@ -1064,7 +1080,7 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer)
for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
; /* do nothing */
if (prefixcmp(msgbuf->buf + i, sob.buf)) {
- if (!i || !ends_rfc2822_footer(msgbuf, ignore_footer))
+ if (!i || !has_conforming_footer(msgbuf, ignore_footer))
strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);
}
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index a6c4168..32c0bb1 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -32,6 +32,10 @@ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
mesg_with_footer_sob="$mesg_with_footer
Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+mesg_with_cherry_footer="$mesg_with_footer_sob
+(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
+Tested-by: C.U. Thor <cuthor@example.com>"
+
test_expect_success setup '
git config advice.detachedhead false &&
@@ -47,6 +51,8 @@ test_expect_success setup '
test_commit "$mesg_with_footer" foo b mesg-with-footer &&
git reset --hard initial &&
test_commit "$mesg_with_footer_sob" foo b mesg-with-footer-sob &&
+ git reset --hard initial &&
+ test_commit "$mesg_with_cherry_footer" foo b mesg-with-cherry-footer &&
pristine_detach initial &&
test_commit conflicting unrelated
'
@@ -98,6 +104,19 @@ test_expect_success 'cherry-pick -s adds sob when last sob doesnt match committe
test_cmp expect actual
'
+test_expect_success 'cherry-pick -x -s adds sob when last sob doesnt match committer' '
+ pristine_detach initial &&
+ sha1=`git rev-parse mesg-with-footer^0` &&
+ git cherry-pick -x -s mesg-with-footer &&
+ cat <<-EOF >expect &&
+ $mesg_with_footer
+ (cherry picked from commit $sha1)
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob' '
pristine_detach initial &&
git cherry-pick -s mesg-with-footer-sob &&
@@ -108,4 +127,40 @@ test_expect_success 'cherry-pick -s refrains from adding duplicate trailing sob'
test_cmp expect actual
'
+test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists for committer' '
+ pristine_detach initial &&
+ sha1=`git rev-parse mesg-with-footer-sob^0` &&
+ git cherry-pick -x -s mesg-with-footer-sob &&
+ cat <<-EOF >expect &&
+ $mesg_with_footer_sob
+ (cherry picked from commit $sha1)
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -x treats "(cherry picked from..." line as part of footer' '
+ pristine_detach initial &&
+ sha1=`git rev-parse mesg-with-cherry-footer^0` &&
+ git cherry-pick -x mesg-with-cherry-footer &&
+ cat <<-EOF >expect &&
+ $mesg_with_cherry_footer
+ (cherry picked from commit $sha1)
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'cherry-pick -s treats "(cherry picked from..." line as part of footer' '
+ pristine_detach initial &&
+ git cherry-pick -s mesg-with-cherry-footer &&
+ cat <<-EOF >expect &&
+ $mesg_with_cherry_footer
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.8.0.284.g959048a
^ permalink raw reply related
* [PATCH 05/11] sequencer.c: always separate "(cherry picked from" from commit body
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>
Start treating the "(cherry picked from" line added by cherry-pick -x
the same way that the s-o-b lines are treated. Namely, separate them
from the main commit message body with an empty line.
Introduce tests to test this functionality.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
sequencer.c | 106 +++++++++++++++++++++++++----------------------
t/t3511-cherry-pick-x.sh | 53 ++++++++++++++++++++++++
2 files changed, 109 insertions(+), 50 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 19eaf11..7c0852a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -20,6 +20,60 @@
const char sign_off_header[] = "Signed-off-by: ";
static const char cherry_picked_prefix[] = "(cherry picked from commit ";
+static int is_rfc2822_line(const char *buf, int len)
+{
+ int i;
+
+ for (i = 0; i < len; i++) {
+ int ch = buf[i];
+ if (ch == ':')
+ break;
+ if (isalnum(ch) || (ch == '-'))
+ continue;
+ return 0;
+ }
+
+ return 1;
+}
+
+static int is_cherry_pick_from_line(const char *buf, int len)
+{
+ return (strlen(cherry_picked_prefix) + 41) <= len &&
+ !prefixcmp(buf, cherry_picked_prefix);
+}
+
+static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+{
+ int hit = 0;
+ int i, k;
+ int len = sb->len - ignore_footer;
+ const char *buf = sb->buf;
+
+ for (i = len - 1; i > 0; i--) {
+ if (hit && buf[i] == '\n')
+ break;
+ hit = (buf[i] == '\n');
+ }
+
+ /* require at least one blank line */
+ if (!hit || buf[i] != '\n')
+ return 0;
+
+ while (i < len - 1 && buf[i] == '\n')
+ i++;
+
+ for (; i < len; i = k) {
+ for (k = i; k < len && buf[k] != '\n'; k++)
+ ; /* do nothing */
+ k++;
+
+ if (!(is_rfc2822_line(buf+i, k-i) ||
+ is_cherry_pick_from_line(buf+i, k-i)))
+ return 0;
+ }
+ return 1;
+}
+
static void remove_sequencer_state(void)
{
struct strbuf seq_dir = STRBUF_INIT;
@@ -493,6 +547,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
if (opts->record_origin) {
+ if (!has_conforming_footer(&msgbuf, 0))
+ strbuf_addch(&msgbuf, '\n');
strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
strbuf_addstr(&msgbuf, ")\n");
@@ -1018,56 +1074,6 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
}
-static int is_rfc2822_line(const char *buf, int len)
-{
- int i;
-
- for (i = 0; i < len; i++) {
- int ch = buf[i];
- if (ch == ':')
- break;
- if (isalnum(ch) || (ch == '-'))
- continue;
- return 0;
- }
-
- return 1;
-}
-
-static int is_cherry_pick_from_line(const char *buf, int len)
-{
- return (strlen(cherry_picked_prefix) + 41) <= len &&
- !prefixcmp(buf, cherry_picked_prefix);
-}
-
-static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
-{
- int hit = 0;
- int i, k;
- int len = sb->len - ignore_footer;
- const char *buf = sb->buf;
-
- for (i = len - 1; i > 0; i--) {
- if (hit && buf[i] == '\n')
- break;
- hit = (buf[i] == '\n');
- }
-
- while (i < len - 1 && buf[i] == '\n')
- i++;
-
- for (; i < len; i = k) {
- for (k = i; k < len && buf[k] != '\n'; k++)
- ; /* do nothing */
- k++;
-
- if (!(is_rfc2822_line(buf+i, k-i) ||
- is_cherry_pick_from_line(buf+i, k-i)))
- return 0;
- }
- return 1;
-}
-
void append_signoff(struct strbuf *msgbuf, int ignore_footer)
{
struct strbuf sob = STRBUF_INIT;
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 32c0bb1..9dd6d5d 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -57,6 +57,19 @@ test_expect_success setup '
test_commit conflicting unrelated
'
+test_expect_success 'cherry-pick -x inserts blank line after one line subject' '
+ pristine_detach initial &&
+ sha1=`git rev-parse mesg-one-line^0` &&
+ git cherry-pick -x mesg-one-line &&
+ cat <<-EOF >expect &&
+ $mesg_one_line
+
+ (cherry picked from commit $sha1)
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cherry-pick -s inserts blank line after one line subject' '
pristine_detach initial &&
git cherry-pick -s mesg-one-line &&
@@ -81,6 +94,19 @@ test_expect_failure 'cherry-pick -s inserts blank line after non-conforming foot
test_cmp expect actual
'
+test_expect_success 'cherry-pick -x inserts blank line when conforming footer not found' '
+ pristine_detach initial &&
+ sha1=`git rev-parse mesg-no-footer^0` &&
+ git cherry-pick -x mesg-no-footer &&
+ cat <<-EOF >expect &&
+ $mesg_no_footer
+
+ (cherry picked from commit $sha1)
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cherry-pick -s inserts blank line when conforming footer not found' '
pristine_detach initial &&
git cherry-pick -s mesg-no-footer &&
@@ -93,6 +119,20 @@ test_expect_success 'cherry-pick -s inserts blank line when conforming footer no
test_cmp expect actual
'
+test_expect_success 'cherry-pick -x -s inserts blank line when conforming footer not found' '
+ pristine_detach initial &&
+ sha1=`git rev-parse mesg-no-footer^0` &&
+ git cherry-pick -x -s mesg-no-footer &&
+ cat <<-EOF >expect &&
+ $mesg_no_footer
+
+ (cherry picked from commit $sha1)
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'cherry-pick -s adds sob when last sob doesnt match committer' '
pristine_detach initial &&
git cherry-pick -s mesg-with-footer &&
@@ -163,4 +203,17 @@ test_expect_success 'cherry-pick -s treats "(cherry picked from..." line as part
test_cmp expect actual
'
+test_expect_success 'cherry-pick -x -s treats "(cherry picked from..." line as part of footer' '
+ pristine_detach initial &&
+ sha1=`git rev-parse mesg-with-cherry-footer^0` &&
+ git cherry-pick -x -s mesg-with-cherry-footer &&
+ cat <<-EOF >expect &&
+ $mesg_with_cherry_footer
+ (cherry picked from commit $sha1)
+ Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+ EOF
+ git log -1 --pretty=format:%B >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.8.0.284.g959048a
^ permalink raw reply related
* [PATCH 06/11] sequencer.c: teach append_signoff how to detect duplicate s-o-b
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>
Teach append_signoff how to detect a duplicate s-o-b in the commit footer.
This is in preparation to unify the append_signoff implementations in
log-tree.c and sequencer.c.
Fixes test in t3511.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
builtin/commit.c | 2 +-
sequencer.c | 43 +++++++++++++++++++++++++++++++------------
sequencer.h | 2 +-
t/t3511-cherry-pick-x.sh | 2 +-
4 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 1dd2ec5..7b9e2ac 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -700,7 +700,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
previous = eol;
}
- append_signoff(&sb, ignore_footer);
+ append_signoff(&sb, ignore_footer, 0);
}
if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
diff --git a/sequencer.c b/sequencer.c
index 7c0852a..3062ad4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -42,12 +42,19 @@ static int is_cherry_pick_from_line(const char *buf, int len)
!prefixcmp(buf, cherry_picked_prefix);
}
-static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
+/* Returns 0 for non-conforming footer
+ * Returns 1 for conforming footer
+ * Returns 2 when sob exists within conforming footer
+ * Returns 3 when sob exists within conforming footer as last entry
+ */
+static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
+ int ignore_footer)
{
int hit = 0;
- int i, k;
+ int i, k = 0;
int len = sb->len - ignore_footer;
const char *buf = sb->buf;
+ int found_sob = 0;
for (i = len - 1; i > 0; i--) {
if (hit && buf[i] == '\n')
@@ -63,14 +70,24 @@ static int has_conforming_footer(struct strbuf *sb, int ignore_footer)
i++;
for (; i < len; i = k) {
+ int found_rfc2822;
+
for (k = i; k < len && buf[k] != '\n'; k++)
; /* do nothing */
k++;
- if (!(is_rfc2822_line(buf+i, k-i) ||
- is_cherry_pick_from_line(buf+i, k-i)))
+ found_rfc2822 = is_rfc2822_line(buf+i, k-i);
+ if (found_rfc2822 && sob &&
+ !strncasecmp(buf+i, sob->buf, sob->len))
+ found_sob = k;
+
+ if (!(found_rfc2822 || is_cherry_pick_from_line(buf+i, k-i)))
return 0;
}
+ if (found_sob == i)
+ return 3;
+ if (found_sob)
+ return 2;
return 1;
}
@@ -291,7 +308,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
rollback_lock_file(&index_lock);
if (opts->signoff)
- append_signoff(msgbuf, 0);
+ append_signoff(msgbuf, 0, 0);
if (!clean) {
int i;
@@ -547,7 +564,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
}
if (opts->record_origin) {
- if (!has_conforming_footer(&msgbuf, 0))
+ if (!has_conforming_footer(&msgbuf, NULL, 0))
strbuf_addch(&msgbuf, '\n');
strbuf_addstr(&msgbuf, cherry_picked_prefix);
strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
@@ -1074,9 +1091,10 @@ int sequencer_pick_revisions(struct replay_opts *opts)
return pick_commits(todo_list, opts);
}
-void append_signoff(struct strbuf *msgbuf, int ignore_footer)
+void append_signoff(struct strbuf *msgbuf, int ignore_footer, int no_dup_sob)
{
struct strbuf sob = STRBUF_INIT;
+ int has_footer = 0;
int i;
strbuf_addstr(&sob, sign_off_header);
@@ -1085,10 +1103,11 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer)
strbuf_addch(&sob, '\n');
for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
; /* do nothing */
- if (prefixcmp(msgbuf->buf + i, sob.buf)) {
- if (!i || !has_conforming_footer(msgbuf, ignore_footer))
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
- strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);
- }
+ if (!i || !(has_footer =
+ has_conforming_footer(msgbuf, &sob, ignore_footer)))
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
+ if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
+ strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
+ sob.buf, sob.len);
strbuf_release(&sob);
}
diff --git a/sequencer.h b/sequencer.h
index 9d57d57..c4c7132 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -48,6 +48,6 @@ int sequencer_pick_revisions(struct replay_opts *opts);
extern const char sign_off_header[];
-void append_signoff(struct strbuf *msgbuf, int ignore_footer);
+void append_signoff(struct strbuf *msgbuf, int ignore_footer, int no_dup_sob);
#endif
diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 9dd6d5d..4b67343 100755
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -82,7 +82,7 @@ test_expect_success 'cherry-pick -s inserts blank line after one line subject' '
test_cmp expect actual
'
-test_expect_failure 'cherry-pick -s inserts blank line after non-conforming footer' '
+test_expect_success 'cherry-pick -s inserts blank line after non-conforming footer' '
pristine_detach initial &&
git cherry-pick -s mesg-broken-footer &&
cat <<-EOF >expect &&
--
1.8.0.284.g959048a
^ permalink raw reply related
* [PATCH 07/11] sequencer.c: teach append_signoff to avoid adding a duplicate newline
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>
Teach append_signoff to detect whether a blank line exists at the position
that the signed-off-by line will be added, and avoid adding an additional
one if one already exists. This is necessary to allow format-patch to add a
s-o-b to a patch with no commit message without adding an extra newline. A
following patch will make format-patch use this function rather than the
append_signoff implementation inside log-tree.c.
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
sequencer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 3062ad4..eb93dd6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1103,8 +1103,8 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, int no_dup_sob)
strbuf_addch(&sob, '\n');
for (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\n'; i--)
; /* do nothing */
- if (!i || !(has_footer =
- has_conforming_footer(msgbuf, &sob, ignore_footer)))
+ if (msgbuf->buf[i] != '\n' && (!i || !(has_footer =
+ has_conforming_footer(msgbuf, &sob, ignore_footer))))
strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, "\n", 1);
if (has_footer != 3 && (!no_dup_sob || has_footer != 2))
strbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,
--
1.8.0.284.g959048a
^ permalink raw reply related
* [PATCH 08/11] t4014: more tests about appending s-o-b lines
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
[bc: fix test 90 "signoff: some random signoff-alike" and mark as failing.
Correct behavior should insert a blank line after message body and
signed-off-by ]
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
t/t4014-format-patch.sh | 146 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 146 insertions(+)
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 16a4ca1..dfe9209 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -963,4 +963,150 @@ test_expect_success 'format patch ignores color.ui' '
test_cmp expect actual
'
+append_signoff()
+{
+ C=`git commit-tree HEAD^^{tree} -p HEAD` &&
+ git format-patch --stdout --signoff ${C}^..${C} |
+ tee append_signoff.patch |
+ sed -n "1,/^---$/p" |
+ grep -n -E "^Subject|Sign|^$"
+}
+
+test_expect_success 'signoff: commit with no body' '
+ append_signoff </dev/null >actual &&
+ cat <<\EOF | sed "s/EOL$//" >expected &&
+4:Subject: [PATCH] EOL
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject' '
+ echo subject | append_signoff >actual &&
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: commit with only subject that does not end with NL' '
+ echo -n subject | append_signoff >actual &&
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: no existing signoffs and no trailing NL' '
+ printf "subject\n\nbody" | append_signoff >actual &&
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: my@house
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: my@house
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_failure 'signoff: some random signoff-alike' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+Fooled-by-me: my@house
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+11:
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff at the end, no trailing NL' '
+ printf "subject\n\nSigned-off-by: C O Mitter <committer@example.com>" |
+ append_signoff >actual &&
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: the same signoff NOT at the end' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: C O Mitter <committer@example.com>
+Signed-off-by: my@house
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+12:Signed-off-by: my@house
+EOF
+ test_cmp expected actual
+'
+
test_done
--
1.8.0.284.g959048a
^ permalink raw reply related
* [PATCH 09/11] format-patch: stricter S-o-b detection
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
S-o-b in the middle of a sentence, at the beginning of the sentence
but it is just because of text wrapping, or a full paragraph of valid
S-o-b in the middle of the message. All these are not S-o-b, but
detected as is. Fix them.
[bc: add two additional tests to demonstrate shortcomings of the current
code:
1. failure to detect non-conforming elements in the footer when last
line matches committer's s-o-b.
2. failure to handle various s-o-b -like elements in the footer as
conforming. e.g. "Change-id: IXXXX or Bug: 1234"
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
log-tree.c | 37 +++++++++++++++++--
t/t4014-format-patch.sh | 95 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 130 insertions(+), 2 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 4f86def..14ebf69 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -260,14 +260,47 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
int has_signoff = 0;
char *cp;
- cp = sb->buf;
+ /*
+ * Only search in the last paragrah, don't be fooled by a
+ * paragraph full of valid S-o-b in the middle.
+ */
+ cp = sb->buf + sb->len - 1;
+ while (cp > sb->buf) {
+ if (cp[0] == '\n' && cp[1] == '\n') {
+ cp += 2;
+ break;
+ }
+ cp--;
+ }
/* First see if we already have the sign-off by the signer */
while ((cp = strstr(cp, signed_off_by))) {
+ const char *s = cp;
+ cp += strlen(signed_off_by);
+
+ if (!has_signoff && s > sb->buf) {
+ /*
+ * S-o-b in the middle of a sentence is not
+ * really S-o-b
+ */
+ if (s[-1] != '\n')
+ continue;
+
+ if (s - 1 > sb->buf && s[-2] == '\n')
+ ; /* the first S-o-b */
+ else if (!detect_any_signoff(sb->buf, s - sb->buf))
+ /*
+ * The previous line looks like an
+ * S-o-b. There's still no guarantee
+ * that it's an actual S-o-b. For that
+ * we need to look back until we find
+ * a blank line, which is too costly.
+ */
+ continue;
+ }
has_signoff = 1;
- cp += strlen(signed_off_by);
if (cp + signoff_len >= sb->buf + sb->len)
break;
if (strncmp(cp, signoff, signoff_len))
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index dfe9209..30e37a7 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1062,6 +1062,60 @@ EOF
test_cmp expected actual
'
+test_expect_success 'signoff: not really a signoff' '
+ append_signoff <<\EOF >actual &&
+subject
+
+I want to mention about Signed-off-by: here.
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:I want to mention about Signed-off-by: here.
+10:
+11:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: not really a signoff (2)' '
+ append_signoff <<\EOF >actual &&
+subject
+
+My unfortunate
+Signed-off-by: example happens to be wrapped here.
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:Signed-off-by: example happens to be wrapped here.
+11:
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: valid S-o-b paragraph in the middle' '
+ append_signoff <<\EOF >actual &&
+subject
+
+Signed-off-by: my@house
+Signed-off-by: your@house
+
+A lot of houses.
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+9:Signed-off-by: my@house
+10:Signed-off-by: your@house
+11:
+13:
+14:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'signoff: the same signoff at the end' '
append_signoff <<\EOF >actual &&
subject
@@ -1109,4 +1163,45 @@ EOF
test_cmp expected actual
'
+test_expect_failure 'signoff: detect garbage in non-conforming footer' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Tested-by: my@house
+Some Trash
+Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+13:Signed-off-by: C O Mitter <committer@example.com>
+14:
+15:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_failure 'signoff: footer begins with non-signoff without @ sign' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Tested-by: my@house
+Change-id: Ideadbeef
+Signed-off-by: C O Mitter <committer@example.com>
+Bug: 1234
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+13:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
test_done
--
1.8.0.284.g959048a
^ permalink raw reply related
* [PATCH 10/11] format-patch: update append_signoff prototype
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>
From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
This is a preparation step for merging with append_signoff from
sequencer.c
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
builtin/log.c | 13 +------------
log-tree.c | 21 +++++++++++++--------
revision.h | 2 +-
3 files changed, 15 insertions(+), 21 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index e7b7db1..bb48344 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1058,7 +1058,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
struct commit *origin = NULL, *head = NULL;
const char *in_reply_to = NULL;
struct patch_ids ids;
- char *add_signoff = NULL;
struct strbuf buf = STRBUF_INIT;
int use_patch_format = 0;
int quiet = 0;
@@ -1154,16 +1153,6 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN |
PARSE_OPT_KEEP_DASHDASH);
- if (do_signoff) {
- const char *committer;
- const char *endpos;
- committer = git_committer_info(IDENT_STRICT);
- endpos = strchr(committer, '>');
- if (!endpos)
- die(_("bogus committer info %s"), committer);
- add_signoff = xmemdupz(committer, endpos - committer + 1);
- }
-
for (i = 0; i < extra_hdr.nr; i++) {
strbuf_addstr(&buf, extra_hdr.items[i].string);
strbuf_addch(&buf, '\n');
@@ -1354,7 +1343,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
total++;
start_number--;
}
- rev.add_signoff = add_signoff;
+ rev.add_signoff = do_signoff;
while (0 <= --nr) {
int shown;
commit = list[nr];
diff --git a/log-tree.c b/log-tree.c
index 14ebf69..be8e7ab 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -253,9 +253,11 @@ static int detect_any_signoff(char *letter, int size)
return seen_head && seen_name;
}
-static void append_signoff(struct strbuf *sb, const char *signoff)
+static void append_signoff(struct strbuf *sb, int flags)
{
- static const char signed_off_by[] = "Signed-off-by: ";
+ char* signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
+ getenv("GIT_COMMITTER_EMAIL")));
+ static const char sign_off_header[] = "Signed-off-by: ";
size_t signoff_len = strlen(signoff);
int has_signoff = 0;
char *cp;
@@ -274,9 +276,9 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
}
/* First see if we already have the sign-off by the signer */
- while ((cp = strstr(cp, signed_off_by))) {
+ while ((cp = strstr(cp, sign_off_header))) {
const char *s = cp;
- cp += strlen(signed_off_by);
+ cp += strlen(sign_off_header);
if (!has_signoff && s > sb->buf) {
/*
@@ -317,9 +319,10 @@ static void append_signoff(struct strbuf *sb, const char *signoff)
if (!has_signoff)
strbuf_addch(sb, '\n');
- strbuf_addstr(sb, signed_off_by);
+ strbuf_addstr(sb, sign_off_header);
strbuf_add(sb, signoff, signoff_len);
strbuf_addch(sb, '\n');
+ free(signoff);
}
static unsigned int digits_in_number(unsigned int number)
@@ -695,8 +698,10 @@ void show_log(struct rev_info *opt)
/*
* And then the pretty-printed message itself
*/
- if (ctx.need_8bit_cte >= 0)
- ctx.need_8bit_cte = has_non_ascii(opt->add_signoff);
+ if (ctx.need_8bit_cte >= 0 && opt->add_signoff)
+ ctx.need_8bit_cte =
+ has_non_ascii(fmt_name(getenv("GIT_COMMITTER_NAME"),
+ getenv("GIT_COMMITTER_EMAIL")));
ctx.date_mode = opt->date_mode;
ctx.date_mode_explicit = opt->date_mode_explicit;
ctx.abbrev = opt->diffopt.abbrev;
@@ -707,7 +712,7 @@ void show_log(struct rev_info *opt)
pretty_print_commit(&ctx, commit, &msgbuf);
if (opt->add_signoff)
- append_signoff(&msgbuf, opt->add_signoff);
+ append_signoff(&msgbuf, 0);
if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
ctx.notes_message && *ctx.notes_message) {
diff --git a/revision.h b/revision.h
index 059bfff..435a60b 100644
--- a/revision.h
+++ b/revision.h
@@ -137,7 +137,7 @@ struct rev_info {
int numbered_files;
char *message_id;
struct string_list *ref_message_ids;
- const char *add_signoff;
+ int add_signoff;
const char *extra_headers;
const char *log_reencode;
const char *subject_prefix;
--
1.8.0.284.g959048a
^ permalink raw reply related
* [PATCH 11/11] Unify appending signoff in format-patch, commit and sequencer
From: Brandon Casey @ 2012-11-26 1:45 UTC (permalink / raw)
To: git; +Cc: pclouds, gitster, Brandon Casey, Brandon Casey
In-Reply-To: <1353894359-6733-1-git-send-email-drafnel@gmail.com>
There are two implementations of append_signoff in log-tree.c and
sequencer.c, which do more or less the same thing. Unify on top of the
sequencer.c implementation.
Add a test in t4014 to demonstrate support for non-s-o-b elements in the
commit footer provided by sequence.c:append_sob. Mark tests fixed as
appropriate.
[Commit message mostly stolen from Nguyễn Thái Ngọc Duy's original
unification patch]
Signed-off-by: Brandon Casey <bcasey@nvidia.com>
---
log-tree.c | 122 +-----------------------------------------------
t/t4014-format-patch.sh | 27 +++++++++--
2 files changed, 26 insertions(+), 123 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index be8e7ab..1fb0a16 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -9,6 +9,7 @@
#include "string-list.h"
#include "color.h"
#include "gpg-interface.h"
+#include "sequencer.h"
struct decoration name_decoration = { "object names" };
@@ -206,125 +207,6 @@ void show_decorations(struct rev_info *opt, struct commit *commit)
putchar(')');
}
-/*
- * Search for "^[-A-Za-z]+: [^@]+@" pattern. It usually matches
- * Signed-off-by: and Acked-by: lines.
- */
-static int detect_any_signoff(char *letter, int size)
-{
- char *cp;
- int seen_colon = 0;
- int seen_at = 0;
- int seen_name = 0;
- int seen_head = 0;
-
- cp = letter + size;
- while (letter <= --cp && *cp == '\n')
- continue;
-
- while (letter <= cp) {
- char ch = *cp--;
- if (ch == '\n')
- break;
-
- if (!seen_at) {
- if (ch == '@')
- seen_at = 1;
- continue;
- }
- if (!seen_colon) {
- if (ch == '@')
- return 0;
- else if (ch == ':')
- seen_colon = 1;
- else
- seen_name = 1;
- continue;
- }
- if (('A' <= ch && ch <= 'Z') ||
- ('a' <= ch && ch <= 'z') ||
- ch == '-') {
- seen_head = 1;
- continue;
- }
- /* no empty last line doesn't match */
- return 0;
- }
- return seen_head && seen_name;
-}
-
-static void append_signoff(struct strbuf *sb, int flags)
-{
- char* signoff = xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
- getenv("GIT_COMMITTER_EMAIL")));
- static const char sign_off_header[] = "Signed-off-by: ";
- size_t signoff_len = strlen(signoff);
- int has_signoff = 0;
- char *cp;
-
- /*
- * Only search in the last paragrah, don't be fooled by a
- * paragraph full of valid S-o-b in the middle.
- */
- cp = sb->buf + sb->len - 1;
- while (cp > sb->buf) {
- if (cp[0] == '\n' && cp[1] == '\n') {
- cp += 2;
- break;
- }
- cp--;
- }
-
- /* First see if we already have the sign-off by the signer */
- while ((cp = strstr(cp, sign_off_header))) {
- const char *s = cp;
- cp += strlen(sign_off_header);
-
- if (!has_signoff && s > sb->buf) {
- /*
- * S-o-b in the middle of a sentence is not
- * really S-o-b
- */
- if (s[-1] != '\n')
- continue;
-
- if (s - 1 > sb->buf && s[-2] == '\n')
- ; /* the first S-o-b */
- else if (!detect_any_signoff(sb->buf, s - sb->buf))
- /*
- * The previous line looks like an
- * S-o-b. There's still no guarantee
- * that it's an actual S-o-b. For that
- * we need to look back until we find
- * a blank line, which is too costly.
- */
- continue;
- }
-
- has_signoff = 1;
-
- if (cp + signoff_len >= sb->buf + sb->len)
- break;
- if (strncmp(cp, signoff, signoff_len))
- continue;
- if (!isspace(cp[signoff_len]))
- continue;
- /* we already have him */
- return;
- }
-
- if (!has_signoff)
- has_signoff = detect_any_signoff(sb->buf, sb->len);
-
- if (!has_signoff)
- strbuf_addch(sb, '\n');
-
- strbuf_addstr(sb, sign_off_header);
- strbuf_add(sb, signoff, signoff_len);
- strbuf_addch(sb, '\n');
- free(signoff);
-}
-
static unsigned int digits_in_number(unsigned int number)
{
unsigned int i = 10, result = 1;
@@ -712,7 +594,7 @@ void show_log(struct rev_info *opt)
pretty_print_commit(&ctx, commit, &msgbuf);
if (opt->add_signoff)
- append_signoff(&msgbuf, 0);
+ append_signoff(&msgbuf, 0, 1);
if ((ctx.fmt != CMIT_FMT_USERFORMAT) &&
ctx.notes_message && *ctx.notes_message) {
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 30e37a7..1dea58b 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1046,7 +1046,28 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'signoff: some random signoff-alike' '
+test_expect_success 'signoff: misc conforming footer elements' '
+ append_signoff <<\EOF >actual &&
+subject
+
+body
+
+Signed-off-by: my@house
+(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)
+Tested-by: Some One <someone@example.com>
+Bug: 1234
+EOF
+ cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+10:
+11:Signed-off-by: my@house
+15:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'signoff: some random signoff-alike' '
append_signoff <<\EOF >actual &&
subject
@@ -1163,7 +1184,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'signoff: detect garbage in non-conforming footer' '
+test_expect_success 'signoff: detect garbage in non-conforming footer' '
append_signoff <<\EOF >actual &&
subject
@@ -1184,7 +1205,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'signoff: footer begins with non-signoff without @ sign' '
+test_expect_success 'signoff: footer begins with non-signoff without @ sign' '
append_signoff <<\EOF >actual &&
subject
--
1.8.0.284.g959048a
^ permalink raw reply related
* Re: Multiple threads of compression
From: Brandon Casey @ 2012-11-26 2:37 UTC (permalink / raw)
To: Thorsten Glaser; +Cc: git@vger.kernel.org
In-Reply-To: <loom.20121125T171702-64@post.gmane.org>
On Sun, Nov 25, 2012 at 8:27 AM, Thorsten Glaser <tg@debian.org> wrote:
> tl;dr: I would like to have a *global* option for git to restrict
> the number of threads of execution it uses. Several subcommands,
> like pack-objects, are already equipped with an optioin for this,
> but unfortunately, these are seldom invoked by hand¹, so this can’t
> work in my situations.
See the git-config man page.
The number of threads that pack uses can be configured in the global
or system gitconfig file by setting pack.threads.
e.g.
$ git config --system pack.threads 1
Also, modern git accepts a '-c' option which allows you to set
configuration options on the command line, e.g. 'git -c pack.threads=1
gc'.
The other setting you should probably look at is pack.windowMemory
which should help you control the amount of memory git uses while
packing. Also look at core.packedGitWindowSize and
core.packedGitLimit if your repository is really large.
>
> ① automatic garbage collection, “git gc --aggressive --prune=now”,
> and cloning are the use cases I have at hand right now.
>
> À propos, while here: is gc --aggressive safe to run on a live,
> online-shared repository, or does it break other users accessing
> the repository concurrently? (If it’s safe I’d very much like to do
> that in a, say weekly, cronjob on FusionForge, our hosting system.)
Running 'git gc' with --aggressive should be as safe as running it
without --aggressive.
But, you should think about whether you really need to run it more
than once, or at all. When you use --aggressive, git will perform the
entire delta search again for _every_ object in the repository. The
general usecase for --aggressive is when you suspect that the original
delta search produced sub-optimal deltas or if you modify the size of
the delta search window or depth and want to regenerate your packed
objects to improve compression or access speed. Even then, you will
not likely get much benefit from running with --aggressive a second
time.
hth,
-Brandon
^ permalink raw reply
* Re: Requirements for integrating a new git subcommand
From: Junio C Hamano @ 2012-11-26 2:57 UTC (permalink / raw)
To: esr; +Cc: git
In-Reply-To: <20121122053012.GA17265@thyrsus.com>
"Eric S. Raymond" <esr@thyrsus.com> writes:
> While the weave operation can build a commit graph with any structure
> desired, an important restriction of the inverse (unraveling)
> operation is that it operates on *master branches only*. The unravel
> operation discards non-master-branch content, emitting a warning
> to standard error when it has to do so.
Imagine that I have a simple four-commit diamond:
I---A
\ \
B---M
where Amy and Bob forked the initial commit made by Ian and created
one commit each, and their branches were merged into one 'master'
branch by a merge commit made by Mac. How many state snapshots will
I see when I ask you to unravel this? Three, or four?
If you are going to give me all four states, then I do not
understand why this needs to be limited to the master branch only.
Even if you start from a single commit at the tip of 'master', once
you hit a merge, you will need to follow all of two (or more) paths
to dig down to the root(s), so supporting to start digging from more
than one commit is not all that different.
If you are going to give me only three states, following the first
parent ancestry chain, then the description needs to state it more
clearly. I am not saying first-parent-only history is useless, but
the user needs to know that merges are flattened in the unraveled
result and the resulting history becomes linear, following the first
parent ancestry chain of the original history (if that is what the
tool does) before deciding if this tool matches what she needs.
As to the procedural stuff, I think others have sufficiently
answered already. If I may add something, a new stuff typically
start its life in contrib/ before it proves to be useful.
^ permalink raw reply
* Re: [PATCH] diff: Fixes shortstat number of files
From: Junio C Hamano @ 2012-11-26 3:28 UTC (permalink / raw)
To: Antoine Pelisse; +Cc: git
In-Reply-To: <1353533210-29684-1-git-send-email-apelisse@gmail.com>
Antoine Pelisse <apelisse@gmail.com> writes:
> Subject: Re: [PATCH] diff: Fixes shortstat number of files
Please replace "Fixes" with "Fix at least (because our log messages
are written as if a patch is giving an order to the codebase, iow,
in imperative mood), but we would prefer to see a concrete
description on what is fixed, when we can. And in this case, I
think we can, perhaps:
diff: do not count unmerged paths twice in --shortstat/--numstat
or something.
> There is a discrepancy between the last line of `git diff --stat`
> and `git diff --shortstat` in case of a merge.
> The unmerged files are actually counted twice, thus doubling the
> value of "file changed".
I think the current 'master' and upward is broken with respect to
this; I am consistently getting two entries for unmerged paths
across --stat, --shortstat and --numstat options (iow, not just
shortstat and numstat but the '--stat' seems to be broken as well).
Thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox