Git development
 help / color / mirror / Atom feed
* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Jacob Keller @ 2016-10-19 23:36 UTC (permalink / raw)
  To: Jeff King; +Cc: Git mailing list
In-Reply-To: <20161019224211.k4anavgqrhmunz6p@sigill.intra.peff.net>

On Wed, Oct 19, 2016 at 3:42 PM, Jeff King <peff@peff.net> 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 <file> to the commit
>> $git commit -s <file>
>> # 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 <files>" 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 <files>" "git commit <other files>"

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

^ permalink raw reply

* Re: [PATCH] submodules: update documentaion for submodule branches
From: Brandon Williams @ 2016-10-19 23:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, sbeller
In-Reply-To: <xmqqzim0hsdz.fsf@gitster.mtv.corp.google.com>

On 10/19, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Brandon Williams <bmwill@google.com> 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.<name>.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

^ permalink raw reply

* Re: [PATCH] submodules: update documentaion for submodule branches
From: Junio C Hamano @ 2016-10-19 22:43 UTC (permalink / raw)
  To: Brandon Williams; +Cc: git, sbeller
In-Reply-To: <xmqqmvi0j9rv.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

> Brandon Williams <bmwill@google.com> 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.<name>.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.



^ permalink raw reply

* Re: tools for easily "uncommitting" parts of a patch I just commited?
From: Jeff King @ 2016-10-19 22:42 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Git mailing list
In-Reply-To: <CA+P7+xqFOn4NSfZ2zpa_y1za3uHZrGGG3ktEtuOcvJLCrAYUhQ@mail.gmail.com>

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 <file> to the commit
> $git commit -s <file>
> # 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

^ permalink raw reply

* Re: [PATCH] rev-list: restore the NUL commit separator in --header mode
From: Junio C Hamano @ 2016-10-19 22:41 UTC (permalink / raw)
  To: Dennis Kaarsemaker
  Cc: git, jacob.e.keller, stefanbeller, peff, j6t, jacob.keller
In-Reply-To: <20161019210448.aupphybw5qar6mqe@hurricane>

Dennis Kaarsemaker <dennis@kaarsemaker.net> 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
> +'

^ permalink raw reply

* Re: [PATCH] rev-list: restore the NUL commit separator in --header mode
From: Junio C Hamano @ 2016-10-19 22:39 UTC (permalink / raw)
  To: Jacob Keller
  Cc: Dennis Kaarsemaker, Git mailing list, Jacob Keller, Stefan Beller,
	Jeff King, Johannes Sixt
In-Reply-To: <CA+P7+xogHOCbPV+rx7yrur85m=HX5ms9kGQYvTpQ7n2i7Hzuvw@mail.gmail.com>

Jacob Keller <jacob.keller@gmail.com> writes:

> Hi,
>
> On Wed, Oct 19, 2016 at 2:04 PM, Dennis Kaarsemaker
> <dennis@kaarsemaker.net> 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 <dennis@kaarsemaker.net>
>> ---
>>  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?

^ permalink raw reply

* Re: [regression] `make profile-install` fails in 2.10.1
From: Jeff King @ 2016-10-19 22:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jan Keromnes, git
In-Reply-To: <xmqqd1iwj7jf.fsf@gitster.mtv.corp.google.com>

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

^ permalink raw reply

* Re: [regression] `make profile-install` fails in 2.10.1
From: Jeff King @ 2016-10-19 22:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jan Keromnes, git
In-Reply-To: <xmqqh988j7oo.fsf@gitster.mtv.corp.google.com>

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

^ permalink raw reply

* Re: [regression] `make profile-install` fails in 2.10.1
From: Junio C Hamano @ 2016-10-19 22:30 UTC (permalink / raw)
  To: Jeff King; +Cc: Jan Keromnes, git
In-Reply-To: <xmqqh988j7oo.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> 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 &&

^ permalink raw reply related

* Re: [regression] `make profile-install` fails in 2.10.1
From: Junio C Hamano @ 2016-10-19 22:27 UTC (permalink / raw)
  To: Jeff King; +Cc: Jan Keromnes, git
In-Reply-To: <20161019210519.ubk5q54rrvbafch7@sigill.intra.peff.net>

Jeff King <peff@peff.net> 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-jeremyhu@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 <gitster@pobox.com>
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 <gitster@pobox.com>
---
 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


^ permalink raw reply related

* tools for easily "uncommitting" parts of a patch I just commited?
From: Jacob Keller @ 2016-10-19 22:26 UTC (permalink / raw)
  To: Git mailing list

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 <file> to the commit
$git commit -s <file>
# 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^ <file>
# update commit removing all changes from this file
$git commit --allow-empty --amend <file>
# now checkout the tree again to the contents of the saved tree
$git checkout $(cat savetree) <file>
# 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

^ permalink raw reply

* Re: Problems with "git svn clone"
From: K Richard Pixley @ 2016-10-19 20:49 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <20161019204118.GA5982@starla>

On 10/19/16 13:41 , Eric Wong wrote:
> K Richard Pixley <cnp637@zebra.com> 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/0BCA1E695085C645B9CD4A27DD59F6FA39AAD5CF@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.

^ permalink raw reply

* Re: [PATCH] rev-list: restore the NUL commit separator in --header mode
From: Jacob Keller @ 2016-10-19 22:15 UTC (permalink / raw)
  To: Dennis Kaarsemaker
  Cc: Git mailing list, Jacob Keller, Stefan Beller, Jeff King,
	Johannes Sixt
In-Reply-To: <20161019210448.aupphybw5qar6mqe@hurricane>

Hi,

On Wed, Oct 19, 2016 at 2:04 PM, Dennis Kaarsemaker
<dennis@kaarsemaker.net> 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 <dennis@kaarsemaker.net>
> ---
>  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

^ permalink raw reply

* Re: [PATCH v12 3/8] graph: add support for --line-prefix on all graph-aware output
From: Jacob Keller @ 2016-10-19 22:13 UTC (permalink / raw)
  To: Dennis Kaarsemaker
  Cc: Jacob Keller, Git mailing list, Junio C Hamano, Stefan Beller,
	Jeff King, Johannes Sixt
In-Reply-To: <1476908699.26043.9.camel@kaarsemaker.net>

Hi,

On Wed, Oct 19, 2016 at 1:24 PM, Dennis Kaarsemaker
<dennis@kaarsemaker.net> wrote:
> On Wed, 2016-08-31 at 16:27 -0700, Jacob Keller wrote:
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Add an extension to git-diff and git-log (and any other graph-aware
>> displayable output) such that "--line-prefix=<string>" 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.

^ permalink raw reply

* Re: [PATCH] submodules: update documentaion for submodule branches
From: Junio C Hamano @ 2016-10-19 21:42 UTC (permalink / raw)
  To: Brandon Williams; +Cc: git, sbeller
In-Reply-To: <20161019204254.97659-1-bmwill@google.com>

Brandon Williams <bmwill@google.com> 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.<name>.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.


^ permalink raw reply

* Re: [PATCH 0/7] Rejecting useless merge bases
From: Junio C Hamano @ 2016-10-19 21:34 UTC (permalink / raw)
  To: git
In-Reply-To: <20161019042345.29766-1-gitster@pobox.com>

Junio C Hamano <gitster@pobox.com> writes:

> This is a continuation of
>
>     http://public-inbox.org/git/xmqqmvi2sj8f.fsf@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

^ permalink raw reply

* Re: [PATCH 1/2] ref-filter: split ref_kind_from_filter
From: Jeff King @ 2016-10-19 21:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Santiago Torres, git, sunshine, walters, Lukas Puehringer
In-Reply-To: <xmqqd1iwkp7m.fsf@gitster.mtv.corp.google.com>

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

> Jeff King <peff@peff.net> 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

^ permalink raw reply

* Re: [PATCH 1/2] ref-filter: split ref_kind_from_filter
From: Junio C Hamano @ 2016-10-19 21:23 UTC (permalink / raw)
  To: Jeff King; +Cc: Santiago Torres, git, sunshine, walters, Lukas Puehringer
In-Reply-To: <20161019203546.dfqmi2czcxopgj6w@sigill.intra.peff.net>

Jeff King <peff@peff.net> 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 <peff@peff.net>
> ---
>  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.

^ permalink raw reply

* Re: What's cooking in git.git (Oct 2016, #04; Mon, 17)
From: Jeff King @ 2016-10-19 21:21 UTC (permalink / raw)
  To: brian m. carlson, Junio C Hamano, Lars Schneider, git
In-Reply-To: <20161019202856.7ld3kpsnjwu6dbwl@vauxhall.crustytoothpaste.net>

On Wed, Oct 19, 2016 at 08:28:56PM +0000, 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

^ permalink raw reply

* Re: [PATCH] Add a knob to abort on die() (was Re: git checkout crashes after ...)
From: Jeff King @ 2016-10-19 21:15 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: git, Raffael Reichelt
In-Reply-To: <20161019134750.GA7256@ash>

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.GA4747@peff.net/T/#u

-Peff

^ permalink raw reply

* Re: [regression] `make profile-install` fails in 2.10.1
From: Jeff King @ 2016-10-19 21:05 UTC (permalink / raw)
  To: Jan Keromnes; +Cc: git
In-Reply-To: <CAA6PgK5vtnZSqqZafMVGoy0Rv38=8e__uQvXaf2SyPePHuPjJA@mail.gmail.com>

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-jeremyhu@apple.com/T/#u,
but it looks like the patch stalled.

-Peff

^ permalink raw reply

* [PATCH] rev-list: restore the NUL commit separator in --header mode
From: Dennis Kaarsemaker @ 2016-10-19 21:04 UTC (permalink / raw)
  To: git; +Cc: jacob.e.keller, stefanbeller, peff, j6t, jacob.keller
In-Reply-To: <1476908699.26043.9.camel@kaarsemaker.net>

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 <dennis@kaarsemaker.net>
---
 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(&buf);
 	} 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 <dennis@kaarsemaker.net>
http://twitter.com/seveas

^ permalink raw reply related

* Re: Drastic jump in the time required for the test suite
From: Jeff King @ 2016-10-19 20:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <xmqqbmygmehv.fsf@gitster.mtv.corp.google.com>

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

^ permalink raw reply

* Re: [PATCH] submodules: update documentaion for submodule branches
From: Stefan Beller @ 2016-10-19 20:45 UTC (permalink / raw)
  To: Brandon Williams; +Cc: git@vger.kernel.org
In-Reply-To: <20161019204254.97659-1-bmwill@google.com>

On Wed, Oct 19, 2016 at 1:42 PM, Brandon Williams <bmwill@google.com> 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 <bmwill@google.com>
> ---
>  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.<name>.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.<name>.update::
>
>  submodule.<name>.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.<name>.fetchRecurseSubmodules::
>         This option can be used to control recursive fetching of this
> --
> 2.10.1
>

^ permalink raw reply

* [PATCH] submodules: update documentaion for submodule branches
From: Brandon Williams @ 2016-10-19 20:42 UTC (permalink / raw)
  To: git; +Cc: sbeller, 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 <bmwill@google.com>
---
 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.<name>.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.<name>.update::
 
 submodule.<name>.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.<name>.fetchRecurseSubmodules::
 	This option can be used to control recursive fetching of this
-- 
2.10.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox