* 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
* 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: 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: [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: 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: 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: 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
* 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: Races on ref .lock files?
From: Junio C Hamano @ 2016-12-16 17:20 UTC (permalink / raw)
To: Andreas Krey; +Cc: git
In-Reply-To: <20161216164751.GA12174@inner.h.apk.li>
Andreas Krey <a.krey@gmx.de> writes:
> 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.
I think (and I think you also think) these messages come from the
Bitbucket side, not your "git push" (or "git fetch"). Not having
seen Bitbucket's sources, I can only guess, but assuming that its
pull-request is triggered from their Web frontend like GitHub's
does, it is quite possible when you try to "push" into (or "fetch"
from, for that matter) a repository, somebody is clicking a button
to create that ref. We do not know what their "receive-pack" that
responds to your "git push" (or "upload-pack" for "git fetch") does
when there are locked refs. I'd naively think that unless you are
pushing to that ref you showed an error message for, the receiving
end shouldn't care if the ref is being written by somebody else, but
who knows ;-) They may have their own reasons wanting to lock that
ref that we think would be irrelevant for the operation, causing
errors.
^ permalink raw reply
* Re: Allow "git shortlog" to group by committer information
From: Junio C Hamano @ 2016-12-16 17:27 UTC (permalink / raw)
To: Jeff King; +Cc: Linus Torvalds, Git Mailing List
In-Reply-To: <20161216135141.yhas67pzfm7bxxum@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> 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).
Yeah, I think it probably is OK.
As it is very clear that "group by author" is the default, there is
no need to add the corresponding "-a/--author" option, either. The
fact that "--no-committer" can countermand an earlier "--committer"
on the command line is just how options work, so it probably does
not deserve a separate mention, either.
Thanks.
> -- >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
^ permalink raw reply
* Re: [PATCH 1/1] mingw: intercept isatty() to handle /dev/null as Git expects it
From: Johannes Sixt @ 2016-12-16 17:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Pranit Bauva
In-Reply-To: <42ddc270ea04e01e899cc479063e5d602e4a4448.1481454992.git.johannes.schindelin@gmx.de>
Am 11.12.2016 um 12:16 schrieb Johannes Schindelin:
> When Git's source code calls isatty(), it really asks whether the
> respective file descriptor is connected to an interactive terminal.
>
> Windows' _isatty() function, however, determines whether the file
> descriptor is associated with a character device. And NUL, Windows'
> equivalent of /dev/null, is a character device.
>
> Which means that for years, Git mistakenly detected an associated
> interactive terminal when being run through the test suite, which
> almost always redirects stdin, stdout and stderr to /dev/null.
>
> This bug only became obvious, and painfully so, when the new
> bisect--helper entered the `pu` branch and made the automatic build & test
> time out because t6030 was waiting for an answer.
>
> For details, see
>
> https://msdn.microsoft.com/en-us/library/f4s0ddew.aspx
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> compat/mingw.h | 3 +++
> compat/winansi.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 36 insertions(+)
>
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 034fff9479..3350169555 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -384,6 +384,9 @@ int mingw_raise(int sig);
> * ANSI emulation wrappers
> */
>
> +int winansi_isatty(int fd);
> +#define isatty winansi_isatty
> +
> void winansi_init(void);
> HANDLE winansi_get_osfhandle(int fd);
>
> diff --git a/compat/winansi.c b/compat/winansi.c
> index db4a5b0a37..cb725fb02f 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -7,6 +7,9 @@
> #include <wingdi.h>
> #include <winreg.h>
>
> +/* In this file, we actually want to use Windows' own isatty(). */
> +#undef isatty
> +
> /*
> ANSI codes used by git: m, K
>
> @@ -570,6 +573,36 @@ static void detect_msys_tty(int fd)
>
> #endif
>
> +int winansi_isatty(int fd)
> +{
> + int res = isatty(fd);
> +
> + if (res) {
> + /*
> + * Make sure that /dev/null is not fooling Git into believing
> + * that we are connected to a terminal, as "_isatty() returns a
> + * nonzero value if the descriptor is associated with a
> + * character device."; for more information, see
> + *
> + * https://msdn.microsoft.com/en-us/library/f4s0ddew.aspx
> + */
> + HANDLE handle = (HANDLE)_get_osfhandle(fd);
> + if (fd == STDIN_FILENO) {
> + DWORD dummy;
> +
> + if (!GetConsoleMode(handle, &dummy))
> + res = 0;
> + } else if (fd == STDOUT_FILENO || fd == STDERR_FILENO) {
> + CONSOLE_SCREEN_BUFFER_INFO dummy;
> +
> + if (!GetConsoleScreenBufferInfo(handle, &dummy))
> + res = 0;
I am sorry to have to report that this check does not work as expected.
I am operating Git from CMD, not mintty, BTW.
It fails with GetLastError() == 6 (invalid handle value). The reason for
this is, I think, that in reality we are not writing to the console
directly, but to a pipe, which is drained by console_thread(), which
writes to the console.
I have little clue what this winansi.c file is doing. Do you have any
pointers where I should start digging?
Wait...
Should we not use winansi_get_osfhandle() instead of _get_osfhandle()?
> + }
> + }
> +
> + return res;
> +}
> +
> void winansi_init(void)
> {
> int con1, con2;
>
^ permalink raw reply
* Re: index-pack outside of repository?
From: Junio C Hamano @ 2016-12-16 17:52 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20161216022904.cjang6napnl2vkc6@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> 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
I think 2/3 is a good change to ensure we get a reasonable error for
"index-pack --stdin", and 3/3 is a very good cleanup. Both of them
of course are enabled by 1/3.
We still fail "nongit git index-pack tmp.pack" with a BUG: though.
^ permalink raw reply
* Re: [PATCH] README: replace gmane link with public-inbox
From: Junio C Hamano @ 2016-12-16 17:57 UTC (permalink / raw)
To: Eric Wong; +Cc: Jeff King, Chiel ten Brinke, git
In-Reply-To: <20161215215702.GA28777@starla>
Eric Wong <e@80x24.org> writes:
> Jeff King <peff@peff.net> wrote:
> ...
>> 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
> ...
> No objections, here.
>
> There's also https://mail-archive.com/git@vger.kernel.org
> Where https://mid.mail-archive.com/<Message-ID> also works
Yes, but do we want to be exhaustive here, of just cite one that is
a useful starting point? I think it is the latter.
Mentioning nntp for those who prefer (including me) may have value,
though.
README.md | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/README.md b/README.md
index c0cd5580ea..91704fe451 100644
--- a/README.md
+++ b/README.md
@@ -34,7 +34,8 @@ requests, comments and patches to git@vger.kernel.org (read
To subscribe to the list, send an email with just "subscribe git" in
the body to majordomo@vger.kernel.org. The mailing list archives are
available at https://public-inbox.org/git,
-http://marc.info/?l=git and other archival sites.
+http://marc.info/?l=git and other archival sites.
+Those who prefer NNTP can use nntp://news.public-inbox.org/inbox.comp.version-control.git
The maintainer frequently sends the "What's cooking" reports that
list the current status of various development topics to the mailing
^ permalink raw reply related
* Re: index-pack outside of repository?
From: Junio C Hamano @ 2016-12-16 17:59 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <xmqqd1gru5fw.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> 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
>
> I think 2/3 is a good change to ensure we get a reasonable error for
> "index-pack --stdin", and 3/3 is a very good cleanup. Both of them
> of course are enabled by 1/3.
>
> We still fail "nongit git index-pack tmp.pack" with a BUG: though.
Wait.
This only happens with a stalled-and-to-be-discarded topic on 'pu'.
Please don't waste time digging it (yet).
^ permalink raw reply
* Re: index-pack outside of repository?
From: Junio C Hamano @ 2016-12-16 18:01 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <xmqq7f6zu55k.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> 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
>>
>> I think 2/3 is a good change to ensure we get a reasonable error for
>> "index-pack --stdin", and 3/3 is a very good cleanup. Both of them
>> of course are enabled by 1/3.
>>
>> We still fail "nongit git index-pack tmp.pack" with a BUG: though.
>
> Wait.
>
> This only happens with a stalled-and-to-be-discarded topic on 'pu'.
> Please don't waste time digging it (yet).
Don't wait ;-). My mistake. We can see t5300 broken with this
change and b1ef400eec ("setup_git_env: avoid blind fall-back to
".git"", 2016-10-20) without anything else. We still need to
address it.
^ permalink raw reply
* Re: [PATCH 1/1] mingw: intercept isatty() to handle /dev/null as Git expects it
From: Junio C Hamano @ 2016-12-16 18:08 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Johannes Schindelin, git, Pranit Bauva
In-Reply-To: <129f000c-49c1-0e75-26b3-c96e9b442443@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> Am 11.12.2016 um 12:16 schrieb Johannes Schindelin:
>> When Git's source code calls isatty(), it really asks whether the
>> respective file descriptor is connected to an interactive terminal.
>> ...
>> + if (!GetConsoleScreenBufferInfo(handle, &dummy))
>> + res = 0;
>
> I am sorry to have to report that this check does not work as
> expected. I am operating Git from CMD, not mintty, BTW.
Ouch.
Sorry for not having waited for you to chime in before agreeing to
fast-track this one, and now this is in 'master'.
I should have known better than that by now, after having seen you
uncover bugs that did not trigger in Dscho's environment and vice
versa to tell you that Windows specific things both of you deem good
have a much higher chance of being correct than a change without an
Ack by the other.
> It fails with GetLastError() == 6 (invalid handle value). The reason
> for this is, I think, that in reality we are not writing to the
> console directly, but to a pipe, which is drained by console_thread(),
> which writes to the console.
>
> I have little clue what this winansi.c file is doing. Do you have any
> pointers where I should start digging?
>
> Wait...
>
> Should we not use winansi_get_osfhandle() instead of _get_osfhandle()?
>
>> + }
>> + }
>> +
>> + return res;
>> +}
>> +
>> void winansi_init(void)
>> {
>> int con1, con2;
>>
^ permalink raw reply
* Segfault in git_config_set_multivar_in_file_gently with direct_io in FUSE filesystem
From: Josh Bleecher Snyder @ 2016-12-16 18:41 UTC (permalink / raw)
To: git
I am using git with a simple in-memory FUSE filesystem. When I enable
direct_io, I get a segfault from
git_config_set_multivar_in_file_gently during git clone.
I have full reproduction instructions using Go and macOS at
https://github.com/josharian/gitbug. It also includes a stack trace in
case anyone wants to try blind debugging. Happy to provide more info
if it will help.
Thanks,
Josh
^ permalink raw reply
* Re: [RFC/PATCH] Makefile: suppress some cppcheck false-positives
From: Junio C Hamano @ 2016-12-16 18:43 UTC (permalink / raw)
To: Chris Packham; +Cc: git, peff, stefan.naewe, gitter.spiros
In-Reply-To: <20161215232240.19427-1-judge.packham@gmail.com>
Chris Packham <judge.packham@gmail.com> writes:
> -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)
Has it been agreed that it is a good idea to tie $(CPPCHECK_ADD)
only to --enable? I somehow thought I saw an objection to this.
> 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
> +
Yuck for both files for multiple reasons.
I do not think it is a good idea to allow these files to clutter the
top-level of tree. How often do we expect that we may have to add
more of these files? Every time we start borrowing code from third
parties?
What is the goal we want to achieve by running cppcheck?
a. Our code must be clean but we do not bother "fixing" [*1*] the
code we borrow from third parties and squelch output instead?
b. Both our own code and third party code we borrow need to be free
of errors and misdetections from cppcheck?
c. Something else?
If a. is what we aim for, perhaps a better option may be not to run
cppcheck for the code we borrowed from third-party at all in the
first place.
If b. is our goal, we need to make sure that the false positive rate
of cppcheck is acceptably low.
[Footnote]
*1* "Fixing" a real problem it uncovers is a good thing, but it may
have to also involve working around false positives reported by
cppcheck, either by rewriting a perfectly correct code to please
it, or adding these line-number based suppression that is
totally unworkable. Like Peff, I'm worried that we will end up
with two static checkers fighting each other, and no good way to
please both.
^ permalink raw reply
* Re: [PATCH 1/1] mingw: intercept isatty() to handle /dev/null as Git expects it
From: Johannes Sixt @ 2016-12-16 18:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, Pranit Bauva
In-Reply-To: <xmqqy3zfsq4q.fsf@gitster.mtv.corp.google.com>
Am 16.12.2016 um 19:08 schrieb Junio C Hamano:
> Sorry for not having waited for you to chime in before agreeing to
> fast-track this one, and now this is in 'master'.
No reason to be sorry, things happen... Dscho's request for
fast-tracking was very reasonable, and the patch looked "obviously
correct". I lacked time to test it promptly, and when I finally did, I
ignored the symptoms because my workflow was a mess and I had to
concentrate on other things. Only after 'git push' did not report the
pack progress, was my suspicion raised...
>> Should we not use winansi_get_osfhandle() instead of _get_osfhandle()?
Unfortunately, I'm away from my Windows box over the weekend. It will
have to wait until Monday before I can test this idea.
-- Hannes
^ permalink raw reply
* Re: index-pack outside of repository?
From: Junio C Hamano @ 2016-12-16 18:54 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <xmqqshpou3wt.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
> ...
>> 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?
I am tempted to suggest an intermediate step that comes before
b1ef400eec ("setup_git_env: avoid blind fall-back to ".git"",
2016-10-20), which is the attached, and publish that as part of an
official release. That way, we'll see what is broken without
hurting people too much (unless they or their scripts care about
extra message given to the standard error stream). I suspect that
released Git has a slightly larger user base than what is cooked on
'next'.
environment.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/environment.c b/environment.c
index 0935ec696e..88f857331e 100644
--- a/environment.c
+++ b/environment.c
@@ -167,8 +167,11 @@ static void setup_git_env(void)
const char *replace_ref_base;
git_dir = getenv(GIT_DIR_ENVIRONMENT);
- if (!git_dir)
+ if (!git_dir) {
+ if (!startup_info->have_repository)
+ warning("BUG: please report this at git@vger.kernel.org");
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
+ }
gitfile = read_gitfile(git_dir);
git_dir = xstrdup(gitfile ? gitfile : git_dir);
if (get_common_dir(&sb, git_dir))
^ permalink raw reply related
* Re: [PATCH v15 08/27] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
From: Pranit Bauva @ 2016-12-16 19:00 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Git List
In-Reply-To: <CAFZEwPOZhO=sXLVwh03C8QN0uVXBUfb=xZ-JS003tgCNLgVOjg@mail.gmail.com>
Hey Stephan,
On Wed, Dec 7, 2016 at 1:03 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>> I don't understand why the return value is int and not void. To avoid a
>> "return 0;" line when calling this function?
>
> Initially I thought I would be using the return value but now I
> realize that it is meaningless to do so. Using void seems better. :)
I just recollected when I was creating the next iteration of this
series that I will need that int.
>>> + case CHECK_EXPECTED_REVS:
>>> + return check_expected_revs(argv, argc);
See this.
Regards,
Pranit Bauva
^ permalink raw reply
* [PATCH v7 0/7] recursively grep across submodules
From: Brandon Williams @ 2016-12-16 19:03 UTC (permalink / raw)
To: git
Cc: peff, sbeller, jonathantanmy, gitster, jacob.keller, j6t,
Brandon Williams
In-Reply-To: <1480555714-186183-1-git-send-email-bmwill@google.com>
Changes in v7:
* Rebased on 'origin/bw/realpath-wo-chdir' in order to fix the race condition
that occurs when verifying a submodule's gitdir.
* Reverted is_submodule_populated() to use resolve_gitdir() now that there is
no race condition.
* Added !MINGW to a test in t7814 so that it won't run on windows. This is due
to testing if colons in filenames are still handled correctly, yet windows
doesn't allow colons in filenames.
Brandon Williams (7):
submodules: add helper to determine if a submodule is populated
submodules: add helper to determine if a submodule is initialized
submodules: load gitmodules file from commit sha1
grep: add submodules as a grep source type
grep: optionally recurse into submodules
grep: enable recurse-submodules to work on <tree> objects
grep: search history of moved submodules
Documentation/git-grep.txt | 14 ++
builtin/grep.c | 386 ++++++++++++++++++++++++++++++++++---
cache.h | 2 +
config.c | 8 +-
git.c | 2 +-
grep.c | 16 +-
grep.h | 1 +
submodule-config.c | 6 +-
submodule-config.h | 3 +
submodule.c | 50 +++++
submodule.h | 3 +
t/t7814-grep-recurse-submodules.sh | 241 +++++++++++++++++++++++
tree-walk.c | 28 +++
13 files changed, 729 insertions(+), 31 deletions(-)
create mode 100755 t/t7814-grep-recurse-submodules.sh
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply
* [PATCH v7 1/7] submodules: add helper to determine if a submodule is populated
From: Brandon Williams @ 2016-12-16 19:03 UTC (permalink / raw)
To: git
Cc: peff, sbeller, jonathantanmy, gitster, jacob.keller, j6t,
Brandon Williams
In-Reply-To: <1481915002-162130-1-git-send-email-bmwill@google.com>
Add the `is_submodule_populated()` helper function to submodules.c.
`is_submodule_populated()` performes a check to see if a submodule has
been checkout out (and has a valid .git directory/file) at the given path.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
submodule.c | 15 +++++++++++++++
submodule.h | 1 +
2 files changed, 16 insertions(+)
diff --git a/submodule.c b/submodule.c
index c85ba50..ee3198d 100644
--- a/submodule.c
+++ b/submodule.c
@@ -198,6 +198,21 @@ void gitmodules_config(void)
}
}
+/*
+ * Determine if a submodule has been populated at a given 'path'
+ */
+int is_submodule_populated(const char *path)
+{
+ int ret = 0;
+ char *gitdir = xstrfmt("%s/.git", path);
+
+ if (resolve_gitdir(gitdir))
+ ret = 1;
+
+ free(gitdir);
+ return ret;
+}
+
int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst)
{
diff --git a/submodule.h b/submodule.h
index d9e197a..c4af505 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,6 +37,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
const char *path);
int submodule_config(const char *var, const char *value, void *cb);
void gitmodules_config(void);
+extern int is_submodule_populated(const char *path);
int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst);
const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v7 3/7] submodules: load gitmodules file from commit sha1
From: Brandon Williams @ 2016-12-16 19:03 UTC (permalink / raw)
To: git
Cc: peff, sbeller, jonathantanmy, gitster, jacob.keller, j6t,
Brandon Williams
In-Reply-To: <1481915002-162130-1-git-send-email-bmwill@google.com>
teach submodules to load a '.gitmodules' file from a commit sha1. This
enables the population of the submodule_cache to be based on the state
of the '.gitmodules' file from a particular commit.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
cache.h | 2 ++
config.c | 8 ++++----
submodule-config.c | 6 +++---
submodule-config.h | 3 +++
submodule.c | 12 ++++++++++++
submodule.h | 1 +
6 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/cache.h b/cache.h
index e12a5d9..de237ca 100644
--- a/cache.h
+++ b/cache.h
@@ -1693,6 +1693,8 @@ extern int git_default_config(const char *, const char *, void *);
extern int git_config_from_file(config_fn_t fn, const char *, void *);
extern int git_config_from_mem(config_fn_t fn, const enum config_origin_type,
const char *name, const char *buf, size_t len, void *data);
+extern int git_config_from_blob_sha1(config_fn_t fn, const char *name,
+ const unsigned char *sha1, void *data);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
extern void git_config(config_fn_t fn, void *);
diff --git a/config.c b/config.c
index 83fdecb..4d78e72 100644
--- a/config.c
+++ b/config.c
@@ -1214,10 +1214,10 @@ int git_config_from_mem(config_fn_t fn, const enum config_origin_type origin_typ
return do_config_from(&top, fn, data);
}
-static int git_config_from_blob_sha1(config_fn_t fn,
- const char *name,
- const unsigned char *sha1,
- void *data)
+int git_config_from_blob_sha1(config_fn_t fn,
+ const char *name,
+ const unsigned char *sha1,
+ void *data)
{
enum object_type type;
char *buf;
diff --git a/submodule-config.c b/submodule-config.c
index 098085b..8b9a2ef 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -379,9 +379,9 @@ static int parse_config(const char *var, const char *value, void *data)
return ret;
}
-static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
- unsigned char *gitmodules_sha1,
- struct strbuf *rev)
+int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
+ unsigned char *gitmodules_sha1,
+ struct strbuf *rev)
{
int ret = 0;
diff --git a/submodule-config.h b/submodule-config.h
index d05c542..78584ba 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -29,6 +29,9 @@ const struct submodule *submodule_from_name(const unsigned char *commit_sha1,
const char *name);
const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
const char *path);
+extern int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,
+ unsigned char *gitmodules_sha1,
+ struct strbuf *rev);
void submodule_free(void);
#endif /* SUBMODULE_CONFIG_H */
diff --git a/submodule.c b/submodule.c
index edffaa1..2600908 100644
--- a/submodule.c
+++ b/submodule.c
@@ -198,6 +198,18 @@ void gitmodules_config(void)
}
}
+void gitmodules_config_sha1(const unsigned char *commit_sha1)
+{
+ struct strbuf rev = STRBUF_INIT;
+ unsigned char sha1[20];
+
+ if (gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {
+ git_config_from_blob_sha1(submodule_config, rev.buf,
+ sha1, NULL);
+ }
+ strbuf_release(&rev);
+}
+
/*
* Determine if a submodule has been initialized at a given 'path'
*/
diff --git a/submodule.h b/submodule.h
index 6ec5f2f..9203d89 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,6 +37,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
const char *path);
int submodule_config(const char *var, const char *value, void *cb);
void gitmodules_config(void);
+extern void gitmodules_config_sha1(const unsigned char *commit_sha1);
extern int is_submodule_initialized(const char *path);
extern int is_submodule_populated(const char *path);
int parse_submodule_update_strategy(const char *value,
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH v7 2/7] submodules: add helper to determine if a submodule is initialized
From: Brandon Williams @ 2016-12-16 19:03 UTC (permalink / raw)
To: git
Cc: peff, sbeller, jonathantanmy, gitster, jacob.keller, j6t,
Brandon Williams
In-Reply-To: <1481915002-162130-1-git-send-email-bmwill@google.com>
Add the `is_submodule_initialized()` helper function to submodules.c.
`is_submodule_initialized()` performs a check to determine if the
submodule at the given path has been initialized.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
submodule.c | 23 +++++++++++++++++++++++
submodule.h | 1 +
2 files changed, 24 insertions(+)
diff --git a/submodule.c b/submodule.c
index ee3198d..edffaa1 100644
--- a/submodule.c
+++ b/submodule.c
@@ -199,6 +199,29 @@ void gitmodules_config(void)
}
/*
+ * Determine if a submodule has been initialized at a given 'path'
+ */
+int is_submodule_initialized(const char *path)
+{
+ int ret = 0;
+ const struct submodule *module = NULL;
+
+ module = submodule_from_path(null_sha1, path);
+
+ if (module) {
+ char *key = xstrfmt("submodule.%s.url", module->name);
+ char *value = NULL;
+
+ ret = !git_config_get_string(key, &value);
+
+ free(value);
+ free(key);
+ }
+
+ return ret;
+}
+
+/*
* Determine if a submodule has been populated at a given 'path'
*/
int is_submodule_populated(const char *path)
diff --git a/submodule.h b/submodule.h
index c4af505..6ec5f2f 100644
--- a/submodule.h
+++ b/submodule.h
@@ -37,6 +37,7 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
const char *path);
int submodule_config(const char *var, const char *value, void *cb);
void gitmodules_config(void);
+extern int is_submodule_initialized(const char *path);
extern int is_submodule_populated(const char *path);
int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
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