Git development
 help / color / mirror / Atom feed
* Re: [PATCH v3 2/6] t5541-http-push.sh: add test for unmatched, non-fast-forwarded refs
From: Jeff King @ 2010-01-06 12:05 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: git, Shawn O. Pearce, Daniel Barkalow, Junio C Hamano
In-Reply-To: <20100105180132.e573fff2.rctay89@gmail.com>

On Tue, Jan 05, 2010 at 06:01:32PM +0800, Tay Ray Chuan wrote:

> > I don't understand what you're testing here. If it's not matched, then
> > how is it a non-fast-forward? Isn't it simply unmatched?
> 
> Let me rephrase this as:
> 
>   Some refs can only be matched to a remote ref with an explicit
>   refspec. When such a ref is a non-fast-forward of its remote ref,
>   test that pushing them (with the explicit refspec specified) fails
>   with a non-fast-foward-type error.

Thanks, that makes sense to me now.

-Peff

^ permalink raw reply

* Re: [PATCH] bash completion: add space between branch name and status flags
From: Roman Fietze @ 2010-01-06 11:59 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Nanako Shiraishi, Junio C Hamano, git
In-Reply-To: <20091230145751.GE6914@spearce.org>

Hello Shawn,

On Wednesday 30 December 2009 15:57:51 Shawn O. Pearce wrote:

> ... but Roman Fietze didn't send
> a revised patch, so it got dropped

Sorry about that, but our Notes "spam filter" has been eating
important mail again.


I'l check the version as proposed by you as soon as I'm back.


Roman

-- 
Roman Fietze                Telemotive AG Büro Mühlhausen
Breitwiesen                              73347 Mühlhausen
Tel.: +49(0)7335/18493-45        http://www.telemotive.de

^ permalink raw reply

* Re: [PATCH v3 5/6] transport-helper.c::push_refs(): ignore helper-reported status if ref is not to be pushed
From: Jeff King @ 2010-01-06 12:04 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: git, Shawn O. Pearce, Daniel Barkalow, Junio C Hamano
In-Reply-To: <20100105180113.6e0572dc.rctay89@gmail.com>

On Tue, Jan 05, 2010 at 06:01:13PM +0800, Tay Ray Chuan wrote:

> > It seems like this should be checking for REF_STATUS_NONE explicitly
> > instead of trying to enumerate the reasons we might not have tried to
> > push. Shouldn't helpers _only_ be pushing REF_STATUS_NONE refs?
> >
> > I think right now the two cases are equivalent, since non-ff and
> > uptodate are the only two states set before the helper is invoked. But
> > we have discussed in the past (and I still have a patch floating around
> > for) a REF_STATUS_REWIND which would treat strict rewinds differently
> > (silently ignoring them instead of making an error). Explicitly checking
> > REF_STATUS_NONE future-proofs against new states being added.
> 
> I'm not really sure if this is true (ie. that if status is not non-ff
> or uptodate, then it is REF_STATUS_NONE), but we could step around this

Well, consider it this way. If it's _not_ REF_STATUS_NONE, then what is
it, and what does it mean to be overwriting it?

Maybe I am misunderstanding the problem the patch is addressing, but the
point of these REF_STATUS feels was to act as a small state machine.
Everything starts as NONE, and then:

  - we compare locally against remote refs. We may transition:
      NONE -> UPTODATE
      NONE -> REJECT_NONFASTFORWARD
      NONE -> REJECT_NODELETE

  - we send the push list
      NONE -> EXPECTING_REPORT (if the remote supports individual status)
      NONE -> OK (otherwise)

  - we get back status responses
      EXPECTING_REPORT -> OK
      EXPECTING_REPORT -> REMOTE_REJECT

I haven't looked closely at the new transport helper code, but I would
think it should stick more or less to those transitions. The exception
would be that some transports don't necessarily handle EXPECTING_REPORT
in the same way, and may transition directly from NONE to
OK/REMOTE_REJECT.

So offhand, I would say that your list should also probably include
REJECT_NODELETE. However, I think that status is just for old servers
which didn't support the delete-refs protocol extension. So presumably
that is none of the new helpers, as they all post-date the addition of
that feature by quite a few years.

> by introducing a property, say, ref->should_push, that is set to 1,
> after all the vetting has been carried out and just before we talk to
> the server.

I'd rather not introduce new state. The point of the status flag was to
encapsulate all of that information, and a new state variable just seems
like introducing extra complexity. If we are not in the NONE state, I
don't see why we would tell the helper about a ref at all.

-Peff

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2010, #01; Mon, 04)
From: Johannes Schindelin @ 2010-01-06 11:29 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, git
In-Reply-To: <20100106191825.6117@nanako3.lavabit.com>

Hi,

On Wed, 6 Jan 2010, Nanako Shiraishi wrote:

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 23ded48..d42cc4f 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -482,6 +482,25 @@ get_saved_options () {
>  	test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
>  }
>  
> +LF='
> +'
> +parse_onto () {
> +	if	expr "$1" : '.*\.\.\.' >/dev/null &&
> +		left=${1%...*} right=${1#*...} &&
> +		: ${left:=HEAD} ${right:=HEAD} &&
> +		onto=$(git merge-base "$left" "$right")
> +	then
> +		case "$onto" in
> +		?*"$LF"?* | '')
> +			exit 1 ;;
> +		esac
> +		echo "$onto"
> +		exit 0
> +	else
> +		git rev-parse --verify "$1^0"
> +	fi
> +}

It might be easier to understand like this:

	case "$1" in
	*...*)
		left=${1%...*} &&
		right=${1#*...} &&
		onto="$(git merge-base "${left:-HEAD}" "${right:-HEAD}")" &&
		test ! -z "$onto" &&
		echo "$onto"
	;;
	*)
		git rev-parse --verify "$1^0"
	;;
	esac

Besides, why do you change the "$1" to "$1^0"?
		
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 6503113..43c62c0 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh

I would separate the patches.  rebase.sh and rebase--interactive.sh are 
fundamentally different.

Thanks,
Dscho

^ permalink raw reply

* Re: git-cherry-pick problem - directory not touched by commit is  marked "added by us"
From: Hakim Cassimally @ 2010-01-06 11:28 UTC (permalink / raw)
  To: git, Sam Vilain
In-Reply-To: <1262727434.22597.8.camel@denix>

2010/1/5 Sam Vilain <sam@vilain.net>:
> On Tue, 2010-01-05 at 12:33 +0000, Hakim Cassimally wrote:
>> I got into a bit of trouble with a git-cherry-pick last night, and
>> though mugwump
>> and others on #git helped me as far as a workaround,
> Yes, that was me.  I'm very confused by the bug, too.
>
>  [...]
>> WHAT HAPPENS
>> ============
>>
>> When I'm in (stable), and try to cherry-pick the change from (experimental),
>> git thinks that I'm making a massive number of changes in a directory that
>> wasn't touched by the relevant commit.
>  [...]
>>     (stable) $ git --version
>>     git version 1.6.6
>>     # I tried previously on 1.6.0.4 but upgraded in case it helped
>>
>>     (stable) $ git status
>>     # On branch stable
>>     # nothing to commit (working directory clean)
>>
>>     (stable) $ git show --stat 301afce1
>>     commit 301afce1c78380276d208077ef4ec76b469c1024
>>     Author: osfameron <...>
>>     Date:   Wed Dec 23 23:45:20 2009 +0000
>>
>>         Proof of concept for import module (parse Excel)
>>
>>      bin/upload_module.pl |  142
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>      1 files changed, 142 insertions(+), 0 deletions(-)
>>
>>     (stable) $ git whatchanged -1 301afce1
>>     commit 301afce1c78380276d208077ef4ec76b469c1024
>>     Author: osfameron <...>
>>     Date:   Wed Dec 23 23:45:20 2009 +0000
>>
>>         Proof of concept for import module (parse Excel)
>>
>>     :000000 100644 0000000... c90e261... A  bin/upload_module.pl
>
> So by here, we know that the commit only affects that single file.  No
> funny stuff like mode changes of other files.
>
>>     (stable) $ git cherry-pick 301afce1
>>     Finished one cherry-pick.
>>     www/client/css/admin.css: needs merge
>>         <...snip>
>>     www/client/css/admin.css: unmerged
>> (8e7cd850bf40d1a921b1f62ce0945abd374fa55d)
>>         <...snip>
>>     ...
>>     error: Error building trees
>
> Then, wham, these files want to be changed.  What is the diff-tree
> inside revert/cherry-pick doing differently to the one in log?

> Hakim, one more useful thing in this situation would be to show the
> state of these files in the index;
>
>  git ls-files -s -u

 $ git ls-files -s -u www/client/css/admin.css # only staged/unmerged
  100755 8e7cd850bf40d1a921b1f62ce0945abd374fa55d 2
www/client/css/admin.css

(i.e. when run after the failed cherry-pick)

> Also take the 'git ls-tree -r HEAD', 'git ls-tree -r 301afce1' and 'git
> ls-tree -r 301afce1^' output, and grep for the files in question.  The
> answer (or at least a bug triage) should be in the output of those
> commands.

  $ git ls-tree HEAD | grep www/client/css/admin.css
  100755 blob 8e7cd850bf40d1a921b1f62ce0945abd374fa55d
www/client/css/admin.css

There's no entry in git ls-tree 301afce1 or 301afce1^1 though.  I'd
guess that's significant, but I don't know what answer it suggests...

(However the file exists in both (stable) and (experimental)...
and is identical in both).

> You can reproduce exactly the merge by making a new branch at the
> position where you were attempting to land the cherry pick before, and
> checking that branch out.

I actually did

  $ cd ..
  $ git clone repo/ repo_backup_4Jan

beforehand, so this should also be an exact reproduction, I think?

> One last test, would be to check that it happens on a clean clone of the
> repository.  I don't think that you're hitting any repository-local
> behaviour (eg, the ability to mark certain files as "always ignore") but
> you never know.  These commands are all being run on the same working
> copy, right?

And see above, should be an entirely clean clone.

Thanks for the suggestions!

Hakim / osfameron

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2010, #01; Mon, 04)
From: Nanako Shiraishi @ 2010-01-06 10:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vskal5c11.fsf@alter.siamese.dyndns.org>

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

> These need a bit more work to go forward.  Help and follow-up are
> appreciated.
>
>  * jc/checkout-merge-base (2009-11-20) 2 commits
>    Users of "rebase -i" might want to teach this to the command.
>    Volunteers?

Let me try. I'll let others to contribute documentation updates.

-- >8 --
Subject: [PATCH] Fix rebase --onto A...B and teach the same to interactive rebase

Signed-off-by: しらいし ななこ <nanako3@lavabit.com>
---
 git-rebase--interactive.sh       |   21 +++++++++++-
 git-rebase.sh                    |    4 +-
 t/t3415-rebase-onto-threedots.sh |   68 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 3 deletions(-)
 create mode 100755 t/t3415-rebase-onto-threedots.sh

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 23ded48..d42cc4f 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -482,6 +482,25 @@ get_saved_options () {
 	test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
 }
 
+LF='
+'
+parse_onto () {
+	if	expr "$1" : '.*\.\.\.' >/dev/null &&
+		left=${1%...*} right=${1#*...} &&
+		: ${left:=HEAD} ${right:=HEAD} &&
+		onto=$(git merge-base "$left" "$right")
+	then
+		case "$onto" in
+		?*"$LF"?* | '')
+			exit 1 ;;
+		esac
+		echo "$onto"
+		exit 0
+	else
+		git rev-parse --verify "$1^0"
+	fi
+}
+
 while test $# != 0
 do
 	case "$1" in
@@ -589,7 +608,7 @@ first and then run 'git rebase --continue' again."
 		;;
 	--onto)
 		shift
-		ONTO=$(git rev-parse --verify "$1") ||
+		ONTO=$(parse_onto "$1") ||
 			die "Does not point to a valid commit: $1"
 		;;
 	--)
diff --git a/git-rebase.sh b/git-rebase.sh
index 6503113..43c62c0 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -419,8 +419,8 @@ fi
 
 # Make sure the branch to rebase onto is valid.
 onto_name=${newbase-"$upstream_name"}
-if	left=$(expr "$onto_name" : '\(.*\)\.\.\.') &&
-	right=$(expr "$onto_name" : '\.\.\.\(.*\)$') &&
+if	expr "$onto_name" : '.*\.\.\.' >/dev/null &&
+	left=${onto_name%...*} right=${onto_name#*...} &&
 	: ${left:=HEAD} ${right:=HEAD} &&
 	onto=$(git merge-base "$left" "$right")
 then
diff --git a/t/t3415-rebase-onto-threedots.sh b/t/t3415-rebase-onto-threedots.sh
new file mode 100755
index 0000000..c243243
--- /dev/null
+++ b/t/t3415-rebase-onto-threedots.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+
+test_description='git rebase --onto A...B'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY/lib-rebase.sh"
+
+#           F---G                     G'
+#          /          -->            /
+# A---B---C---D---E         A---B---C---D---E
+
+test_expect_success setup '
+	test_commit A &&
+	test_commit B &&
+	test_commit C &&
+	git branch topic &&
+	test_commit D &&
+	test_commit E &&
+	git checkout topic &&
+	test_commit F &&
+	test_commit G
+'
+
+test_expect_success 'rebase --onto A...B' '
+	git reset --hard &&
+	git checkout topic &&
+	git reset --hard G &&
+
+	git rebase --onto master...topic HEAD^ &&
+	git rev-parse HEAD^ >actual &&
+	git rev-parse C^0 >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rebase --onto A...' '
+	git reset --hard &&
+	git checkout topic &&
+	git reset --hard G &&
+
+	git rebase --onto master... HEAD^ &&
+	git rev-parse HEAD^ >actual &&
+	git rev-parse C^0 >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rebase -i --onto A...B' '
+	git reset --hard &&
+	git checkout topic &&
+	git reset --hard G &&
+	set_fake_editor &&
+	EXPECT_COUNT=1 git rebase -i --onto master...topic HEAD^ &&
+	git rev-parse HEAD^ >actual &&
+	git rev-parse C^0 >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rebase -i --onto A...' '
+	git reset --hard &&
+	git checkout topic &&
+	git reset --hard G &&
+	set_fake_editor &&
+	EXPECT_COUNT=1 git rebase -i --onto master... HEAD^ &&
+	git rev-parse HEAD^ >actual &&
+	git rev-parse C^0 >expect &&
+	test_cmp expect actual
+'
+
+test_done
-- 
1.6.6

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply related

* Re: [PATCH/RFC 0/2] Lazily generate header dependencies
From: Jonathan Nieder @ 2010-01-06  9:36 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Nanako Shiraishi, Johannes Sixt, Git Mailing List
In-Reply-To: <alpine.DEB.1.00.1001061024400.11013@intel-tinevez-2-302>

Hi,

Johannes Schindelin wrote:
> On Tue, 5 Jan 2010, Junio C Hamano wrote:

>> I was mildly interested in the series, but after this:
>> 
>> http://thread.gmane.org/gmane.comp.version-control.git/133872/focus=133911
>> 
>> the progress seems to have stopped.
>
> Like I said yesterday, I do not want to spend time chasing old threads.  
> But if you spend just two more minutes to summarize what came out of that 
> thread (two well spent minutes in the global time balance, if you ask me), 
> I will gladly comment.

I received some feedback (a suggestion to tuck the makefile snippets in
a separate directory and a follow-up suggestion to make the list of
directories needed for this more maintainable) but have not sent an updated
patch yet.  Hopefully tomorrow I will have more to comment on.

Apologies again for the long silence,
Jonathan

^ permalink raw reply

* Re: [PATCH/RFC 0/2] Lazily generate header dependencies
From: Johannes Schindelin @ 2010-01-06  9:26 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nanako Shiraishi, Jonathan Nieder, Johannes Sixt,
	Git Mailing List
In-Reply-To: <7vwrzwnirz.fsf@alter.siamese.dyndns.org>

Hi,

On Tue, 5 Jan 2010, Junio C Hamano wrote:

> Nanako Shiraishi <nanako3@lavabit.com> writes:
> 
> > Junio, could you tell us what happened to this thread?
> >
> > Makefile improvements.  No discussion.
> 
> I was mildly interested in the series, but after this:
> 
> http://thread.gmane.org/gmane.comp.version-control.git/133872/focus=133911
> 
> the progress seems to have stopped.

Like I said yesterday, I do not want to spend time chasing old threads.  
But if you spend just two more minutes to summarize what came out of that 
thread (two well spent minutes in the global time balance, if you ask me), 
I will gladly comment.

Ciao,
Dscho

^ permalink raw reply

* Re: cmake, was Re: submodules' shortcomings
From: Johannes Schindelin @ 2010-01-06  9:24 UTC (permalink / raw)
  To: Pau Garcia i Quiles; +Cc: Git Mailing List
In-Reply-To: <3af572ac1001051717u7757f0dep9392fbb7b02cbbca@mail.gmail.com>

Hi,

On Wed, 6 Jan 2010, Pau Garcia i Quiles wrote:

> On Wed, Jan 6, 2010 at 12:06 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Tue, 5 Jan 2010, Pau Garcia i Quiles wrote:
> >
> >> For instance, I'd like to have a 'cmake' repository where I store all 
> >> the FindBlah.cmake modules, so that I can share them from every 
> >> repository, and not worry about users changing and committing in the 
> >> main project instead of the submodule.
> >
> > ... which reminds me... it was you who wanted to provide a working 
> > recipe to compile and install CMake on msysGit, right?
> 
> Right
> 
> > What happened in the meantime?
> 
> What happened is I was very busy until November. Now I've got some free 
> time.
> 
> At this moment, what stops me from beginning this project is a simple 
> question: is it worth my time?

Well, I thought you wanted to show that CMake is superior to what we have 
right now, and for me as msysGit maintainer, that implies that CMake 
actually works within msysGit.

Now, I do not think that it is hard to get CMake to compile in msysGit, 
but then, I just lost access to the last Windows computer, so I cannot do 
that myself.

As Miles said, it is up to you to decide whether it is so complicated, or 
whether CMake is likely not to convince, that the time balance turns out 
positive or negative.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH v2 0/5] Makefile: fix generation of assembler listings
From: Linus Torvalds @ 2010-01-06  9:07 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git, Shawn O. Pearce
In-Reply-To: <20100106080216.GA7298@progeny.tock>



On Wed, 6 Jan 2010, Jonathan Nieder wrote:
> 
> Patch 1 fixes a problem I noticed when tweaking the Makefile to
> automatically generate dependencies for the %.o targets.  The problem
> is that the dependencies for the corresponding %.s (code listing)
> targets are not included in the Makefile at all, automatically or not.

Patches 1-3 (which were the ones I was cc'd on) look sane to me. Having 
real dependencies might be prettier, but I agree that since a *.s file is 
only generated on demand (and useful mainly to see code generation), just 
forcing the build makes sense.

		Linus

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2010, #01; Mon, 04)
From: Johannes Sixt @ 2010-01-06  9:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzl4r7jyu.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> Regarding your "[PATCH 8/6] t4030, t4031", I have two questions:
> 
>     Recall that MSYS bash converts POSIX style absolute paths to Windows style
>     absolute paths. Unfortunately, it converts a program argument that begins
>     with a double-quote and otherwise looks like an absolute POSIX path, but
>     in doing so, it strips everything past the second double-quote[*]. This
>     case is triggered in the two test scripts. The work-around is to place the
>     Windows style path between the quotes to avoid the path conversion.
> 
> (1) Does "Windows style path" here mean what $(pwd) returns as opposed to
>     what is in $PWD?

Yes. $PWD is of the form /c/foo/bar; pwd is a function in test-lib.sh that 
ensures it returns the form c:/foo/bar.

> (2) The patch reads like this:
> 
> -	git config diff.foo.textconv "\"$PWD\""/hexdump &&
> +	git config diff.foo.textconv "\"$(pwd)\""/hexdump &&
> 
>     Does "strips everything past the second dq" mean "drops '/hexdump'"?

Yes.

>     If so, would this also work (I am not suggesting to change it, just
>     asking for information)?
> 
> -	git config diff.foo.textconv "\"$PWD\""/hexdump &&
> +	git config diff.foo.textconv "\"$PWD/hexdump\"" &&

It would work, too, but it would depend on very bogus behavior of the MSYS 
bash.

-- Hannes

^ permalink raw reply

* Re: [PATCH 9/9] rerere forget path: forget recorded resolution
From: Johannes Sixt @ 2010-01-06  8:55 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4on0oxcs.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> Johannes Sixt <j6t@kdbg.org> writes:
>> When you simply call ll_merge(), will it obey any merge drivers that
>> are defined in .gitattributes? Do we care about them?
>>
>> I already had an implementation of "rerere forget" before you
>> presented this solution, but it relies on that the user calls
>> "checkout --conflict=merge" first. One reason (besides its simplicity)
>> was that it does not have to care how the merge is computed.
> 
> Doesn't "checkout --conflict=merge" use the same ll_merge() machinery?

It does, without setting up .gitattributes, either (IIUC), which is a bug IMO.

That said, I consider your solution superior because it works without 
depending on conflict markers in the file in the worktree. Nevertheless, 
we should think about whether merge drivers of .gitattributes should be 
obeyed. I think they should: For example, a specialized XML merge driver 
could leave conflicts that are different from those that merge-recursive 
generates.

-- Hannes

^ permalink raw reply

* Re: checkout -m dumping core
From: Daniel @ 2010-01-06  8:11 UTC (permalink / raw)
  To: Git List, Tomas Carnecky
In-Reply-To: <4B4381CA.1080504@dbservice.com>

Tomas Carnecky <tom@dbservice.com> wrote:

> git version 1.6.6.78.gbd757c
> 
> HEAD points to a non-existent branch refs/heads/master. Normal checkout 
> origin fails with:
> error: Entry '.cvsignore' would be overwritten by merge. Cannot merge.
> (the working tree does indeed contain this file). So I tried checkout -m 
> and git crashed. Workaround for me was reset --hard origin; checkout 
> origin. I have the repository backed up in case someone wants to try 
> themselves.
> 
> $ gdb `which git`
> GNU gdb 6.8
> Copyright (C) 2008 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later 
> <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "i386-pc-solaris2.11"...
> (gdb) run checkout -m origin
> Starting program: /export/home/tomc/local/git/bin/git checkout -m origin
> warning: Lowest section in /lib/libpthread.so.1 is .dynamic at 00000074
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x080788fa in cmd_checkout (argc=0, argv=0x8047538, prefix=0x0) at 
> builtin-checkout.c:450
> 450                             merge_trees(&o, new->commit->tree, work,
> (gdb) list
> 445                             ret = reset_tree(new->commit->tree, 
> opts, 1);
> 446                             if (ret)
> 447                                     return ret;
> 448                             o.branch1 = new->name;
> 449                             o.branch2 = "local";
> 450                             merge_trees(&o, new->commit->tree, work,
> 451                                     old->commit->tree, &result);
> 452                             ret = reset_tree(new->commit->tree, 
> opts, 0);
> 453                             if (ret)
> 454                                     return ret;
> (gdb) p o
> $1 = {branch1 = 0x8047650 "origin", branch2 = 0x0, subtree_merge = 0, 
> buffer_output = 1, verbosity = 0, diff_rename_limit = -1, 
> merge_rename_limit = -1, call_depth = 0, obuf = {alloc = 0, len = 0, buf 
> = 0x81643ac ""}, current_file_set = {
>      items = 0x0, nr = 0, alloc = 0, strdup_strings = 1}, 
> current_directory_set = {items = 0x0, nr = 0, alloc = 0, strdup_strings 
> = 1}}
> (gdb) p new
> $2 = {name = 0x8047650 "origin", path = 0x8166438 "refs/heads/origin", 
> commit = 0x8168f48}
> (gdb) p work
> $3 = (struct tree *) 0x8174f90
> (gdb) p old
> No symbol "old" in current context.
> (gdb) p result
> $4 = (struct tree *) 0xfefc81be
> (gdb)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Does this patch help?

---
From b2203bded22db1a496ee3c9f6f5f4a384a8ccefa Mon Sep 17 00:00:00 2001
From: Daniel Baranski <mjucde@o2.pl>
Date: Wed, 6 Jan 2010 08:58:21 +0100
Subject: [PATCH] checkout -m: Fix SEGFAULT if HEAD is not valid.

Signed-off-by: Daniel Barański <mjucde@o2.pl>
---
 builtin-checkout.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 64f3a11..0ab59b2 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -422,7 +422,8 @@ static int merge_working_tree(struct checkout_opts *opts,
                        struct merge_options o;
                        if (!opts->merge)
                                return 1;
-                       parse_commit(old->commit);
+                       if (!parse_commit(old->commit))
+                               die("Couldn't parse commit '%s'", old->path);

                        /* Do more real merge */

-- 
1.6.5.6

^ permalink raw reply related

* Re: checkout -m dumping core
From: Daniel @ 2010-01-06  8:20 UTC (permalink / raw)
  To: git; +Cc: Tomas Carnecky

Argh, this one is better... (does not contain !). Stupid typo :/

---
From b2203bded22db1a496ee3c9f6f5f4a384a8ccefa Mon Sep 17 00:00:00 2001
From: Daniel Baranski <mjucde@o2.pl>
Date: Wed, 6 Jan 2010 08:58:21 +0100
Subject: [PATCH] checkout -m: Fix SEGFAULT if HEAD is not valid.

Signed-off-by: Daniel Barański <mjucde@o2.pl>
---
 builtin-checkout.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 64f3a11..0ab59b2 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -422,7 +422,8 @@ static int merge_working_tree(struct checkout_opts
*opts,
                        struct merge_options o;
                        if (!opts->merge)
                                return 1;
-                       parse_commit(old->commit);
+                       if (parse_commit(old->commit))
+                               die("Couldn't parse commit '%s'",
old->path);

                        /* Do more real merge */

-- 
1.6.5.6

^ permalink raw reply related

* [PATCH git-gui 5/5] git-gui/Makefile: consolidate .FORCE-* targets
From: Jonathan Nieder @ 2010-01-06  8:16 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Junio C Hamano
In-Reply-To: <20100106080216.GA7298@progeny.tock>

Providing multiple targets to force a rebuild is unnecessary
complication.

Avoid using a name that could conflict with future special
targets in GNU make (a leading period followed by uppercase
letters).

Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
This is my first git-gui patch, so it is likely I have missed some
conventions.  If that is the case, please let me know.

Thanks,
Jonathn

 Makefile |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index b3580e9..197b55e 100644
--- a/Makefile
+++ b/Makefile
@@ -7,7 +7,7 @@ all::
 # TCL_PATH must be vaild for this to work.
 #
 
-GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
+GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
 -include GIT-VERSION-FILE
 
@@ -270,7 +270,7 @@ TRACK_VARS = \
 	GITGUI_MACOSXAPP=$(GITGUI_MACOSXAPP) \
 #end TRACK_VARS
 
-GIT-GUI-VARS: .FORCE-GIT-GUI-VARS
+GIT-GUI-VARS: FORCE
 	@VARS='$(TRACK_VARS)'; \
 	if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
 		echo 1>&2 "    * new locations or Tcl/Tk interpreter"; \
@@ -340,5 +340,4 @@ ifdef GITGUI_WINDOWS_WRAPPER
 endif
 
 .PHONY: all install uninstall dist-version clean
-.PHONY: .FORCE-GIT-VERSION-FILE
-.PHONY: .FORCE-GIT-GUI-VARS
+.PHONY: FORCE
-- 
1.6.6.rc2

^ permalink raw reply related

* [PATCH 4/5] Makefile: consolidate .FORCE-* targets
From: Jonathan Nieder @ 2010-01-06  8:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20100106080216.GA7298@progeny.tock>

Providing multiple targets to force a rebuild is unnecessary
complication.

Avoid using a name that could conflict with future special
targets in GNU make (a leading period followed by uppercase
letters).

The corresponding change to the git-gui Makefile is left for
another patch.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Documentation/Makefile |    4 ++--
 Makefile               |   15 ++++++---------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 4797b2d..8a8a395 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -204,7 +204,7 @@ install-pdf: pdf
 install-html: html
 	'$(SHELL_PATH_SQ)' ./install-webdoc.sh $(DESTDIR)$(htmldir)
 
-../GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
+../GIT-VERSION-FILE: FORCE
 	$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE
 
 -include ../GIT-VERSION-FILE
@@ -337,4 +337,4 @@ quick-install-man:
 quick-install-html:
 	'$(SHELL_PATH_SQ)' ./install-doc-quick.sh $(HTML_REF) $(DESTDIR)$(htmldir)
 
-.PHONY: .FORCE-GIT-VERSION-FILE
+.PHONY: FORCE
diff --git a/Makefile b/Makefile
index 3d774c6..fd79cd6 100644
--- a/Makefile
+++ b/Makefile
@@ -222,7 +222,7 @@ all::
 #   DEFAULT_EDITOR='$GIT_FALLBACK_EDITOR',
 #   DEFAULT_EDITOR='"C:\Program Files\Vim\gvim.exe" --nofork'
 
-GIT-VERSION-FILE: .FORCE-GIT-VERSION-FILE
+GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
 -include GIT-VERSION-FILE
 
@@ -1632,7 +1632,7 @@ git.o git.spec \
 
 %.o: %.c GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
-%.s: %.c GIT-CFLAGS .FORCE-LISTING
+%.s: %.c GIT-CFLAGS FORCE
 	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
 %.o: %.S
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
@@ -1723,7 +1723,7 @@ cscope:
 TRACK_CFLAGS = $(subst ','\'',$(ALL_CFLAGS)):\
              $(bindir_SQ):$(gitexecdir_SQ):$(template_dir_SQ):$(prefix_SQ)
 
-GIT-CFLAGS: .FORCE-GIT-CFLAGS
+GIT-CFLAGS: FORCE
 	@FLAGS='$(TRACK_CFLAGS)'; \
 	    if test x"$$FLAGS" != x"`cat GIT-CFLAGS 2>/dev/null`" ; then \
 		echo 1>&2 "    * new build flags or prefix"; \
@@ -1733,7 +1733,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
 # We need to apply sq twice, once to protect from the shell
 # that runs GIT-BUILD-OPTIONS, and then again to protect it
 # and the first level quoting from the shell that runs "echo".
-GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
+GIT-BUILD-OPTIONS: FORCE
 	@echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@
 	@echo PERL_PATH=\''$(subst ','\'',$(PERL_PATH_SQ))'\' >>$@
 	@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@
@@ -1745,14 +1745,12 @@ GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
 ifndef NO_TCLTK
 TRACK_VARS = $(subst ','\'',-DTCLTK_PATH='$(TCLTK_PATH_SQ)')
 
-GIT-GUI-VARS: .FORCE-GIT-GUI-VARS
+GIT-GUI-VARS: FORCE
 	@VARS='$(TRACK_VARS)'; \
 	    if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
 		echo 1>&2 "    * new Tcl/Tk interpreter location"; \
 		echo "$$VARS" >$@; \
             fi
-
-.PHONY: .FORCE-GIT-GUI-VARS
 endif
 
 ### Testing rules
@@ -1972,8 +1970,7 @@ endif
 
 .PHONY: all install clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
-.PHONY: .FORCE-GIT-VERSION-FILE TAGS tags cscope .FORCE-GIT-CFLAGS
-.PHONY: .FORCE-GIT-BUILD-OPTIONS .FORCE-LISTING
+.PHONY: FORCE TAGS tags cscope
 
 ### Check documentation
 #
-- 
1.6.6.rc2

^ permalink raw reply related

* [PATCH 3/5] Makefile: learn to generate listings for targets requiring special flags
From: Jonathan Nieder @ 2010-01-06  8:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <20100106080216.GA7298@progeny.tock>

'make git.s' to debug code generation of main() fails because
git.c makes use of preprocessor symbols such as GIT_VERSION that
are not set.  make does not generate code listings for
builtin_help.c, exec_cmd.c, builtin-init-db.c, config.c, http.c,
or http-walker.c either, for the same reason.

So pass the flags used to generate each .o file when generating
the corresponding assembler listing.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 81190a6..3d774c6 100644
--- a/Makefile
+++ b/Makefile
@@ -1468,7 +1468,7 @@ strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
 git.o: common-cmds.h
-git.o: ALL_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
+git.s git.o: ALL_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
 	'-DGIT_HTML_PATH="$(htmldir_SQ)"'
 
 git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
@@ -1476,7 +1476,7 @@ git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
 		$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)
 
 builtin-help.o: common-cmds.h
-builtin-help.o: ALL_CFLAGS += \
+builtin-help.s builtin-help.o: ALL_CFLAGS += \
 	'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
 	'-DGIT_MAN_PATH="$(mandir_SQ)"' \
 	'-DGIT_INFO_PATH="$(infodir_SQ)"'
@@ -1637,21 +1637,21 @@ git.o git.spec \
 %.o: %.S
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
-exec_cmd.o: ALL_CFLAGS += \
+exec_cmd.s exec_cmd.o: ALL_CFLAGS += \
 	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
 	'-DBINDIR="$(bindir_relative_SQ)"' \
 	'-DPREFIX="$(prefix_SQ)"'
 
-builtin-init-db.o: ALL_CFLAGS += \
+builtin-init-db.s builtin-init-db.o: ALL_CFLAGS += \
 	-DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"'
 
-config.o: ALL_CFLAGS += -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
+config.s config.o: ALL_CFLAGS += -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
 
-http.o: ALL_CFLAGS += -DGIT_USER_AGENT='"git/$(GIT_VERSION)"'
+http.s http.o: ALL_CFLAGS += -DGIT_USER_AGENT='"git/$(GIT_VERSION)"'
 
 ifdef NO_EXPAT
 http-walker.o: http.h
-http-walker.o: ALL_CFLAGS += -DNO_EXPAT
+http-walker.s http-walker.o: ALL_CFLAGS += -DNO_EXPAT
 endif
 
 git-%$X: %.o $(GITLIBS)
-- 
1.6.6.rc2

^ permalink raw reply related

* [PATCH 2/5] Makefile: use target-specific variable to pass flags to cc
From: Jonathan Nieder @ 2010-01-06  8:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <20100106080216.GA7298@progeny.tock>

This allows reusing the standard %.o: %.c pattern rule even for
targets that require special flags to be set.  Thus after this
change, any changes in the command for compilation only have to
be performed in one place.

Target-specific variables have been supported in GNU make since
version 3.77, which has been available since 1998.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |   41 ++++++++++++++++++-----------------------
 1 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/Makefile b/Makefile
index ba4d071..81190a6 100644
--- a/Makefile
+++ b/Makefile
@@ -1467,20 +1467,19 @@ shell_compatibility_test: please_set_SHELL_PATH_to_a_more_modern_shell
 strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
-git.o: git.c common-cmds.h GIT-CFLAGS
-	$(QUIET_CC)$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
-		'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
-		$(ALL_CFLAGS) -o $@ -c $(filter %.c,$^)
+git.o: common-cmds.h
+git.o: ALL_CFLAGS += -DGIT_VERSION='"$(GIT_VERSION)"' \
+	'-DGIT_HTML_PATH="$(htmldir_SQ)"'
 
 git$X: git.o $(BUILTIN_OBJS) $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \
 		$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)
 
-builtin-help.o: builtin-help.c common-cmds.h GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) \
-		'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
-		'-DGIT_MAN_PATH="$(mandir_SQ)"' \
-		'-DGIT_INFO_PATH="$(infodir_SQ)"' $<
+builtin-help.o: common-cmds.h
+builtin-help.o: ALL_CFLAGS += \
+	'-DGIT_HTML_PATH="$(htmldir_SQ)"' \
+	'-DGIT_MAN_PATH="$(mandir_SQ)"' \
+	'-DGIT_INFO_PATH="$(infodir_SQ)"'
 
 $(BUILT_INS): git$X
 	$(QUIET_BUILT_IN)$(RM) $@ && \
@@ -1638,25 +1637,21 @@ git.o git.spec \
 %.o: %.S
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
-exec_cmd.o: exec_cmd.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) \
-		'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
-		'-DBINDIR="$(bindir_relative_SQ)"' \
-		'-DPREFIX="$(prefix_SQ)"' \
-		$<
+exec_cmd.o: ALL_CFLAGS += \
+	'-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \
+	'-DBINDIR="$(bindir_relative_SQ)"' \
+	'-DPREFIX="$(prefix_SQ)"'
 
-builtin-init-db.o: builtin-init-db.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"' $<
+builtin-init-db.o: ALL_CFLAGS += \
+	-DDEFAULT_GIT_TEMPLATE_DIR='"$(template_dir_SQ)"'
 
-config.o: config.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"' $<
+config.o: ALL_CFLAGS += -DETC_GITCONFIG='"$(ETC_GITCONFIG_SQ)"'
 
-http.o: http.c GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DGIT_USER_AGENT='"git/$(GIT_VERSION)"' $<
+http.o: ALL_CFLAGS += -DGIT_USER_AGENT='"git/$(GIT_VERSION)"'
 
 ifdef NO_EXPAT
-http-walker.o: http-walker.c http.h GIT-CFLAGS
-	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DNO_EXPAT $<
+http-walker.o: http.h
+http-walker.o: ALL_CFLAGS += -DNO_EXPAT
 endif
 
 git-%$X: %.o $(GITLIBS)
-- 
1.6.6.rc2

^ permalink raw reply related

* [PATCH 1/5] Makefile: regenerate assembler listings when asked
From: Jonathan Nieder @ 2010-01-06  8:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds
In-Reply-To: <20100106080216.GA7298@progeny.tock>

'make var.s' fails to regenerate an assembler listing if var.c
has not changed but a header it includes has:

	$ make var.s
	    CC var.s
	$ touch cache.h
	$ make var.s
	$

The corresponding problem for 'make var.o' does not occur because
the Makefile lists dependencies for each .o target explicitly;
analogous dependency rules for the .s targets are not present.
Rather than add some, it seems better to force 'make' to always
regenerate assembler listings, since the assembler listing
targets are only invoked when specifically requested on the make
command line.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index c11719c..ba4d071 100644
--- a/Makefile
+++ b/Makefile
@@ -1633,7 +1633,7 @@ git.o git.spec \
 
 %.o: %.c GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
-%.s: %.c GIT-CFLAGS
+%.s: %.c GIT-CFLAGS .FORCE-LISTING
 	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
 %.o: %.S
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
@@ -1978,7 +1978,7 @@ endif
 .PHONY: all install clean strip
 .PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
 .PHONY: .FORCE-GIT-VERSION-FILE TAGS tags cscope .FORCE-GIT-CFLAGS
-.PHONY: .FORCE-GIT-BUILD-OPTIONS
+.PHONY: .FORCE-GIT-BUILD-OPTIONS .FORCE-LISTING
 
 ### Check documentation
 #
-- 
1.6.6.rc2

^ permalink raw reply related

* [PATCH v2 0/5] Makefile: fix generation of assembler listings
From: Jonathan Nieder @ 2010-01-06  8:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Linus Torvalds, Shawn O. Pearce
In-Reply-To: <20091128113709.GD10059@progeny.tock>

Jonathan Nieder wrote:

> This adds yet another phony .FORCE-foo target.  Wouldn’t it be simpler
> to use a single target called .FORCE, or is there something I am
> missing that that would break?

I didn’t hear any screams when I suggested this about a month ago, so
let’s try it out.

Patch 1 fixes a problem I noticed when tweaking the Makefile to
automatically generate dependencies for the %.o targets.  The problem
is that the dependencies for the corresponding %.s (code listing)
targets are not included in the Makefile at all, automatically or not.
Thus the command "make var.s var.o && touch cache.h && make var.s var.o"
produces the output

CC var.s
CC var.o
CC var.o

not regenerating var.s to reflect potential changes in cache.h.

"make git.s" previously did not work at all; patches 2-3 fix that.

Jonathan Nieder (5):
  Makefile: regenerate assembler listings when asked
  Makefile: use target-specific variable to pass flags to cc
  Makefile: learn to generate listings for targets requiring special
    flags
  Makefile: consolidate .FORCE-* targets
  git-gui/Makefile: consolidate .FORCE-* targets

 Documentation/Makefile |    4 +-
 Makefile               |   56 ++++++++++++++++++++---------------------------
 git-gui/Makefile       |    7 ++---
 3 files changed, 29 insertions(+), 38 deletions(-)

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2010, #01; Mon, 04)
From: Junio C Hamano @ 2010-01-06  7:47 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git
In-Reply-To: <4B43A5CA.7090104@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

> Junio C Hamano schrieb:
>> While you are technically correct that the change you made in t4030 is not
>> justified by the commit log message in the sense that the "hexdump" script
>> will go through run_command() interface and is not subject to the special
>> rules filter writers need to keep in mind, the patch text itself is a good
>> change, isn't it?
>
> The patch text is good, but since it will not make a difference (and
> there are a ton of other places that use /bin/sh successfully), the
> change is not warrented at this time, IMO.

You are right (and Peff also corrected me).

>> As "run-command: convert simple callsites to use_shell" is the one that
>> changes the filter_buffer(), do you want to have t0021 patch before that
>> one, to prepare the test for the coming change?
>
> Well, the test will break on Windows only after "run-command: optimize
> out useless shell calls", and I wrote the commit message
> accordingly. If you move it before that one (and if you are picky) the
> commit message should be changed as well.

Yeah, I've reworded that one with a phrase "futureproof".

Regarding your "[PATCH 8/6] t4030, t4031", I have two questions:

    Recall that MSYS bash converts POSIX style absolute paths to Windows style
    absolute paths. Unfortunately, it converts a program argument that begins
    with a double-quote and otherwise looks like an absolute POSIX path, but
    in doing so, it strips everything past the second double-quote[*]. This
    case is triggered in the two test scripts. The work-around is to place the
    Windows style path between the quotes to avoid the path conversion.

(1) Does "Windows style path" here mean what $(pwd) returns as opposed to
    what is in $PWD?

(2) The patch reads like this:

-	git config diff.foo.textconv "\"$PWD\""/hexdump &&
+	git config diff.foo.textconv "\"$(pwd)\""/hexdump &&

    Does "strips everything past the second dq" mean "drops '/hexdump'"?
    If so, would this also work (I am not suggesting to change it, just
    asking for information)?

-	git config diff.foo.textconv "\"$PWD\""/hexdump &&
+	git config diff.foo.textconv "\"$PWD/hexdump\"" &&


Thanks.

^ permalink raw reply

* Re: [PATCH] Documentation: reset: add some missing tables
From: Christian Couder @ 2010-01-06  7:31 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Linus Torvalds, Johannes Schindelin, Stephan Beyer,
	Daniel Barkalow, Jakub Narebski, Paolo Bonzini, Johannes Sixt,
	Stephen Boyd
In-Reply-To: <7vwrzx3v4z.fsf@alter.siamese.dyndns.org>

On mardi 05 janvier 2010, Junio C Hamano wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
> > and while at it also explain why --merge option is disallowed in some
> > cases.
> >
> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> > ---
> >  Documentation/git-reset.txt |   35 +++++++++++++++++++++++++++++------
> >  1 files changed, 29 insertions(+), 6 deletions(-)
> >
> > I must say that I find it a bit strange (and difficult to explain) that
> > we have:
> >
> >       working index HEAD target         working index HEAD
> >       ----------------------------------------------------
> >        B       C     C    C     --merge   B       C     C
> >
> > while in the other cases, when it is allowed, --merge is like --hard.
>
> That is probably because you don't explain what --merge option is _for_
> well enough to your readers.  If the reader understands it is to reset
> away a half-merged conflicted result, starting from a potentially dirty
> work tree, then it would be very obvious that the above is the right
> thing to do.
>
> As a prerequisite, the reader should be aware (otherwise they should read
> some introductory git books, or
> http://gitster.livejournal.com/29060.html) that a mergy operation can
> stop without completing a merge in two ways:
>
>  - If a path that is involved in a mergy operation has local changes in
>    the work tree, or if the index is dirty, the operation stops _without_
>    doing anything.
>
>  - If all paths that are involved in a mergy operation are clean in the
>    work tree, the operation is attempted.  If a conflict happens at the
>    content level, the operation leaves the paths in conflicted state in
>    the index and leaves the conflict markers in the files in the work
>    tree.  Be _very_ aware that even in this case, cleanly automerged
> paths are updated in the index and the work tree.
>
> In the first case, you do not have to run "reset --merge", as nothing was
> done by the mergy operation (it happens to be safe to "reset --merge", as
> the only thing you lose is a partial add, which you can easily redo from
> the files in the working tree).
>
> In the latter case, there are four classes of paths:
>
>  (1) Ones that are not involved in the merge at all, and were clean from
>      the beginning.  The work tree file, the index and the HEAD would
>      match.
>
>      w=C i=C H=C
>
>  (2) Ones that are not involved in the merge at all, but were dirty when
>      you started the mergy operation.  They have your local changes in
> the work tree that you wanted to keep across the mergy operation.
>
>      w=B i=C H=C
>
>  (3) Ones that are involved in the merge, and were cleanly merged.  By
>      definition, these paths did _not_ have local changes in the work
> tree (otherwise the mergy operation would have stopped without doing
> anything).  These are updated in the index and the files in the work tree
> matches the index after the mergy operation stops.
>
>      w=B i=B H=C
>
>  (4) Ones that are involved in the merge, and were conflicted.  Again, by
>      definition, these paths did _not_ have local changes in the work
> tree These are left in the index as conflicted, and the files in the work
> tree have conflict markers after the mergy operation stops.
>
>      w=X i=U H=C
>
> "reset --merge HEAD" is about going back to the state before you started
> this mergy operation.  You don't need to do anything to paths in (1), and
> you want to reset paths in (3) and (4) back to the HEAD.
>
> Think what you want to do to (2).  By definition, they weren't involved
> in the mergy operation (otherwise you couldn't have come this far), so
> the difference between the index and the work tree is purely your local
> changes, untouched by the mergy operation, and have not even been updated
> in "cvs update" style.  The right thing to do is simply leave them as
> they are.
>
>     Side note.  Explained in the opposite way, if the work tree file is
>     different from the index and the index is not unmerged, the
> difference _only_ could have come from the local change before you
> started your mergy operation.  Any other change to the work tree files
> done by any mergy operation will be matched to the index.  So w=B i=C in
> (2) will immediately tell you that the change is a local one that is
> unrelated to the merge.
>
> By the way, people often say that the index is good because it allows you
> to make partial commits (i.e. "add -p"), but at the same time have this
> mistaken notion that it is the _primary_ benefit of the index.  Actually,
> a lot more important benefit of the index is (3) above.  When you are
> dealing with a large merge with many paths, often a lot of them automerge
> cleanly while some gives you conflicts.  The automerged results are added
> to the index and you do not have to see them in "git diff" (as their
> files and the index match), to allow you concentrate on the conflicted
> ones very easily.

Thanks for your explanations.
Christian.

^ permalink raw reply

* [PATCH v2] Makefile: make ppc/sha1ppc.o depend on GIT-CFLAGS
From: Jonathan Nieder @ 2010-01-06  6:37 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Nanako Shiraishi
In-Reply-To: <20091128113323.GC10059@progeny.tock>

The %.o: %.S pattern rule should depend on GIT-CFLAGS to avoid
trouble when ALL_CFLAGS changes.

The pattern only applies to one file (ppc/sha1ppc.S) and that
file does not use any #ifdefs, so leaving the dependency out is
probably harmless.  Nevertheless, it is safer to include the
dependency in case future code's behavior does depend on the
build flags.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hopefully the justification is a little clearer this time.

This is not a high-priority change.  The problem it addresses is only
an aesthetic one as far as I can tell.  Still, I would be happy to see
it fixed; thanks for the reminder.

 Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index c11719c..015bfab 100644
--- a/Makefile
+++ b/Makefile
@@ -1635,7 +1635,7 @@ git.o git.spec \
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 %.s: %.c GIT-CFLAGS
 	$(QUIET_CC)$(CC) -S $(ALL_CFLAGS) $<
-%.o: %.S
+%.o: %.S GIT-CFLAGS
 	$(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) $<
 
 exec_cmd.o: exec_cmd.c GIT-CFLAGS
-- 
1.6.6.rc2

^ permalink raw reply related

* Re: cmake, was Re: submodules' shortcomings
From: Miles Bader @ 2010-01-06  4:25 UTC (permalink / raw)
  To: Pau Garcia i Quiles; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <3af572ac1001051717u7757f0dep9392fbb7b02cbbca@mail.gmail.com>

Pau Garcia i Quiles <pgquiles@elpauer.org> writes:
> At this moment, what stops me from beginning this project is a simple
> question: is it worth my time? From the discussion a few months ago,
> it looked like it would the a second-class citizen and never replace
> the existing buildsystems, so I really wonder if I should spend me
> time porting git to CMake, or I should focus on other projects which
> would gladly receive my contributions. If you honestly think it's
> worth it, just tell me and I'll start the port to CMake immediately.

It sounds like it's you who want it, so aren't you the best person to
make that judgement...?  It seems very unlikely for cmake to replace
anything.

-Miles

-- 
Politeness, n. The most acceptable hypocrisy.

^ permalink raw reply

* Re: [PATCH 0/4] Makefile fixes
From: Jonathan Nieder @ 2010-01-06  4:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7vpr5onir5.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
> 
>> Junio, could you tell us what happened to this thread?
>>
>> Makefile improvements.  No discussion.

These had some issues and instead of following up, I simply forgot
about them.

> I took 4/4, and after looking at them again, I think 2/4 looks sensible,
> too.

I also think the patch for 2/4 looks sensible, but the commit message
does not make much sense.  Optimization flags do not affect
compilation of assembler code as far as I can tell.  It would have
made more sense to say something like "Since the only .S file in git
does not have any #ifdefs, leaving the dependency out was mostly
harmless."  (Will resend.)

> I was puzzled by 3/4 and I still am; the dependency rules are the same for
> %.o and %.s yet the patch changes only %.s.  Either it leaves the same
> breakage for %.o (which is much more important in practice), or the
> problem Jonathan has with %.s may have other causes, but it was unclear to
> me.

The Makefile lists dependencies for each .o target elsewhere.  While
cleaning up those other dependency rules, I noticed there was nothing
analogous for the .s targets.  You can reproduce this by running
"make var.o var.s && touch cache.h && make var.o var.s".

Of course, I should have mentioned this in the commit message.  Will
resend as well.  Sorry to leave these standing for so long.

Sincerely,
Jonathan

^ permalink raw reply


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