Git development
 help / color / mirror / Atom feed
* Re: [PATCH] [git-p4] Detect exec bit in more cases.
From: David Brown @ 2007-09-19 21:03 UTC (permalink / raw)
  To: git
In-Reply-To: <1190232768445-git-send-email-git@davidb.org>

On Wed, Sep 19, 2007 at 01:12:48PM -0700, David Brown wrote:
>git-p4 was missing the execute bit setting if the file had other attribute
>bits set.
>---
> contrib/fast-import/git-p4 |   10 +++++++++-
> 1 files changed, 9 insertions(+), 1 deletions(-)

I've tested this patch on our fairly large P4 repo, and at least the tip
exactly matches the files that P4 finds.  So, it at least should be better.
git-p4 still has some problems with case-insensitive servers.

Dave

^ permalink raw reply

* Re: Problem with merge when renaming
From: Alex Riesen @ 2007-09-19 21:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: David Euresti, git
In-Reply-To: <7vejgvtv0a.fsf@gitster.siamese.dyndns.org>

Junio C Hamano, Wed, Sep 19, 2007 01:27:01 +0200:
> > it happens with very recent git (as of today)
> >
> >>  (2) if you do "git merge -s resolve" instead of just "git
> >>      merge", do you see a difference?
> >
> > yes: it has more error output.
> >
> > Trying really trivial in-index merge...
> > warning: Merge requires file-level merging
> > Nope, a really trivial in-index merge not possible
> > Trying simple merge.
> > Simple merge failed, trying Automatic merge.
> > error: init: is a directory - add individual files instead
> > fatal: Unable to process path init
> > fatal: merge program failed
> > Automatic merge failed; fix conflicts and then commit the result.
> 
> Isn't this the same as the known issue here?
> 
> 	<http://permalink.gmane.org/gmane.comp.version-control.git/53402>
> 

Yes, looks similar:

    .../git/t$ ./t3031-df-3way.sh -d -v -i
    ...
    Created commit 992a330: side moves and makes changes
     6 files changed, 155 insertions(+), 153 deletions(-)
     create mode 100644 D
     create mode 100644 D.A
     delete mode 100644 D/A
     delete mode 100644 D/B
     delete mode 100644 D/C
     create mode 100644 D0C
    *   ok 1: setup

    * expecting success: 

	    git read-tree -m initial side master


    fatal: Entry 'D/A' would be overwritten by merge. Cannot merge.
    * FAIL 2: threeway


		    git read-tree -m initial side master

    .../git/t$

^ permalink raw reply

* Re: State of Perforce importing.
From: Reece Dunn @ 2007-09-19 21:20 UTC (permalink / raw)
  To: Sam Vilain; +Cc: Git
In-Reply-To: <46F06C0F.3040609@vilain.net>

On 19/09/2007, Sam Vilain <sam@vilain.net> wrote:
> Reece Dunn wrote:
> > There is no need to create yet another Perforce importing tool, git-p4
> > works well in most cases. If we focus on improving git-p4, extending
> > it to support the functionality mentioned here, fix the issues that
> > there are with it, then that will be more beneficial to the community
> > as they will not have to learn another tool with a different set of
> > bugs and issues.
>
> I like my approach; it's clean and I think shows a tasteful level of
> distrust towards the sanity and integrity of the data held by Perforce.
>  Actually it really helped me understand what was really going on;
> because the information as displayed by for instance "p4 integrate" is
> a lot more confusing than the underlying tables (IMHO).

I agree. What I wasn't clear about in that paragraph, but had eluded
to in other comments in that email, is that having both git-p4 and
git-p4raw is a good thing as they operate on two differing use cases.
What I was referring to there is to have another equivalent of git-p4
that interfaced using the p4 client.

- Reece

^ permalink raw reply

* Re: [RFC] Convert builin-mailinfo.c to use The Better String Library.
From: Steven Burns @ 2007-09-19 19:56 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.LFD.0.999.0709070203200.5626@evo.linux-foundation.org>

To me, the only thing that C++ has that all other mentioned languages lack 
is the power you get from the templates and generic programming.
Sorting will always be faster if you can call the comparison function 
directly without using a function pointer, and the only way you can create a 
generic sorting algorithm is that way.

Thinking about it with a cold head, most things to hate about C++ are not in 
the language but in its libraries.
The only feature I hate from the language itself is the preprocessor 
(macros), which you get in C too.

And maybe I also hate the fact that C++ allows for unexperienced programmers 
to create a bunch of classes and hierarchies that make sense to nobody but 
them. Or even worse, unexperienced programmers start writing their own 
frameworks, wrapping and re-wrapping, the same good old C function one 
thousand times.

I guess that is why most C++ based projects out there have a strict list of 
rules and conventions, you cannot have a stable project without them.

But, nothing prevents anybody from programming in C++ the way you describe, 
using simple and clear core structures with some basic methods that 
complement them (not obscure them) and make it easier to write the 
algorithms.
Sadly, once you start using std::string, their overly complicated and fancy 
iostreams, and bulky classes that hide too much from you, I have no other 
choice than to agree and call the whole thing a mess.

Steven Burns


"Linus Torvalds" <torvalds@linux-foundation.org> wrote in message 
news:alpine.LFD.0.999.0709070203200.5626@evo.linux-foundation.org...
>
>
> On Fri, 7 Sep 2007, Linus Torvalds wrote:
>>
>> The fact is, git is better than the other SCM's. And good taste (and C) 
>> is
>> one of the reasons for that.
>
> To be very specific:
> - simple and clear core datastructures, with *very* lean and aggressive
>   code to manage them that takes the whole approach of "simplicity over
>   fancy" to the extreme.
> - a willingness to not abstract away the data structures and algorithms,
>   because those are the *whole*point* of core git.
>
> And if you want a fancier language, C++ is absolutely the worst one to
> choose. If you want real high-level, pick one that has true high-level
> features like garbage collection or a good system integration, rather than
> something that lacks both the sparseness and straightforwardness of C,
> *and* doesn't even have the high-level bindings to important concepts.
>
> IOW, C++ is in that inconvenient spot where it doesn't help make things
> simple enough to be truly usable for prototyping or simple GUI
> programming, and yet isn't the lean system programming language that C is
> that actively encourags you to use simple and direct constructs.
>
> Linus 

^ permalink raw reply

* quote/strbuf series, take 3
From: Pierre Habouzit @ 2007-09-19 22:42 UTC (permalink / raw)
  To: gitster; +Cc: git

  Here is the take 3. wrt last time: the need for strbuf_addvf has been
avoided. It was not really needed for many places, and I chose ad-hoc
solutions for each. It has been done in a new patch (inserted at slot
2).

  Note that this patch 2/7 looks curious, especially the:

  write_fd_or_whine(..., "\n", 1, ...)

  it is rewritten in a following patch in the series, it's just to have
an intermediate working state. Don't really mind the bad look of it,
please :).


  As a consequence, strbuf_addvf is no longer added nor used in the
series.

  I just rebased it on the last next (with the new git-fetch).

^ permalink raw reply

* Re: [EGIT PATCH] Change to simplified icon.
From: Ben Konrath @ 2007-09-19 22:49 UTC (permalink / raw)
  To: Robin Rosenberg; +Cc: git
In-Reply-To: <200709192019.13096.robin.rosenberg.lists@dewire.com>

On Wed, 2007-09-19 at 20:19 +0200, Robin Rosenberg wrote:
> onsdag 19 september 2007 skrev Ben Konrath:
> > Hi Robin,
> > 
> > Here's a patch that changes the icon to something that is a little more
> > aesthetically pleasing than the one I iniitially submitted. Feel free to
> > use the one you like best.
> 
> The icon looks very similar to the one in GitWeb. The simplified is, without,
> making a bitwise compare, is seems identical to the Git icon. Is that 
> appropriate? What do the author of that icon, and other Git developers, think?

I did actually make this icon with gimp but it is heavily inspired by
the GitWeb icon. But I think this is moot because the icon you just made
seems to be a better fit :)

Cheers, Ben

> The plugin. after all, does steal ideas and repository format from Git,
> but is in fact a completely separate implementation. 
> 
> I'll leave this one in my patch tree for now. The Egit icon contest is stil open to all.
> 
> -- robin
> 

^ permalink raw reply

* Re: Latest builtin-commit series
From: Junio C Hamano @ 2007-09-19 23:16 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: Git Mailing List
In-Reply-To: <1190129009.23692.24.camel@hinata.boston.redhat.com>

This series somewhat collided with Pierre's strbuf clean-ups.  I
queued it in 'pu' for now after fixing the conflicts up.

I haven't looked at the code enough to give any meaningful
comments yet, but I agree with most of the comments Dscho and
others raised on the topic already.

^ permalink raw reply

* Re: [PATCH 6/7] Export rerere() and launch_editor().
From: Junio C Hamano @ 2007-09-19 23:52 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: git
In-Reply-To: <1190074016669-git-send-email-krh@redhat.com>

Kristian Høgsberg <krh@redhat.com> writes:

> +/* Export for builtin-commit. */
> +int rerere(void)
> +{
> +	struct path_list merge_rr = { NULL, 0, 0, 1 };
> +	int fd;
> +
> +	git_config(git_rerere_config);
> +	if (!is_rerere_enabled())
> +		return 0;
> +
> +	merge_rr_path = xstrdup(git_path("rr-cache/MERGE_RR"));
> +	fd = hold_lock_file_for_update(&write_lock, merge_rr_path, 1);
> +	read_rr(&merge_rr);
> +	return do_plain_rerere(&merge_rr, fd);
> +}

Is it just me who sees a suboptimal cut and paste here?

BTW, [1-5/7] look good so far.

diff --git a/builtin-rerere.c b/builtin-rerere.c
index 29d057c..2f51ae0 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -415,18 +415,39 @@ static int is_rerere_enabled(void)
 	return 1;
 }
 
-int cmd_rerere(int argc, const char **argv, const char *prefix)
+static int setup_rerere(struct path_list *merge_rr)
 {
-	struct path_list merge_rr = { NULL, 0, 0, 1 };
-	int i, fd = -1;
+	int fd;
 
 	git_config(git_rerere_config);
 	if (!is_rerere_enabled())
-		return 0;
+		return -1;
 
 	merge_rr_path = xstrdup(git_path("rr-cache/MERGE_RR"));
 	fd = hold_lock_file_for_update(&write_lock, merge_rr_path, 1);
-	read_rr(&merge_rr);
+	read_rr(merge_rr);
+	return fd;
+}
+
+int rerere(void)
+{
+	struct path_list merge_rr = { NULL, 0, 0, 1 };
+	int fd;
+
+	fd = setup_rerere(&merge_rr);
+	if (fd < 0)
+		return 0;
+	return do_plain_rerere(&merge_rr, fd);
+}
+
+int cmd_rerere(int argc, const char **argv, const char *prefix)
+{
+	struct path_list merge_rr = { NULL, 0, 0, 1 };
+	int i, fd;
+
+	fd = setup_rerere(&merge_rr);
+	if (fd < 0)
+		return 0;
 
 	if (argc < 2)
 		return do_plain_rerere(&merge_rr, fd);

^ permalink raw reply related

* [PATCH] User Manual: add a chapter for submodules
From: Miklos Vajna @ 2007-09-20  0:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <7vbqbyphil.fsf@gitster.siamese.dyndns.org>

Signed-off-by: Michael Smith <msmith@cbnco.com>
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

[ 2nd version ]

On Wed, Sep 19, 2007 at 12:44:18PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> > +To see how submodule support works, create (for example) four example
> > +repository that can be used later as a submodule:
> 
> s/repository/repositories/;

fixed.

> > +$ for i in a b c d
> > +do
> > +	mkdir $i
> > +	cd $i
> > +	git init
> > +	echo "module $i" > $i.txt
> > +	git add $i.txt
> > +	git commit -m "Initial commit, submodule $mod"
> 
> s/\$mod/$i/;

fixed

> > +$ echo hi > super.txt
> > +$ git add super.txt
> > +$ git commit -m "Initial commit of empty superproject"
> 
> This is not *empty*; do you even need this step?

in fact, no :) removed.

> > +-------------------------------------------------
> > +$ git commit -m "Add submodules a, b, c, d."
> > +-------------------------------------------------
> 
> s/c, d./c and d./;

fixed.

> > +$ ls -a a
> > +.  ..
> > +$ git submodule status
> > +-d266b9873ad50488163457f025db7cdd9683d88b a
> > +-e81d457da15309b4fef4249aba9b50187999670d b
> > +-c1536a972b9affea0f16e0680ba87332dc059146 c
> > +-d96249ff5d57de5de093e6baff9e0aafa5276a74 d
> > +-------------------------------------------------
> 
> You might want to mention...
> 
>     Note: the commit object names shown above would be different for
>     you, but they should match the HEAD commit object names of your
>     repositories.  You can check it by doing:
> 
>     $ git ls-remote ../a

i've added such a note.

> > +If you want to make a change within a submodule, you should first check out a
> > +branch, make your changes, publish the change within the submodule, and then
> > +update the superproject to reference the new commit:
> > +
> > +-------------------------------------------------
> > +$ git branch
> > +* (no branch)
> > +  master
> > +$ git checkout master
> 
> I am not so sure about this advice.  Don't you want to see how
> the detached HEAD and 'master' (or any other branches) are
> related before doing this?  You might even want to create a
> "fix-up" branch that is rooted at the detached HEAD if the
> change you are making is to fix minor details of the submodule
> to suit what superproject wants (i.e. "little feature that is
> applicable to the submodule as a standalone project, meant to
> be pushed back to the submodule upstream").

right, changed it to two examples, one about "git checkout master", and
the other about "git checkout -b fix-up"

> > +$ git push
> > +$ cd ..
> > +$ git add a
> 
> Before doing this "git add", it would be educational to have the
> user do
> 
>     $ git diff
> 
> which would say that now the submodule is being updated.

added.

> > +$ git commit -m "Updated submodule a."
> > +$ git push
> > +-------------------------------------------------
> > +
> > +NOTE: This means that you have to run `git submodule update` after `git pull`
> > +if you want to update the subprojects, too.
> 
> I do not quite understand this note.  I do understand the part
> after "that you have to", but I do not know how the above
> example relates to that --- iow, the above example does not
> seem to "mean" such thing to me.
> 
> Also "submodule" is used consistently in the rest of the
> document but this sentence talks about "subproject".

yes, it was illogical to use a 'note' here as it's not related. i've
just added a separte sentence about this without the 'this means'.

> > +Problems with submodules
> > +------------------------
> 
> The list is good, but are they Problems or pitfalls to watch out
> for?

the later :) title changed.

> > +Always publish the submodule change before publishing the change to the
> > +superproject that references it. If you forget to publish the submodule change,
> > +others won't be able to clone the repository:
> 
> The same caution applies not to rewind branches in submodule
> beyond commits that were ever recorded in any superproject.

i've added a paragraph about this issue.

> > +-------------------------------------------------
> > +$ echo i added another line to this file >> a.txt
> > +$ git commit -a -m "doing it wrong this time"
> 
> It is not clear in which repository you are supposed to try this
> command in the example sequence, as we crossed the section
> boundary.  I am _guessing_ that the above two commands are to be
> run inside ~/git/cloned-2/a after redoing ~/git/cloned-2 the
> same way as you did ~/git/cloned, but you may want to make it
> clear.

right, i forgot the 'cd ~/git/super/a'. provided the repos are
symmetric, i think it would just make the situation unnecessary
complicated to clone a third repo.

> > +$ git submodule update
> > +error: pathspec '261dfac35cb99d380eb966e102c1197139f7fa24' did not match any file(s) known to git.
> > +Did you forget to 'git add'?
> 
> I sense there is a bug in "git submodule update" here.  I do not
> have time to look at what it's doing right now, but this is a
> symptom of running:
> 
>     $ git checkout 261dfac35...
> 
> without checking if that is a valid commit object name, and git
> checkout cannot decide if the request is about detaching the
> head at commit 261dfac35 or checking the path 261dfac35 out of
> the current index.  Porcelains should check with "cat-file -t"
> first.

hm yes. i think as long as this behaviour is not changed, it would not
be good to remove the "Did you forget to 'git add'?" part from the
example

> > +Unable to checkout '261dfac35cb99d380eb966e102c1197139f7fa24' in submodule path 'a'
> > +-------------------------------------------------
> > +
> > +It's not safe to run `git submodule update` if you've made changes within a
> > +submodule. They will be silently overwritten:
> 
> You want to distinguish two cases here.  Local uncommitted
> changes, and committed changes.  The latter case is switched
> away without too much noise, which is what you are demonstrating
> here.  I _think_ local uncommitted changes will not be
> overwritten at all, unless "submodule update" does "reset
> --hard", which I suspect it doesn't.

i've just checked that it does a git checkout, so uncommitted changes
are preserved. i've updated this part to mention this fact.

i've also added the suggestions by skimo.

VMiklos

 Documentation/user-manual.txt |  175 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 175 insertions(+), 0 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index ecb2bf9..ce0cf38 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -3155,6 +3155,181 @@ a tree which you are in the process of working on.
 If you blow the index away entirely, you generally haven't lost any
 information as long as you have the name of the tree that it described.
 
+[[submodules]]
+Submodules
+==========
+
+This tutorial explains how to create and publish a repository with submodules
+using the gitlink:git-submodule[1] command.
+
+Submodules maintain their own identity; the submodule support just stores the
+submodule repository location and commit ID, so other developers who clone the
+superproject can easily clone all the submodules at the same revision.
+
+To see how submodule support works, create (for example) four example
+repository that can be used later as a submodule:
+
+-------------------------------------------------
+$ mkdir ~/git
+$ cd ~/git
+$ for i in a b c d
+do
+	mkdir $i
+	cd $i
+	git init
+	echo "module $i" > $i.txt
+	git add $i.txt
+	git commit -m "Initial commit, submodule $mod"
+	cd ..
+done
+-------------------------------------------------
+
+Now create the superproject and add all the submodules:
+
+-------------------------------------------------
+$ mkdir super
+$ cd super
+$ git init
+$ echo hi > super.txt
+$ git add super.txt
+$ git commit -m "Initial commit of empty superproject"
+$ for i in a b c d
+do
+	git submodule add ~/git/$i
+done
+-------------------------------------------------
+
+See what files `git submodule` created:
+
+-------------------------------------------------
+$ ls -a
+.  ..  .git  .gitmodules  a  b  c  d  super.txt
+-------------------------------------------------
+
+The `git submodule add` command does a couple of things:
+
+- It clones the submodule under the current directory and by default checks out
+  the master branch.
+- It adds the submodule's clone path to the `.gitmodules` file and adds this
+  file to the index, ready to be committed.
+- It adds the submodule's current commit ID to the index, ready to be
+  committed.
+
+Commit the superproject:
+
+-------------------------------------------------
+$ git commit -m "Add submodules a, b, c, d."
+-------------------------------------------------
+
+Now clone the superproject:
+
+-------------------------------------------------
+$ cd ..
+$ git clone super cloned
+$ cd cloned
+-------------------------------------------------
+
+The submodule directories are there, but they're empty:
+
+-------------------------------------------------
+$ ls -a a
+.  ..
+$ git submodule status
+-d266b9873ad50488163457f025db7cdd9683d88b a
+-e81d457da15309b4fef4249aba9b50187999670d b
+-c1536a972b9affea0f16e0680ba87332dc059146 c
+-d96249ff5d57de5de093e6baff9e0aafa5276a74 d
+-------------------------------------------------
+
+Pulling down the submodules is a two-step process. First run `git submodule
+init` to add the submodule repository URLs to `.git/config`:
+
+-------------------------------------------------
+$ git submodule init
+-------------------------------------------------
+
+Now use `git submodule update` to clone the repositories and check out the
+commits specified in the superproject:
+
+-------------------------------------------------
+$ git submodule update
+$ cd a
+$ ls -a
+.  ..  .git  a.txt
+-------------------------------------------------
+
+One major difference between `git submodule update` and `git submodule add` is
+that `git submodule update` checks out a specific commit, rather than the tip
+of a branch. It's like checking out a tag: the head is detached, so you're not
+working on a branch.
+
+-------------------------------------------------
+$ git branch
+* (no branch)
+  master
+-------------------------------------------------
+
+If you want to make a change within a submodule, you should first check out a
+branch, make your changes, publish the change within the submodule, and then
+update the superproject to reference the new commit:
+
+-------------------------------------------------
+$ git branch
+* (no branch)
+  master
+$ git checkout master
+$ echo "adding a line again" >> a.txt
+$ git commit -a -m "Updated the submodule from within the superproject."
+$ git push
+$ cd ..
+$ git add a
+$ git commit -m "Updated submodule a."
+$ git push
+-------------------------------------------------
+
+NOTE: This means that you have to run `git submodule update` after `git pull`
+if you want to update the subprojects, too.
+
+Problems with submodules
+------------------------
+
+Always publish the submodule change before publishing the change to the
+superproject that references it. If you forget to publish the submodule change,
+others won't be able to clone the repository:
+
+-------------------------------------------------
+$ echo i added another line to this file >> a.txt
+$ git commit -a -m "doing it wrong this time"
+$ cd ..
+$ git add a
+$ git commit -m "Updated submodule a again."
+$ git push
+$ cd ~/git/cloned
+$ git pull
+$ git submodule update
+error: pathspec '261dfac35cb99d380eb966e102c1197139f7fa24' did not match any file(s) known to git.
+Did you forget to 'git add'?
+Unable to checkout '261dfac35cb99d380eb966e102c1197139f7fa24' in submodule path 'a'
+-------------------------------------------------
+
+It's not safe to run `git submodule update` if you've made changes within a
+submodule. They will be silently overwritten:
+
+-------------------------------------------------
+$ cat a.txt
+module a
+$ echo line added from private2 >> a.txt
+$ git commit -a -m "line added inside private2"
+$ cd ..
+$ git submodule update
+Submodule path 'a': checked out 'd266b9873ad50488163457f025db7cdd9683d88b'
+$ cd a
+$ cat a.txt
+module a
+-------------------------------------------------
+
+NOTE: The changes are still visible in the submodule's reflog.
+
 [[low-level-operations]]
 Low-level git operations
 ========================
-- 
1.5.3.1.1.g1e61-dirty

^ permalink raw reply related

* [PATCH] User Manual: add a chapter for submodules
From: Miklos Vajna @ 2007-09-20  0:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <7vbqbyphil.fsf@gitster.siamese.dyndns.org>

Signed-off-by: Michael Smith <msmith@cbnco.com>
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---

Sorry, I sent the original patch again. So here is the updated second version.

 Documentation/user-manual.txt |  202 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 202 insertions(+), 0 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index ecb2bf9..aee654c 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -3155,6 +3155,208 @@ a tree which you are in the process of working on.
 If you blow the index away entirely, you generally haven't lost any
 information as long as you have the name of the tree that it described.
 
+[[submodules]]
+Submodules
+==========
+
+This tutorial explains how to create and publish a repository with submodules
+using the gitlink:git-submodule[1] command.
+
+Submodules maintain their own identity; the submodule support just stores the
+submodule repository location and commit ID, so other developers who clone the
+superproject can easily clone all the submodules at the same revision.
+
+To see how submodule support works, create (for example) four example
+repositories that can be used later as a submodule:
+
+-------------------------------------------------
+$ mkdir ~/git
+$ cd ~/git
+$ for i in a b c d
+do
+	mkdir $i
+	cd $i
+	git init
+	echo "module $i" > $i.txt
+	git add $i.txt
+	git commit -m "Initial commit, submodule $i"
+	cd ..
+done
+-------------------------------------------------
+
+Now create the superproject and add all the submodules:
+
+-------------------------------------------------
+$ mkdir super
+$ cd super
+$ git init
+$ for i in a b c d
+do
+	git submodule add ~/git/$i
+done
+-------------------------------------------------
+
+NOTE: Do not use local URLs here if you plan to publish your superproject!
+
+See what files `git submodule` created:
+
+-------------------------------------------------
+$ ls -a
+.  ..  .git  .gitmodules  a  b  c  d
+-------------------------------------------------
+
+The `git submodule add` command does a couple of things:
+
+- It clones the submodule under the current directory and by default checks out
+  the master branch.
+- It adds the submodule's clone path to the `.gitmodules` file and adds this
+  file to the index, ready to be committed.
+- It adds the submodule's current commit ID to the index, ready to be
+  committed.
+
+Commit the superproject:
+
+-------------------------------------------------
+$ git commit -m "Add submodules a, b, c and d."
+-------------------------------------------------
+
+Now clone the superproject:
+
+-------------------------------------------------
+$ cd ..
+$ git clone super cloned
+$ cd cloned
+-------------------------------------------------
+
+The submodule directories are there, but they're empty:
+
+-------------------------------------------------
+$ ls -a a
+.  ..
+$ git submodule status
+-d266b9873ad50488163457f025db7cdd9683d88b a
+-e81d457da15309b4fef4249aba9b50187999670d b
+-c1536a972b9affea0f16e0680ba87332dc059146 c
+-d96249ff5d57de5de093e6baff9e0aafa5276a74 d
+-------------------------------------------------
+
+NOTE: The commit object names shown above would be different for you, but they
+should match the HEAD commit object names of your repositories.  You can check
+it by running `git ls-remote ../a`.
+
+Pulling down the submodules is a two-step process. First run `git submodule
+init` to add the submodule repository URLs to `.git/config`:
+
+-------------------------------------------------
+$ git submodule init
+-------------------------------------------------
+
+Now use `git submodule update` to clone the repositories and check out the
+commits specified in the superproject:
+
+-------------------------------------------------
+$ git submodule update
+$ cd a
+$ ls -a
+.  ..  .git  a.txt
+-------------------------------------------------
+
+One major difference between `git submodule update` and `git submodule add` is
+that `git submodule update` checks out a specific commit, rather than the tip
+of a branch. It's like checking out a tag: the head is detached, so you're not
+working on a branch.
+
+-------------------------------------------------
+$ git branch
+* (no branch)
+  master
+-------------------------------------------------
+
+If you want to make a change within a submodule and you have a detached head,
+then you should create or checkout a branch, make your changes, publish the
+change within the submodule, and then update the superproject to reference the
+new commit:
+
+-------------------------------------------------
+$ git checkout master
+-------------------------------------------------
+
+or
+
+-------------------------------------------------
+$ git checkout -b fix-up
+-------------------------------------------------
+
+then
+
+-------------------------------------------------
+$ echo "adding a line again" >> a.txt
+$ git commit -a -m "Updated the submodule from within the superproject."
+$ git push
+$ cd ..
+$ git diff
+diff --git a/a b/a
+index d266b98..261dfac 160000
+--- a/a
++++ b/a
+@@ -1 +1 @@
+-Subproject commit d266b9873ad50488163457f025db7cdd9683d88b
++Subproject commit 261dfac35cb99d380eb966e102c1197139f7fa24
+$ git add a
+$ git commit -m "Updated submodule a."
+$ git push
+-------------------------------------------------
+
+You have to run `git submodule update` after `git pull` if you want to update
+submodules, too.
+
+Pitfalls with submodules
+------------------------
+
+Always publish the submodule change before publishing the change to the
+superproject that references it. If you forget to publish the submodule change,
+others won't be able to clone the repository:
+
+-------------------------------------------------
+$ cd ~/git/super/a
+$ echo i added another line to this file >> a.txt
+$ git commit -a -m "doing it wrong this time"
+$ cd ..
+$ git add a
+$ git commit -m "Updated submodule a again."
+$ git push
+$ cd ~/git/cloned
+$ git pull
+$ git submodule update
+error: pathspec '261dfac35cb99d380eb966e102c1197139f7fa24' did not match any file(s) known to git.
+Did you forget to 'git add'?
+Unable to checkout '261dfac35cb99d380eb966e102c1197139f7fa24' in submodule path 'a'
+-------------------------------------------------
+
+You also should not rewind branches in a submodule beyond commits that were
+ever recorded in any superproject.
+
+It's not safe to run `git submodule update` if you've made and committed
+changes within a submodule without checking out a branch first. They will be
+silently overwritten:
+
+-------------------------------------------------
+$ cat a.txt
+module a
+$ echo line added from private2 >> a.txt
+$ git commit -a -m "line added inside private2"
+$ cd ..
+$ git submodule update
+Submodule path 'a': checked out 'd266b9873ad50488163457f025db7cdd9683d88b'
+$ cd a
+$ cat a.txt
+module a
+-------------------------------------------------
+
+NOTE: The changes are still visible in the submodule's reflog.
+
+This is not the case if you did not commit your changes.
+
 [[low-level-operations]]
 Low-level git operations
 ========================
-- 
1.5.3.1.1.g1e61-dirty

^ permalink raw reply related

* Re: [PATCH 7/7] Implement git commit as a builtin command.
From: Junio C Hamano @ 2007-09-20  1:27 UTC (permalink / raw)
  To: Kristian Høgsberg; +Cc: git
In-Reply-To: <11900740163661-git-send-email-krh@redhat.com>

Kristian Høgsberg <krh@redhat.com> writes:

> diff --git a/builtin-commit.c b/builtin-commit.c
> new file mode 100644
> index 0000000..ee98de9
> --- /dev/null
> +++ b/builtin-commit.c
> @@ -0,0 +1,740 @@
> +/*
> + * Builtin "git commit"
> + *
> + * Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>
> + * Based on git-commit.sh by Junio C Hamano and Linus Torvalds
> + */
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +

With 85023577a8f4b540aa64aa37f6f44578c0c305a3 (simplify
inclusion of system header files.), we introduced a rule against
these lines.  We probably would want a Coding Guideline (aka
"Hacking Git") document somewhere.

> +#include "cache.h"
> +#include "cache-tree.h"
> +#include "builtin.h"
> +#include "diff.h"
> +#include "diffcore.h"
> +#include "commit.h"
> +#include "revision.h"
> +#include "wt-status.h"
> +#include "run-command.h"
> +#include "refs.h"
> +#include "log-tree.h"
> +#include "strbuf.h"
> +#include "utf8.h"
> +
> +static const char builtin_commit_usage[] =
> +	"[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [--template <file>] [[-i | -o] <path>...]";
> +
> +static unsigned char head_sha1[20], merge_head_sha1[20];
> +static char *use_message_buffer;
> +static const char commit_editmsg[] = "COMMIT_EDITMSG";
> +static struct lock_file lock_file;
> +
> +enum option_type {
> +    OPTION_NONE,
> +    OPTION_STRING,
> +    OPTION_INTEGER,
> +    OPTION_LAST,
> +};
> +
> +struct option {
> +    enum option_type type;
> +    const char *long_name;
> +    char short_name;
> +    void *value;
> +};
> +
> +static int scan_options(const char ***argv, struct option *options)
> +{
> +	const char *value, *eq;
> +	int i;
> +
> +	if (**argv == NULL)
> +		return 0;
> +	if ((**argv)[0] != '-')
> +		return 0;
> +	if (!strcmp(**argv, "--"))
> +		return 0;
> +
> +	value = NULL;
> +	for (i = 0; options[i].type != OPTION_LAST; i++) {
> +		if ((**argv)[1] == '-') {
> +			if (!prefixcmp(options[i].long_name, **argv + 2)) {
> +				if (options[i].type != OPTION_NONE)
> +					value = *++(*argv);
> +				goto match;
> +			}
> +
> +			eq = strchr(**argv + 2, '=');
> +			if (eq && options[i].type != OPTION_NONE &&
> +			    !strncmp(**argv + 2, 
> +				     options[i].long_name, eq - **argv - 2)) {
> +				value = eq + 1;
> +				goto match;
> +			}
> +		}
> +
> +		if ((**argv)[1] == options[i].short_name) {
> +			if ((**argv)[2] == '\0') {
> +				if (options[i].type != OPTION_NONE)
> +					value = *++(*argv);
> +				goto match;
> +			}
> +
> +			if (options[i].type != OPTION_NONE) {
> +				value = **argv + 2;
> +				goto match;
> +			}
> +		}
> +	}

How do you disambiguate between "--author <me>" and "--amend"?
"The order in *options list matters" is an acceptable answer but
it needs to be documented.

> +
> +	usage(builtin_commit_usage);
> +
> + match:
> +	switch (options[i].type) {
> +	case OPTION_NONE:
> +		*(int *)options[i].value = 1;
> +		break;
> +	case OPTION_STRING:
> +		if (value == NULL)
> +			die("option %s requires a value.", (*argv)[-1]);
> +		*(const char **)options[i].value = value;
> +		break;
> +	case OPTION_INTEGER:
> +		if (value == NULL)
> +			die("option %s requires a value.", (*argv)[-1]);
> +		*(int *)options[i].value = atoi(value);
> +		break;
> +	default:
> +		assert(0);
> +	}
> +
> +	(*argv)++;
> +
> +	return 1;
> +}

I do not particularly like this OPTION_LAST convention, but that
is a minor detail.  I also suspect in the longer term we might
be better off using getopt() or popt(), but that would be a
larger project.

In any case, if you want to use this option parser, you would
need to add a new file, perhaps parse_options.c, and move this
part there, so that other parts of the system can reuse it.

And the same comment goes for the launch_editor still in
builtin-tag.c.  We should move it to editor.c or something.

> +
> +static char *logfile, *force_author, *message, *template_file;
> +static char *edit_message, *use_message;
> +static int all, edit_flag, also, interactive, only, no_verify, amend, signoff;
> +static int quiet, verbose, untracked_files;
> +
> +static int no_edit, initial_commit, in_merge;
> +const char *only_include_assumed;
> +
> +static struct option commit_options[] = {
> +	{ OPTION_STRING, "file", 'F', (void *) &logfile },
> +	{ OPTION_NONE, "all", 'a', &all },
> +	{ OPTION_STRING, "author", 0, (void *) &force_author },
> +	{ OPTION_NONE, "edit", 'e', &edit_flag },

Why do some get casted to (void*) and others don't?  It doesn't
seem to have any pattern.  I am puzzled...

> +	{ OPTION_NONE, "include", 'i', &also },
> +	{ OPTION_NONE, "interactive", 0, &interactive },
> +	{ OPTION_NONE, "only", 'o', &only },
> +	{ OPTION_STRING, "message", 'm', &message },
> +	{ OPTION_NONE, "no-verify", 'n', &no_verify },
> +	{ OPTION_NONE, "amend", 0, &amend },
> +	{ OPTION_STRING, "reedit-message", 'c', &edit_message },
> +	{ OPTION_STRING, "reuse-message", 'C', &use_message },
> +	{ OPTION_NONE, "signoff", 's', &signoff },
> +	{ OPTION_NONE, "quiet", 'q', &quiet },
> +	{ OPTION_NONE, "verbose", 'v', &verbose },
> +	{ OPTION_NONE, "untracked-files", 0, &untracked_files },
> +	{ OPTION_STRING, "template", 't', &template_file },
> +	{ OPTION_LAST },
> +};
> +
> +/* FIXME: Taken from builtin-add, should be shared. */

You're darn right.  I thought you have some other patch that
touches builtin-add already...

> +
> +static void update_callback(struct diff_queue_struct *q,
> +			    struct diff_options *opt, void *cbdata)
> +{
> +	int i, verbose;
> +
> +	verbose = *((int *)cbdata);
> +	for (i = 0; i < q->nr; i++) {
> +		struct diff_filepair *p = q->queue[i];
> +		const char *path = p->one->path;
> +		switch (p->status) {
> +		default:
> +			die("unexpacted diff status %c", p->status);
> +		case DIFF_STATUS_UNMERGED:
> +		case DIFF_STATUS_MODIFIED:
> +		case DIFF_STATUS_TYPE_CHANGED:
> +			add_file_to_cache(path, verbose);
> +			break;
> +		case DIFF_STATUS_DELETED:
> +			remove_file_from_cache(path);
> +			cache_tree_invalidate_path(active_cache_tree, path);

I've updated remove_file_from_cache() to invalidate the path in
cache-tree so this does not hurt but is no longer necessary.  I
removed this line in the version I queued in 'pu'.

> +			if (verbose)
> +				printf("remove '%s'\n", path);
> +			break;
> +		}
> +	}
> +}
> +
> +static void
> +add_files_to_cache(int fd, const char **files, const char *prefix)
> +{
> +	struct rev_info rev;
> +
> +	init_revisions(&rev, "");
> +	setup_revisions(0, NULL, &rev, NULL);
> +	rev.prune_data = get_pathspec(prefix, files);
> +	rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
> +	rev.diffopt.format_callback = update_callback;
> +	rev.diffopt.format_callback_data = &verbose;
> +
> +	run_diff_files(&rev, 0);
> +	refresh_cache(REFRESH_QUIET);

Your update_callback() does add_file_to_cache() which picks up
the stat information from the filesystem already, so I do not
see much point calling refresh_cache() afterwards.  On the other
hand, refreshing before running diff_files() might make sense,
as you can avoid a lot of unnecessary work when you are told to
do "git commit ."

Have you tested this with an unmerged index?

> +
> +	if (write_cache(fd, active_cache, active_nr) || close(fd))
> +		die("unable to write new index file");
> +}
> +
> +static char *
> +prepare_index(const char **files, const char *prefix)
> +{
> +	int fd;
> +	struct tree *tree;
> +	struct lock_file *next_index_lock;
> +
> +	fd = hold_locked_index(&lock_file, 1);
> +	if (read_cache() < 0)
> +		die("index file corrupt");
> +
> +	if (all) {
> +		add_files_to_cache(fd, files, NULL);
> +		return lock_file.filename;

Should you be passing files, which is used as pathspec to decide
if your update_callback() should be called, under --all option?

> +	} else if (also) {
> +		add_files_to_cache(fd, files, prefix);
> +		return lock_file.filename;
> +	}
> +
> +	if (interactive)
> +		interactive_add();

Don't you need to

 (1) abort if the user aborts out of interactive add with ^C?
 (2) re-read the index after interactive_add() returns?

> +	if (*files == NULL) {
> +		/* Commit index as-is. */
> +		rollback_lock_file(&lock_file);
> +		return get_index_file();
> +	}
> +
> +	/*
> +	 * FIXME: Warn on unknown files.  Shell script does
> +	 *
> +	 *   commit_only=`git-ls-files --error-unmatch -- "$@"`
> +	 */

This should be much easier to do than from the shell script, as
you have active_cache[] (aka "the_index.cache[]") in-core.

> +	/*
> +	 * FIXME: shell script does
> +	 *
> +	 *   git-read-tree --index-output="$TMP_INDEX" -i -m HEAD
> +	 *
> +	 * which warns about unmerged files in the index.
> +	 */

I think "unmerged warning" is an unintended side effect.  The
point of the command is to have a copy of index, as there is no
way for shell script to work with more than one index at a time
without using a temporary file.

> +
> +	/* update the user index file */
> +	add_files_to_cache(fd, files, prefix);
> +
> +	if (!initial_commit) {
> +		tree = parse_tree_indirect(head_sha1);
> +		if (!tree)
> +			die("failed to unpack HEAD tree object");
> +		if (read_tree(tree, 0, NULL))
> +			die("failed to read HEAD tree object");
> +	}

Huh?  Doesn't this read_tree() defeat the add_files_to_cache()
you did earlier?

> +
> +	/* Uh oh, abusing lock_file to create a garbage collected file */
> +	next_index_lock = xmalloc(sizeof(*next_index_lock));
> +	fd = hold_lock_file_for_update(next_index_lock,
> +				       git_path("next-index-%d", getpid()), 1);

That's not an abuse, but I wonder what happened to the fd you
got at the beginning of the function.

> +	add_files_to_cache(fd, files, prefix);

and then this is puzzling to me.

I am starting to suspect that you might be better off if you do
not follow the use of temporary index file that was in the shell
script version to the letter.  You can use more than one index
in the core at the same time, now you are built-in.

> +
> +	return next_index_lock->filename;
> +}

... and if we were to go that route, wt_status structure would
have a pointer to the "struct index_state" instead of the
filename of the index... oops, wt_status_print() will discard
and re-read the cache from file, so that approach would not work
right now without fixing wt-status first.  Hmm.

> +static int run_status(FILE *fp, const char *index_file)
> +{
> +	struct wt_status s;
> +
> +	wt_status_prepare(&s);
> +
> +	if (amend) {
> +		s.amend = 1;
> +		s.reference = "HEAD^1";
> +	}
> +	s.verbose = verbose;
> +	s.untracked = untracked_files;
> +	s.index_file = index_file;
> +	s.fp = fp;
> +
> +	wt_status_print(&s);
> +
> +	return s.commitable;
> +}

I did not look at the rest of the patch; it should be either
obviously correct or outright incorrect --- anybody would notice
the breakage immediately.

The prepare_index() part is the most (and only) "interesting"
part of the puzzle.  I need to look at this again and think
about it a bit more.  It needs to:

 * save the current index and restore in case the whole
   "git-commit" is aborted in any way (e.g. ^C, empty log
   message from the editor); This is easy to do from C with
   hold_locked_index().

 * when doing a partial commit,
   - read HEAD in a temporary index, if not initial commit;
   - update the temporary index with the given paths;
   - write out the temporary index as a tree to build a commit;

   - update the real index the same way with the given paths;
   - if all goes well, commit the updated real index; again,
     this is easy in C with commit_locked_index();

 * when not doing a partial commit,
   - update the real index with given paths (if "--also") or all
     what diff-files reports (if "--all");
   - write out the real index as a tree to build a commit;
   - if all goes well, commit the updated real index; again,
     this is easy in C with commit_locked_index();

A thing to keep in mind is that you can write out the temporary
index as a tree from the core without writing it out first to a
temporary index file at all.  Perhaps the code should be
structured like this.

    - parse options and all the other necessary setup;

    - hold_locked_index() to lock the "real" index;

    - prepare_index() is responsibile for doing two things

      1. build, write out and return the tree object to be
         committed;

      2. leave the_index[] in the state to become the "real"
         index if this commit command is not aborted;

    - take the tree object, put commit log message around it and
      make a commit object;

    - commit_locked_index() to write out the "real" index.

Now, the task #1 for prepare_index() is simpler for "also" and
"all" case.  You do the equivalent of "git-add -u" or "git add
<path>" and run cache_tree_update() to write out the tree, and
when you are done, both the_index.cache and the_index.cache_tree
are up-to-date, ready to be written out by the caller at the
very end.

For a partial commit, the task is a bit more convoluted, as
writing out the tree needs to be done from outside the_index.  I
would say something like this:

	int prepare_index(unsigned char *sha1) {
		if (partial commit) {
                       struct index_state tmp_index;

                       /* "temporary index" only in-core */
                       memset(&tmp_index, 0, sizeof(tmp_index));
                       read_index(&tmp_index);
                       add_files_to_index(files, prefix, &tmp_index);
                       write_index_as_tree(&tmp_index, sha1);
                       discard_index(&tmp_index);

                       /* update the index the same way */
                       add_files_to_index(files, prefix, &the_index);
                       return ok;
		}
                /* otherwise */
                if (files && *files)
                	add_files_to_index(files, prefix, &the_index);
		else if (all)
                	add_files_to_index(NULL, prefix, &the_index);
		write_index_as_tree(&the_index, sha1);
                return ok;
	}

where

 * add_files_to_index() is similar to your add_files_to_cache()
   but takes struct index_state; the callback can still diff
   with the_index.cache (aka active_cache) as that diff is what
   we are interested in updating anyway;

 * write_index_as_tree() would be a new function that takes
   struct index_state and writes that as a tree.  It would
   essentially be the builtin-write-tree.c::write_tree()
   function except the opening and reading the index (or
   updating the cache-tree) part, something like:

write_index_as_tree(struct index_state *istate, unsigned char *sha1)
{
       	if (!cache_tree_fully_valid(istate->cache_tree))
        	cache_tree_update(istate->cache_tree,
                		  istate->cache,
                                  istate->cache_nr, 0, 0);
	hashcpy(sha1, istate->cache_tree->sha1);
}

^ permalink raw reply

* Re: [PATCH 2/5] Refactor struct transport_ops inlined into struct transport
From: Daniel Barkalow @ 2007-09-20  2:33 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Shawn O. Pearce, Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0709191409590.28395@racer.site>

On Wed, 19 Sep 2007, Johannes Schindelin wrote:

> Hi,
> 
> On Wed, 19 Sep 2007, Shawn O. Pearce wrote:
> 
> > diff --git a/transport.c b/transport.c
> > index cc76e3f..d8458dc 100644
> > --- a/transport.c
> > +++ b/transport.c
> > @@ -44,8 +44,6 @@ static int disconnect_walker(struct transport *transport)
> >  	return 0;
> >  }
> >  
> > -static const struct transport_ops rsync_transport;
> > -
> >  static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags) {
> >  	const char **argv;
> >  	int argc;
> > @@ -431,18 +406,31 @@ struct transport *transport_get(struct remote *remote, const char *url)
> >  	ret->url = url;
> >  
> >  	if (!prefixcmp(url, "rsync://")) {
> > -		ret->ops = &rsync_transport;
> > +		/* not supported; don't populate any ops */
> > +
> 
> That is sneaky.  What are the reasons to remove rsync support?  I know it 
> is deprecated, but I'd still like to have it, especially for initial 
> clones on small-RAMed machines.

It never got implemented in a way called from C. This is just removing the 
pointer to the empty struct where support would go. If anybody knows 
enough about interfacing with rsync to write the necessary functions, it 
can be restored.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH 0/5] Yet another builtin-fetch round
From: Daniel Barkalow @ 2007-09-20  2:40 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20070919044923.GP3099@spearce.org>

On Wed, 19 Sep 2007, Shawn O. Pearce wrote:

> Another short series for db/fetch-pack, still in pu.  Aside from
> optimizing the pipeline on the native transport (so we only invoke
> the remote process we need once vs. twice) I'm actually now quite
> comfortable with this whole series and think it is ready for next.

While it's still in pu, should these series of corrections be amended into 
the original series (for the ones that correct new code)? Most of the 
before-fixing states aren't worth saving as project history.

> I'm certainly running it in production, and will be until it is
> merged.  The performance difference is too big for me (and at least
> some of my coworkers) to not be doing so.  If there are any specific
> reasons why this topic is not ready for next or is unsuitable for
> merging please let me know so I can take the time to correct it.

Good to hear that it's working for you. It's been working for me since 
July, for my usage patterns, but they're not very extensive, and I was 
mostly relying on the tests.

> 1/5  Rename remote.uri to remote.url within remote handling internals
> 2/5  Refactor struct transport_ops inlined into struct transport
> 3/5  Always obtain fetch-pack arguments from struct fetch_pack_args
> 
>   These three are basic code cleanups for small issues that
>   bothered me about the original implementation of builtin-fetch.
>   Now is just as good of a time as any to cleanup the code and make
>   it more maintainable.  I think the overall total line count is
>   reduced by these three patches.

All of these look right.

> * Ensure builtin-fetch honors {fetch,transfer}.unpackLimit
> * Fix memory leaks when disconnecting transport instances
> 
>   Fixes two known (but minor) outstanding bugs.  At this point
>   I do not know of any other bugs in builtin-fetch so I would
>   really appreciate testing reports from other people, especially
>   those whose uses cases might stray outside of my workflow.  Hah,
>   I did not tell you my workflow.  ;-)

Looks good, but I didn't really check over them.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* [PATCH] Add git-rev-list --invert-match
From: Bart Trojanowski @ 2007-09-20  2:52 UTC (permalink / raw)
  To: git
In-Reply-To: <20070919202615.GK3076@jukie.net>

Example usage:

  git log --invert-match --grep="uninteresting"

  This command will prune out all commits that match the grep pattern.

How it works:

  The --invert-match flag sets invert_match, in rev_info.  This boolean
  is later checked in commit_match() and if set it inverts the result of
  grep_buffer().

Signed-off-by: Bart Trojanowski <bart@jukie.net>
---

This patch includes the Documentation update I missed in the first
patch.

 Documentation/git-rev-list.txt |    6 ++++++
 revision.c                     |   11 ++++++++++-
 revision.h                     |    3 ++-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 7cd0e89..f7a4891 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -29,6 +29,7 @@ SYNOPSIS
 	     [ \--(author|committer|grep)=<pattern> ]
 	     [ \--regexp-ignore-case | \-i ]
 	     [ \--extended-regexp | \-E ]
+	     [ \--invert-match ]
 	     [ \--date={local|relative|default|iso|rfc|short} ]
 	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
 	     [ \--pretty | \--header ]
@@ -238,6 +239,11 @@ limiting may be applied.
 	Consider the limiting patterns to be extended regular expressions
 	instead of the default basic regular expressions.
 
+--invert-match::
+
+	Show those parts of history that do not match any of the regular
+        expression patterns.
+
 --remove-empty::
 
 	Stop when a given path disappears from the tree.
diff --git a/revision.c b/revision.c
index 33d092c..57b2d0f 100644
--- a/revision.c
+++ b/revision.c
@@ -1182,6 +1182,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				regflags |= REG_ICASE;
 				continue;
 			}
+			if (!strcmp(arg, "--invert-match")) {
+				revs->invert_match = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--all-match")) {
 				all_match = 1;
 				continue;
@@ -1383,11 +1387,16 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
 
 static int commit_match(struct commit *commit, struct rev_info *opt)
 {
+	int result;
+
 	if (!opt->grep_filter)
 		return 1;
-	return grep_buffer(opt->grep_filter,
+
+	result = grep_buffer(opt->grep_filter,
 			   NULL, /* we say nothing, not even filename */
 			   commit->buffer, strlen(commit->buffer));
+
+	return opt->invert_match ? !result : result;
 }
 
 static struct commit *get_revision_1(struct rev_info *revs)
diff --git a/revision.h b/revision.h
index 98a0a8f..ead04a7 100644
--- a/revision.h
+++ b/revision.h
@@ -48,7 +48,8 @@ struct rev_info {
 			parents:1,
 			reverse:1,
 			cherry_pick:1,
-			first_parent_only:1;
+			first_parent_only:1,
+			invert_match:1;
 
 	/* Diff flags */
 	unsigned int	diff:1,
-- 
1.5.3.1.154.g734e65

^ permalink raw reply related

* Re: [PATCH] Add git-rev-list --invert-match
From: Junio C Hamano @ 2007-09-20  4:05 UTC (permalink / raw)
  To: Bart Trojanowski; +Cc: git
In-Reply-To: <20070920025225.GA12076@jukie.net>

> diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
> index 7cd0e89..f7a4891 100644
> --- a/Documentation/git-rev-list.txt
> +++ b/Documentation/git-rev-list.txt
> @@ -29,6 +29,7 @@ SYNOPSIS
>  	     [ \--(author|committer|grep)=<pattern> ]
>  	     [ \--regexp-ignore-case | \-i ]
>  	     [ \--extended-regexp | \-E ]
> +	     [ \--invert-match ]
>  	     [ \--date={local|relative|default|iso|rfc|short} ]
>  	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
>  	     [ \--pretty | \--header ]
> @@ -238,6 +239,11 @@ limiting may be applied.
>  	Consider the limiting patterns to be extended regular expressions
>  	instead of the default basic regular expressions.
>  
> +--invert-match::
> +
> +	Show those parts of history that do not match any of the regular
> +        expression patterns.
> +

Tabs and spaces?

I am afraid people may not understand what "any of the regular
expression patterns" means.  How about being a bit more
explicit, like this?

	When filtering the commits based on log message (`--grep`)
	or people (`--author` and `--committer`), show commits
	that do not match the criteria.

^ permalink raw reply

* Re: [PATCH 0/5] Yet another builtin-fetch round
From: Junio C Hamano @ 2007-09-20  4:08 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Shawn O. Pearce, git
In-Reply-To: <Pine.LNX.4.64.0709192233360.21941@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Wed, 19 Sep 2007, Shawn O. Pearce wrote:
>
>> Another short series for db/fetch-pack, still in pu.  Aside from
>> optimizing the pipeline on the native transport (so we only invoke
>> the remote process we need once vs. twice) I'm actually now quite
>> comfortable with this whole series and think it is ready for next.
>
> While it's still in pu, should these series of corrections be amended into 
> the original series (for the ones that correct new code)? Most of the 
> before-fixing states aren't worth saving as project history.

Yeah, I was wondering if that is a sane thing to do.  It is
merely additional work to arrive at the same tree state, but
might be a good investment in the longer term.

^ permalink raw reply

* Re: [PATCH] User Manual: add a chapter for submodules
From: Junio C Hamano @ 2007-09-20  4:15 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git, J. Bruce Fields
In-Reply-To: <20070920003413.GJ16235@genesis.frugalware.org>

Miklos Vajna <vmiklos@frugalware.org> writes:

> Signed-off-by: Michael Smith <msmith@cbnco.com>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>
> Sorry, I sent the original patch again. So here is the updated second version.

Looks Ok to me, although I didn't verify the examples by
actually running them myself this time (last round I did).

^ permalink raw reply

* Re: [PATCH 2/7] nfv?asprintf are broken without va_copy, workaround them.
From: Junio C Hamano @ 2007-09-20  4:27 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: gitster, git
In-Reply-To: <1190241736-30449-3-git-send-email-madcoder@debian.org>

Pierre Habouzit <madcoder@debian.org> writes:

> * drop nfasprintf.
> * move nfvasprintf into imap-send.c back, and let it work on a 8k buffer,
>   and die() in case of overflow. It should be enough for imap commands, if
>   someone cares about imap-send, he's welcomed to fix it properly.
> * replace nfvasprintf use in merge-recursive with a copy of the strbuf_addf
>   logic, it's one place, we'll live with it.
>   To ease the change, output_buffer string list is replaced with a strbuf ;)

While I'd agree with all of the above,

> * rework trace.c API's so that only one of the trace functions takes a
>   vararg. It's used to format strerror()s and git command names, it should
>   never be more than a few octets long, let it work on a 8k static buffer
>   with vsnprintf or die loudly.

and I'd agree with this in principle, there is a minor nit with
the implementation and use in trace.c.  E.g.

> diff --git a/exec_cmd.c b/exec_cmd.c
> index 9b74ed2..c0f954e 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -97,7 +97,8 @@ int execv_git_cmd(const char **argv)
>  		tmp = argv[0];
>  		argv[0] = git_command;
>  
> -		trace_argv_printf(argv, -1, "trace: exec:");
> +		trace_printf("trace: exec:");
> +		trace_argv(argv, -1);

This used to be a single call into trace.c which would format a
single string to write(2) out.  Now these two messages go
through separate write(2) and can be broken up.  I think the
atomicity of the log/trace message was the primary reason the
original had such a strange calling convention.

^ permalink raw reply

* Re: [PATCH 2/7] nfv?asprintf are broken without va_copy, workaround them.
From: Christian Couder @ 2007-09-20  4:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pierre Habouzit, git
In-Reply-To: <7vwsumkll8.fsf@gitster.siamese.dyndns.org>

Le jeudi 20 septembre 2007, Junio C Hamano a écrit :
> > diff --git a/exec_cmd.c b/exec_cmd.c
> > index 9b74ed2..c0f954e 100644
> > --- a/exec_cmd.c
> > +++ b/exec_cmd.c
> > @@ -97,7 +97,8 @@ int execv_git_cmd(const char **argv)
> >               tmp = argv[0];
> >               argv[0] = git_command;
> >  
> > -             trace_argv_printf(argv, -1, "trace: exec:");
> > +             trace_printf("trace: exec:");
> > +             trace_argv(argv, -1);
>
> This used to be a single call into trace.c which would format a
> single string to write(2) out.  Now these two messages go
> through separate write(2) and can be broken up.  I think the
> atomicity of the log/trace message was the primary reason the
> original had such a strange calling convention.

That's right.

Christian.

^ permalink raw reply

* Re: [PATCH 0/5] Yet another builtin-fetch round
From: Junio C Hamano @ 2007-09-20  5:09 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Shawn O. Pearce, git
In-Reply-To: <7v6426m110.fsf@gitster.siamese.dyndns.org>

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
>
>> On Wed, 19 Sep 2007, Shawn O. Pearce wrote:
>>
>>> Another short series for db/fetch-pack, still in pu.  Aside from
>>> optimizing the pipeline on the native transport (so we only invoke
>>> the remote process we need once vs. twice) I'm actually now quite
>>> comfortable with this whole series and think it is ready for next.
>>
>> While it's still in pu, should these series of corrections be amended into 
>> the original series (for the ones that correct new code)? Most of the 
>> before-fixing states aren't worth saving as project history.
>
> Yeah, I was wondering if that is a sane thing to do.  It is
> merely additional work to arrive at the same tree state, but
> might be a good investment in the longer term.

Heh, I did not realize that they are now all part of 'next' so
that's moot.

^ permalink raw reply

* [PATCH] rev-list --bisect: Fix best == NULL case.
From: Christian Couder @ 2007-09-20  5:23 UTC (permalink / raw)
  To: Junio Hamano; +Cc: git

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-rev-list.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

 Not sure if the "if (best)" check is really needed, but it looks safer
 with this patch.

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 899a31d..3894633 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -436,10 +436,10 @@ static struct commit_list *find_bisection(struct commit_list *list,
 	/* Do the real work of finding bisection commit. */
 	best = do_find_bisection(list, nr, weights);
 
-	if (best)
+	if (best) {
 		best->next = NULL;
-
-	*reaches = weight(best);
+		*reaches = weight(best);
+	}
 	free(weights);
 
 	return best;
-- 
1.5.3.2.80.g077d6f

^ permalink raw reply related

* Re: State of Perforce importing.
From: Dmitry Kakurin @ 2007-09-20  6:12 UTC (permalink / raw)
  To: David Brown; +Cc: Git
In-Reply-To: <20070918154918.GA19106@old.davidb.org>

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

I had to import quite a big Perforce depot too. And after some struggle it 
went fine.
For case mismatch: 
http://kb.perforce.com/AdminTasks/SuperuserTasks/CrossPlatfor..erMigration, 
bullet 10. It requires server access.
For a@b and for excluding one p4 path during migration you could use my 
quick-and-dirty fix (attached).
You can easily extend it to include and exclude multiple paths. Then it 
becomes as flexible as p4 client mapping.

- Dmitry
----- Original Message ----- 
From: "David Brown" <git@davidb.org>
Newsgroups: gmane.comp.version-control.git
To: "Sam Vilain" <sam@vilain.net>
Cc: "Git" <git@vger.kernel.org>
Sent: Tuesday, 18 September 2007 8:49
Subject: Re: State of Perforce importing.


> On Tue, Sep 18, 2007 at 07:27:13PM +1200, Sam Vilain wrote:
>
>>I'm pretty close to giving a newer one a spin, that actually imports
>>from the raw perforce back-end files without needing the perforce
>>server.  I am hoping that this should give a very clean import and will
>>be very fast and efficient, sending files that share ancestry to gfi in
>>sequence so that the on-the-fly delta system works.
>
> Unfortunately, this isn't something I'm going to be able to use.  The
> Perforce server will remain live, and resides on a machine I don't have
> access to.
>
>>It could possibly be adapted to use the p4 client (though I'd expect
>>that to be relatively slow per-revision), and possibly be extended to be
>>bidirectional as all of the upstream change number information is
>>recorded, a la git-svn.
>
> I was able to get 'git-p4' to work a lot better by using @all, but it 
> still
> has some problems, at least bad interactions with P4.
>
>   - It doesn't use any client spec.  Our P4 server space is a complete
>     mismash and has to be fixed up to get a sane directory layout.  For
>     example, some revisions have hundred-MB tar files sitting in the root
>     directory and I don't want that in the repo.  I also need to exclude
>     directories, and in some cases completely rearrange the directory
>     layout.
>
>   - Our P4 server is set to be case insensitive.  'git-p4' ignores paths
>     that come back from the server that are specified using a different
>     case.  Unfortunately, this means that a handful of files just get
>     randomly dropped from each revision.
>
>     I tried importing a client path instead of a depot path, but the names
>     that come back from 'p4 files' are depot based so none ever match.  I
>     end up with a nice revision history of entirely empty trees.
>
> I'm probably going to end up writing an importer that uses an actual 
> client
> workspace to let Perforce do the client mapping.  I'm also going to have 
> to
> put some work into some code to clean up the log messages, since most of
> our changes have as a first line "New Features:", which makes for a rather
> uninformative shortlog.
>
> But, I did learn about 'p4 -G' from git-p4 so that will help in getting
> information from the repository.
>
> Thanks,
> David 

[-- Attachment #2: 0001-git-p4-Added-exclude-option-and-use-P4-client-nam.patch --]
[-- Type: application/octet-stream, Size: 2664 bytes --]

>From a089b02239c3bc310956964d61075b093d26549f Mon Sep 17 00:00:00 2001
From: Dmitry Kakurin <Dmitry.Kakurin@gmail.com>
Date: Sun, 9 Sep 2007 13:58:12 -0700
Subject: [PATCH] git-p4: Added --exclude option and use P4 client name in commits

---
 contrib/fast-import/git-p4 |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index adaaae6..337854f 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -769,6 +769,7 @@ class P4Sync(Command):
         self.keepRepoPath = False
         self.depotPaths = None
         self.p4BranchesInGit = []
+        self.cloneExclude = None
 
         if gitConfig("git-p4.syncFromOrigin") == "false":
             self.syncWithOrigin = False
@@ -779,8 +780,12 @@ class P4Sync(Command):
         while commit.has_key("depotFile%s" % fnum):
             path =  commit["depotFile%s" % fnum]
 
-            found = [p for p in self.depotPaths
-                     if path.startswith (p)]
+            if self.cloneExclude and path.startswith( self.cloneExclude ):
+                found = False
+            else:
+                found = [p for p in self.depotPaths
+                         if path.startswith (p)]
+
             if not found:
                 fnum = fnum + 1
                 continue
@@ -905,6 +910,8 @@ class P4Sync(Command):
         else:
             committer = "%s <a@b> %s %s" % (author, epoch, self.tz)
 
+        committer = "%s <%s@%s> %s %s" % (author, details["user"], details["client"], epoch, self.tz)
+
         self.gitStream.write("committer %s\n" % committer)
 
         self.gitStream.write("data <<EOT\n")
@@ -1540,10 +1547,14 @@ class P4Clone(P4Sync):
         P4Sync.__init__(self)
         self.description = "Creates a new git repository and imports from Perforce into it"
         self.usage = "usage: %prog [options] //depot/path[@revRange]"
-        self.options.append(
+        self.options += [
             optparse.make_option("--destination", dest="cloneDestination",
                                  action='store', default=None,
-                                 help="where to leave result of the clone"))
+                                 help="where to leave result of the clone"),
+            optparse.make_option("--exclude", dest="cloneExclude",
+                                 action='store', default=None,
+                                 help="exclude depot path")
+        ]
         self.cloneDestination = None
         self.needsGit = False
 
-- 
1.5.3.mingw.1.1.g01e3a1


^ permalink raw reply related

* Re: [PATCH 1/5] strbuf API additions and enhancements.
From: Johannes Sixt @ 2007-09-20  6:17 UTC (permalink / raw)
  To: Edgar Toernig
  Cc: Pierre Habouzit, Junio C Hamano, Shawn O. Pearce,
	Git Mailing List
In-Reply-To: <20070919144604.7deca4f7.froese@gmx.de>

Edgar Toernig schrieb:
> Pierre Habouzit wrote:
>> +void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)
>> +{
>> +	int len;
>> +
>> +	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
>> +	if (len < 0) {
>> +		len = 0;
>> +	}
>> +	if (len > strbuf_avail(sb)) {
>> +		strbuf_grow(sb, len);
>> +		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
>> +		if (len > strbuf_avail(sb)) {
>> +			die("this should not happen, your snprintf is broken");
>> +		}
>> +	}
>> +	strbuf_setlen(sb, sb->len + len);
>> +}
> 
> The second vsnprintf won't work as the first one consumed all args
> from va_list ap.  You need to va_copy the ap.

Your analysis is not correct. The second vsnprintf receives the same 
argument pointer as the first, and, hence, consumes the same set of arguments.

You have to use va_copy in a variadic function, ie. if you are using 
va_start+va_end in the same function, but not in a function with a fixed 
list of arguments like this one.

-- Hannes

^ permalink raw reply

* Re: [PATCH] rev-list --bisect: Fix best == NULL case.
From: Junio C Hamano @ 2007-09-20  6:46 UTC (permalink / raw)
  To: Christian Couder; +Cc: git
In-Reply-To: <20070920072301.689b96c9.chriscool@tuxfamily.org>

The patch makes sense.  In fact you can crash any released
version like this:

    $ git rev-list --bisect v1.5.2..v1.5.0

I'll need to apply a moral equivalent to 'maint'.

By the way, you may find the attached patch an interesting food
for thought, on top of your patch.

    $ git rev-list --bisect-all --pretty=oneline v1.5.2.5..v1.5.3.2

---

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 3894633..e4ef172 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -9,6 +9,7 @@
 #include "revision.h"
 #include "list-objects.h"
 #include "builtin.h"
+#include "log-tree.h"
 
 /* bits #0-15 in revision.h */
 
@@ -74,6 +75,7 @@ static void show_commit(struct commit *commit)
 			parents = parents->next;
 		}
 	}
+	show_decorations(commit);
 	if (revs.commit_format == CMIT_FMT_ONELINE)
 		putchar(' ');
 	else
@@ -278,6 +280,57 @@ static struct commit_list *best_bisection(struct commit_list *list, int nr)
 	return best;
 }
 
+struct commit_dist {
+	struct commit *commit;
+	int distance;
+};
+
+static int compare_commit_dist(const void *a_, const void *b_)
+{
+	struct commit_dist *a, *b;
+
+	a = (struct commit_dist *)a_;
+	b = (struct commit_dist *)b_;
+	if (a->distance != b->distance)
+		return b->distance - a->distance; /* desc sort */
+	return hashcmp(a->commit->object.sha1, b->commit->object.sha1);
+}
+
+static struct commit_list *best_bisection_sorted(struct commit_list *list, int nr)
+{
+	struct commit_list *p;
+	struct commit_dist *array = xcalloc(nr, sizeof(*array));
+	int cnt, i;
+
+	for (p = list, cnt = 0; p; p = p->next) {
+		int distance;
+		unsigned flags = p->item->object.flags;
+
+		if (revs.prune_fn && !(flags & TREECHANGE))
+			continue;
+		distance = weight(p);
+		if (nr - distance < distance)
+			distance = nr - distance;
+		array[cnt].commit = p->item;
+		array[cnt].distance = distance;
+		cnt++;
+	}
+	qsort(array, cnt, sizeof(*array), compare_commit_dist);
+	for (p = list, i = 0; i < cnt; i++) {
+		struct name_decoration *r = xmalloc(sizeof(*r) + 100);
+		struct object *obj = &(array[i].commit->object);
+
+		sprintf(r->name, "dist=%d", array[i].distance);
+		r->next = add_decoration(&name_decoration, obj, r);
+		p->item = array[i].commit;
+		p = p->next;
+	}
+	if (p)
+		p->next = NULL;
+	free(array);
+	return list;
+}
+
 /*
  * zero or positive weight is the number of interesting commits it can
  * reach, including itself.  Especially, weight = 0 means it does not
@@ -292,7 +345,8 @@ static struct commit_list *best_bisection(struct commit_list *list, int nr)
  * or positive distance.
  */
 static struct commit_list *do_find_bisection(struct commit_list *list,
-					     int nr, int *weights)
+					     int nr, int *weights,
+					     int find_all)
 {
 	int n, counted;
 	struct commit_list *p;
@@ -351,7 +405,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 		clear_distance(list);
 
 		/* Does it happen to be at exactly half-way? */
-		if (halfway(p, nr))
+		if (!find_all && halfway(p, nr))
 			return p;
 		counted++;
 	}
@@ -389,19 +443,22 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 				weight_set(p, weight(q));
 
 			/* Does it happen to be at exactly half-way? */
-			if (halfway(p, nr))
+			if (!find_all && halfway(p, nr))
 				return p;
 		}
 	}
 
 	show_list("bisection 2 counted all", counted, nr, list);
 
-	/* Then find the best one */
-	return best_bisection(list, nr);
+	if (!find_all)
+		return best_bisection(list, nr);
+	else
+		return best_bisection_sorted(list, nr);
 }
 
 static struct commit_list *find_bisection(struct commit_list *list,
-					  int *reaches, int *all)
+					  int *reaches, int *all,
+					  int find_all)
 {
 	int nr, on_list;
 	struct commit_list *p, *best, *next, *last;
@@ -434,14 +491,12 @@ static struct commit_list *find_bisection(struct commit_list *list,
 	weights = xcalloc(on_list, sizeof(*weights));
 
 	/* Do the real work of finding bisection commit. */
-	best = do_find_bisection(list, nr, weights);
-
-	if (best) {
+	best = do_find_bisection(list, nr, weights, find_all);
+	if (!find_all && best) {
 		best->next = NULL;
 		*reaches = weight(best);
 	}
 	free(weights);
-
 	return best;
 }
 
@@ -468,6 +523,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	int i;
 	int read_from_stdin = 0;
 	int bisect_show_vars = 0;
+	int bisect_find_all = 0;
 
 	git_config(git_default_config);
 	init_revisions(&revs, prefix);
@@ -490,6 +546,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 			bisect_list = 1;
 			continue;
 		}
+		if (!strcmp(arg, "--bisect-all")) {
+			bisect_list = 1;
+			bisect_find_all = 1;
+			continue;
+		}
 		if (!strcmp(arg, "--bisect-vars")) {
 			bisect_list = 1;
 			bisect_show_vars = 1;
@@ -536,7 +597,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (bisect_list) {
 		int reaches = reaches, all = all;
 
-		revs.commits = find_bisection(revs.commits, &reaches, &all);
+		revs.commits = find_bisection(revs.commits, &reaches, &all,
+					      bisect_find_all);
 		if (bisect_show_vars) {
 			int cnt;
 			if (!revs.commits)
diff --git a/log-tree.c b/log-tree.c
index a642371..32e360b 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -15,7 +15,7 @@ static void show_parents(struct commit *commit, int abbrev)
 	}
 }
 
-static void show_decorations(struct commit *commit)
+void show_decorations(struct commit *commit)
 {
 	const char *prefix;
 	struct name_decoration *decoration;
diff --git a/log-tree.h b/log-tree.h
index e82b56a..b33f7cd 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -12,5 +12,6 @@ int log_tree_diff_flush(struct rev_info *);
 int log_tree_commit(struct rev_info *, struct commit *);
 int log_tree_opt_parse(struct rev_info *, const char **, int);
 void show_log(struct rev_info *opt, const char *sep);
+void show_decorations(struct commit *commit);
 
 #endif

^ permalink raw reply related

* Re: [PATCH] rev-list --bisect: Fix best == NULL case.
From: Junio C Hamano @ 2007-09-20  7:08 UTC (permalink / raw)
  To: Christian Couder; +Cc: git
In-Reply-To: <7vmyvhltpy.fsf@gitster.siamese.dyndns.org>

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

> The patch makes sense.  In fact you can crash any released
> version like this:
>
>     $ git rev-list --bisect v1.5.2..v1.5.0
>
> I'll need to apply a moral equivalent to 'maint'.

I spoke too fast.  This is not broken in any released version.

Only with the patch from somebody that broke it with commit
ce0cbad7727457854d631a6314d9aee7c837cb65 (rev-list --bisect:
Move finding bisection into do_find_bisection.), the "empty set"
case would segfault ;-).

^ 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