Git development
 help / color / mirror / Atom feed
* Re: Rebase & Trailing Whitespace
From: Jeff King @ 2011-09-01  2:31 UTC (permalink / raw)
  To: Hilco Wijbenga; +Cc: Git Users
In-Reply-To: <CAE1pOi0rY4kRR4rvEdFhzzTgfhUczHMX=H5_9+o5aHnv4vTadw@mail.gmail.com>

On Wed, Aug 31, 2011 at 04:55:03PM -0700, Hilco Wijbenga wrote:

> hilco@centaur ~/workspaces/project-next project-next (next $)$ git rebase master
> [...]
> <stdin>:721810: trailing whitespace.
> [...]
> Note the trailing whitespace warnings. How do I find out which file(s)
> generated these warnings? Would it be possible to add the file name
> causing the warnings to be output? By default? (Using --verbose
> doesn't seem to make any difference where the whitespace warnings are
> concerned.)

You can see any whitespace warnings in your repository (and in which
commit they were introduced) by doing something like:

  git log --oneline --check

The "--check" option is just another diff output format, so you use it
with "log" or "diff". For example, if you want to see just whitespace
problems that still exist in your current tree, you can diff against the
empty tree with --check, like:

  git diff --check 4b825dc642cb6eb9a060e54bf8d69288fbee4904

where "4b825dc..." is the well-known sha1 of an empty tree[1].

> Furthermore, why didn't I get these or similar warnings when I
> committed/pushed that particular commit ("Use static WAR for SWF files
> and assets.")? I did just find "[core] whitespace = trailing-space"
> which I will add to my .gitconfig, I suppose. So I guess what I really
> mean to ask is, why do rebase (and merge?) behave differently from
> commit?

Because these warnings are only triggered by default when applying
patches. And the idea of rebase is to apply patches from one place to
another. I thought we squelched whitespace warnings for rebase these
days (since they are somewhat pointless; you are moving around your own
commits, not applying commits from some other contributor), but maybe I
am misremembering. Or maybe you have an older version of git. Which
version are you using?

If you want to enforce whitespace checks during commit or push, you can
do so with a hook. The sample "pre-commit" hook, for example, uses "diff
--check" for just this purpose. See "git help hooks" for more
information.

-Peff

[1] If you don't remember the empty tree sha1, you can always derive it
    with:

        git hash-object -t tree /dev/null

^ permalink raw reply

* Re: [PATCH 1/2] remote: write correct fetch spec when renaming remote 'remote'
From: Jeff King @ 2011-09-01  2:42 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git, Junio C Hamano
In-Reply-To: <1314841843-19868-1-git-send-email-martin.von.zweigbergk@gmail.com>

On Wed, Aug 31, 2011 at 09:50:42PM -0400, Martin von Zweigbergk wrote:

> When renaming a remote whose name is contained in a configured fetch
> refspec for that remote, we currently replace the first occurrence of
> the remote name in the refspec. This is correct in most cases, but
> breaks if the remote name occurs in the fetch refspec before the
> expected place. For example, we currently change
> 
> [remote "remote"]
> 	url = git://git.kernel.org/pub/scm/git/git.git
> 	fetch = +refs/heads/*:refs/remotes/remote/*
> 
> into
> 
> [remote "origin"]
> 	url = git://git.kernel.org/pub/scm/git/git.git
> 	fetch = +refs/heads/*:refs/origins/remote/*

Oops.

> Reduce the risk of changing incorrect sections of the refspec by
> requiring the string to be matched to have leading and trailing
> slashes, i.e. match "/<name>/" instead of just "<name>".

Doesn't this just mean that:

  git remote rename remotes foo

will break in the same way?

> We could have required even a leading ":refs/remotes/", but that would
> mean that we would limit the types of refspecs we could help the user
> update.

Actually, I think it's better to be more conservative, and rename only:

  refs/remotes/$OLD/

into

  refs/remotes/$NEW/

If we are tweaking the refspecs, it's because we assume that the refspec
follows a certain naming convention (i.e., the one we set up with "git
remote add"). If somebody has tweaked this to be:

  refs/heads/$OLD/*

or even:

  refs/heads/foo/$OLD/bar

then we are just guessing about what they want. And I suspect such a
person would not use "git remote rename", anyway, but would instead edit
the config themselves. I'd rather make it safe for people using the
default config, and people who want to stray from that can deal with
updating the config themselves (since they would have to have done so to
get into that state in the first place).

Maybe we should even print a warning in that case.

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] remote: "rename o foo" should not rename ref "origin/bar"
From: Jeff King @ 2011-09-01  2:46 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: git, Junio C Hamano
In-Reply-To: <1314841843-19868-2-git-send-email-martin.von.zweigbergk@gmail.com>

On Wed, Aug 31, 2011 at 09:50:43PM -0400, Martin von Zweigbergk wrote:

> When renaming a remote called 'o' using 'git remote rename o foo', git
> should also rename any remote-tracking branches for the remote. This
> does happen, but any remote-tracking branches starting with
> 'refs/remotes/o', such as 'refs/remotes/origin/bar', will also be
> renamed (to 'refs/remotes/foorigin/bar' in this case).

To be totally correct, shouldn't this check each ref against the RHS of
the remote's old refspec, and rename it according to the remote's new
refspec?

Maybe that is just being pedantic, though. This should work fine with
the default config[1].

-Peff

[1] Since this part of the renaming process obviously depends heavily on
    refs/remotes/$OLD being the naming convention, shouldn't the
    renaming of the refspecs do the same thing? I.e., it's another
    reason that your patch 1/2 should only tweak refs/remotes/$OLD.
    Otherwise you will get renamed refspecs, but the actual refs won't
    be moved.

^ permalink raw reply

* Re: [PATCH 1/2] remote: write correct fetch spec when renaming remote 'remote'
From: Junio C Hamano @ 2011-09-01  3:18 UTC (permalink / raw)
  To: Jeff King; +Cc: Martin von Zweigbergk, git, Junio C Hamano
In-Reply-To: <20110901024211.GC31838@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Actually, I think it's better to be more conservative, and rename only:
>
>   refs/remotes/$OLD/
>
> into
>
>   refs/remotes/$NEW/

Thanks.

I wholeheartedly agree with this. It would let me sleep better if we did
this rewriting only when remote.$name.fetch is set exactly to its default
value, "+refs/heads/*:refs/origin/$name/*" (it is Ok if the leading '+' is
missing, though), and we didn't touch either config nor the existing
tracking refs at all otherwise, and gave warning saying we didn't do
anything to them.

^ permalink raw reply

* Re: Idea: "git format-patch" should get more information out of git
From: Michael Haggerty @ 2011-09-01  4:32 UTC (permalink / raw)
  To: Johan Herland
  Cc: Michael J Gruber, Jeff King, Junio C Hamano, git, Jonathan Nieder
In-Reply-To: <201108301939.53487.johan@herland.net>

On 08/30/2011 07:39 PM, Johan Herland wrote:
> On Tuesday 30. August 2011, Michael J Gruber wrote:
>> Reminds me of the ref namespace restructuring which could help
>> sharing notes... Oh, lots to do before git 3.0!
> 
> Indeed. I am very sorry to not have been able to properly follow up on 
> that proposal [...]

What "ref rename restructuring" are you talking about?  Can you give a
mailing list link?

(I've been working on refs lately and want to know whether your proposal
is relevant to my work.)

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: [spf:guess] Re: [PATCH] Teach dcommit --mergeinfo to handle multiple lines
From: Sam Vilain @ 2011-09-01  4:40 UTC (permalink / raw)
  To: Eric Wong; +Cc: Bryan Jacobs, git
In-Reply-To: <20110901013734.GA31207@dcvr.yhbt.net>

On 31/08/11 18:37, Eric Wong wrote:
> Sam Vilain <sam@vilain.net> wrote:
>> Ok, well I guess this is a useful intermediate feature then.  Feel
>> free to copy in any further changes you may come up with in this
>> area to me, if you decide to do that.
> Shall I consider this an Acked-by?

Acked-by: Sam Vilain <sam@vilain.net>

Cheers,
Sam

^ permalink raw reply

* Re: [PATCH v6] Add a remote helper to interact with mediawiki (fetch & push)
From: Matthieu Moy @ 2011-09-01  5:26 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Sverre Rabbelier, git, Jeremie Nikaes, Arnaud Lacurie,
	Claire Fousse, David Amouyal
In-Reply-To: <7vobz5xg7u.fsf@alter.siamese.dyndns.org>

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

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Here:
>>
>> +	for my $refspec (@refsspecs) {
>> +		unless ($refspec =~ m/^(\+?)([^:]*):([^:]*)$/) {
>> +			die("Invalid refspec for push. Expected <src>:<dst> or +<src>:<dst>");
>> +		}
>> +		my ($force, $local, $remote) = ($1 eq "+", $2, $3);
>>
>> At this point, $force is a boolean saying whether there were a +, and
>> $local and $remote are as you can guess.
>
> It may be slightly more Perl-ish to hoist the "0-or-1" outside the group
> and rely on $1 becoming undef, like this:
>
>         my ($force, $local, $remote) = $refspec =~ /^(\+)?([^:]*):([^:]*)$/
> 		or die(...);

Thanks, I didn't know I could do this.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: Idea: "git format-patch" should get more information out of git
From: Michael J Gruber @ 2011-09-01  6:44 UTC (permalink / raw)
  To: Michael Haggerty
  Cc: Johan Herland, Jeff King, Junio C Hamano, git, Jonathan Nieder
In-Reply-To: <4E5F0ACD.8050206@alum.mit.edu>

Michael Haggerty venit, vidit, dixit 01.09.2011 06:32:
> On 08/30/2011 07:39 PM, Johan Herland wrote:
>> On Tuesday 30. August 2011, Michael J Gruber wrote:
>>> Reminds me of the ref namespace restructuring which could help
>>> sharing notes... Oh, lots to do before git 3.0!
>>
>> Indeed. I am very sorry to not have been able to properly follow up on 
>> that proposal [...]
> 
> What "ref rename restructuring" are you talking about?  Can you give a
> mailing list link?
> 
> (I've been working on refs lately and want to know whether your proposal
> is relevant to my work.)

Searching for "namespace" in the subject should give you all relevant
proposals plus the new "namespaces" which might affect you also:

http://permalink.gmane.org/gmane.comp.version-control.git/140681
http://permalink.gmane.org/gmane.comp.version-control.git/165813
http://permalink.gmane.org/gmane.comp.version-control.git/165799

http://permalink.gmane.org/gmane.comp.version-control.git/176808

Michael

^ permalink raw reply

* Re: [PATCH] for-each-ref: add split message parts to %(contents:*).
From: Michał Górny @ 2011-09-01  7:34 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Michael J Gruber
In-Reply-To: <20110831232201.GA29296@sigill.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 805 bytes --]

On Wed, 31 Aug 2011 19:22:01 -0400
Jeff King <peff@peff.net> wrote:

> On Wed, Aug 31, 2011 at 03:54:35PM -0700, Junio C Hamano wrote:
> 
> > > +The complete message in a commit and tag object is `contents`.
> > > +Its first line is `contents:subject`, the remaining lines
> > > +are `contents:body` and the optional GPG signature
> > > +is `contents:signature`.
> > 
> > To match the parsing of commit objects, I would prefer to see
> > "subject" to mean "the first paragraph" (usually the first line
> > alone but that is purely from convention), but that probably is a
> > separate topic.
> 
> Good idea. I suspect pretty.c:format_subject can be reused here.

Should I fix regular 'subject' and 'body' as well, or just
the 'contents:' variants?

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

^ permalink raw reply

* Re: git-svn and mergeinfo
From: Michael Haggerty @ 2011-09-01  8:59 UTC (permalink / raw)
  To: Bryan Jacobs; +Cc: git
In-Reply-To: <20110829132052.0ad7a088@robyn.woti.com>

On 08/29/2011 07:20 PM, Bryan Jacobs wrote:
> I have been (ab)using git-svn for committing to a central SVN
> repository while doing my work locally with git. To this end, I've
> written a set of scripts and hooks which perform squash merges locally
> and then dcommit them with proper svn:mergeinfo annotations. The final
> result is the perfect appearance of having done a native SVN merge in
> the central repository, while using only local git commands and
> gaining the full benefit of git's conflict resolution and developer
> convenience.
> 
> However, to make this work with git 1.7.6, I needed to make *one* change
> to the git internals: --merge-info does not allow setting mergeinfo for
> more than one branch. Because it's a complete overwrite operation
> instead of an update, this is a serious issue preventing its use for
> nontrivial branches.
> 
> Might I suggest adding a block like the following around line 552 of
> git-svn?
> 
>     if (defined($_merge_info))
>     {  
>         $_merge_info =~ tr{ }{\n};
>     }

Naive question: why can't you pass a newline (properly quoted, of
course) directly within the string argument to the --mergeinfo option?

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: [PATCH] git-remote-helpers.txt: explain how import works with multiple refs
From: Sverre Rabbelier @ 2011-09-01 11:24 UTC (permalink / raw)
  To: Matthieu Moy, Jonathan Nieder; +Cc: git, gitster
In-Reply-To: <1314809222-30528-1-git-send-email-Matthieu.Moy@imag.fr>

Heya,

[+Jonathan Nieder]

On Wed, Aug 31, 2011 at 18:47, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> This is important for two reasons:
>
> * when two "import" lines follow each other, only one "done" command
>  should be issued in the fast-import stream, not one per "import".
>
> * The blank line terminating an import command should not be confused
>  with the one terminating the sequence of commands.
>
> While we're there, illustrate the corresponding explanation for push
> batches with an example.

Thank you!

Acked-by: Sverre Rabbelier <srabbelier@gmail.com

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH] (short) documentation for the testgit remote helper
From: Sverre Rabbelier @ 2011-09-01 11:27 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, gitster
In-Reply-To: <1314814498-13699-1-git-send-email-Matthieu.Moy@imag.fr>

Heya,

On Wed, Aug 31, 2011 at 20:14, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
> +# To understand better the way things work, one can set the variable
> +# "static int debug" in transport-helper.c to 1, and/or the "DEBUG"
> +# variable in git_remote_helpers/util.py to True, and try various
> +# commands.

Both are controlled by an environmental variable, it would be better
to mention these directly.

I think that for remote-testgit it is GIT_DEBUG_TESTGIT=1.

Other than that:

Acked-by: Sverre Rabbelier <srabbelier@gmail.com>

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [PATCH] test-lib: save test counts across invocations
From: Thomas Rast @ 2011-09-01 13:08 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano

Under 'prove' we can get progress status for tests running in
parallel.  However we never told it the total number of tests in a
file in advance, and thus the progress only showed the tests already
executed.

Save the number of tests run ($test_count) in a file under
test-counts/.  Then when sourcing test-lib.sh the next time, compare
the timestamps.  If the counts file is older than the test, discard.
Otherwise use the count that we saved and give prove the test plan
("1..N") up front.

This results in 'make prove' giving progress output like

  ===(    2884;121   8/12   24/147  1/8  1/3  )============================

if you have already run the tests before.

Prerequisite changes can mean that a whole test group is skipped
(e.g. NO_SVN_TESTS=1).  We thus need to be somewhat careful to only
emit the "full" plan once we know we're not going to $skip_all.

t9700 needs special treatment on top of $test_external_has_tap because
the latter can only be set once we know that the external test will
run.  If a prerequisite fails, we still need to emit the plan.

The Makefile changes are required because we want to keep that
test-count subdirectory unless the *user* invokes 'make clean', but we
previously ran the latter ourselves after every successful test run.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Sparked by a discussion on G+.  I think this is the "simple" approach.
The "cute" approach would be to let test-lib.sh define test_* as
test-counting dummies once, source the test script itself (avoiding
the sourcing loop with test-lib) to count what it does, then do the
real work.


 t/.gitignore        |    1 +
 t/Makefile          |    9 ++++++---
 t/t9700-perl-git.sh |    1 +
 t/test-lib.sh       |   27 +++++++++++++++++++++++++--
 4 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/t/.gitignore b/t/.gitignore
index 4e731dc..7de845f 100644
--- a/t/.gitignore
+++ b/t/.gitignore
@@ -1,3 +1,4 @@
 /trash directory*
 /test-results
+/test-counts
 /.prove
diff --git a/t/Makefile b/t/Makefile
index 9046ec9..c70de07 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -36,11 +36,14 @@ $(T):
 pre-clean:
 	$(RM) -r test-results
 
-clean:
+post-clean:
 	$(RM) -r 'trash directory'.* test-results
 	$(RM) -r valgrind/bin
 	$(RM) .prove
 
+clean: post-clean
+	$(RM) -r test-counts
+
 test-lint: test-lint-duplicates test-lint-executable
 
 test-lint-duplicates:
@@ -55,7 +58,7 @@ test-lint-executable:
 
 aggregate-results-and-cleanup: $(T)
 	$(MAKE) aggregate-results
-	$(MAKE) clean
+	$(MAKE) post-clean
 
 aggregate-results:
 	for f in test-results/t*-*.counts; do \
@@ -111,4 +114,4 @@ smoke_report: smoke
 		http://smoke.git.nix.is/app/projects/process_add_report/1 \
 	| grep -v ^Redirecting
 
-.PHONY: pre-clean $(T) aggregate-results clean valgrind smoke smoke_report
+.PHONY: pre-clean $(T) aggregate-results clean valgrind smoke smoke_report post-clean
diff --git a/t/t9700-perl-git.sh b/t/t9700-perl-git.sh
index 3787186..20ec097 100755
--- a/t/t9700-perl-git.sh
+++ b/t/t9700-perl-git.sh
@@ -4,6 +4,7 @@
 #
 
 test_description='perl interface (Git.pm)'
+test_disable_saved_count=1
 . ./test-lib.sh
 
 if ! test_have_prereq PERL; then
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bdd9513..374cdb2 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -522,11 +522,19 @@ test_skip () {
 	esac
 }
 
+test_emit_plan () {
+	if [ -z "$test_plan_emitted" -a -n "$test_count_saved" ]; then
+		say "1..$test_count_saved"
+		test_plan_emitted=y
+	fi
+}
+
 test_expect_failure () {
 	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
 	test "$#" = 2 ||
 	error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
 	export test_prereq
+	test_emit_plan
 	if ! test_skip "$@"
 	then
 		say >&3 "checking known breakage: $2"
@@ -545,6 +553,7 @@ test_expect_success () {
 	test "$#" = 2 ||
 	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
 	export test_prereq
+	test_emit_plan
 	if ! test_skip "$@"
 	then
 		say >&3 "expecting success: $2"
@@ -860,12 +869,14 @@ test_done () {
 	fi
 	case "$test_failure" in
 	0)
+		[ -z "$skip_all" ] && echo "$test_count" > "$test_count_file"
+
 		# Maybe print SKIP message
 		[ -z "$skip_all" ] || skip_all=" # SKIP $skip_all"
 
 		if test $test_external_has_tap -eq 0; then
 			say_color pass "# passed all $msg"
-			say "1..$test_count$skip_all"
+			[ -z "$test_plan_emitted" ] && say "1..$test_count$skip_all"
 		fi
 
 		test -d "$remove_trash" &&
@@ -877,7 +888,7 @@ test_done () {
 	*)
 		if test $test_external_has_tap -eq 0; then
 			say_color error "# failed $test_failure among $msg"
-			say "1..$test_count"
+			[ -z "$test_plan_emitted" ] && say "1..$test_count"
 		fi
 
 		exit 1 ;;
@@ -896,6 +907,18 @@ then
 fi
 GIT_BUILD_DIR="$TEST_DIRECTORY"/..
 
+mkdir -p "$TEST_DIRECTORY"/test-counts
+test_count_file="$TEST_DIRECTORY"/test-counts/$(basename "$0" .sh)
+test_count_saved=$(
+	if [ -n "$test_disable_saved_count" ]; then
+		:
+	# the saved count is only valid if the file is newer than the test
+	elif [ -f "$test_count_file" -a "$test_count_file" -nt "$0" ]; then
+		cat "$test_count_file" 2>/dev/null
+	fi
+	# otherwise we leave the variable empty
+)
+
 if test -n "$valgrind"
 then
 	make_symlink () {
-- 
1.7.7.rc0.420.g468b7

^ permalink raw reply related

* Re: git-svn and mergeinfo
From: Bryan Jacobs @ 2011-09-01 14:43 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git
In-Reply-To: <4E5F4987.5040205@alum.mit.edu>

On Thu, 01 Sep 2011 10:59:51 +0200
Michael Haggerty <mhagger@alum.mit.edu> wrote:

> On 08/29/2011 07:20 PM, Bryan Jacobs wrote:
> > I have been (ab)using git-svn for committing to a central SVN
> > repository while doing my work locally with git. To this end, I've
> > written a set of scripts and hooks which perform squash merges
> > locally and then dcommit them with proper svn:mergeinfo
> > annotations. The final result is the perfect appearance of having
> > done a native SVN merge in the central repository, while using only
> > local git commands and gaining the full benefit of git's conflict
> > resolution and developer convenience.
> > 
> > However, to make this work with git 1.7.6, I needed to make *one*
> > change to the git internals: --merge-info does not allow setting
> > mergeinfo for more than one branch. Because it's a complete
> > overwrite operation instead of an update, this is a serious issue
> > preventing its use for nontrivial branches.
> > 
> > Might I suggest adding a block like the following around line 552 of
> > git-svn?
> > 
> >     if (defined($_merge_info))
> >     {  
> >         $_merge_info =~ tr{ }{\n};
> >     }
> 
> Naive question: why can't you pass a newline (properly quoted, of
> course) directly within the string argument to the --mergeinfo option?

The only way I know of to do that in bash is to assign the
newline-bearing string to a variable, and then use the variable in a
command line option. Extremely awkward.

I think the long-term solution for this issue is probably to have
git-svn populate the mergeinfo on its own, reducing the need for
users manipulating the value directly. This could in theory be done for
both cherry picks and merges, provided that the merge was --no-ff or
bears a body (so there is a commit object to carry the property
change) and both parents are tagged with SVN revs at the time the merge
is dcommitted (or, correspondingly, that the cherry-pick source carries
an SVN revision number). I will send patches for some to all of this
shortly as I pull my bash scripts into git-svn.perl and clean up the
code.

The cost of the automatic svn:mergeinfo pushing will be an SVN property
retrieval before each dcommit operation. I plan to have this behavior
disabled by default.

Bryan Jacobs

^ permalink raw reply

* [PATCH] Fix git-completion.bash for use in zsh
From: Alex Merry @ 2011-09-01 13:47 UTC (permalink / raw)
  To: Shawn O. Pearce, git

Certain versions (or option combinations) of zsh appear to treat
things like
local some_var=()
as a function declaration.  This makes errors appear when using it in
combination with the GIT_PS1_SHOWUPSTREAM option.

Signed-off-by: Alex Merry <dev@randomguy3.me.uk>
---
 contrib/completion/git-completion.bash |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5a83090..89de45d 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -106,8 +106,9 @@ __gitdir ()
 __git_ps1_show_upstream ()
 {
        local key value
-       local svn_remote=() svn_url_pattern count n
+       local svn_remote svn_url_pattern count n
        local upstream=git legacy="" verbose=""
+       svn_remote=()

        # get some config options from git-config
        while read key value; do
-- 
1.7.6

^ permalink raw reply related

* git-checkout silently throws away the dirty status of index without a warning?
From: Tzu-Jung Lee @ 2011-09-01 15:47 UTC (permalink / raw)
  To: git

Hi guys,

Correct me if I'm wrong:

    git-checkout saves the changes to index and working-tree, and
tries to apply them to the destined commit.
    If the changes are applicable, then git-checkout the destined
commit and apply the changes.
    Otherwise, git-checkout fails with warnings and leaves the current
status untouched.

If the above correct. Please help me clarify if the following corner
case an intended or unexpected behavior.

Setup and git repo with two commits to illustrate the scenario:

    $ git init
    $ echo aaa >> aaa.txt
    $ echo bbb >> bbb.txt
    $ git add .
    $ git commit -a -m commit1

    $ echo bbb >> bbb.txt
    $ echo aaa >> aaa.txt
    $ git commit -a -m commit2

Forge a unclean index with changes that are subset of the destined
commit we are about to switching to.

    $ git checkout -b br1
    $ git reset HEAD^
    Unstaged changes after reset:
    M       aaa.txt
    M       bbb.txt

    $ git checkout HEAD aaa.txt
    $ git status --short
    M bbb.txt
    $ git add bbb.txt
    $ git status

    # On branch br1
    # Changes to be committed:
    #   (use "git reset HEAD <file>..." to unstage)
    #
    #       modified:   bbb.txt
    #

    $ git checkout master
    Switched to branch 'master'

git silently switch to master without warning against the index are
"RESTORE/RESET" to clean.

    $ git checkout br1
    $ git status
    # On branch br1
    nothing to commit (working directory clean)

Is this an intended behavior?
Though the status and changes can be safely restore from the database
with some manipulation.
But it may become difficult and tedious if the number of involved
files are large.


Regards,
Roy

^ permalink raw reply

* Cannot rewrite branch(es) with a dirty working directory
From: James Blackburn @ 2011-09-01 15:52 UTC (permalink / raw)
  To: git

Hi All,

I get a spurious:
"Cannot rewrite branch(es) with a dirty working directory."
trying to filter-branch in a clean git repo (having done a reset). The
error disappears when I do git status.

Log of the shell commands:
bash:jamesb:xl-cbga-20:33083> mkdir org.eclipse.cdt.core.linux.ia64
bash:jamesb:xl-cbga-20:33084> cp -r
../../../CDT_HEAD_GIT/org.eclipse.cdt/.git
org.eclipse.cdt.core.linux.ia64/
bash:jamesb:xl-cbga-20:33085> cd org.eclipse.cdt.core.linux.ia64/
mbash:jamesb:xl-cbga-20:33086> git reset --hard
Checking out files: 100% (11879/11879), done.
HEAD is now at a03d454 Build against a local mirror of the 3.7 p2 repo
bash:jamesb:xl-cbga-20:33087> git filter-branch --subdirectory-filter
core/org.eclipse.cdt.core.linux.ia64 -- master
Cannot rewrite branch(es) with a dirty working directory.
bash:jamesb:xl-cbga-20:33088> git status
# On branch master
nothing to commit (working directory clean)
bash:jamesb:xl-cbga-20:33089> git filter-branch --subdirectory-filter
core/org.eclipse.cdt.core.linux.ia64 -- master
Rewrite d7092b12c93925f6f7c4725a5abc72e55650621c (16/16)
Ref 'refs/heads/master' was rewritten
bash:jamesb:xl-cbga-20:33090> git --version
git version 1.7.3.2

Is there a particular reason why filter-branch thinks the tree is
dirty, and status magically fixes this?

Cheers,
James

^ permalink raw reply

* Re: [PATCH] (short) documentation for the testgit remote helper
From: Matthieu Moy @ 2011-09-01 15:52 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: git, gitster
In-Reply-To: <CAGdFq_grmJLoTt7JMUuoXrd02Gx8JdcEL-wa7YQ=-FkurRqfWA@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Heya,
>
> On Wed, Aug 31, 2011 at 20:14, Matthieu Moy <Matthieu.Moy@imag.fr> wrote:
>> +# To understand better the way things work, one can set the variable
>> +# "static int debug" in transport-helper.c to 1, and/or the "DEBUG"
>> +# variable in git_remote_helpers/util.py to True, and try various
>> +# commands.
>
> Both are controlled by an environmental variable, it would be better
> to mention these directly.
>
> I think that for remote-testgit it is GIT_DEBUG_TESTGIT=1.

Oops, I had missed it. Will resend.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: git-svn and mergeinfo
From: Junio C Hamano @ 2011-09-01 16:00 UTC (permalink / raw)
  To: Bryan Jacobs; +Cc: Michael Haggerty, git
In-Reply-To: <20110901104327.14d4dba6@robyn.woti.com>

Bryan Jacobs <bjacobs@woti.com> writes:

>> Naive question: why can't you pass a newline (properly quoted, of
>> course) directly within the string argument to the --mergeinfo option?
>
> The only way I know of to do that in bash is to assign the
> newline-bearing string to a variable, and then use the variable in a
> command line option. Extremely awkward.

Hmm, I think Michael meant by "properly quoted" something like this:

    $ git commit -s -m 'Fix blorb
    > 
    > As it stands, blorb feature is totally broken for such and
    > such reasons. Fix it by restructuring frotz and nitfol to
    > use the same xyzzy helper function.'

which is not all that awkward, even for a free-form text argument like
commit log. In this case, you are talking about svn merge-info that is a
lot more structured (it is much less likely to see a single-quote in there
than my commit log message example above, for example) so...

^ permalink raw reply

* Re: [PATCH] for-each-ref: add split message parts to %(contents:*).
From: Junio C Hamano @ 2011-09-01 16:00 UTC (permalink / raw)
  To: Michał Górny; +Cc: Jeff King, Junio C Hamano, git, Michael J Gruber
In-Reply-To: <20110901093450.57512480@pomiocik.lan>

Michał Górny <mgorny@gentoo.org> writes:

> On Wed, 31 Aug 2011 19:22:01 -0400
> Jeff King <peff@peff.net> wrote:
>
>> On Wed, Aug 31, 2011 at 03:54:35PM -0700, Junio C Hamano wrote:
>> 
>> > > +The complete message in a commit and tag object is `contents`.
>> > > +Its first line is `contents:subject`, the remaining lines
>> > > +are `contents:body` and the optional GPG signature
>> > > +is `contents:signature`.
>> > 
>> > To match the parsing of commit objects, I would prefer to see
>> > "subject" to mean "the first paragraph" (usually the first line
>> > alone but that is purely from convention), but that probably is a
>> > separate topic.
>> 
>> Good idea. I suspect pretty.c:format_subject can be reused here.
>
> Should I fix regular 'subject' and 'body' as well, or just
> the 'contents:' variants?

I thought you made them synonyms...

^ permalink raw reply

* Re: [PATCH] test-lib: save test counts across invocations
From: Junio C Hamano @ 2011-09-01 16:14 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Jeff King
In-Reply-To: <8fe5381a6b69079b8c20452fd4d99a128764dd52.1314882443.git.trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

> Save the number of tests run ($test_count) in a file under
> test-counts/.  Then when sourcing test-lib.sh the next time, compare
> the timestamps.

... which is this logic ...

> +test_count_file="$TEST_DIRECTORY"/test-counts/$(basename "$0" .sh)
> +test_count_saved=$(
> +	if [ -n "$test_disable_saved_count" ]; then
> +		:
> +	# the saved count is only valid if the file is newer than the test
> +	elif [ -f "$test_count_file" -a "$test_count_file" -nt "$0" ]; then
> +		cat "$test_count_file" 2>/dev/null
> +	fi
> +	# otherwise we leave the variable empty
> +)

I think the patch is cute, but I however do not think this is sufficient
to catch prerequisite changes, unfortunately. I'd rather leave the total
unknown than giving incorrect numbers.

^ permalink raw reply

* Re: [PATCH] for-each-ref: add split message parts to %(contents:*).
From: Jeff King @ 2011-09-01 16:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Michał Górny, git, Michael J Gruber
In-Reply-To: <7vbov4xnfc.fsf@alter.siamese.dyndns.org>

On Thu, Sep 01, 2011 at 09:00:39AM -0700, Junio C Hamano wrote:

> >> > To match the parsing of commit objects, I would prefer to see
> >> > "subject" to mean "the first paragraph" (usually the first line
> >> > alone but that is purely from convention), but that probably is a
> >> > separate topic.
> >> 
> >> Good idea. I suspect pretty.c:format_subject can be reused here.
> >
> > Should I fix regular 'subject' and 'body' as well, or just
> > the 'contents:' variants?
> 
> I thought you made them synonyms...

No, %(body) retains its historical usage as body+signature. If you think
it's OK to change that.

We could either leave %(subject) with its historical behavior, or fix it
to handle multi-line subjects. Although it's technically a regression to
change it, I tend to think it is simply a bug, as it doesn't match what
the rest of git (like "git log --format=%s") does.

-Peff

^ permalink raw reply

* Re: [PATCH] test-lib: save test counts across invocations
From: Jeff King @ 2011-09-01 16:38 UTC (permalink / raw)
  To: Thomas Rast; +Cc: git, Junio C Hamano
In-Reply-To: <8fe5381a6b69079b8c20452fd4d99a128764dd52.1314882443.git.trast@student.ethz.ch>

On Thu, Sep 01, 2011 at 03:08:45PM +0200, Thomas Rast wrote:

> Save the number of tests run ($test_count) in a file under
> test-counts/.  Then when sourcing test-lib.sh the next time, compare
> the timestamps.  If the counts file is older than the test, discard.
> Otherwise use the count that we saved and give prove the test plan
> ("1..N") up front.

Hmm. What happens when we're wrong? Does our eye-candy just print
something non-sensical like "13/12", or does prove actually care that we
run the right number of tests?

> Sparked by a discussion on G+.  I think this is the "simple" approach.
> The "cute" approach would be to let test-lib.sh define test_* as
> test-counting dummies once, source the test script itself (avoiding
> the sourcing loop with test-lib) to count what it does, then do the
> real work.

I don't think the "cute" approach will ever be accurate. Deciding
whether to run later tests sometimes depends on the results of earlier
tests, in at least two cases:

  1. Some tests find out which capabilities the system has, and set
     prerequisites. You need to actually run those tests, not make them
     counting dummies.

  2. Some tests create state that we then iterate on. For example, I
     think the mailinfo tests do something like:

        test_expect_success 'split' '
                git mailsplit -o patches mbox
        '
        for i in patches/*; do
          test_expect_success "check patch $i" '
                  git mailinfo $i >output
                  ...
          '
        done

      You'd get an inaccurate count if you didn't actually run the
      mailsplit command.

Anyway, this whole thing is a cute idea, and I do love eye candy, but I
wonder if it's worth the complexity. All this is telling us is how far
into each of the scripts it is. But we have literally hundreds of test
scripts, all with varying numbers of tests of varying speeds, and you're
probably running 16 or more at one time. So it doesn't tell you what you
really want to know, which is: how soon will the test suite probably be
done running.

-Peff

^ permalink raw reply

* Re: [PATCH] for-each-ref: add split message parts to %(contents:*).
From: Michał Górny @ 2011-09-01 16:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, Michael J Gruber
In-Reply-To: <20110901162222.GC15018@sigill.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

On Thu, 1 Sep 2011 12:22:22 -0400
Jeff King <peff@peff.net> wrote:

> On Thu, Sep 01, 2011 at 09:00:39AM -0700, Junio C Hamano wrote:
> 
> > >> > To match the parsing of commit objects, I would prefer to see
> > >> > "subject" to mean "the first paragraph" (usually the first line
> > >> > alone but that is purely from convention), but that probably
> > >> > is a separate topic.
> > >> 
> > >> Good idea. I suspect pretty.c:format_subject can be reused here.
> > >
> > > Should I fix regular 'subject' and 'body' as well, or just
> > > the 'contents:' variants?
> > 
> > I thought you made them synonyms...
> 
> No, %(body) retains its historical usage as body+signature. If you
> think it's OK to change that.
> 
> We could either leave %(subject) with its historical behavior, or fix
> it to handle multi-line subjects. Although it's technically a
> regression to change it, I tend to think it is simply a bug, as it
> doesn't match what the rest of git (like "git log --format=%s") does.

Ok, I'll go with fixing it. If we want to have old behavior back, it's
as simple as putting the line copying function in the right place.

Sadly, I had to add a few magical '-1's and '+1's to get whitespace
in-place. It seems that signed, unannotated signatures glue to subject.

-- 
Best regards,
Michał Górny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 316 bytes --]

^ permalink raw reply

* [PATCH] for-each-ref: add split message parts to %(contents:*).
From: Michał Górny @ 2011-09-01 16:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Michael J Gruber, Jeff King,
	Michał Górny
In-Reply-To: <20110901184815.2cd8b472@pomiocik.lan>

Now %(contents:subject) contains the message subject, %(contents:body)
main body part and %(contents:signature) GPG signature.
---
 Documentation/git-for-each-ref.txt |    7 +++--
 builtin/for-each-ref.c             |   42 ++++++++++++++++++++++++------------
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 152e695..c872b88 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -101,9 +101,10 @@ Fields that have name-email-date tuple as its value (`author`,
 `committer`, and `tagger`) can be suffixed with `name`, `email`,
 and `date` to extract the named component.
 
-The first line of the message in a commit and tag object is
-`subject`, the remaining lines are `body`.  The whole message
-is `contents`.
+The complete message in a commit and tag object is `contents`.
+Its first line is `contents:subject`, the remaining lines
+are `contents:body` and the optional GPG signature
+is `contents:signature`.
 
 For sorting purposes, fields with numeric values sort in numeric
 order (`objectsize`, `authordate`, `committerdate`, `taggerdate`).
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 89e75c6..e320ba2 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -69,6 +69,9 @@ static struct {
 	{ "subject" },
 	{ "body" },
 	{ "contents" },
+	{ "contents:subject" },
+	{ "contents:body" },
+	{ "contents:signature" },
 	{ "upstream" },
 	{ "symref" },
 	{ "flag" },
@@ -458,8 +461,9 @@ static void grab_person(const char *who, struct atom_value *val, int deref, stru
 	}
 }
 
-static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body)
+static void find_subpos(const char *buf, unsigned long sz, const char **sub, const char **body, const char **signature)
 {
+	*signature = buf + parse_signature(buf, sz);
 	while (*buf) {
 		const char *eol = strchr(buf, '\n');
 		if (!eol)
@@ -475,21 +479,21 @@ static void find_subpos(const char *buf, unsigned long sz, const char **sub, con
 	if (!*buf)
 		return;
 	*sub = buf; /* first non-empty line */
-	buf = strchr(buf, '\n');
-	if (!buf) {
-		*body = "";
-		return; /* no body */
-	}
-	while (*buf == '\n')
-		buf++; /* skip blank between subject and body */
-	*body = buf;
+	buf = format_subject(NULL, buf, NULL);
+
+	/* When having a signed tag without body, format_subject()
+	 * will start to eat the signature. */
+	if (buf > *signature)
+		*body = *signature;
+	else /* - 1 to get a trailing newline to strip */
+		*body = buf - 1;
 }
 
 /* See grab_values */
 static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
 {
 	int i;
-	const char *subpos = NULL, *bodypos = NULL;
+	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		const char *name = used_atom[i];
@@ -500,19 +504,29 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
 			name++;
 		if (strcmp(name, "subject") &&
 		    strcmp(name, "body") &&
-		    strcmp(name, "contents"))
+		    strcmp(name, "contents") &&
+		    strcmp(name, "contents:subject") &&
+		    strcmp(name, "contents:body") &&
+		    strcmp(name, "contents:signature"))
 			continue;
 		if (!subpos)
-			find_subpos(buf, sz, &subpos, &bodypos);
+			find_subpos(buf, sz, &subpos, &bodypos, &sigpos);
 		if (!subpos)
 			return;
 
-		if (!strcmp(name, "subject"))
-			v->s = copy_line(subpos);
+		if (!strcmp(name, "subject") || !strcmp(name, "contents:subject"))
+			v->s = xstrndup(subpos, bodypos - subpos - 1);
 		else if (!strcmp(name, "body"))
 			v->s = xstrdup(bodypos);
 		else if (!strcmp(name, "contents"))
 			v->s = xstrdup(subpos);
+		else if (!strcmp(name, "contents:body")) {
+			if (sigpos - bodypos > 0)
+				v->s = xstrndup(bodypos + 1, sigpos - bodypos - 1);
+			else
+				v->s = xstrdup("");
+		} else if (!strcmp(name, "contents:signature"))
+			v->s = xstrdup(sigpos);
 	}
 }
 
-- 
1.7.6.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