* Races on ref .lock files?
From: Andreas Krey @ 2016-12-16 16:47 UTC (permalink / raw)
To: git
Hi all,
We're occasionally seeing a lot of
error: cannot lock ref 'stash-refs/pull-requests/18978/to': Unable to create '/opt/apps/..../repositories/68/stash-refs/pull-requests/18978/to.lock': File exists.
from the server side with fetches as well as pushes. (Bitbucket server.)
What I find strange is that neither the fetches nor the pushes even
touch these refs (but the bitbucket triggers underneath might).
But my question is whether there are race conditions that can cause
such messages in regular operation - they continue with 'If no other git
process is currently running, this probably means a git process crashed
in this repository earlier.' which indicates some level of anticipation.
- Andreas
--
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800
^ permalink raw reply
* Re: Is there a way to have local changes in a branch 'bake' while working in different branches?
From: John Keeping @ 2016-12-16 15:22 UTC (permalink / raw)
To: Larry Minton; +Cc: git@vger.kernel.org
In-Reply-To: <14b481f95c5043aca6cdfddfe4728fa9@BLUPR79MB001.MGDADSK.autodesk.com>
On Thu, Dec 15, 2016 at 08:14:58PM +0000, Larry Minton wrote:
> My question:
>
> Let's say I have a code change that I want to 'bake' for a while
> locally, just to make sure some edge case doesn't pop up while I am
> working on other things. Is there any practical way of doing that? I
> could constantly merge that 'bake me' branch into other branches as I
> work on them and then remove those changes from the branches before
> sending them out for code review, but sooner or later pretty much
> guaranteed to screw that up....
I wrote a tool [1] a while ago to manage integration branches so I use a
personal integration branch to pull together various in-progress
branches.
It means you can keep each topic in its own branch but work/test on top
of a unified branch by running:
git integration --rebuild my-integration-branch
whenever you change one of the topic branches.
I also use the instruction sheet to keep track of abandoned topics that
I might want to go back to but which are currently in a broken state,
you can see an example of that in my CGit integration branch [2].
[1] http://johnkeeping.github.io/git-integration/
[2] https://github.com/johnkeeping/cgit/blob/d01ce31ed3dfa9b05ef971464da2af5b9d6f2756/GIT-INTEGRATION-INSN
^ permalink raw reply
* Re: Allow "git shortlog" to group by committer information
From: Jeff King @ 2016-12-16 13:51 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20161216133940.hu474phggdslh6ka@sigill.intra.peff.net>
On Fri, Dec 16, 2016 at 08:39:40AM -0500, Jeff King wrote:
> I'm OK with the approach your patch takes, but I think there were some
> unresolved issues:
>
> - are we OK taking the short "-c" for this, or do we want
> "--group-by=committer" or something like it?
>
> - no tests; you can steal the general form from my [1]
>
> - no documentation (can also be stolen from [1], though the syntax is
> quite different)
Being moved by the holiday spirit, I wrote a patch to address the latter
two. ;)
It obviously would need updating if we switch away from "-c", but I
think I am OK with the short "-c" (even if we add a more exotic grouping
option later, this can remain as a short synonym).
-- >8 --
Subject: [PATCH] shortlog: test and document --committer option
This puts the final touches on the feature added by
fbfda15fb8 (shortlog: group by committer information,
2016-10-11).
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git-shortlog.txt | 4 ++++
t/t4201-shortlog.sh | 13 +++++++++++++
2 files changed, 17 insertions(+)
diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index 31af7f2736..ee6c5476c1 100644
--- a/Documentation/git-shortlog.txt
+++ b/Documentation/git-shortlog.txt
@@ -47,6 +47,10 @@ OPTIONS
Each pretty-printed commit will be rewrapped before it is shown.
+-c::
+--committer::
+ Collect and show committer identities instead of authors.
+
-w[<width>[,<indent1>[,<indent2>]]]::
Linewrap the output by wrapping each line at `width`. The first
line of each entry is indented by `indent1` spaces, and the second
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
index ae08b57712..6c7c637481 100755
--- a/t/t4201-shortlog.sh
+++ b/t/t4201-shortlog.sh
@@ -190,4 +190,17 @@ test_expect_success 'shortlog with --output=<file>' '
test_line_count = 3 shortlog
'
+test_expect_success 'shortlog --committer (internal)' '
+ cat >expect <<-\EOF &&
+ 3 C O Mitter
+ EOF
+ git shortlog -nsc HEAD >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'shortlog --committer (external)' '
+ git log --format=full | git shortlog -nsc >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.348.g960a0b554
^ permalink raw reply related
* Re: Allow "git shortlog" to group by committer information
From: Jeff King @ 2016-12-16 13:39 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <CA+55aFxSQ2wxU3cA+8uqS-W8mbobF35dVCZow2BcixGOOvGVFQ@mail.gmail.com>
On Thu, Dec 15, 2016 at 01:29:47PM -0800, Linus Torvalds wrote:
> On Tue, Oct 11, 2016 at 11:45 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > In some situations you may want to group the commits not by author,
> > but by committer instead.
> >
> > For example, when I just wanted to look up what I'm still missing from
> > linux-next in the current merge window [..]
>
> It's another merge window later for the kernel, and I just re-applied
> this patch to my git tree because I still want to know teh committer
> information rather than the authorship information, and it still seems
> to be the simplest way to do that.
>
> Jeff had apparently done something similar as part of a bigger
> patch-series, but I don't see that either. I really don't care very
> much how this is done, but I do find this very useful, I do things
> like
Sorry if I de-railed the earlier conversation. The shortlog
group-by-trailer work didn't seem useful enough for me to make it a
priority.
I'm OK with the approach your patch takes, but I think there were some
unresolved issues:
- are we OK taking the short "-c" for this, or do we want
"--group-by=committer" or something like it?
- no tests; you can steal the general form from my [1]
- no documentation (can also be stolen from [1], though the syntax is
quite different)
-Peff
[1] http://public-inbox.org/git/20151229073515.GK8842@sigill.intra.peff.net/
^ permalink raw reply
* Re: [PATCH] printk: Remove no longer used second struct cont
From: Joe Perches @ 2016-12-16 6:04 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, git, Sergey Senozhatsky, Petr Mladek,
Geert Uytterhoeven, Steven Rostedt, Mark Rutland, Andrew Morton,
Linux Kernel Mailing List
In-Reply-To: <xmqqtwa4tqnc.fsf@gitster.mtv.corp.google.com>
On Thu, 2016-12-15 at 21:00 -0800, Junio C Hamano wrote:
> Joe Perches <joe@perches.com> writes:
>
> > grep 2.5.4 was the last version that supported the -P option to
> > grep through for multiple lines.
>
> Does anybody know why it was dropped?
perl compatible regexes in grep have always been "experimental"
and never officially supported.
From the grep manual https://www.gnu.org/software/grep/manual/grep.html
--perl-regexp
Interpret the pattern as a Perl-compatible regular expression
(PCRE). This is highly experimental, particularly when combined with
the -z (--null-data) option, and ‘grep -P’ may warn of unimplemented
features. See Other Options.
It wasn't dropped so much as "enhanced" away.
Oh well.
^ permalink raw reply
* Re: [PATCH] printk: Remove no longer used second struct cont
From: Junio C Hamano @ 2016-12-16 5:00 UTC (permalink / raw)
To: Joe Perches
Cc: Linus Torvalds, git, Sergey Senozhatsky, Petr Mladek,
Geert Uytterhoeven, Steven Rostedt, Mark Rutland, Andrew Morton,
Linux Kernel Mailing List
In-Reply-To: <1481855446.29291.80.camel@perches.com>
Joe Perches <joe@perches.com> writes:
> grep 2.5.4 was the last version that supported the -P option to
> grep through for multiple lines.
Does anybody know why it was dropped?
^ permalink raw reply
* Re: Allow "git shortlog" to group by committer information
From: Junio C Hamano @ 2016-12-16 4:56 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <CA+55aFySBc1Nd_xYZmXF9tdynjW+udsEz+PtkQpkrPjeFVcfDw@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Thu, Dec 15, 2016 at 4:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> This fell off the radar partly because of the distractions like
>> "there are other attempts and other ways", and also because the
>> message was not a text-plain that can be reviewed inline. Let me
>> try to dig it up from the mail archive to see if I can find it.
>
> Sorry, I'll just re-send it without the attachment. I prefer inline
> myself, but I thought you didn't care (and gmail makes it
> unnecessarily hard).
Thanks.
I do care, but I try to be lenient for inexperienced contriburors,
which you don't qualify ;-) Experienced ones are held to a higher
standard.
^ permalink raw reply
* Re: [PATCH] printk: Remove no longer used second struct cont
From: Joe Perches @ 2016-12-16 2:30 UTC (permalink / raw)
To: Linus Torvalds, git, Junio C Hamano
Cc: Sergey Senozhatsky, Petr Mladek, Geert Uytterhoeven,
Steven Rostedt, Mark Rutland, Andrew Morton,
Linux Kernel Mailing List
In-Reply-To: <CA+55aFxaOFoh+Zrm5tNhU4hWu4Z032+nqV3vXK=QPJyhZsU3_A@mail.gmail.com>
On Thu, 2016-12-15 at 18:10 -0800, Linus Torvalds wrote:
> On Thu, Dec 15, 2016 at 5:57 PM, Joe Perches <joe@perches.com> wrote:
> > >
> > > In fact, I thought we already upped the check-patch limit to 100?
> >
> > Nope, CodingStyle neither.
> >
> > Last time I tried was awhile ago.
>
> Ok, it must have been just talked about, and with the exceptions for
> strings etc I may not have seen as many of the really annoying line
> breaks lately.
>
> I don't mind a 80-column "soft limit" per se: if some code
> consistently goes over 80 columns, there really is something seriously
> wrong there. So 80 columns may well be the right limit for that kind
> of check (or even less).
Newspaper column widths were relatively small for a good reason.
I think most of the uses of simple statements should be on a single
line. I'd rather see just a few arguments on a single line than a
dozen though. Especially those with long identifiers, functions
with many arguments are just difficult to visually scan.
> But if we have just a couple of lines that are longer (in a file that
> is 3k+ lines), I'd rather not break those.
>
> I tend use "git grep" a lot, and it's much easier to see function
> argument use if it's all on one line.
>
> Of course, some function calls really are *so* long that they have to
> be broken up, but that's where the "if it's a couple of lines that go
> a bit over the 80 column limit..." exception basically comes in.
>
> Put another way: long lines definitely aren't good. But breaking long
> lines has some downsides too, so there should be a balance between the
> two, rather than some black-and-white limit.
>
> In fact, we've seldom had cases where black-and-white limits work well.
One thing that _would_ be useful is some enhancement to git grep
that would look for multi-line statements more easily.
The git grep -P option doesn't span lines.
grep 2.5.4 was the last version that supported the -P option to
grep through for multiple lines.
It'd be nice to have something like
git grep --code_style=c90 --function <foo>
that'd show all multiple line uses/definitions/declarations of a
particular function.
I played with extending git grep a bit once, mostly to get the \s
mechanism to span lines. It kinda worked.
Still, it seems like real work to implement well.
^ permalink raw reply
* [PATCH 3/3] t: use nongit() function where applicable
From: Jeff King @ 2016-12-16 2:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20161216022904.cjang6napnl2vkc6@sigill.intra.peff.net>
Many tests want to run a command outside of any git repo;
with the nongit() function this is now a one-liner. It saves
a few lines, but more importantly, it's immediately obvious
what the code is trying to accomplish.
This doesn't convert every such case in the test suite; it
just covers those that want to do a one-off command. Other
cases, such as the ones in t4035, are part of a larger
scheme of outside-repo files, and it's less confusing for
them to stay consistent with the surrounding tests.
Signed-off-by: Jeff King <peff@peff.net>
---
This one is obviously not necessary for the rest of the series, but the
diffstat is certainly pleasing.
t/t1308-config-set.sh | 10 ++--------
t/t9100-git-svn-basic.sh | 17 ++---------------
t/t9902-completion.sh | 7 +------
3 files changed, 5 insertions(+), 29 deletions(-)
diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh
index 7655c94c28..ff50960cca 100755
--- a/t/t1308-config-set.sh
+++ b/t/t1308-config-set.sh
@@ -219,14 +219,8 @@ test_expect_success 'check line errors for malformed values' '
'
test_expect_success 'error on modifying repo config without repo' '
- mkdir no-repo &&
- (
- GIT_CEILING_DIRECTORIES=$(pwd) &&
- export GIT_CEILING_DIRECTORIES &&
- cd no-repo &&
- test_must_fail git config a.b c 2>err &&
- grep "not in a git directory" err
- )
+ nongit test_must_fail git config a.b c 2>err &&
+ grep "not in a git directory" err
'
cmdline_config="'foo.bar=from-cmdline'"
diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index 92a3aa8063..8a8ba65a2a 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -17,25 +17,12 @@ case "$GIT_SVN_LC_ALL" in
;;
esac
-deepdir=nothing-above
-ceiling=$PWD
-
test_expect_success 'git svn --version works anywhere' '
- mkdir -p "$deepdir" && (
- GIT_CEILING_DIRECTORIES="$ceiling" &&
- export GIT_CEILING_DIRECTORIES &&
- cd "$deepdir" &&
- git svn --version
- )
+ nongit git svn --version
'
test_expect_success 'git svn help works anywhere' '
- mkdir -p "$deepdir" && (
- GIT_CEILING_DIRECTORIES="$ceiling" &&
- export GIT_CEILING_DIRECTORIES &&
- cd "$deepdir" &&
- git svn help
- )
+ nongit git svn help
'
test_expect_success \
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 2ba62fbc17..a34e55f874 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -257,12 +257,7 @@ test_expect_success SYMLINKS '__gitdir - resulting path avoids symlinks' '
'
test_expect_success '__gitdir - not a git repository' '
- (
- cd subdir/subsubdir &&
- GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY" &&
- export GIT_CEILING_DIRECTORIES &&
- test_must_fail __gitdir
- )
+ nongit test_must_fail __gitdir
'
test_expect_success '__gitcomp - trailing space - options' '
--
2.11.0.348.g960a0b554
^ permalink raw reply related
* [PATCH 2/3] index-pack: complain when --stdin is used outside of a repo
From: Jeff King @ 2016-12-16 2:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20161216022904.cjang6napnl2vkc6@sigill.intra.peff.net>
The index-pack builtin is marked as RUN_SETUP_GENTLY,
because it's perfectly fine to index a pack in the
filesystem outside of any repository. However, --stdin mode
will write the result to the object database, which does not
make sense outside of a repository. Doing so creates a bogus
".git" directory with nothing in it except the newly-created
pack and its index.
Instead, let's flag this as an error and abort.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/index-pack.c | 2 ++
t/t5300-pack-object.sh | 15 +++++++++++++++
2 files changed, 17 insertions(+)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 0a27bab11b..d450a6ada2 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1730,6 +1730,8 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
usage(index_pack_usage);
if (fix_thin_pack && !from_stdin)
die(_("--fix-thin cannot be used without --stdin"));
+ if (from_stdin && !startup_info->have_repository)
+ die(_("--stdin requires a git repository"));
if (!index_name && pack_name)
index_name = derive_filename(pack_name, ".idx", &index_name_buf);
if (keep_msg && !keep_name && pack_name)
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 899e52d50f..43a672c345 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -406,6 +406,21 @@ test_expect_success 'verify resulting packs' '
git verify-pack test-11-*.pack
'
+test_expect_success 'set up pack for non-repo tests' '
+ # make sure we have a pack with no matching index file
+ cp test-1-*.pack foo.pack
+'
+
+test_expect_success 'index-pack --stdin complains of non-repo' '
+ nongit test_must_fail git index-pack --stdin <foo.pack &&
+ test_path_is_missing non-repo/.git
+'
+
+test_expect_success 'index-pack <pack> works in non-repo' '
+ nongit git index-pack ../foo.pack &&
+ test_path_is_file foo.idx
+'
+
#
# WARNING!
#
--
2.11.0.348.g960a0b554
^ permalink raw reply related
* [PATCH 1/3] t5000: extract nongit function to test-lib-functions.sh
From: Jeff King @ 2016-12-16 2:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20161216022904.cjang6napnl2vkc6@sigill.intra.peff.net>
This function abstracts the idea of running a command
outside of any repository (which is slightly awkward to do
because even if you make a non-repo directory, git may keep
walking up outside of the trash directory). There are
several scripts that use the same technique, so let's make
the function available for everyone.
Signed-off-by: Jeff King <peff@peff.net>
---
I waffled on the name. Something like test_outside_repo() is more
descriptive, but as this is prepended to existing commands, the lines
already end up quite long.
t/t5000-tar-tree.sh | 14 --------------
t/test-lib-functions.sh | 14 ++++++++++++++
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 830bf2a2f6..886b6953e4 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -94,20 +94,6 @@ check_tar() {
'
}
-# run "$@" inside a non-git directory
-nongit () {
- test -d non-repo ||
- mkdir non-repo ||
- return 1
-
- (
- GIT_CEILING_DIRECTORIES=$(pwd) &&
- export GIT_CEILING_DIRECTORIES &&
- cd non-repo &&
- "$@"
- )
-}
-
test_expect_success \
'populate workdir' \
'mkdir a &&
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fdaeb3a96b..adab7f51f4 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -994,3 +994,17 @@ test_copy_bytes () {
}
' - "$1"
}
+
+# run "$@" inside a non-git directory
+nongit () {
+ test -d non-repo ||
+ mkdir non-repo ||
+ return 1
+
+ (
+ GIT_CEILING_DIRECTORIES=$(pwd) &&
+ export GIT_CEILING_DIRECTORIES &&
+ cd non-repo &&
+ "$@"
+ )
+}
--
2.11.0.348.g960a0b554
^ permalink raw reply related
* Re: index-pack outside of repository?
From: Jeff King @ 2016-12-16 2:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20161216013728.in2dazshtarrnnq3@sigill.intra.peff.net>
On Thu, Dec 15, 2016 at 08:37:28PM -0500, Jeff King wrote:
> But if this case really is just "if (from_stdin)" that's quite easy,
> too.
So here is that patch (with some associated refactoring and cleanups).
This is conceptually independent of jk/no-looking-at-dotgit-outside-repo-final,
though it should be fine to merge with that topic. The BUG will actually
pass the new test, because it calls die, too. I wonder if we should die
with a unique error code on BUGs, and catch them in test_must_fail
similar to the way we catch signal death.
[1/3]: t5000: extract nongit function to test-lib-functions.sh
[2/3]: index-pack: complain when --stdin is used outside of a repo
[3/3]: t: use nongit() function where applicable
builtin/index-pack.c | 2 ++
t/t1308-config-set.sh | 10 ++--------
t/t5000-tar-tree.sh | 14 --------------
t/t5300-pack-object.sh | 15 +++++++++++++++
t/t9100-git-svn-basic.sh | 17 ++---------------
t/t9902-completion.sh | 7 +------
t/test-lib-functions.sh | 14 ++++++++++++++
7 files changed, 36 insertions(+), 43 deletions(-)
-Peff
^ permalink raw reply
* Re: Allow "git shortlog" to group by committer information
From: Stephen & Linda Smith @ 2016-12-16 1:51 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <xmqqoa0cu3nn.fsf@gitster.mtv.corp.google.com>
On Thursday, December 15, 2016 5:39:53 PM MST Linus Torvalds wrote:
> On Thu, Dec 15, 2016 at 4:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Sorry, I'll just re-send it without the attachment. I prefer inline
> myself, but I thought you didn't care (and gmail makes it
> unnecessarily hard).
>
> Linus
Why does gmail make it unnecessarily hard?
I thought that a good percentage of the kernel maintainers use git send-email.
what would make that command easier to use with gmail?
sps
^ permalink raw reply
* Re: Allow "git shortlog" to group by committer information
From: Linus Torvalds @ 2016-12-16 2:00 UTC (permalink / raw)
To: Stephen & Linda Smith; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <3720429.U3o1zloj4W@thunderbird>
On Thu, Dec 15, 2016 at 5:51 PM, Stephen & Linda Smith <ischis2@cox.net> wrote:
>
> Why does gmail make it unnecessarily hard?
I read email with the gmail web interface, which is wonderful because
of the server-side searching etc. The only real downside is the weak
threading, but you get used to it.
I personally find IMAP and POP to be a tool of the devil, and have
never had a good experience with them as a mail interface. In theory
IMAP is supposed to support server-side searches, in practice it never
worked for me.
But the problem with sending patches using the web interface is that
you cannot attach things inline without gmail screwing up whitespace.
I suggested to some googler that a "attach inline" checkbox in the
would be a wonderful option for text attachments, but considering that
the android gmail app still has no text-only option I don't think that
suggestion went anywhere.
> I thought that a good percentage of the kernel maintainers use git send-email.
> what would make that command easier to use with gmail?
Oh, I can send inline stuff (as I just re-sent that patch), but then I
have to fire up alpine and do it the old-fashioned way. So it's an
extra step. So since I spend all my time at the gmail web interface
_anyway_, the attachment model ends up being the slightly more
convenient one.
And sure, I could use git-send-email as that extra step instead, but
I'd rather just use alpine. That's the extra step I do for some other
things (ie the "200-email patch-bomb from Andrew Morton" things - I'm
not using the web interface for _that_).
Linus
^ permalink raw reply
* [PATCH 1/1] Allow "git shortlog" to group by committer information
From: Linus Torvalds @ 2016-12-16 1:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqoa0cu3nn.fsf@gitster.mtv.corp.google.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Subject: Allow "git shortlog" to group by committer information
Date: Tue, 11 Oct 2016 11:45:58 -0700
In some situations you may want to group the commits not by author, but by
committer instead.
For example, when I just wanted to look up what I'm still missing from
linux-next in the current merge window, I don't care so much about who
wrote a patch, as what git tree it came from, which generally boils down
to "who committed it".
So make git shortlog take a "-c" or "--committer" option to switch
grouping to that. During the merge window this allows me to do things like
git shortlog -cnse linus..next |
head -20 |
cut -f2 |
sed 's/$/,/'
to easily create a list of the top-20 committers that I haven't gotten
pull requests from yet (the committer is not necessarily the person who
will send the pull request, but it's a reasonably good approximation).
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
builtin/shortlog.c | 15 ++++++++++++---
shortlog.h | 1 +
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index ba0e1154a..c9585d475 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -117,11 +117,15 @@ static void read_from_stdin(struct shortlog *log)
{
struct strbuf author = STRBUF_INIT;
struct strbuf oneline = STRBUF_INIT;
+ static const char *author_match[2] = { "Author: ", "author " };
+ static const char *committer_match[2] = { "Commit: ", "committer " };
+ const char **match;
+ match = log->committer ? committer_match : author_match;
while (strbuf_getline_lf(&author, stdin) != EOF) {
const char *v;
- if (!skip_prefix(author.buf, "Author: ", &v) &&
- !skip_prefix(author.buf, "author ", &v))
+ if (!skip_prefix(author.buf, match[0], &v) &&
+ !skip_prefix(author.buf, match[1], &v))
continue;
while (strbuf_getline_lf(&oneline, stdin) != EOF &&
oneline.len)
@@ -140,6 +144,7 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
struct strbuf author = STRBUF_INIT;
struct strbuf oneline = STRBUF_INIT;
struct pretty_print_context ctx = {0};
+ const char *fmt;
ctx.fmt = CMIT_FMT_USERFORMAT;
ctx.abbrev = log->abbrev;
@@ -148,7 +153,9 @@ void shortlog_add_commit(struct shortlog *log, struct commit *commit)
ctx.date_mode.type = DATE_NORMAL;
ctx.output_encoding = get_log_output_encoding();
- format_commit_message(commit, "%an <%ae>", &author, &ctx);
+ fmt = log->committer ? "%cn <%ce>" : "%an <%ae>";
+
+ format_commit_message(commit, fmt, &author, &ctx);
if (!log->summary) {
if (log->user_format)
pretty_print_commit(&ctx, commit, &oneline);
@@ -238,6 +245,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
int nongit = !startup_info->have_repository;
const struct option options[] = {
+ OPT_BOOL('c', "committer", &log.committer,
+ N_("Group by committer rather than author")),
OPT_BOOL('n', "numbered", &log.sort_by_number,
N_("sort output according to the number of commits per author")),
OPT_BOOL('s', "summary", &log.summary,
diff --git a/shortlog.h b/shortlog.h
index 5a326c686..5d64cfe92 100644
--- a/shortlog.h
+++ b/shortlog.h
@@ -13,6 +13,7 @@ struct shortlog {
int in2;
int user_format;
int abbrev;
+ int committer;
char *common_repo_prefix;
int email;
^ permalink raw reply related
* Re: Allow "git shortlog" to group by committer information
From: Linus Torvalds @ 2016-12-16 1:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqoa0cu3nn.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 15, 2016 at 4:19 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> This fell off the radar partly because of the distractions like
> "there are other attempts and other ways", and also because the
> message was not a text-plain that can be reviewed inline. Let me
> try to dig it up from the mail archive to see if I can find it.
Sorry, I'll just re-send it without the attachment. I prefer inline
myself, but I thought you didn't care (and gmail makes it
unnecessarily hard).
Linus
^ permalink raw reply
* Re: index-pack outside of repository?
From: Jeff King @ 2016-12-16 1:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqshpou3wt.fsf@gitster.mtv.corp.google.com>
On Thu, Dec 15, 2016 at 04:13:38PM -0800, Junio C Hamano wrote:
> > So I think complaining to the user is the right thing to do here. I
> > started to write a patch to have index-pack notice when it needs a repo
> > and doesn't have one, but the logic is actually a bit unclear. Do we
> > need to complain early _just_ when --stdin is specified, or does that
> > miss somes cases? Likewise, are there cases where --stdin can operate
> > without a repo? I couldn't think of any.
>
> I think there are two and only two major modes; --stdin wants to put
> the result in the repository it is working on, while the other mode
> takes a filename to deposit the result in, so the latter does not
> technically need a repository.
OK. That's easy to check for, then. Reverse-engineering that logic from
the actual calls in index-pack.c:final() is complicated. But certainly
basing it on --stdin is what I would have expected.
> > That strategy _might_ be a problem for some programs, which would want
> > to notice the issue early before doing work. But it seems like a
> > reasonable outcome for index-pack. Thoughts?
>
> That is, once we know which codepaths should require a repository, I
> think it is reasonable to add a check that is done earlier than the
> place where we currently try to see where we have one (which could
> be deep in the callchain). But we are all human and can miss things,
> so the BUG() thing is probably fine. We are cooking it exactly because
> we would want to find such corner cases we missed, no?
Right, that was my original intent in adding the BUG(): to catch
unhandled cases, and then do the appropriate thing earlier. I was just
questioning whether the appropriate thing in some cases might be dying
at the BUG(), just with a more friendly message. That has the benefit of
being very easy to implement, and never wrong (e.g., forbidding a case
that actually _doesn't_ need to look at the repo).
But if this case really is just "if (from_stdin)" that's quite easy,
too.
-Peff
^ permalink raw reply
* Re: Allow "git shortlog" to group by committer information
From: Junio C Hamano @ 2016-12-16 0:19 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <CA+55aFxSQ2wxU3cA+8uqS-W8mbobF35dVCZow2BcixGOOvGVFQ@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Just a ping on this patch..
>
> Jeff had apparently done something similar as part of a bigger
> patch-series, but I don't see that either. I really don't care very
> much how this is done, but I do find this very useful, ...
>
> Yes, I can just maintain this myself, and maybe nobody else needs it,
> but it's pretty simple and straightforward, and there didn't seem to
> be any real reason not to have the option..
This fell off the radar partly because of the distractions like
"there are other attempts and other ways", and also because the
message was not a text-plain that can be reviewed inline. Let me
try to dig it up from the mail archive to see if I can find it.
^ permalink raw reply
* Re: index-pack outside of repository?
From: Junio C Hamano @ 2016-12-16 0:13 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161215204000.avlcfaqjwstkptu2@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> In older versions of git will just blindly write into
> ".git/objects/pack", even though there's no repository there.
>
> So I think complaining to the user is the right thing to do here. I
> started to write a patch to have index-pack notice when it needs a repo
> and doesn't have one, but the logic is actually a bit unclear. Do we
> need to complain early _just_ when --stdin is specified, or does that
> miss somes cases? Likewise, are there cases where --stdin can operate
> without a repo? I couldn't think of any.
I think there are two and only two major modes; --stdin wants to put
the result in the repository it is working on, while the other mode
takes a filename to deposit the result in, so the latter does not
technically need a repository.
> I'm actually wondering if the way it calls die() in 'next' is a pretty
> reasonable way for things to work in general. It happens when we lazily
> try to ask for the repository directory. So we don't have to replicate
> logic to say "are we going to need a repo"; at the moment we need it, we
> notice we don't have it and die. The only problem is that it says "BUG"
> and not "this operation must be run in a git repository".
Isn't what we currently have is a good way to discover which
codepaths we missed to add a check to issue the latter error?
> That strategy _might_ be a problem for some programs, which would want
> to notice the issue early before doing work. But it seems like a
> reasonable outcome for index-pack. Thoughts?
That is, once we know which codepaths should require a repository, I
think it is reasonable to add a check that is done earlier than the
place where we currently try to see where we have one (which could
be deep in the callchain). But we are all human and can miss things,
so the BUG() thing is probably fine. We are cooking it exactly because
we would want to find such corner cases we missed, no?
^ permalink raw reply
* Re: Is there a way to have local changes in a branch 'bake' while working in different branches?
From: Junio C Hamano @ 2016-12-16 0:07 UTC (permalink / raw)
To: Larry Minton; +Cc: git@vger.kernel.org
In-Reply-To: <14b481f95c5043aca6cdfddfe4728fa9@BLUPR79MB001.MGDADSK.autodesk.com>
Larry Minton <larry.minton@autodesk.com> writes:
> Lars Schneider recommended I ask you this question.
>
> My question:
>
> Let's say I have a code change that I want to 'bake' for a while
> locally, just to make sure some edge case doesn't pop up while I
> am working on other things. Is there any practical way of doing
> that?
That sounds exactly like what I have been doing for the past several
years in public around here ;-)
^ permalink raw reply
* [RFC/PATCH] Makefile: suppress some cppcheck false-positives
From: Chris Packham @ 2016-12-15 23:22 UTC (permalink / raw)
To: git; +Cc: peff, stefan.naewe, gitter.spiros, Chris Packham
In-Reply-To: <20161214112401.mq3n5kui5eeebdtk@sigill.intra.peff.net>
Pass a list of suppressions to cppcheck so that legitimate errors are
more obvious.
Signed-off-by: Chris Packham <judge.packham@gmail.com>
---
On Thu, Dec 15, 2016 at 12:24 AM, Jeff King <peff@peff.net> wrote:
> The patch itself is OK to me, I guess. The interesting part will be
> whether people start actually _using_ cppcheck and squelching the false
> positives. I'm not sure how I feel about the in-code annotations. I'd
> have to see a patch first.
So here's a patch that adds supression files. It would work well for
things in contrib/compat that don't change that often. It would be a
nightmare to maintain for high-touch code.
Makefile | 7 ++++++-
nedmalloc.supp | 4 ++++
regcomp.supp | 8 ++++++++
3 files changed, 18 insertions(+), 1 deletion(-)
create mode 100644 nedmalloc.supp
create mode 100644 regcomp.supp
diff --git a/Makefile b/Makefile
index e5c86decf..bb335ca0f 100644
--- a/Makefile
+++ b/Makefile
@@ -2637,7 +2637,12 @@ cover_db_html: cover_db
.PHONY: cppcheck
-CPPCHECK_FLAGS = --force --quiet --inline-suppr $(if $(CPPCHECK_ADD),--enable=$(CPPCHECK_ADD))
+CPPCHECK_SUPP = --suppressions-list=nedmalloc.supp \
+ --suppressions-list=regcomp.supp
+
+CPPCHECK_FLAGS = --force --quiet --inline-suppr \
+ $(if $(CPPCHECK_ADD),--enable=$(CPPCHECK_ADD)) \
+ $(CPPCHECK_SUPP)
cppcheck:
@cppcheck --version
diff --git a/nedmalloc.supp b/nedmalloc.supp
new file mode 100644
index 000000000..37bd54def
--- /dev/null
+++ b/nedmalloc.supp
@@ -0,0 +1,4 @@
+nullPointer:compat/nedmalloc/malloc.c.h:4093
+nullPointer:compat/nedmalloc/malloc.c.h:4106
+memleak:compat/nedmalloc/malloc.c.h:4646
+
diff --git a/regcomp.supp b/regcomp.supp
new file mode 100644
index 000000000..3ae023c26
--- /dev/null
+++ b/regcomp.supp
@@ -0,0 +1,8 @@
+memleak:compat/regex/regcomp.c:3086
+memleak:compat/regex/regcomp.c:3634
+memleak:compat/regex/regcomp.c:3086
+memleak:compat/regex/regcomp.c:3634
+uninitvar:compat/regex/regcomp.c:2802
+uninitvar:compat/regex/regcomp.c:2805
+memleak:compat/regex/regcomp.c:532
+
--
2.11.0.24.ge6920cf
^ permalink raw reply related
* Re: Is there a way to have local changes in a branch 'bake' while working in different branches?
From: Aaron Schrab @ 2016-12-15 22:44 UTC (permalink / raw)
To: Larry Minton; +Cc: git@vger.kernel.org
In-Reply-To: <14b481f95c5043aca6cdfddfe4728fa9@BLUPR79MB001.MGDADSK.autodesk.com>
At 20:14 +0000 15 Dec 2016, Larry Minton <larry.minton@autodesk.com> wrote:
>Let's say I have a code change that I want to 'bake' for a while
>locally, just to make sure some edge case doesn't pop up while I am
>working on other things. Is there any practical way of doing that?
>I could constantly merge that 'bake me' branch into other branches as I
>work on them and then remove those changes from the branches before
>sending them out for code review, but sooner or later pretty much
>guaranteed to screw that up....
That sounds like the best way to me. How do you envision screwing it up?
If you anticipate messing up while removing the changes, that's only
likely if there are conflicts and any other strategy is likely to be
worse there.
If you suspect you'll forget to remove them before sending for code
review there may be ways to help with that. Easiest you can add a large
notice in the commit message(s) and/or comments; this may not prevent
going for review but reviewers should catch it pretty quickly. To help
prevent it even getting that far you may be able to add a pre-push hook
to prevent such commits from being pushed to somewhere other than a
private fork or a branch with a name that clearly indicates that it
contains experimental code.
^ permalink raw reply
* Re: [PATCH] README: replace gmane link with public-inbox
From: Eric Wong @ 2016-12-15 21:57 UTC (permalink / raw)
To: Jeff King; +Cc: Chiel ten Brinke, git
In-Reply-To: <20161215141719.52peppv5pbjk3nuf@sigill.intra.peff.net>
Jeff King <peff@peff.net> wrote:
> On Thu, Dec 15, 2016 at 02:57:18PM +0100, Chiel ten Brinke wrote:
>
> > Btw, the link in the README
> > http://news.gmane.org/gmane.comp.version-control.git/ is dead.
>
> Yes, the status of gmane was up in the air for a while, but I think we
> can give it up as dead now (at least for our purposes).
s/http/nntp/ still works for gmane.
> -- >8 --
> Subject: README: replace gmane link with public-inbox
>
> The general status and future of gmane is unclear at this
> point, but certainly it does not seem to be carrying
> gmane.comp.version-control.git at all anymore. Let's point
> to public-inbox.org, which seems to be the favored archive
> on the list these days (and which uses message-ids in its
> URLs, making the links somewhat future-proof).
No objections, here.
There's also https://mail-archive.com/git@vger.kernel.org
Where https://mid.mail-archive.com/<Message-ID> also works
^ permalink raw reply
* Re: Allow "git shortlog" to group by committer information
From: Linus Torvalds @ 2016-12-15 21:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <CA+55aFzWkE43rSm-TJNKkHq4F3eOiGR0-Bo9V1=a1s=vQ0KPqQ@mail.gmail.com>
Just a ping on this patch..
On Tue, Oct 11, 2016 at 11:45 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> In some situations you may want to group the commits not by author,
> but by committer instead.
>
> For example, when I just wanted to look up what I'm still missing from
> linux-next in the current merge window [..]
It's another merge window later for the kernel, and I just re-applied
this patch to my git tree because I still want to know teh committer
information rather than the authorship information, and it still seems
to be the simplest way to do that.
Jeff had apparently done something similar as part of a bigger
patch-series, but I don't see that either. I really don't care very
much how this is done, but I do find this very useful, I do things
like
git shortlog -cnse linus..next |
head -20 |
cut -f2 |
sed 's/$/,/'
to generate a nice list of the top-20 committers that I haven't gotten
pull requests from yet.
Yes, I can just maintain this myself, and maybe nobody else needs it,
but it's pretty simple and straightforward, and there didn't seem to
be any real reason not to have the option..
Linus
^ permalink raw reply
* index-pack outside of repository?
From: Jeff King @ 2016-12-15 20:40 UTC (permalink / raw)
To: git
Running git on 'next', you can trigger a BUG:
$ cd /some/repo
$ git pack-objects --all --stdout </dev/null >/tmp/foo.pack
$ cd /not-a-git-repo
$ git index-pack --stdin </tmp/foo.pack
fatal: BUG: setup_git_env called without repository
This obviously comes from my b1ef400eec (setup_git_env: avoid blind
fall-back to ".git", 2016-10-20). What's going on is that index-pack
uses RUN_SETUP_GENTLY, but never actually handles the out-of-repo case.
When we use the internal git_dir to make "objects/pack/pack-xxx.pack",
it barfs.
In older versions of git will just blindly write into
".git/objects/pack", even though there's no repository there.
So I think complaining to the user is the right thing to do here. I
started to write a patch to have index-pack notice when it needs a repo
and doesn't have one, but the logic is actually a bit unclear. Do we
need to complain early _just_ when --stdin is specified, or does that
miss somes cases? Likewise, are there cases where --stdin can operate
without a repo? I couldn't think of any.
I'm actually wondering if the way it calls die() in 'next' is a pretty
reasonable way for things to work in general. It happens when we lazily
try to ask for the repository directory. So we don't have to replicate
logic to say "are we going to need a repo"; at the moment we need it, we
notice we don't have it and die. The only problem is that it says "BUG"
and not "this operation must be run in a git repository".
That strategy _might_ be a problem for some programs, which would want
to notice the issue early before doing work. But it seems like a
reasonable outcome for index-pack. Thoughts?
-Peff
^ 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