Re: tools for easily "uncommitting" parts of a patch I just commited?

2016-10-19 Thread Jacob Keller
On Wed, Oct 19, 2016 at 7:13 PM, Jeff King  wrote:
> I suspect both of those would complain about legitimate workflows.
>
> I dunno.  I do not ever use "git commit " myself. I almost
> invariably use one of "git add -p" (to review changes as I add them) or
> "git add -u" (when I know everything is in good shape, such as after
> merge resolution; I'll sometimes just "git commit -a", too).
>
> But it sounds like you want a third mode besides "--include" and
> "--only". Basically "commit what has been staged already, but restrict
> the commit to the paths I mentioned". Something like "--only-staged" or
> something. I do not think we would want to change the default from
> --only, but I could see a config option or something to select that
> behavior.
>
> I suspect nobody has really asked for such a thing before because
> separate staging and "git commit " are really two distinct
> workflows. Your pain comes from mix-and-matching them.
>
> -Peff

No. What I want is to *prevent* mix-and-match from happening.
Basically, sometimes I use "git add -p" to stage changes. But if I
just did a "git diff" and I know all the changes that I want are in
the file I will just do "git commit " or "git commit -a". The
problem is that sometimes I stage stuff, forget or just make a brain
mistake, and I go ahead and use "git commit "

What I want is to make it so that when you run "git commit "
that it is smart enough to go "hey! You already staged something from
that file in the index and it doesn't match what you're asking me to
commit now, so I'm going to stop and make sure you either reset, don't
run "git commit " or run "git commit --only " or similar.

It's just about making it so that if I happen to make the mistake in
the future it doesn't generate a commit and instead tells me that I
was being an idiot. I don't want this check to just be "you can't
stage with the index and then tell me to commit -a or commit "
because I think that's too restrictive and might make people complain.

Essentially, I want the tool to become smart enough to make it so that
an obvious mistake is caught early before I have to undo things.

That being said, it's much less of a pain point now that I know I can
go "git reset -p HEAD^". It never occurred to me that git reset -p
would work that way, so I didn't even try it.

Thanks,
Jake


Re: tools for easily "uncommitting" parts of a patch I just commited?

2016-10-19 Thread Jeff King
On Wed, Oct 19, 2016 at 04:36:36PM -0700, Jacob Keller wrote:

> >   # undo selectively
> >   git reset -p HEAD^
> >   git commit --amend
> 
> AHA! I knew about git reset -p but I didn't know about git reset -p
> allowed passing a treeish. Does this reset modify my local files at
> all? I think it doesn't, right?

Correct. If you wanted to modify the working tree, too, use "git
checkout -p HEAD^".

> I still think it's worth while to add a check for git-commit which
> does something like check when we say "git commit " and if the
> index already has those files marked as being changed, compare them
> with the current contents of the file as in the checkout and quick
> saying "please don't do that" so as to avoid the problem in the first
> place.
> 
> A naive approach would just be "if index already has staged
> differences dont allow path selection" but that doesn't let me do
> something like "git add -p " "git commit "

I suspect both of those would complain about legitimate workflows.

I dunno.  I do not ever use "git commit " myself. I almost
invariably use one of "git add -p" (to review changes as I add them) or
"git add -u" (when I know everything is in good shape, such as after
merge resolution; I'll sometimes just "git commit -a", too).

But it sounds like you want a third mode besides "--include" and
"--only". Basically "commit what has been staged already, but restrict
the commit to the paths I mentioned". Something like "--only-staged" or
something. I do not think we would want to change the default from
--only, but I could see a config option or something to select that
behavior.

I suspect nobody has really asked for such a thing before because
separate staging and "git commit " are really two distinct
workflows. Your pain comes from mix-and-matching them.

-Peff


Re: Problems with "git svn clone"

2016-10-19 Thread Eric Wong
K Richard Pixley  wrote:
> On 10/19/16 13:41 , Eric Wong wrote:
> >K Richard Pixley  wrote:
> >>error: git-svn died of signal 11
> >>
> >>This seems awfully early and blatant for a core dump.  What can I do to
> >>get this running?
> >Can you show us a backtrace?  Thanks.
> There is none.  I ran it in gdb and bt produced no results.  Nor did the
> thread send bt command.

You probably need to install the debug package(s) for subversion.
In case it's a different bug, maybe the debug package for Perl, too.

> >>Initially discovered on git-2.7.4, (ubuntu-16.04), but also reproduced
> >>on freshly built top of tree git-2.10.1.445.g3cdd5d1.
> >This could be a problem with the SVN Perl libraries, and should
> >be fixed in newer versions (not sure if it's made it to distros,
> >yet):
> >
> >https://public-inbox.org/git/0bca1e695085c645b9cd4a27dd59f6fa39aad...@gbwgceuhubd0101.rbsres07.net/T/
> >
> >Seems like it is fixed in latest Debian, maybe it needs to trickle
> >into Ubuntu: https://bugs.debian.org/780246
> Thanks.  I'll try adding that.
> 
> Er... which debian would that be?  testing?  Or sid?

It looks like it (1.9.4-3) has trickled into stretch (testing)
by now:

https://packages.debian.org/src:subversion


[ANNOUNCE] git-log-compact v1.0

2016-10-19 Thread Kyle J. McKay
> NOTE: If you read the excellent Git Rev News [1], then you
> already know all about git-log-compact :)

The git-log-compact script provides a compact alternative to the
`git log --oneline` output that includes dates, times and author
and/or committer initials in a space efficient output format.

`git-log-compact` is intended to fill the gap between the single line
`--oneline` log output format and the multiline `--pretty=short` /
`--pretty=medium` output formats.

It is not strictly a one line per commit output format (but almost) and
while it only shows the title of each commit (like the short format) it
does include author and date information (similarly to the medium format)
except that timestamps are shown very compactly and author/committer
names are shown as initials.

This allows one to get a complete view of repository activity in a very
compact output format that can show many commits per screen full and is
fully compatible with the `--graph` option.

Simple example output from the Git repository:

git log-compact --graph --date-order --decorate --no-merges -n 5 v2.5.3

=== 2015-09-17 ===
  * ee6ad5f4 12:16 jch (tag: v2.5.3) Git 2.5.3
=== 2015-09-09 ===
  * b9d66899 14:22 js  am --skip/--abort: merge HEAD/ORIG_HEAD tree into index
  |   === 2015-09-04 ===
  | * 27ea6f85 10:46 jch (tag: v2.5.2) Git 2.5.2
  * 74b67638 10:36 jch (tag: v2.4.9) Git 2.4.9
   ..
  * ecad27cf 10:32 jch (tag: v2.3.9) Git 2.3.9

I have been wanting a compact one line output format that included dates,
times and initials for some time that is compatible with --graph, clearly
shows root commits and eliminates confusion over whether or not two adjacent
lines in the output are related as parent/child (the --show-linear-break
option does not work with --graph).

The git-log-compact utility is the result.  Except for --notes, --pretty and
--format options (which would make the output a non-oneline format) any
other `git log` option may be used (including things like --cherry-mark,
--patch, --raw, --stat, --summary, --show-linear-break etc.),

There are a few new options specific to git-log-compact which are described
in the README and the `git-log-compact -h` output that can be used to alter
the dates, times and/or initials displayed.

The project page with detailed help and many screen shots is located at:

  https://mackyle.github.io/git-log-compact/

Alternatively the repository can be cloned from:

  https://github.com/mackyle/git-log-compact.git

Or the script file itself (which is really all you need) can be
viewed/fetched from:

  https://github.com/mackyle/git-log-compact/blob/HEAD/git-log-compact

The git-log-compact script should work with any version of Git released
in the last several years.

--Kyle

[1] https://git.github.io/rev_news/2016/10/19/edition-20/


Re: tools for easily "uncommitting" parts of a patch I just commited?

2016-10-19 Thread Jacob Keller
On Wed, Oct 19, 2016 at 3:42 PM, Jeff King  wrote:
> On Wed, Oct 19, 2016 at 03:26:18PM -0700, Jacob Keller wrote:
>
>> I recently (and in the past) had an issue where I was using git add
>> --interactive and accidentally did something like the following:
>>
>> # hack lots of randmo changes, then begin trying to commit then separately
>> git add -i
>> # set only the changes I want
>> # accidentally add  to the commit
>> $git commit -s 
>> # type up a long commit message
>> # notice that I committed everything
>>
>> At this point I'd like to be able to do something like:
>> $git unstage -i
>> # select each hunk to unstage
>
> I'd usually do one of:
>
>   # undo selectively
>   git reset -p HEAD^
>   git commit --amend

AHA! I knew about git reset -p but I didn't know about git reset -p
allowed passing a treeish. Does this reset modify my local files at
all? I think it doesn't, right?

>
> or:
>
>   # roll back the whole commit
>   git reset HEAD
>   # do it right this time
>   git add -p
>   # and steal the commit message from the previous attempt
>   git commit -c HEAD@{1}
>
> -Peff

Also nice to know about git commit -c

Thanks a lot! This should save me some headaches.

I still think it's worth while to add a check for git-commit which
does something like check when we say "git commit " and if the
index already has those files marked as being changed, compare them
with the current contents of the file as in the checkout and quick
saying "please don't do that" so as to avoid the problem in the first
place.

A naive approach would just be "if index already has staged
differences dont allow path selection" but that doesn't let me do
something like "git add -p " "git commit "

We could even make it work so that "commit --only" doesn't run this so
that way people can easily override, and we can give suggestions for
how to fix it in the output of the message. I can't really think if a
reasonable objection to such a change. I'll try to code something up
in the next few days when I can find some spare time.

Regards,
Jake


Re: [PATCH] submodules: update documentaion for submodule branches

2016-10-19 Thread Brandon Williams
On 10/19, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
> > Brandon Williams  writes:
> >
> >> Update the documentaion for the the special value `.` to indicate that
> >> it signifies that the tracking branch in the submodule should be the
> >> same as the current branch in the superproject.
> >
> > Thanks.  Will typofix while extending with info supplied by Stefan
> > like so:
> 
> Ugh.  Should have proof-read before sending it out.
> 
> > 4d7bc52b17 ("submodule update: allow '.' for branch value",
> > 2016-08-03) adopted from Gerrit a feature to set "." as a special
> > value of "submodule..branch" in .gitmodules file to indicate
> > that it signifies that the tracking branch in the submodule should
> > be the same as the current branch in the superproject.
> 
> ... in .gitmodules file to indicate that the tracking branch in
> the submodule should be ...
> 
> "to indicate that it signifies that" was overly redundant.

Sorry that seems to have stemed from my poor commit message.

-- 
Brandon Williams


Re: [PATCH] submodules: update documentaion for submodule branches

2016-10-19 Thread Junio C Hamano
Junio C Hamano  writes:

> Brandon Williams  writes:
>
>> Update the documentaion for the the special value `.` to indicate that
>> it signifies that the tracking branch in the submodule should be the
>> same as the current branch in the superproject.
>
> Thanks.  Will typofix while extending with info supplied by Stefan
> like so:

Ugh.  Should have proof-read before sending it out.

> 4d7bc52b17 ("submodule update: allow '.' for branch value",
> 2016-08-03) adopted from Gerrit a feature to set "." as a special
> value of "submodule..branch" in .gitmodules file to indicate
> that it signifies that the tracking branch in the submodule should
> be the same as the current branch in the superproject.

... in .gitmodules file to indicate that the tracking branch in
the submodule should be ...

"to indicate that it signifies that" was overly redundant.




Re: tools for easily "uncommitting" parts of a patch I just commited?

2016-10-19 Thread Jeff King
On Wed, Oct 19, 2016 at 03:26:18PM -0700, Jacob Keller wrote:

> I recently (and in the past) had an issue where I was using git add
> --interactive and accidentally did something like the following:
> 
> # hack lots of randmo changes, then begin trying to commit then separately
> git add -i
> # set only the changes I want
> # accidentally add  to the commit
> $git commit -s 
> # type up a long commit message
> # notice that I committed everything
> 
> At this point I'd like to be able to do something like:
> $git unstage -i
> # select each hunk to unstage

I'd usually do one of:

  # undo selectively
  git reset -p HEAD^
  git commit --amend

or:

  # roll back the whole commit
  git reset HEAD
  # do it right this time
  git add -p
  # and steal the commit message from the previous attempt
  git commit -c HEAD@{1}

-Peff


Re: [PATCH] rev-list: restore the NUL commit separator in --header mode

2016-10-19 Thread Junio C Hamano
Dennis Kaarsemaker  writes:

> + touch expect &&
> + printf "\0" > expect &&

What's the point of that "touch", especially if you are going to
overwrite it immediately after?

> + git rev-list --header --max-count=1 HEAD | tail -n1 >actual &&

As "tail" is a tool for text files, it is likely unportable to use
"tail -n1" to grab the "last incomplete line that happens to contain
a single NUL".

> + test_cmp_bin expect actual
> +'


Re: [PATCH] rev-list: restore the NUL commit separator in --header mode

2016-10-19 Thread Junio C Hamano
Jacob Keller  writes:

> Hi,
>
> On Wed, Oct 19, 2016 at 2:04 PM, Dennis Kaarsemaker
>  wrote:
>> Commit 660e113 (graph: add support for --line-prefix on all graph-aware
>> output) changed the way commits were shown. Unfortunately this dropped
>> the NUL between commits in --header mode. Restore the NUL and add a test
>> for this feature.
>>
>
> Oops! Thanks for the bug fix.
>
>> Signed-off-by: Dennis Kaarsemaker 
>> ---
>>  builtin/rev-list.c   | 4 
>>  t/t6000-rev-list-misc.sh | 7 +++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
>> index 8479f6e..cfa6a7d 100644
>> --- a/builtin/rev-list.c
>> +++ b/builtin/rev-list.c
>> @@ -157,6 +157,10 @@ static void show_commit(struct commit *commit, void 
>> *data)
>> if (revs->commit_format == CMIT_FMT_ONELINE)
>> putchar('\n');
>> }
>> +   if (revs->commit_format == CMIT_FMT_RAW) {
>> +   putchar(info->hdr_termination);
>> +   }
>> +
>
> This seems right to me. My one concern is that we make sure we restore
> it for every case (in case it needs to be there for other formats?)
> I'm not entirely sure about whether other non-raw modes need this or
> not?

Right.  The original didn't do anything special for CMIT_FMT_RAW,
and 660e113 did not remove anything special for CMIT_FMT_RAW, so it
isn't immediately obvious why this patch is sufficient.  

Dennis, care to elaborate?


Re: [regression] `make profile-install` fails in 2.10.1

2016-10-19 Thread Jeff King
On Wed, Oct 19, 2016 at 03:30:44PM -0700, Junio C Hamano wrote:

> > Ouch.  Thanks for a reminder.  How about doing this for now?
> 
> And the hack I used to quickly test it looks like this:
> 
> $ cd t
> $ GIT_I_AM_INSANE=Yes sh ./t3700-add.sh
> 
> We may want a more general 
> 
> GIT_OVERRIDE_PREREQ='!SANITY,!POSIXPERM,MINGW' make test
> 
> or something like that, though.

I don't think I've ever wanted to do that myself, but I can see how it
might be useful (e.g., claiming we don't support symlinks is another
one).

I usually just try to recreate the actual environment (e.g., run the
tests as root, run them on a loopback case-insensitive fs, etc) as that
gives a more realistic recreation.

-Peff


Re: [regression] `make profile-install` fails in 2.10.1

2016-10-19 Thread Jeff King
On Wed, Oct 19, 2016 at 03:27:35PM -0700, Junio C Hamano wrote:

> Ouch.  Thanks for a reminder.  How about doing this for now?
> [...]
>  test_expect_success 'git add --chmod=[+-]x changes index with already added 
> file' '
> + rm -f foo3 xfoo3 &&
>   echo foo >foo3 &&

Yeah, this makes sense. It does make me feel like the test should simply
be using xfoo27, or some name that is not otherwise used in the rest of
the script, but I don't mind doing the minimal thing.

-Peff


Re: [regression] `make profile-install` fails in 2.10.1

2016-10-19 Thread Junio C Hamano
Junio C Hamano  writes:

> Ouch.  Thanks for a reminder.  How about doing this for now?

And the hack I used to quickly test it looks like this:

$ cd t
$ GIT_I_AM_INSANE=Yes sh ./t3700-add.sh

We may want a more general 

GIT_OVERRIDE_PREREQ='!SANITY,!POSIXPERM,MINGW' make test

or something like that, though.

 t/test-lib.sh | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0055ebba46..9c5bcd9d1d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -89,6 +89,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
UNZIP
PERF_
CURL_VERBOSE
+   I_AM_INSANE
));
my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
print join("\n", @vars);
@@ -1081,6 +1082,12 @@ test_lazy_prereq NOT_ROOT '
 # containing directory doesn't have read or execute permissions.
 
 test_lazy_prereq SANITY '
+
+   if test -n "$GIT_I_AM_INSANE"
+   then
+   return 1
+   fi &&
+
mkdir SANETESTD.1 SANETESTD.2 &&
 
chmod +w SANETESTD.1 SANETESTD.2 &&


Re: [regression] `make profile-install` fails in 2.10.1

2016-10-19 Thread Junio C Hamano
Jeff King  writes:

> I can't reproduce any problems with raciness there, but there is a known
> problem with running the script as root (which I guess you might be
> doing from your "make prefix=/usr" call). There's some discussion in
> http://public-inbox.org/git/20161010035756.38408-1-jerem...@apple.com/T/#u,
> but it looks like the patch stalled.

Ouch.  Thanks for a reminder.  How about doing this for now?

-- >8 --
From: Junio C Hamano 
Date: Mon, 10 Oct 2016 10:41:51 -0700
Subject: [PATCH] t3700: fix broken test under !SANITY

An "add --chmod=+x" test recently added by 610d55af0f ("add: modify
already added files when --chmod is given", 2016-09-14) used "xfoo3"
as a test file.  The paths xfoo[1-3] were used by earlier tests for
symbolic links but they were expected to have been removed by the
test script reached this new test.

The removal with "git reset --hard" however happened in tests that
are protected by POSIXPERM,SANITY prerequisites.  Platforms and test
environments that lacked these would have seen xfoo3 as a leftover
symbolic link, pointing somewhere else, and chmod test would have
given a wrong result.

Signed-off-by: Junio C Hamano 
---
 t/t3700-add.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 924a266126..53c0cb6dea 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -350,6 +350,7 @@ test_expect_success POSIXPERM,SYMLINKS 'git add --chmod=+x 
with symlinks' '
 '
 
 test_expect_success 'git add --chmod=[+-]x changes index with already added 
file' '
+   rm -f foo3 xfoo3 &&
echo foo >foo3 &&
git add foo3 &&
git add --chmod=+x foo3 &&
-- 
2.10.1-633-g7f0e449216



tools for easily "uncommitting" parts of a patch I just commited?

2016-10-19 Thread Jacob Keller
Hi,

I recently (and in the past) had an issue where I was using git add
--interactive and accidentally did something like the following:

# hack lots of randmo changes, then begin trying to commit then separately
git add -i
# set only the changes I want
# accidentally add  to the commit
$git commit -s 
# type up a long commit message
# notice that I committed everything

At this point I'd like to be able to do something like:
$git unstage -i
# select each hunk to unstage

and end up with a commit that only has what I originally wanted,
without having to re-write the commit message, nor having to do a lot
of weird things, or anything.

I can ofcourse use reset HEAD^ and lose my commit message, but then
I'd have to retype that out or copy paste it from somewhere else.

I ended up doing something like:

# save the current tree
$git rev-parse HEAD >savetree
# checkout the old files and re-write
$git checkout HEAD^ 
# update commit removing all changes from this file
$git commit --allow-empty --amend 
# now checkout the tree again to the contents of the saved tree
$git checkout $(cat savetree) 
# now add only the parts I wanted before
$git add -i
# finally amend the commit
$git commit --amend

That's a lot of steps and forces me to save my own file.

I thought of a few alternatives:

1. Create an advice setting which basically allows me to say "git,
please prevent me from staging files + an index if the files I marked
also conflict with paths already added to the index, maybe unless I
passed a force option"

or

2. somehow streamline the process of what I did above so I could just
do something like:

git commit --amend --set-tree=HEAD^

which would force the commit tree up one and avoid the double checkout
stuff, without actually changing my checked out copy at all

Then I'd be able to quickly re-add what I wanted.


3. somehow allow an unstage option.

So for the TL;DR; .. does anyone know of any tools which would help
automate the process so I could simply do

"git uncommit -i" and run a tool just like git add interactive or git add -p?

Thanks,
Jake


Re: Problems with "git svn clone"

2016-10-19 Thread K Richard Pixley

On 10/19/16 13:41 , Eric Wong wrote:

K Richard Pixley  wrote:

error: git-svn died of signal 11

This seems awfully early and blatant for a core dump.  What can I do to
get this running?

Can you show us a backtrace?  Thanks.

There is none.  I ran it in gdb and bt produced no results.  Nor did the
thread send bt command.

Initially discovered on git-2.7.4, (ubuntu-16.04), but also reproduced
on freshly built top of tree git-2.10.1.445.g3cdd5d1.

This could be a problem with the SVN Perl libraries, and should
be fixed in newer versions (not sure if it's made it to distros,
yet):

https://public-inbox.org/git/0bca1e695085c645b9cd4a27dd59f6fa39aad...@gbwgceuhubd0101.rbsres07.net/T/

Seems like it is fixed in latest Debian, maybe it needs to trickle
into Ubuntu: https://bugs.debian.org/780246

Thanks.  I'll try adding that.

Er... which debian would that be?  testing?  Or sid?

--rich



- CONFIDENTIAL-

This email and any files transmitted with it are confidential, and may also be 
legally privileged. If you are not the intended recipient, you may not review, 
use, copy, or distribute this message. If you receive this email in error, 
please notify the sender immediately by reply email and then delete this email.


Re: [PATCH] rev-list: restore the NUL commit separator in --header mode

2016-10-19 Thread Jacob Keller
Hi,

On Wed, Oct 19, 2016 at 2:04 PM, Dennis Kaarsemaker
 wrote:
> Commit 660e113 (graph: add support for --line-prefix on all graph-aware
> output) changed the way commits were shown. Unfortunately this dropped
> the NUL between commits in --header mode. Restore the NUL and add a test
> for this feature.
>

Oops! Thanks for the bug fix.

> Signed-off-by: Dennis Kaarsemaker 
> ---
>  builtin/rev-list.c   | 4 
>  t/t6000-rev-list-misc.sh | 7 +++
>  2 files changed, 11 insertions(+)
>
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 8479f6e..cfa6a7d 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -157,6 +157,10 @@ static void show_commit(struct commit *commit, void 
> *data)
> if (revs->commit_format == CMIT_FMT_ONELINE)
> putchar('\n');
> }
> +   if (revs->commit_format == CMIT_FMT_RAW) {
> +   putchar(info->hdr_termination);
> +   }
> +

This seems right to me. My one concern is that we make sure we restore
it for every case (in case it needs to be there for other formats?)
I'm not entirely sure about whether other non-raw modes need this or
not?

Thanks,
Jake


Re: [PATCH v12 3/8] graph: add support for --line-prefix on all graph-aware output

2016-10-19 Thread Jacob Keller
Hi,

On Wed, Oct 19, 2016 at 1:24 PM, Dennis Kaarsemaker
 wrote:
> On Wed, 2016-08-31 at 16:27 -0700, Jacob Keller wrote:
>> From: Jacob Keller 
>>
>> Add an extension to git-diff and git-log (and any other graph-aware
>> displayable output) such that "--line-prefix=" will print the
>> additional line-prefix on every line of output.
>
> This patch breaks git rev-list --header, also breaking gitweb.
>

Oops! Is it possible you have a test case already?

> The NUL between commits has gone missing, causing gitweb to interpret
> the output of git rev-list as one commit.
>

That is obviously not what we want!

> Sorry for not catching this earlier, I actually encountered this early
> september but thought it was caused by us running an ancient gitweb
> with a modern git. Finally managed to upgrade gitweb today, and the bug
> didn't go away. git bisect says 660e113ce is the culprit. Checking out
> 'next' and reverting this single patch makes the problem disappear.
>

Ok.

> Haven't yet tried to fix the bug, but this hunk looks suspicious:



>
> -   if (revs->commit_format != CMIT_FMT_USERFORMAT ||
> -   buf.len) {
> -   fwrite(buf.buf, 1, buf.len, stdout);
> -   putchar(info->hdr_termination);
> -   }
> +   /*
> +* If the message buffer is empty, just show
> +* the rest of the graph output for this
> +* commit.
> +*/
> +   if (graph_show_remainder(revs->graph))
> +   putchar('\n');

Most likely this should have been "putchar(info->hdr_termination);" I
think? Not entirely sure.

If we can get a test case in we can use that to help debug the issue.

Thanks,
Jake

> +   if (revs->commit_format == CMIT_FMT_ONELINE)
> +
>
> D.


Re: [PATCH] submodules: update documentaion for submodule branches

2016-10-19 Thread Junio C Hamano
Brandon Williams  writes:

> Update the documentaion for the the special value `.` to indicate that
> it signifies that the tracking branch in the submodule should be the
> same as the current branch in the superproject.

Thanks.  Will typofix while extending with info supplied by Stefan
like so:

submodules doc: update documentation for "." used for submodule branches

4d7bc52b17 ("submodule update: allow '.' for branch value",
2016-08-03) adopted from Gerrit a feature to set "." as a special
value of "submodule..branch" in .gitmodules file to indicate
that it signifies that the tracking branch in the submodule should
be the same as the current branch in the superproject.

Update the documentation to describe this.



Re: [PATCH 0/7] Rejecting useless merge bases

2016-10-19 Thread Junio C Hamano
Junio C Hamano  writes:

> This is a continuation of
>
> http://public-inbox.org/git/xmqqmvi2sj8f@gitster.mtv.corp.google.com

I scanned all two-parent merges in git.git leading to 'next' and
'pu' and reproduced them with or without the new --fp-base-only
option.  I also did the same experiment in linux.git, but only for
the latest 10,000 merges.

This exercise yielded some interesting numbers.

Among the total 10,995 merges, 10,487 merges had only one merge base
and that merge base was on the first-parent chain, i.e. the result
of reproduction is the same with or without "--fp-base-only".

There were 65 merges with multiple merge-bases but all of these
merge bases were on the first-parent chain, i.e. again the result is
the same with or without "--fp-base-only".

The remaining 443 merges had merge bases that are not on the
first-parent chain.  These merges, when recreated with
"--fp-base-only", would use different/reduced set of merge bases to
drive merge-recursive machinery and could produce different results.

Among these 443, "git merge" with or without the "--fp-base-only"
option successfully auto-resolved and produced the same result as
recorded in the real history for 214 of them.  They stopped in
conflicts but "git ls-files -s" output in their conflicted states
were the same, i.e. they left the identical conflicts, for 221 of
them.

The remaining 8 merges were auto-resolved the same way with or
without the "--fp-base-only" option, but they were different from
the merge in the real history (i.e. the real history had an evil
merge there to adjust for non-textual conflicts).

The most important numbers was 0.  There was no merges whose
reproduction with and without "--fp-base-only" produced different
results.


For linux.git the numbers are:

 - total merges looked at: 10,000
 - single merge base:   9,705
 - multi merge base all on the first parent chain:123
 - fp-base-only eligible: 172
 - cleanly resolved the same way w/ or w/o fp-only:   118
 - conflicted the same way w/ or w/o fp-only:  54
 - evil merges among fp-base-only eligible merges:  0
 - fp-base-only mismerges:  0


Re: [PATCH 1/2] ref-filter: split ref_kind_from_filter

2016-10-19 Thread Jeff King
On Wed, Oct 19, 2016 at 02:23:41PM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > Subject: Re: [PATCH 1/2] ref-filter: split ref_kind_from_filter
> 
> I think you meant ref_kind_from_refname() ;-)
> 
> Looks like a good idea.

Heh, I actually meant filter_ref_kind(), which is the original function.
But any name that is actually a real function would do. :)

-Peff


Re: [PATCH 1/2] ref-filter: split ref_kind_from_filter

2016-10-19 Thread Junio C Hamano
Jeff King  writes:

> Subject: Re: [PATCH 1/2] ref-filter: split ref_kind_from_filter

I think you meant ref_kind_from_refname() ;-)

Looks like a good idea.

> This function does two things: if we know we are filtering
> only a certain kind of ref, then we can immediately know
> that we have that kind. If not, then we compute the kind
> from the fully-qualified refname. The latter half is useful
> for other callers; let's split it out.
>
> Signed-off-by: Jeff King 
> ---
>  ref-filter.c | 17 +++--
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index cfbcd73..77ec9de 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1329,7 +1329,7 @@ static struct ref_array_item *new_ref_array_item(const 
> char *refname,
>   return ref;
>  }
>  
> -static int filter_ref_kind(struct ref_filter *filter, const char *refname)
> +static int ref_kind_from_refname(const char *refname)
>  {
>   unsigned int i;
>  
> @@ -1342,11 +1342,7 @@ static int filter_ref_kind(struct ref_filter *filter, 
> const char *refname)
>   { "refs/tags/", FILTER_REFS_TAGS}
>   };
>  
> - if (filter->kind == FILTER_REFS_BRANCHES ||
> - filter->kind == FILTER_REFS_REMOTES ||
> - filter->kind == FILTER_REFS_TAGS)
> - return filter->kind;
> - else if (!strcmp(refname, "HEAD"))
> + if (!strcmp(refname, "HEAD"))
>   return FILTER_REFS_DETACHED_HEAD;
>  
>   for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
> @@ -1357,6 +1353,15 @@ static int filter_ref_kind(struct ref_filter *filter, 
> const char *refname)
>   return FILTER_REFS_OTHERS;
>  }
>  
> +static int filter_ref_kind(struct ref_filter *filter, const char *refname)
> +{
> + if (filter->kind == FILTER_REFS_BRANCHES ||
> + filter->kind == FILTER_REFS_REMOTES ||
> + filter->kind == FILTER_REFS_TAGS)
> + return filter->kind;
> + return ref_kind_from_refname(refname);
> +}
> +
>  /*
>   * A call-back given to for_each_ref().  Filter refs and keep them for
>   * later object processing.


Re: What's cooking in git.git (Oct 2016, #04; Mon, 17)

2016-10-19 Thread Jeff King
On Wed, Oct 19, 2016 at 08:28:56PM +, brian m. carlson wrote:

> On Wed, Oct 19, 2016 at 03:46:48AM -0400, Jeff King wrote:
> > FWIW, I gave it a fairly thorough read-over (something I'd been meaning
> > to do for quite a while, but kept never quite getting around to). I
> > think overall it is OK for next. I did find one or two nits, but I think
> > they are things we can fix up in-tree if and when they become a problem
> > (e.g., I noticed that test-genrandom gets piped to "perl -pe". I'm not
> > sure if perl will complain about funny multibyte characters on some
> > systems. I suggest we ignore it until somebody demonstrates that it
> > actually matters).
> 
> I just looked, and that use is fine.  perl -pe is always going to treat
> its data as bytes unless you use -C or explicitly enable Unicode
> functionality.

Thanks. I have vague memories of multibyte warnings, but I think they
may have been on _output_ when passing through binary data that came
on stdin.

-Peff


Re: [PATCH] Add a knob to abort on die() (was Re: git checkout crashes after ...)

2016-10-19 Thread Jeff King
On Wed, Oct 19, 2016 at 08:47:50PM +0700, Duy Nguyen wrote:

> On Wed, Oct 19, 2016 at 08:27:43PM +0700, Duy Nguyen wrote:
> > If you set the environment variable GIT_ALLOC_LIMIT ...  git
> > attempts to allocate more than that ... then it's caught and we get
> > a glimpse of how much memory git may need. Unfortunately we can't
> > get a stack trace or anything like that unless you rebuild Git from
> > source.
> 
> It's moments like this that I wish we had a knob to force core
> dumps. And I often modify die_builtin() to add '*(char*)0 = 1;' to
> force a core dump when I can't figure out some problem based on the
> error message alone.
> 
> So.. how about we do something like this? We could extend it to abort
> on error() as well as die(). Aborting on warning() may be a bit too
> much though. On glibc systems we could even print the back trace
> without aborting, which helps in some cases.

I have been tempted by something like this, too, and have occasionally
resorted to patching in an abort() call to get cores from sub-processes.

I'm not sure how useful it would be in practice in deployed versions of
git, though. You'd have to coach the user into finding the core file and
generating the backtrace from it. Something that called backtrace(3)
automatically would be more seamless (but provides worse information
than gdb), and otherwise it is probably just as easy to ship the user
cut-and-paste instructions to use gdb.

See the previous discussion in this subthread:

  http://public-inbox.org/git/20150424201734.ga4...@peff.net/T/#u

-Peff


Re: [regression] `make profile-install` fails in 2.10.1

2016-10-19 Thread Jeff King
On Wed, Oct 19, 2016 at 01:15:56PM +0200, Jan Keromnes wrote:

> - Problem: Is there a way to `make profile-install` but ignore
> occasional test failures, as these are not critical to get a useful
> hardware profile? (Note: In a previous thread, Dennis Kaarsemaker
> mentioned this is fixing a symptom, not the root cause, but it would
> still be great to get a working profile in spite of occasional test
> failures.)

No, there isn't a way currently. I agree with Dennis that we do want to
ultimately fix the flaky tests, but at the same time, it does seem
reasonable to use a partial test run for your profile results.

I suspect you'd need to switch the call to:

  $(MAKE) PROFILE=GEN -j1 -k test || true

to make it best-effort.

> - Related problem: `t3700-add.sh` fails again in 2.10.1 for me. More
> details below, and I can provide further debug information if you
> don't already know the problem.

I can't reproduce any problems with raciness there, but there is a known
problem with running the script as root (which I guess you might be
doing from your "make prefix=/usr" call). There's some discussion in
http://public-inbox.org/git/20161010035756.38408-1-jerem...@apple.com/T/#u,
but it looks like the patch stalled.

-Peff


[PATCH] rev-list: restore the NUL commit separator in --header mode

2016-10-19 Thread Dennis Kaarsemaker
Commit 660e113 (graph: add support for --line-prefix on all graph-aware
output) changed the way commits were shown. Unfortunately this dropped
the NUL between commits in --header mode. Restore the NUL and add a test
for this feature.

Signed-off-by: Dennis Kaarsemaker 
---
 builtin/rev-list.c   | 4 
 t/t6000-rev-list-misc.sh | 7 +++
 2 files changed, 11 insertions(+)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8479f6e..cfa6a7d 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -157,6 +157,10 @@ static void show_commit(struct commit *commit, void *data)
if (revs->commit_format == CMIT_FMT_ONELINE)
putchar('\n');
}
+   if (revs->commit_format == CMIT_FMT_RAW) {
+   putchar(info->hdr_termination);
+   }
+
strbuf_release();
} else {
if (graph_show_remainder(revs->graph))
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 3e752ce..a2acff3 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -100,4 +100,11 @@ test_expect_success '--bisect and --first-parent can not 
be combined' '
test_must_fail git rev-list --bisect --first-parent HEAD
 '
 
+test_expect_success '--header shows a NUL after each commit' '
+   touch expect &&
+   printf "\0" > expect &&
+   git rev-list --header --max-count=1 HEAD | tail -n1 >actual &&
+   test_cmp_bin expect actual
+'
+
 test_done
-- 
2.10.1-449-gab0f84c


-- 
Dennis Kaarsemaker 
http://twitter.com/seveas


Re: Drastic jump in the time required for the test suite

2016-10-19 Thread Jeff King
On Wed, Oct 19, 2016 at 10:32:12AM -0700, Junio C Hamano wrote:

> > Maybe we should start optimizing the tests...
> 
> Yup, two things that come to mind are to identify long ones and see
> if each of them can be split into two halves that can be run in
> parallel, and to go through the tests with fine toothed comb and
> remove the ones that test exactly the same thing as another test.
> The latter would be very time consuming, though.

FWIW, I have made attempts at "split long ones into two" before, and
didn't come up with much. There _are_ some tests that are much longer
than others[1], but they are not longer than the whole suite takes to
run. So running in slow-to-fast order means they start first, are run in
parallel with the other tests, and the CPUs stay relatively full through
the whole run.

Of course YMMV; the long tests on Windows may be different, or
proportionally much longer (I note the worst cases almost all involve
rebase, which as a shell script is presumably worse on Windows than
elsewhere). And of course any reasoning about slow-to-fast order does
not apply if you are not using a tool to do that for you. :)

-Peff

[1] After running "make prove" (time are in seconds):

$ perl -MYAML -e '
$_ = do { local $/; <> };
# prove puts this non-YAML cruft at the end
s/\.\.\.$//s;

my $t = YAML::Load($_)->{tests};
print "$_->[1] $_->[0]\n" for
  sort { $b->[1] <=> $a->[1] }
  map { [$_, $t->{$_}->{elapsed}] }
  keys(%$t);
  ' .prove | head

43.216765165329 t3404-rebase-interactive.sh
30.6568658351898 t3421-rebase-topology-linear.sh
27.92564702034 t9001-send-email.sh
15.5906939506531 t9500-gitweb-standalone-no-errors.sh
15.4882569313049 t6030-bisect-porcelain.sh
14.487174987793 t7610-mergetool.sh
13.8276169300079 t3425-rebase-topology-merges.sh
12.7450480461121 t3426-rebase-submodule.sh
12.4915001392365 t3415-rebase-autosquash.sh
11.8122401237488 t5572-pull-submodule.sh


Re: [PATCH] submodules: update documentaion for submodule branches

2016-10-19 Thread Stefan Beller
On Wed, Oct 19, 2016 at 1:42 PM, Brandon Williams  wrote:
> Update the documentaion for the the special value `.` to indicate that
> it signifies that the tracking branch in the submodule should be the
> same as the current branch in the superproject.

For reference of other reviewers:
See 4d7bc52b178bffe9e484c4dcd92d5353e2ce716f
as well as 
https://gerrit-review.googlesource.com/Documentation/user-submodules.html

Thanks,
Stefan

>
> Signed-off-by: Brandon Williams 
> ---
>  Documentation/git-submodule.txt | 4 +++-
>  Documentation/gitmodules.txt| 7 +--
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index bf3bb37..d841573 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -259,7 +259,9 @@ OPTIONS
>  --branch::
> Branch of repository to add as submodule.
> The name of the branch is recorded as `submodule..branch` in
> -   `.gitmodules` for `update --remote`.
> +   `.gitmodules` for `update --remote`.  A special value of `.` is used 
> to
> +   indicate that the name of the branch in the submodule should be the
> +   same name as the current branch in the current repository.
>
>  -f::
>  --force::
> diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
> index 10dcc08..8f7c50f 100644
> --- a/Documentation/gitmodules.txt
> +++ b/Documentation/gitmodules.txt
> @@ -50,8 +50,11 @@ submodule..update::
>
>  submodule..branch::
> A remote branch name for tracking updates in the upstream submodule.
> -   If the option is not specified, it defaults to 'master'.  See the
> -   `--remote` documentation in linkgit:git-submodule[1] for details.
> +   If the option is not specified, it defaults to 'master'.  A special
> +   value of `.` is used to indicate that the name of the branch in the
> +   submodule should be the same name as the current branch in the
> +   current repository.  See the `--remote` documentation in
> +   linkgit:git-submodule[1] for details.
>
>  submodule..fetchRecurseSubmodules::
> This option can be used to control recursive fetching of this
> --
> 2.10.1
>


[PATCH] submodules: update documentaion for submodule branches

2016-10-19 Thread Brandon Williams
Update the documentaion for the the special value `.` to indicate that
it signifies that the tracking branch in the submodule should be the
same as the current branch in the superproject.

Signed-off-by: Brandon Williams 
---
 Documentation/git-submodule.txt | 4 +++-
 Documentation/gitmodules.txt| 7 +--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index bf3bb37..d841573 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -259,7 +259,9 @@ OPTIONS
 --branch::
Branch of repository to add as submodule.
The name of the branch is recorded as `submodule..branch` in
-   `.gitmodules` for `update --remote`.
+   `.gitmodules` for `update --remote`.  A special value of `.` is used to
+   indicate that the name of the branch in the submodule should be the
+   same name as the current branch in the current repository.
 
 -f::
 --force::
diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt
index 10dcc08..8f7c50f 100644
--- a/Documentation/gitmodules.txt
+++ b/Documentation/gitmodules.txt
@@ -50,8 +50,11 @@ submodule..update::
 
 submodule..branch::
A remote branch name for tracking updates in the upstream submodule.
-   If the option is not specified, it defaults to 'master'.  See the
-   `--remote` documentation in linkgit:git-submodule[1] for details.
+   If the option is not specified, it defaults to 'master'.  A special
+   value of `.` is used to indicate that the name of the branch in the
+   submodule should be the same name as the current branch in the
+   current repository.  See the `--remote` documentation in
+   linkgit:git-submodule[1] for details.
 
 submodule..fetchRecurseSubmodules::
This option can be used to control recursive fetching of this
-- 
2.10.1



Re: Problems with "git svn clone"

2016-10-19 Thread Eric Wong
K Richard Pixley  wrote:
> error: git-svn died of signal 11
> 
> This seems awfully early and blatant for a core dump.  What can I do to
> get this running?

Can you show us a backtrace?  Thanks.

> Initially discovered on git-2.7.4, (ubuntu-16.04), but also reproduced
> on freshly built top of tree git-2.10.1.445.g3cdd5d1.

This could be a problem with the SVN Perl libraries, and should
be fixed in newer versions (not sure if it's made it to distros,
yet):

https://public-inbox.org/git/0bca1e695085c645b9cd4a27dd59f6fa39aad...@gbwgceuhubd0101.rbsres07.net/T/

Seems like it is fixed in latest Debian, maybe it needs to trickle
into Ubuntu: https://bugs.debian.org/780246


[PATCH 2/2] tag: send fully qualified refnames to verify_tag_and_format

2016-10-19 Thread Jeff King
The ref-filter code generally expects to see fully qualified
refs, so that things like "%(refname)" and "%(refname:short)"
work as expected. We can do so easily from git-tag, which
always works with refnames in the refs/tags namespace. As a
bonus, we can drop the "kind" parameter from
pretty_print_ref() and just deduce it automatically.

Unfortunately, things are not so simple for verify-tag,
which takes an arbitrary sha1 expression. It has no clue if
a refname as used or not, and whether it was in the
refs/tags namespace.

In an ideal world, get_sha1_with_context() would optionally
tell us about any refs we resolved while it was working, and
we could just feed that refname (and then in cases where we
didn't use a ref at all, like a bare sha1, we could fallback
to just showing the sha1 name the user gave us).

Signed-off-by: Jeff King 
---
I think you'd really just squash the various bits of this into your
series at the right spots, though I don't mind it on top, either.

The big question is to what degree we should care about the verify-tag
case. I don't think it's any worse off with this change than it is with
your series (its "kind" becomes "OTHER", but I don't think that is
actually used for display at all; the name remains the same). I'd be OK
with leaving it like this, as a known bug, until get_sha1_with_context()
learns to tell us about the ref. It's an unhandled corner case in a
brand-new feature, not a regression in an existing one.

 builtin/tag.c | 2 +-
 ref-filter.c  | 4 ++--
 ref-filter.h  | 6 +-
 tag.c | 2 +-
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 49aeb50..18eab7e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -114,7 +114,7 @@ static int verify_tag(const char *name, const char *ref,
if (fmt_pretty)
flags = GPG_VERIFY_QUIET;
 
-   return verify_and_format_tag(sha1, name, fmt_pretty, flags);
+   return verify_and_format_tag(sha1, ref, fmt_pretty, flags);
 }
 
 static int do_sign(struct strbuf *buffer)
diff --git a/ref-filter.c b/ref-filter.c
index 77ec9de..74da17a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1643,11 +1643,11 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format, int qu
 }
 
 void pretty_print_ref(const char *name, const unsigned char *sha1,
-   const char *format, unsigned kind)
+ const char *format)
 {
struct ref_array_item *ref_item;
ref_item = new_ref_array_item(name, sha1, 0);
-   ref_item->kind = kind;
+   ref_item->kind = ref_kind_from_refname(name);
show_ref_array_item(ref_item, format, 0);
free_array_item(ref_item);
 }
diff --git a/ref-filter.h b/ref-filter.h
index 3d23090..fed2f5e 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -107,7 +107,11 @@ struct ref_sorting *ref_default_sorting(void);
 /*  Function to parse --merged and --no-merged options */
 int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
unset);
 
+/*
+ * Print a single ref, outside of any ref-filter. Note that the
+ * name must be a fully qualified refname.
+ */
 void pretty_print_ref(const char *name, const unsigned char *sha1,
-   const char *format, unsigned kind);
+ const char *format);
 
 #endif /*  REF_FILTER_H  */
diff --git a/tag.c b/tag.c
index d3512c0..d5a7cfb 100644
--- a/tag.c
+++ b/tag.c
@@ -62,7 +62,7 @@ int verify_and_format_tag(const unsigned char *sha1, const 
char *name,
free(buf);
 
if (fmt_pretty)
-   pretty_print_ref(name, sha1, fmt_pretty, FILTER_REFS_TAGS);
+   pretty_print_ref(name, sha1, fmt_pretty);
 
return ret;
 }
-- 
2.10.1.619.g16351a7


Re: [PATCH v4 2/7] ref-filter: add function to print single ref_array_item

2016-10-19 Thread Jeff King
On Wed, Oct 19, 2016 at 01:07:34PM -0400, Santiago Torres wrote:

> > I guess that may complicate things for the caller you add in this
> > series, which may not have a fully-qualified refname (which is obviously
> > how filter_ref_kind() figures it out). I'd argue that is a bug, though,
> > as things like "%(refname)" are generally expected to print out the
> > fully refname ("git tag --format=%(refname)" does so, and you can use
> > "%(refname:short)" if you want the shorter part).
> 
> Hmm, I hadn't actually noticed that. Do you have any suggestions in how to
> address this?
> 
> In general this feels like a consequence of disambiguating .git/tags/*
> within builtin/tag.c rather than letting ref-filter figure it out.

The partial solution would look like something below. It's not too bad
because git-tag always knows that it's working a ref in the refs/tags
namespace (and we don't even have to qualify it ourselves,
for_each_tag_name already does it for us).

But verify-tag feeds arbitrary sha1 expressions. See the notes in the
second patch.

  [1/2]: ref-filter: split ref_kind_from_filter
  [2/2]: tag: send fully qualified refnames to verify_tag_and_format

 builtin/tag.c |  2 +-
 ref-filter.c  | 21 +
 ref-filter.h  |  6 +-
 tag.c |  2 +-
 4 files changed, 20 insertions(+), 11 deletions(-)

-Peff


[PATCH 1/2] ref-filter: split ref_kind_from_filter

2016-10-19 Thread Jeff King
This function does two things: if we know we are filtering
only a certain kind of ref, then we can immediately know
that we have that kind. If not, then we compute the kind
from the fully-qualified refname. The latter half is useful
for other callers; let's split it out.

Signed-off-by: Jeff King 
---
 ref-filter.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index cfbcd73..77ec9de 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1329,7 +1329,7 @@ static struct ref_array_item *new_ref_array_item(const 
char *refname,
return ref;
 }
 
-static int filter_ref_kind(struct ref_filter *filter, const char *refname)
+static int ref_kind_from_refname(const char *refname)
 {
unsigned int i;
 
@@ -1342,11 +1342,7 @@ static int filter_ref_kind(struct ref_filter *filter, 
const char *refname)
{ "refs/tags/", FILTER_REFS_TAGS}
};
 
-   if (filter->kind == FILTER_REFS_BRANCHES ||
-   filter->kind == FILTER_REFS_REMOTES ||
-   filter->kind == FILTER_REFS_TAGS)
-   return filter->kind;
-   else if (!strcmp(refname, "HEAD"))
+   if (!strcmp(refname, "HEAD"))
return FILTER_REFS_DETACHED_HEAD;
 
for (i = 0; i < ARRAY_SIZE(ref_kind); i++) {
@@ -1357,6 +1353,15 @@ static int filter_ref_kind(struct ref_filter *filter, 
const char *refname)
return FILTER_REFS_OTHERS;
 }
 
+static int filter_ref_kind(struct ref_filter *filter, const char *refname)
+{
+   if (filter->kind == FILTER_REFS_BRANCHES ||
+   filter->kind == FILTER_REFS_REMOTES ||
+   filter->kind == FILTER_REFS_TAGS)
+   return filter->kind;
+   return ref_kind_from_refname(refname);
+}
+
 /*
  * A call-back given to for_each_ref().  Filter refs and keep them for
  * later object processing.
-- 
2.10.1.619.g16351a7



Re: What's cooking in git.git (Oct 2016, #04; Mon, 17)

2016-10-19 Thread brian m. carlson
On Wed, Oct 19, 2016 at 03:46:48AM -0400, Jeff King wrote:
> FWIW, I gave it a fairly thorough read-over (something I'd been meaning
> to do for quite a while, but kept never quite getting around to). I
> think overall it is OK for next. I did find one or two nits, but I think
> they are things we can fix up in-tree if and when they become a problem
> (e.g., I noticed that test-genrandom gets piped to "perl -pe". I'm not
> sure if perl will complain about funny multibyte characters on some
> systems. I suggest we ignore it until somebody demonstrates that it
> actually matters).

I just looked, and that use is fine.  perl -pe is always going to treat
its data as bytes unless you use -C or explicitly enable Unicode
functionality.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature


Re: [PATCH v12 3/8] graph: add support for --line-prefix on all graph-aware output

2016-10-19 Thread Dennis Kaarsemaker
On Wed, 2016-08-31 at 16:27 -0700, Jacob Keller wrote:
> From: Jacob Keller 
> 
> Add an extension to git-diff and git-log (and any other graph-aware
> displayable output) such that "--line-prefix=" will print the
> additional line-prefix on every line of output.

This patch breaks git rev-list --header, also breaking gitweb.

The NUL between commits has gone missing, causing gitweb to interpret
the output of git rev-list as one commit.

Sorry for not catching this earlier, I actually encountered this early
september but thought it was caused by us running an ancient gitweb
with a modern git. Finally managed to upgrade gitweb today, and the bug
didn't go away. git bisect says 660e113ce is the culprit. Checking out
'next' and reverting this single patch makes the problem disappear.

Haven't yet tried to fix the bug, but this hunk looks suspicious:

-   if (revs->commit_format != CMIT_FMT_USERFORMAT ||
-   buf.len) {
-   fwrite(buf.buf, 1, buf.len, stdout);
-   putchar(info->hdr_termination);
-   }
+   /*
+* If the message buffer is empty, just show
+* the rest of the graph output for this
+* commit.
+*/
+   if (graph_show_remainder(revs->graph))
+   putchar('\n');
+   if (revs->commit_format == CMIT_FMT_ONELINE)
+  

D.


Re: Integrating submodules with no side effects

2016-10-19 Thread Robert Dailey
On Wed, Oct 19, 2016 at 2:45 PM, Stefan Beller  wrote:
> On Wed, Oct 19, 2016 at 12:19 PM, Robert Dailey
>  wrote:
>> On Wed, Oct 19, 2016 at 11:23 AM, Stefan Beller  wrote:
>>> You could try this patch series:
>>> https://github.com/jlehmann/git-submod-enhancements/tree/git-checkout-recurse-submodules
>>> (rebased to a newer version; no functional changes:)
>>> https://github.com/stefanbeller/git/tree/submodule-co
>>> (I'll rebase that later to origin/master)
>>>

 Do you have any info on how I can prevent that error? Ideally I want
 the integration to go smoothly and transparently, not just for the
 person doing the actual transition (me) but for everyone else that
 gets those changes from upstream. They should not even notice that it
 happened (i.e. no failed commands, awkward behavior, or manual steps).
>>>
>>> It depends on how long you want to postpone the transition, but I plan to
>>> upstream the series referenced above in the near future,
>>> which would enable your situation to Just Work (tm). ;)
>>
>> At first glance, what you've linked to essentially looks like
>> automated `git submodule update` for every `git checkout`. Am I
>> misunderstanding?
>
> Essentially yes, except with stricter rules than the actual submodule update
> IIRC.
>
>>
>> If I'm correct, this is not the same as what I'm talking about. The
>> problem appears to be more internal: When a submodule is removed, the
>> physical files that were there are not removed by Git.
>
> That is also done by that series: submodules ought to be treated as files:
> If you checkout a new version where a file is deleted, the checkout command
> will actually remove the file for you (and e.g. solve any
> directory/file conflicts
> that may happen in the transition.)
>
>> It leaves them
>> there in the working copy as untracked files.
>
> That is the current behavior as checkout tries hard to ignore submodules.
>
>> The next step Git takes
>> (again, just from outside observation) is to add those very same files
>> to the working copy, since they were added to a commit. However, at
>> this point Git fails because it's trying to create (write) files to
>> the working copy when an exact file of that name already exists there.
>> Git will not overwrite untracked files, so at this point it fails.
>>
>> What needs to happen, somehow, is Git sees that the files were
>> actually part of a submodule (which was removed) and remove the
>> physical files as well, assuming that they were not modified in the
>> submodule itself. This will ensure that the next step (creating the
>> files) will succeed since the files no longer block it.
>
> Yep.

It's great we're finally on the same page ;-)

However, I don't see how this problem can be solved with your script,
or solved in general outside of that. Does this mean that Git needs to
change to treat submodules as it does normal files, per your previous
assertion, which means submodules should *not* be left behind in the
working copy as untracked files?


Re: Integrating submodules with no side effects

2016-10-19 Thread Stefan Beller
On Wed, Oct 19, 2016 at 12:19 PM, Robert Dailey
 wrote:
> On Wed, Oct 19, 2016 at 11:23 AM, Stefan Beller  wrote:
>> You could try this patch series:
>> https://github.com/jlehmann/git-submod-enhancements/tree/git-checkout-recurse-submodules
>> (rebased to a newer version; no functional changes:)
>> https://github.com/stefanbeller/git/tree/submodule-co
>> (I'll rebase that later to origin/master)
>>
>>>
>>> Do you have any info on how I can prevent that error? Ideally I want
>>> the integration to go smoothly and transparently, not just for the
>>> person doing the actual transition (me) but for everyone else that
>>> gets those changes from upstream. They should not even notice that it
>>> happened (i.e. no failed commands, awkward behavior, or manual steps).
>>
>> It depends on how long you want to postpone the transition, but I plan to
>> upstream the series referenced above in the near future,
>> which would enable your situation to Just Work (tm). ;)
>
> At first glance, what you've linked to essentially looks like
> automated `git submodule update` for every `git checkout`. Am I
> misunderstanding?

Essentially yes, except with stricter rules than the actual submodule update
IIRC.

>
> If I'm correct, this is not the same as what I'm talking about. The
> problem appears to be more internal: When a submodule is removed, the
> physical files that were there are not removed by Git.

That is also done by that series: submodules ought to be treated as files:
If you checkout a new version where a file is deleted, the checkout command
will actually remove the file for you (and e.g. solve any
directory/file conflicts
that may happen in the transition.)

> It leaves them
> there in the working copy as untracked files.

That is the current behavior as checkout tries hard to ignore submodules.

> The next step Git takes
> (again, just from outside observation) is to add those very same files
> to the working copy, since they were added to a commit. However, at
> this point Git fails because it's trying to create (write) files to
> the working copy when an exact file of that name already exists there.
> Git will not overwrite untracked files, so at this point it fails.
>
> What needs to happen, somehow, is Git sees that the files were
> actually part of a submodule (which was removed) and remove the
> physical files as well, assuming that they were not modified in the
> submodule itself. This will ensure that the next step (creating the
> files) will succeed since the files no longer block it.

Yep.


Re: [PATCH v3 5/6] trailer: allow non-trailers in trailer block

2016-10-19 Thread Junio C Hamano
Jonathan Tan  writes:

> Would this work then:
> - at least one trailer line generated by Git ("(cherry picked by" or
>   "Signed-off-by: ") or configured in the "trailer" section in
>   git config AND at least 25% logical trailer lines
> OR
> - 100% logical trailer lines

Sure.  

At that point, I doubt that the latter "100% logical trailer" makes
much difference, though, because at least one of them is likely to
be in the former set, or the users can easily make it so by throwing
what they use like "Bug: ", "Test: " and "Change-ID: " in the
"configured in the trailer section" category.  FWIW, I do not think
we mind terribly to tweak the definition of "generated by Git" class
to "commonly used in Git managed projects" to include "Change-ID:"
and friends.


Re: Integrating submodules with no side effects

2016-10-19 Thread Robert Dailey
On Wed, Oct 19, 2016 at 11:23 AM, Stefan Beller  wrote:
> You could try this patch series:
> https://github.com/jlehmann/git-submod-enhancements/tree/git-checkout-recurse-submodules
> (rebased to a newer version; no functional changes:)
> https://github.com/stefanbeller/git/tree/submodule-co
> (I'll rebase that later to origin/master)
>
>>
>> Do you have any info on how I can prevent that error? Ideally I want
>> the integration to go smoothly and transparently, not just for the
>> person doing the actual transition (me) but for everyone else that
>> gets those changes from upstream. They should not even notice that it
>> happened (i.e. no failed commands, awkward behavior, or manual steps).
>
> It depends on how long you want to postpone the transition, but I plan to
> upstream the series referenced above in the near future,
> which would enable your situation to Just Work (tm). ;)

At first glance, what you've linked to essentially looks like
automated `git submodule update` for every `git checkout`. Am I
misunderstanding?

If I'm correct, this is not the same as what I'm talking about. The
problem appears to be more internal: When a submodule is removed, the
physical files that were there are not removed by Git. It leaves them
there in the working copy as untracked files. The next step Git takes
(again, just from outside observation) is to add those very same files
to the working copy, since they were added to a commit. However, at
this point Git fails because it's trying to create (write) files to
the working copy when an exact file of that name already exists there.
Git will not overwrite untracked files, so at this point it fails.

What needs to happen, somehow, is Git sees that the files were
actually part of a submodule (which was removed) and remove the
physical files as well, assuming that they were not modified in the
submodule itself. This will ensure that the next step (creating the
files) will succeed since the files no longer block it.


Re: [PATCH v4 05/14] i18n: add--interactive: mark plural strings

2016-10-19 Thread Junio C Hamano
Vasco Almeida  writes:

> @@ -669,12 +669,18 @@ sub status_cmd {
>  sub say_n_paths {
>   my $did = shift @_;
>   my $cnt = scalar @_;
> - print "$did ";
> - if (1 < $cnt) {
> - print "$cnt paths\n";
> - }
> - else {
> - print "one path\n";
> + if ($did eq 'added') {
> + printf(__n("added %d path\n", "added %d paths\n",
> +$cnt), $cnt);
> + } elsif ($did eq 'updated') {
> + printf(__n("updated %d path\n", "updated %d paths\n",
> +$cnt), $cnt);
> + } elsif ($did eq 'reverted') {
> + printf(__n("reverted %d path\n", "reverted %d paths\n",
> +$cnt), $cnt);
> + } else {
> + printf(__n("touched %d path\n", "touched %d paths\n",
> +$cnt), $cnt);
>   }
>  }

Nice to see you covered all verbs currently in use and then
future-proofed by adding a fallback "touched" here.

Thanks.



Re: [PATCH v4 01/14] i18n: add--interactive: mark strings for translation

2016-10-19 Thread Junio C Hamano
Vasco Almeida  writes:

>   } else {
> - print "No untracked files.\n";
> + print __("No untracked files.\n");
>   }

Not a big deal, but this makes me wonder if we want to do this
instead

print __("No untracked files.") . "\n";

so that translators do not have to remember to keep the final LF.

It seems there are some more hits of strings that end with "\n"
inside __(...) in this patch, so it probably is OK.


Re: [PATCH v3 5/6] trailer: allow non-trailers in trailer block

2016-10-19 Thread Jonathan Tan

On 10/18/2016 09:36 AM, Junio C Hamano wrote:

Jonathan Tan  writes:


* rs/c-auto-resets-attributes:
  pretty: avoid adding reset for %C(auto) if output is empty

And neither of the two colon containing line remotely resembles how
a typical RFC-822 header is formatted.  So that may serve as a hint
to how we can tighten it without introducing false negative.


The only "offending" character is the space (according to RFC 822),
but that sounds like a good rule to have.


I suspect that we should be willing to deviate from the letter of
RFC to reject misidentification.  I saw things like

Thanks to: Jonathan Tan 
Signed-off-by: A U Thor 

in the wild (notice the SP between Thanks and to), for example.
Rejecting leading whitespace as a line that does *not* start the
header (hence its colon does not count) may be a good compromise.


Good point.


I think that "Signed-off-by:" is not guaranteed to be
present.


But do we really care about that case where there is no S-o-b:?  I
personally do not think so.


I think we should - the use case I had in mind when I started this is 
the Android Git repository, which does not use Signed-off-by. For 
example, I quoted this commit in an earlier e-mail [1]:


https://android.googlesource.com/platform/frameworks/base/+/4c5281862f750cbc9d7355a07ef1a5545b9b3523

which has the footer:

  Bug: http://b/30562229
  Test: readelf --dyn-sym app_process32 and check that bsd_signal is 
exported
readelf --dyn-sym app_process64 and check that bsd_signal is 
not exported

  Change-Id: Iec584070b42bc7fa43b114c0f884aff2db5a6858


Defining a trailer line as "a line starting with a token,
then optional whitespace, then separator", maybe the following rule:
- at least one trailer line generated by Git ("(cherry picked by" or
"Signed-off-by") or configured in the "trailer" section in gitconfig
OR
- at least 3/4 logical trailer lines (I'm wondering if this should be
100% trailer lines)


I'd strongly suggest turning that OR to AND.  We will not safely be
able to write a commit log message that describes how S-o-b lines
are handled in its last paragraph otherwise.

I do not care too deeply about 3/4, but I meant to allow 75% cruft
but no more than that, and the fact that the threashold is set at
way more than 50% is important.  IOW, if you have

Ordinary log message here...

S-o-b: A U Thor 
[a short description that is typically a single liner
in the real world use pattern we saw in the world, but
could overflow to become multi line cruft]
S-o-b: R E Layer 

"last paragraph" is 5 lines long, among which 60% are cruft that is
below the 75% threshold, and "am -s" can still add the S-o-b of the
committer at the end of that existing last paragraph.  Making it too
strict would raise the false negative ratio.


Ah, sorry, I misread your original suggestion.

Would this work then:
- at least one trailer line generated by Git ("(cherry picked by" or
  "Signed-off-by: ") or configured in the "trailer" section in
  git config AND at least 25% logical trailer lines
OR
- 100% logical trailer lines

The first part is your original suggestion except that I think that it 
is more consistent to allow other trailer lines as well (but I do not 
feel too strongly about this). The second part would satisfy the Android 
Git use case above, and also retain existing behavior when 
"Signed-off-by" (for example) is added to an existing footer that does 
not contain "Signed-off-by" yet.


What do you think?

[1] Message ID <29cb0f55-f729-80af-cdca-64e927fa9...@google.com>


Re: [PATCH 5/7] merge-base: mark bases that are on first-parent chain

2016-10-19 Thread Junio C Hamano
Junio C Hamano  writes:

> In a workflow where topic branches are first merged to the 'next'
> integration branch to be tested before getting merged down to the
> 'master' integration branch to be consumed by the end users, merging
> the 'master' branch back to the 'next' happens after topics graduate
> to 'master' and release notes entries are written for them.
>
> Git finds many merge bases between 'master' and 'next' while
> creating this merge.  In addition to the tip of 'master' back when
> we made such a merge back from 'master' to 'next' was made the last
> time, which is the most reasonable merge base to explain the
> histories of both branches, all the tips of topic branches that
> graduated recently are merge bases.  Because these tips of topic
> branches were already in 'next', the tip of 'next' reaches them, and
> because they just graduated to 'master', the tip of 'master' reaches
> them, too.  And these topics are independent most of the time (that
> is the point of employing the topic-branch workflow), so they cannot
> be reduced.

The idea here is to exclude tips of topic branches as "obviously
less useful as merge bases than the mainline commit".  Aside from
the fact that the approach is purely a heuristic that heavily relies
on convention aka "topic branch workflow", there is a caveat (or
more that I haven't thought of yet).  

One is that there may be no merge base found that is on the first
parent chain with this particular patch.  When any of the topics
that just graduated to 'master' was forked off of 'master' after the
latest merge from 'master' to 'next' was made, then the merge base
on the first parent chain, i.e. the commit on 'master' that was
merged to 'next' the last time, will be an ancestor of the tip of
that recently forked topic, which is another common ancestor between
'master' and 'next' being merged (hence removed as redundant).  We
will be left with only the merge bases that are off first-parent
chain, and I think "git merge" will say "no related histories; I
won't merge them".

I do not know if that is a problem in practice, but if it turns out
to be, it probably can be corrected by updating the way how the
paint_down_to_common() function works.  It still stops traversal,
even with this patch, when a commit is found to be common and place
it to the "potential merge bases" list, but instead we could keep
digging until we hit a commit that is common between PARENT1 and
PARENT2 and also is on the first-parent chain, without declaring
that we found one result.

But that probably ends up being the same as ignoring all side
branches from merges and finding merge bases only paying attention
to the first-parent chain.


Re: Drastic jump in the time required for the test suite

2016-10-19 Thread Junio C Hamano
Johannes Schindelin  writes:

> What I can also say for certain is that the version from yesterday (commit
> 4ef44ce) was the first one in a week that built successfully and hence
> reached the test phase, and it was the first version of `pu` ever to time
> out after running for 3h.

I am sympathetic, but I'd be lying if I said I can feel your pain.

Admittedly I do not run _all_ the tests (I certainly know that I
exclude the ones behind EXPENSIVE prerequisite), but after every
rebuilding of 'jch' and 'pu', I run the testsuite (and also rebuild
docs) before pushing them out, and "make test && make doc && make
install install-doc" run sequentially for the four integration
branches finishes within 15 minutes, even when I run them after
"make clean".

Perhaps the recent change to run the tests in parallel from slower
to faster under prove may be helping my case.

> Maybe we should start optimizing the tests...

Yup, two things that come to mind are to identify long ones and see
if each of them can be split into two halves that can be run in
parallel, and to go through the tests with fine toothed comb and
remove the ones that test exactly the same thing as another test.
The latter would be very time consuming, though.


Re: [PATCH v4 2/7] ref-filter: add function to print single ref_array_item

2016-10-19 Thread Santiago Torres
On Wed, Oct 19, 2016 at 05:16:42AM -0400, Jeff King wrote:
> On Wed, Oct 19, 2016 at 04:55:43AM -0400, Jeff King wrote:
> 
> > > diff --git a/ref-filter.h b/ref-filter.h
> > > index 14d435e..3d23090 100644
> > > --- a/ref-filter.h
> > > +++ b/ref-filter.h
> > > @@ -107,4 +107,7 @@ struct ref_sorting *ref_default_sorting(void);
> > >  /*  Function to parse --merged and --no-merged options */
> > >  int parse_opt_merge_filter(const struct option *opt, const char *arg, 
> > > int unset);
> > >  
> > > +void pretty_print_ref(const char *name, const unsigned char *sha1,
> > > + const char *format, unsigned kind);
> > > +
> > 
> > What are the possible values for "kind"? I guess these should come from
> > FILTER_REFS_TAGS, BRANCHES, etc. It's probably worth documenting that.
> > Alternatively, is it possible to just determine this from the name? It
> > looks like filter_ref_kind() is how it happens for a normal ref-filter.
> 
> I guess that may complicate things for the caller you add in this
> series, which may not have a fully-qualified refname (which is obviously
> how filter_ref_kind() figures it out). I'd argue that is a bug, though,
> as things like "%(refname)" are generally expected to print out the
> fully refname ("git tag --format=%(refname)" does so, and you can use
> "%(refname:short)" if you want the shorter part).

Hmm, I hadn't actually noticed that. Do you have any suggestions in how to
address this?

In general this feels like a consequence of disambiguating .git/tags/*
within builtin/tag.c rather than letting ref-filter figure it out.

Thanks,
-Santiago.


signature.asc
Description: PGP signature


Re: [PATCH] sha1_name: remove ONELINE_SEEN bit

2016-10-19 Thread Johannes Schindelin
Hi Junio,

On Tue, 18 Oct 2016, Junio C Hamano wrote:

> 28a4d94044 ("object name: introduce ':/' notation",
> 2007-02-24) started using its own bit from object->flags to mark
> commits used while parsing the ":/token" extended SHA-1 syntax to
> name a commit temporarily, and this was kept even when f7bff00314
> ("sha1_name.c: fix parsing of ":/token" syntax", 2010-08-02) found
> and fixed a bug in its implementation.
> 
> The use of that flag bit, however, is limited to a single function,
> get_sha1_oneline(), which first sets it for the commits sitting at
> the tips of refs, uses the bit to avoid duplicate traversal while
> walking the history, and then cleans the bit from all commits it
> walked.
> 
> Which is exactly what the general-purpose TMP_MARK bit meant to be
> used for isolated case was invented for.  Replace ONELINE_SEEN with
> TMP_MARK and retire the former.

ACK,
Dscho


Re: Integrating submodules with no side effects

2016-10-19 Thread Stefan Beller
On Wed, Oct 19, 2016 at 6:27 AM, Robert Dailey  wrote:
> On Tue, Oct 18, 2016 at 4:17 PM, Stefan Beller  wrote:
>> On Tue, Oct 18, 2016 at 12:35 PM, Robert Dailey
>>  wrote:
>>> Hello git experts,
>>>
>>> I have in the past attempted to integrate submodules into my primary
>>> repository using the same directory name. However, this has always
>>> caused headache when going to and from branches that take you between
>>> when this integration occurred and when it didn't. It's a bit hard to
>>> explain. Basically, if I have a submodule "foo", and I delete that
>>> submodule and physically add its files under the same directory "foo",
>>> when I do a pull to get this change from another clone, it fails
>>> saying:
>>>
>>> error: The following untracked working tree files would be overwritten
>>> by checkout:
>>> foo/somefile.txt
>>> Please move or remove them before you switch branches.
>>> Aborting
>>> could not detach HEAD
>>>
>>>
>>> Obviously, git can't delete the submodule because the files have also
>>> been added directly. I don't think it is built to handle this
>>> scenario. Here is the series of commands I ran to "integrate" the
>>> submodule (replace the submodule with a directory containing the exact
>>> contents of the submodule itself):
>>>
>>> #!/usr/bin/env bash
>>> mv "$1" "${1}_"
>>> git submodule deinit "$1"
>>
>> This removes the submodule entries from .git/config
>> (and it would remove the contents of that submodule, but they are moved)
>>
>>> git rm "$1"
>>
>> Removing the git link here.
>>
>> So we still have the entries in the .gitmodules file there.
>> Maybe add:
>>
>> name=$(git submodule-helper name $1)
>> git config -f .gitmodules --unset submodule.$name.*
>> git add .gitmodules
>>
>> ? (Could be optional)
>
> Actually I verified that it seems `git rm` is specialized for
> submodules somewhere, because when I run that command on a submodule
> the relevant entries in the .gitmodules file are removed. I did not
> have to do this as a separate step.
>
>>> mv "${1}_" "$1"
>>> git add "$1/**"
>>
>> Moving back into place and adding all files in there.
>>
>>>
>>> The above script is named git-integrate-submodule, I run it like so:
>>>
>>> $ git integrate-submodule foo
>>>
>>> Then I do:
>>>
>>> $ git commit -m 'Integrated foo submodule'
>>>
>>> Is there any way to make this work nicely?
>>
>> I think you can just remove the gitlink from the index and not from the 
>> working
>> tree ("git rm --cached $1")
>
> What is the goal of doing it this way? What does this simplify?

You don't have to mv it back and forth with an underscore I would imagine?

>
>>> The only solution I've
>>> found is to obviously rename the directory before adding the physical
>>> files, for example name it foo1. Because they're different, they never
>>> "clash".
>>
>> Also look at the difference between plumbing and porcelain commands[1],
>> as plumbing is more stable than the porcelain, so it will be easier to 
>> maintain
>> this script.
>
> Which plumbing commands did you have in mind?

None specifically. I write scripts using porcelain all the time for
personal use.
But if you were planning to publish this seriously then I'd recommend looking at
plumbing commands.

>
>> I think this would be an actually reasonable feature, which Git itself
>> could support via "git submodule [de]integrate", but then we'd also want
>> to see the reverse, i.e. take a sub directory and make it a submodule.
>
> Integrating this as a feature might be fine, I think when you bring up
> the question of retaining history makes things much harder.
> Fortunately for me that is not a requirement in this case, so I'm able
> to do things with much less effort.

That reminds me of subtree merging, which could be used for this case.
(see 'git subtree')

>
> However the primary purpose of my post was to find out how to
> integrate the submodule without the error on next pull by other
> collaborators of my repository. It's a real pain to recover your
> working copy when going inbetween commits where the submodule
> integration happened inbetween them. I did quote the exact error
> message I got in my original post.

You could try this patch series:
https://github.com/jlehmann/git-submod-enhancements/tree/git-checkout-recurse-submodules
(rebased to a newer version; no functional changes:)
https://github.com/stefanbeller/git/tree/submodule-co
(I'll rebase that later to origin/master)

>
> Do you have any info on how I can prevent that error? Ideally I want
> the integration to go smoothly and transparently, not just for the
> person doing the actual transition (me) but for everyone else that
> gets those changes from upstream. They should not even notice that it
> happened (i.e. no failed commands, awkward behavior, or manual steps).

It depends on how long you want to postpone the transition, but I plan to
upstream the series referenced above in the near 

[PATCH] Add a knob to abort on die() (was Re: git checkout crashes after ...)

2016-10-19 Thread Duy Nguyen
On Wed, Oct 19, 2016 at 08:27:43PM +0700, Duy Nguyen wrote:
> If you set the environment variable GIT_ALLOC_LIMIT ...  git
> attempts to allocate more than that ... then it's caught and we get
> a glimpse of how much memory git may need. Unfortunately we can't
> get a stack trace or anything like that unless you rebuild Git from
> source.

It's moments like this that I wish we had a knob to force core
dumps. And I often modify die_builtin() to add '*(char*)0 = 1;' to
force a core dump when I can't figure out some problem based on the
error message alone.

So.. how about we do something like this? We could extend it to abort
on error() as well as die(). Aborting on warning() may be a bit too
much though. On glibc systems we could even print the back trace
without aborting, which helps in some cases.

The long variable name and value are on purpose to hopefully not
trigger this by mistake.

diff --git a/git.c b/git.c
index ab5c99c..5fea224 100644
--- a/git.c
+++ b/git.c
@@ -622,15 +622,34 @@ static int run_argv(int *argcp, const char ***argv)
return done_alias;
 }
 
+static NORETURN void die_by_aborting(const char *err, va_list params)
+{
+   vreportf("fatal: ", err, params);
+   abort();
+}
+
+static NORETURN void die_silently_by_aborting(const char *err, va_list params)
+{
+   abort();
+}
+
 int cmd_main(int argc, const char **argv)
 {
const char *cmd;
int done_help = 0;
+   const char *die_abort_env = getenv("GIT_ABORT_ON_FATAL_ERRORS");
 
cmd = argv[0];
if (!cmd)
cmd = "git-help";
 
+   if (die_abort_env) {
+   if (!strcmp(die_abort_env, "yes please"))
+   set_die_routine(die_by_aborting);
+   else if (!strcmp(die_abort_env, "just die"))
+   set_die_routine(die_silently_by_aborting);
+   }
+
trace_command_performance(argv);
 
/*
--
Duy


Drastic jump in the time required for the test suite

2016-10-19 Thread Johannes Schindelin
Hi Junio,

I know you are a fan of testing things thoroughly in the test suite, but I
have to say that it is getting out of hand, in particular due to our
over-use of shell script idioms (which really only run fast on Linux, not
a good idea for a portable software).

My builds of `pu` now time out, after running for 3h straight in the VM
dedicated to perform the daily routine of building and testing the git.git
branches in Git for Windows' SDK. For comparison, `next` passes build &
tests in 2.6h. That is quite the jump.

Unfortunately, I cannot tell you precisely when this massive increase
happened (we are talking about half an hour, after all), because there
were build failures preventing the test from being run for the last 40
days (and my build job only retains the results for 7 days currently,
anyway, in addition to the last successful/unsuccessful build).

What I can say for certain is that the version from 41 days ago (commit
7837d4a) ran for only 2.6h, just as `next` does today, and passed without
failure.

Every single revision of `pu` since then has been broken in one way or
another. Mostly already the build, so that the tests would not even start
to run.

What I can also say for certain is that the version from yesterday (commit
4ef44ce) was the first one in a week that built successfully and hence
reached the test phase, and it was the first version of `pu` ever to time
out after running for 3h.

I will increase the time-out, of course, but we are walking into a
dangerous territory here: the build & test requires such an incredibly
long time now that the VM will start to take more than 24h to complete all
8 tasks (building & testing the `pu`, `next`, `master` and `maint`
branches, as well as trying to rebase Git for Windows' patches onto each
of them).

As those tasks are scheduled to run once every 24h, I will no longer be
able to notify you and the mailing list in a timely manner, if at all,
because the queue will clog up.

Maybe we should start optimizing the tests to become more useful again, by
forcing them not to take such an insane amount of time?

Ciao,
Dscho


Re: [PATCH v4 1/7] gpg-interface, tag: add GPG_VERIFY_QUIET flag

2016-10-19 Thread Jeff King
On Fri, Oct 07, 2016 at 05:07:15PM -0400, santi...@nyu.edu wrote:

> From: Lukas Puehringer 
> 
> Functions that print git object information may require that the
> gpg-interface functions be silent. Add GPG_VERIFY_QUIET flag and prevent
> print_signature_buffer from being called if flag is set.

The layering here is a little funny. The gpg-interface code allocates a
new flag, but none of its functions do anything with it.  Instead, it's
acted on only by the run_gpg_verify() command local to tag.c.

I guess this "flags" variable comes to us from other callsites via
gpg_verify_tag. That's still in tag.c, but it's arguably part of the gpg
interface.

So I think it's OK.

-Peff


Re: [PATCH v4 2/7] ref-filter: add function to print single ref_array_item

2016-10-19 Thread Jeff King
On Fri, Oct 07, 2016 at 05:07:16PM -0400, santi...@nyu.edu wrote:

> From: Lukas Puehringer 
> 
> ref-filter functions are useful for printing git object information
> using a format specifier. However, some other modules may not want to use
> this functionality on a ref-array but only print a single item.
> 
> Expose a pretty_print_ref function to create, pretty print and free
> individual ref-items.

Makes sense.

> diff --git a/ref-filter.h b/ref-filter.h
> index 14d435e..3d23090 100644
> --- a/ref-filter.h
> +++ b/ref-filter.h
> @@ -107,4 +107,7 @@ struct ref_sorting *ref_default_sorting(void);
>  /*  Function to parse --merged and --no-merged options */
>  int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
> unset);
>  
> +void pretty_print_ref(const char *name, const unsigned char *sha1,
> + const char *format, unsigned kind);
> +

What are the possible values for "kind"? I guess these should come from
FILTER_REFS_TAGS, BRANCHES, etc. It's probably worth documenting that.
Alternatively, is it possible to just determine this from the name? It
looks like filter_ref_kind() is how it happens for a normal ref-filter.

-Peff


Re: [PATCH v4 2/7] ref-filter: add function to print single ref_array_item

2016-10-19 Thread Jeff King
On Wed, Oct 19, 2016 at 04:55:43AM -0400, Jeff King wrote:

> > diff --git a/ref-filter.h b/ref-filter.h
> > index 14d435e..3d23090 100644
> > --- a/ref-filter.h
> > +++ b/ref-filter.h
> > @@ -107,4 +107,7 @@ struct ref_sorting *ref_default_sorting(void);
> >  /*  Function to parse --merged and --no-merged options */
> >  int parse_opt_merge_filter(const struct option *opt, const char *arg, int 
> > unset);
> >  
> > +void pretty_print_ref(const char *name, const unsigned char *sha1,
> > +   const char *format, unsigned kind);
> > +
> 
> What are the possible values for "kind"? I guess these should come from
> FILTER_REFS_TAGS, BRANCHES, etc. It's probably worth documenting that.
> Alternatively, is it possible to just determine this from the name? It
> looks like filter_ref_kind() is how it happens for a normal ref-filter.

I guess that may complicate things for the caller you add in this
series, which may not have a fully-qualified refname (which is obviously
how filter_ref_kind() figures it out). I'd argue that is a bug, though,
as things like "%(refname)" are generally expected to print out the
fully refname ("git tag --format=%(refname)" does so, and you can use
"%(refname:short)" if you want the shorter part).

-Peff


Re: [PATCH v4 5/7] builtin/tag: add --format argument for tag -v

2016-10-19 Thread Jeff King
On Fri, Oct 07, 2016 at 05:07:19PM -0400, santi...@nyu.edu wrote:

> Adding --format to git tag -v mutes the default output of the GPG
> verification and instead prints the formatted tag object.

The same comments apply to "mutes" here, as to the previous patch (which
is to say I think we may want something more, but this is probably OK
for now).

> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 7ecca8e..3bb5e3c 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -15,7 +15,7 @@ SYNOPSIS
>  'git tag' [-n[]] -l [--contains ] [--points-at ]
>   [--column[=] | --no-column] [--create-reflog] [--sort=]
>   [--format=] [--[no-]merged []] [...]
> -'git tag' -v ...
> +'git tag' -v [--format=] ...

Just thinking out loud, but if we had ref-filter placeholders that
triggered GPG verification, you could do this all with the listing mode,
like:

  git tag --format='%(gpgstatus) %(tag) %(refname:short)'

and verify multiple tags (or give a single tag to limit it to just one).

I don't think that's any kind of blocker for this series. We already
have "-v", and adding --format to it is reasonable, even if we
eventually move to a world where people can use the listing mode. Like I
said, just thinking out loud.

> +static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
> + void *cb_data)
>  {
>   const char **p;
>   char ref[PATH_MAX];
>   int had_error = 0;
>   unsigned char sha1[20];
>  
> +
>   for (p = argv; *p; p++) {

Extra space?

>  static int verify_tag(const char *name, const char *ref,
> - const unsigned char *sha1)
> + const unsigned char *sha1, void *cb_data)
>  {
> - return verify_and_format_tag(sha1, name, NULL, GPG_VERIFY_VERBOSE);
> + int flags;

Probably doesn't matter much, but these flags are defined as "unsigned"
elsewhere.

-Peff


Re: [PATCH v4 6/7] t/t7030-verify-tag: Add --format specifier tests

2016-10-19 Thread Jeff King
On Fri, Oct 07, 2016 at 05:07:20PM -0400, santi...@nyu.edu wrote:

> Verify-tag now provides --format specifiers to inspect and ensure the
> contents of the tag are proper. We add two tests to ensure this
> functionality works as expected: the return value should indicate if
> verification passed, and the format specifiers must be respected.

Sounds good.

> +test_expect_success 'verifying tag with --format' '
> + cat >expect <<-\EOF &&
> + tagname : fourth-signed
> + EOF
> + git verify-tag --format="tagname : %(tag)" "fourth-signed" >actual &&
> + test_cmp expect actual
> +'

I suppose it's a matter of style, but for a single-line expectation I
would just do:

  echo "tagname : fourth-signed" >expect &&

which is shorter and saves a process invocation.

Ditto on the next patch (which IMHO could just be squashed into a single
patch).

-Peff


Re: What's cooking in git.git (Oct 2016, #03; Tue, 11)

2016-10-19 Thread Jeff King
On Tue, Oct 18, 2016 at 08:53:44AM -0700, Junio C Hamano wrote:

> >> * st/verify-tag (2016-10-10) 7 commits
> [...]
> > Hi, I saw this on the previous "what's cooking." Is there anything I
> > need to do on my side to make sure this is ready for next?
> 
> Posting this exact message to the list would be an excellent way
> ;-).
> 
> Hopefully some competent reviewer comes and points me at a thread
> where s/he says the series was already reviewed and in good shape
> soonish, and your message may be a good trigger to make it happen.

Another from my list of "things I have been meaning to review". :)

I think this is close, but I had a few nits to pick with the ref-filter
printing interface. I sent comments for each patch in the v4 thread.

-Peff


[BUG] git-stash fails when tracked file is replaced with directory

2016-10-19 Thread Alex C. Reed, IV
When a file is deleted from Git and replaced with a directory+file(s),
git-stash has two unexpected behaviors.  This is tested against
versions 2.8.1 and 2.10.1:


1) Old file is removed from index and newly-added directory+file(s)
are added to index.
In this scenario, 'git stash' fails with the message:

error: : is a directory - add individual files instead
fatal: Unable to process path 
Cannot save the current work tree state

The expected result would be that Git properly records the state
currently stored in index.


2) Old file is removed and new-added directory+file(s) are present
only in worktree, but not added to the index.
In this scenario, 'git stash -u' works fine, but 'git stash apply'
fails with the message:

fatal: cannot create directory at '': File exists
Could not restore untracked files from stash

The expected result would be that Git removes 'file' and replaces with
the untracked contents recorded in the original stash.


It is worth noting that Git does properly handle the scenario where
the stash operation of (1) is replaced with a commit to a temporary
branch, so this quirk seems to be restricted to stashes only.


I found a similar issue reported 22 April 2016 titled "possible bug of
git stash deleting uncommitted files in corner case".  The thread-view
of the message is here:

http://marc.info/?t=14613256862=1=2


Here's a quick-and-dirty bash script to re-create all three scenarios
(1, 2, and 'commit to branch'):

= BEGIN stash-test.sh
#!/bin/bash

set -x
export GIT_TRACE=1

setup() {
  repo=$1

  # Prepare repo
  rm -rf $repo
  git init $repo
  cd $repo

  # Add initial file (symlink to external assets)
  ln -fs /external/dir dir
  git add dir
  git commit -m "Add symlink to /external/dir"

  # Replace symlink with local copy of assets
  rm dir
  mkdir -p dir
  touch dir/local_copy_of_dir_files
}

{
  ( setup stash-test-unstaged
git stash -u
git stash apply )

  ( setup stash-test-staged
git add .
git stash )

  ( setup stash-test-commit
git add .
git co -b stash-branch
git commit -m "commit to branch instead of stash" )

} 2>&1 | tee stash-test.log
= END stash-test.sh


Thanks,
-Alex Reed


[regression] `make profile-install` fails in 2.10.1

2016-10-19 Thread Jan Keromnes
Hello,

TL;DR - Probably a regression of a previously reported bug. [0]

I'm trying to `profile-install` Git from source on Ubuntu 16.04, to
have the latest stable Git optimized for my machine.

However, this often fails, because the profile build runs all Git
tests (to get an idea of how Git behaves on my hardware, and optimize
for it), but it bails out if there are any test failures (for me, this
has happened on most Git version upgrades this year, see also [0] and
[1]).

- Problem: Is there a way to `make profile-install` but ignore
occasional test failures, as these are not critical to get a useful
hardware profile? (Note: In a previous thread, Dennis Kaarsemaker
mentioned this is fixing a symptom, not the root cause, but it would
still be great to get a working profile in spite of occasional test
failures.)

- Related problem: `t3700-add.sh` fails again in 2.10.1 for me. More
details below, and I can provide further debug information if you
don't already know the problem.

Thanks,
Jan

[0] "`make profile-install` fails in 2.9.3" -
https://marc.info/?l=git=147274608823171=2

[1] "Fwd: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile
build" - https://marc.info/?l=git=146193443529229=2

---

Steps to reproduce:

curl https://www.kernel.org/pub/software/scm/git/git-2.10.1.tar.xz
| tar xJ \
 && cd git-2.10.1 \
 && make prefix=/usr profile-install install-man -j18

Expected result:

- runs all tests to get a profile (ignoring occasional failures)
- rebuilds Git with the profile
- installs Git

Actual result:

- runs all tests to get a profile
- at least one test fails, interrupting the whole process
- Git is not installed

Failure log:

(snip)
*** t3700-add.sh ***
ok 1 - Test of git add
ok 2 - Post-check that foo is in the index
ok 3 - Test that "git add -- -q" works
ok 4 - git add: Test that executable bit is not used if core.filemode=0
ok 5 - git add: filemode=0 should not get confused by symlink
(snip)
ok 38 - git add --chmod=[+-]x stages correctly
ok 39 - git add --chmod=+x with symlinks
not ok 40 - git add --chmod=[+-]x changes index with already added file
#
#   echo foo >foo3 &&
#   git add foo3 &&
#   git add --chmod=+x foo3 &&
#   test_mode_in_index 100755 foo3 &&
#   echo foo >xfoo3 &&
#   chmod 755 xfoo3 &&
#   git add xfoo3 &&
#   git add --chmod=-x xfoo3 &&
#   test_mode_in_index 100644 xfoo3
#
ok 41 - git add --chmod=[+-]x does not change the working tree
ok 42 - no file status change if no pathspec is given
ok 43 - no file status change if no pathspec is given in subdir
ok 44 - all statuses changed in folder if . is given
# failed 1 among 44 test(s)
1..44
Makefile:43: recipe for target 't3700-add.sh' failed
make[3]: Leaving directory '/tmp/git/git-2.10.1/t'
make[3]: *** [t3700-add.sh] Error 1
Makefile:36: recipe for target 'test' failed
make[2]: Leaving directory '/tmp/git/git-2.10.1/t'
make[2]: *** [test] Error 2
Makefile:2273: recipe for target 'test' failed
make[1]: *** [test] Error 2
make[1]: Leaving directory '/tmp/git/git-2.10.1'
Makefile:1679: recipe for target 'profile' failed
make: *** [profile] Error 2
The command '/bin/sh -c mkdir /tmp/git  && cd /tmp/git  && curl
https://www.kernel.org/pub/software/scm/git/git-2.10.1.tar.xz | tar xJ
 && cd git-2.10.1  && make prefix=/usr profile-install install-man
-j18  && rm -rf /tmp/git' returned a non-zero code: 2


Re: Integrating submodules with no side effects

2016-10-19 Thread Robert Dailey
On Tue, Oct 18, 2016 at 4:17 PM, Stefan Beller  wrote:
> On Tue, Oct 18, 2016 at 12:35 PM, Robert Dailey
>  wrote:
>> Hello git experts,
>>
>> I have in the past attempted to integrate submodules into my primary
>> repository using the same directory name. However, this has always
>> caused headache when going to and from branches that take you between
>> when this integration occurred and when it didn't. It's a bit hard to
>> explain. Basically, if I have a submodule "foo", and I delete that
>> submodule and physically add its files under the same directory "foo",
>> when I do a pull to get this change from another clone, it fails
>> saying:
>>
>> error: The following untracked working tree files would be overwritten
>> by checkout:
>> foo/somefile.txt
>> Please move or remove them before you switch branches.
>> Aborting
>> could not detach HEAD
>>
>>
>> Obviously, git can't delete the submodule because the files have also
>> been added directly. I don't think it is built to handle this
>> scenario. Here is the series of commands I ran to "integrate" the
>> submodule (replace the submodule with a directory containing the exact
>> contents of the submodule itself):
>>
>> #!/usr/bin/env bash
>> mv "$1" "${1}_"
>> git submodule deinit "$1"
>
> This removes the submodule entries from .git/config
> (and it would remove the contents of that submodule, but they are moved)
>
>> git rm "$1"
>
> Removing the git link here.
>
> So we still have the entries in the .gitmodules file there.
> Maybe add:
>
> name=$(git submodule-helper name $1)
> git config -f .gitmodules --unset submodule.$name.*
> git add .gitmodules
>
> ? (Could be optional)

Actually I verified that it seems `git rm` is specialized for
submodules somewhere, because when I run that command on a submodule
the relevant entries in the .gitmodules file are removed. I did not
have to do this as a separate step.

>> mv "${1}_" "$1"
>> git add "$1/**"
>
> Moving back into place and adding all files in there.
>
>>
>> The above script is named git-integrate-submodule, I run it like so:
>>
>> $ git integrate-submodule foo
>>
>> Then I do:
>>
>> $ git commit -m 'Integrated foo submodule'
>>
>> Is there any way to make this work nicely?
>
> I think you can just remove the gitlink from the index and not from the 
> working
> tree ("git rm --cached $1")

What is the goal of doing it this way? What does this simplify?

>> The only solution I've
>> found is to obviously rename the directory before adding the physical
>> files, for example name it foo1. Because they're different, they never
>> "clash".
>
> Also look at the difference between plumbing and porcelain commands[1],
> as plumbing is more stable than the porcelain, so it will be easier to 
> maintain
> this script.

Which plumbing commands did you have in mind?

> I think this would be an actually reasonable feature, which Git itself
> could support via "git submodule [de]integrate", but then we'd also want
> to see the reverse, i.e. take a sub directory and make it a submodule.

Integrating this as a feature might be fine, I think when you bring up
the question of retaining history makes things much harder.
Fortunately for me that is not a requirement in this case, so I'm able
to do things with much less effort.

However the primary purpose of my post was to find out how to
integrate the submodule without the error on next pull by other
collaborators of my repository. It's a real pain to recover your
working copy when going inbetween commits where the submodule
integration happened inbetween them. I did quote the exact error
message I got in my original post.

Do you have any info on how I can prevent that error? Ideally I want
the integration to go smoothly and transparently, not just for the
person doing the actual transition (me) but for everyone else that
gets those changes from upstream. They should not even notice that it
happened (i.e. no failed commands, awkward behavior, or manual steps).


Re: git checkout crashes after server being updated to Debian X86_64

2016-10-19 Thread Duy Nguyen
On Tue, Oct 18, 2016 at 10:17 PM, Raffael Reichelt
 wrote:
> Hello!
>
> I have a serious problem with git, After my provider had updated to a X86_64 
> architecture git crashes with various memory-related errors. This is 
> happening remote when pushing to the repository from my local machine as well 
> as trying it on a shell on the server itself.
>
> This are the error-messages:
>
> fatal: Out of memory, realloc failed
> fatal: recursion detected in die handler
> fatal: recursion detected in die handler

You other mail said memory is capped at 600MB, which should be a lot
for normal repositories. If you set the environment variable
GIT_ALLOC_LIMIT to maybe 500MB or lower (convert it to kilobytes
first) and git attempts to allocate more than that (just that one
time, not total mem) then it's caught and we get a glimpse of how much
memory git may need. Unfortunately we can't get a stack trace or
anything like that unless you rebuild Git from source.

> or
> fatal: unable to create threaded lstat
> fatal: recursion detected in die handler

Hmm.. with "max user processes (-u) 42" we should be fine because we
only create 20 threads max. What happens if you set core.preloadindex
to false? Can it run until the end or hit some other fatal errors?

There's room for improvement in preload_index(). If we hit resource
limit like this, it's not the end of the world and we should be able
to keep going. But threaded lstat has been available for a long time
and this is the first time I see a report like this, not sure if it's
worth fixing.
-- 
Duy


Re: What's cooking in git.git (Oct 2016, #04; Mon, 17)

2016-10-19 Thread Jeff King
On Tue, Oct 18, 2016 at 01:31:27PM -0700, Junio C Hamano wrote:

> >> * ls/filter-process (2016-10-17) 14 commits
> [...]
> > what do you think about v11? Do you feel the series is becoming mature
> > enough for `next`?
> 
> I've already had that feeling a few rounds ago, but I haven't had a
> chance to read the most recent one carefully myself to answer that
> question honestly.

FWIW, I gave it a fairly thorough read-over (something I'd been meaning
to do for quite a while, but kept never quite getting around to). I
think overall it is OK for next. I did find one or two nits, but I think
they are things we can fix up in-tree if and when they become a problem
(e.g., I noticed that test-genrandom gets piped to "perl -pe". I'm not
sure if perl will complain about funny multibyte characters on some
systems. I suggest we ignore it until somebody demonstrates that it
actually matters).

-Peff


Re: git checkout crashes after server being updated to Debian X86_64

2016-10-19 Thread Raffael Reichelt

> Am 19.10.2016 um 15:27 schrieb Duy Nguyen :
> 
> On Tue, Oct 18, 2016 at 10:17 PM, Raffael Reichelt
>  wrote:
>> Hello!
>> 
>> I have a serious problem with git, After my provider had updated to a X86_64 
>> architecture git crashes with various memory-related errors. This is 
>> happening remote when pushing to the repository from my local machine as 
>> well as trying it on a shell on the server itself.
>> 
>> This are the error-messages:
>> 
>> fatal: Out of memory, realloc failed
>> fatal: recursion detected in die handler
>> fatal: recursion detected in die handler
> 
> You other mail said memory is capped at 600MB, which should be a lot
> for normal repositories. If you set the environment variable
> GIT_ALLOC_LIMIT to maybe 500MB or lower (convert it to kilobytes
> first) and git attempts to allocate more than that (just that one
> time, not total mem) then it's caught and we get a glimpse of how much
> memory git may need. Unfortunately we can't get a stack trace or
> anything like that unless you rebuild Git from source.

This was no change: crashed with the same errors …

> 
>> or
>> fatal: unable to create threaded lstat
>> fatal: recursion detected in die handler
> 
> Hmm.. with "max user processes (-u) 42" we should be fine because we
> only create 20 threads max. What happens if you set core.preloadindex
> to false? Can it run until the end or hit some other fatal errors?
> 

This did the trick :) I just repeatedly did a forced checkout and it went until 
the end without errors
THX a lot!

Raffael



Re: [PATCH] daemon, path.c: fix a bug with ~ in repo paths

2016-10-19 Thread Duy Nguyen
On Wed, Oct 19, 2016 at 1:05 AM, Luke Shumaker  wrote:
>> I am not sure if it is even a bug.  As you can easily lose that
>> tilde that appears in front of subdirectory of /srv/git/ or replace
>> it with something else (e.g. "u/"), this smells like "Don't do it if
>> it hurts" thing to me.
>
> I buy into "Don't do it if it hurts", but that doesn't mean it's not a
> bug on an uncommon edge-case.

The amount of changes is unbelievable for fixing such a rare case
though. I wonder if we can just detect this in daemon.c and pass
"./~foo/bar" instead of "~foo/bar" to enter_repo() in non-strict mode
to "disable" expand_user_path(). If it works, it's much simpler
changes (even though a bit hacky)
-- 
Duy


[ANNOUNCE] Git Rev News edition 20

2016-10-19 Thread Christian Couder
Hi everyone,

I'm happy announce that the 20th edition of Git Rev News is now published:

https://git.github.io/rev_news/2016/10/19/edition-20/

Thanks a lot to all the contributors and helpers, especially Jakub,
Dennis, Dscho, Lars and Peff!

Enjoy,
Christian and Thomas.


Re: [PATCH v4 4/7] builtin/verify-tag: add --format to verify-tag

2016-10-19 Thread Jeff King
On Fri, Oct 07, 2016 at 05:07:18PM -0400, santi...@nyu.edu wrote:

> @@ -46,12 +50,17 @@ int cmd_verify_tag(int argc, const char **argv, const 
> char *prefix)
>   if (verbose)
>   flags |= GPG_VERIFY_VERBOSE;
>  
> + if (fmt_pretty) {
> + verify_ref_format(fmt_pretty);
> + flags |= GPG_VERIFY_QUIET;
> + }

I see why you would want to disable the normal output when there is a
custom format. But it seems a shame that the GPG information cannot be
retrieved as part of that format.

I think in the long run we'd want something like pretty.c's "%G"
placeholders for the ref-filter pretty-printer. But I think we are OK to
do this patch without it. It allows at least:

  tag=v2.0.0
  got=$(git verify-tag --format='%(tag)' "$tag") &&
  test "$tag" = "$got" ||
  echo >&2 "verification failed"

which is better than what we have now, and can be extended in the
future.

-Peff