Git development
 help / color / mirror / Atom feed
* Re: Bizarre missing changes (git bug?)
From: Roman Zippel @ 2008-07-30  1:14 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Tim Harper, git
In-Reply-To: <alpine.LFD.1.10.0807290838360.3334@nehalem.linux-foundation.org>

Hi,

On Tue, 29 Jul 2008, Linus Torvalds wrote:

> On Tue, 29 Jul 2008, Roman Zippel wrote:
> > 
> > I'm not dismissing it, but your focus is on how to get this result.
> 
> No, you misunderstand.
> 
> My focus is really on one single thing:
> 
>  - performance
> 
> with a smaller focus on the fact that I simply don't see how it's 
> _possible_ to do better than our current all-or-nothing approach of 
> simplification (eg either extreme simplification or none at all: nothing 
> or --full-history).

That's exactly what I'm not dismissing as you claim, but I've hit the 
problem where this approach simply produces crap, so I'm foremost 
interested in getting a useful result, only after that I'm interested in 
the performance (which I think is possible).

> So here's my challenge again, which you seem to have TOTALLY MISSED.
> 
> Make this be fast:
> 
> 	time sh -c "git log <filename> | head"
> 
> nothing else matters. If you can make that one be fast, I'm happy. 

I already explained it, but you simply dismissed it. It's possible, but it 
requires a bit of cached information (e.g. as part of the pack file, which 
is needed for decent performance anyway).

> In fact, you can see what I'm talking about by trying --topo-order in the 
> above timing test.

Please give me full example.
gitk --topo-order kernel/printk.c shows no difference (e.g. it doesn't 
show 02630a12c7f72fa294981c8d86e38038781c25b7), several experiments with 
git-rev-list show no improvement either.

> > > And quite frankly, I've seen that behaviour from you before, when it comes 
> > > to other things.
> > 
> > What exact behaviour is that? That I dare to disagree with you?
> 
> No. The fact that you like arguing _pointlessly_, and just being abrasive, 
> without actually helping or understanding the big picture.

The problem is that your picture doesn't include my specific problem, I'm 
very interested in the big picture, but I'd like to be in it.

> I'm thinking 
> back on the whole scheduler thing. You weren't arguing with _me_, but you 
> had the same modus operandi.

Well, it seems I have talent for finding the special cases, e.g. last time 
I tested the scheduler it was performing twice as bad as the old scheduler 
on m68k. I've also seen cases where it sacrifices throughput for 
interactivity.
Anyway, this is the wrong place for it anyway, the problem I'm hitting is 
these "good enough" solutions, which work in most situations, but fail in 
a few special situations, but nobody is interested to get these right 
unless your name is Linus.

bye, Roman

^ permalink raw reply

* Re: [trivial fast-export PATCH] Fix typo in documentation
From: Nick Andrew @ 2008-07-30  1:05 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0807291404390.4631@eeepc-johanness>

On Tue, Jul 29, 2008 at 02:06:13PM +0200, Johannes Schindelin wrote:
> On Tue, 29 Jul 2008, Nick Andrew wrote:
> 
> >  hg-fast-export.txt |   10 +++++-----
> 
> I do not see this in git.git.  Maybe you meant to Cc: Simon Hausmann, 
> Rocco Rutte or Han-Wen?
> 
> Or Chris Lee, who seems to be the owner of fast-export.git.

Thanks; I forwarded the patch to Chris Lee.

Nick.

^ permalink raw reply

* Re: [PATCH] Documentation: Remove mentions of git-svnimport.
From: Brian Gernhardt @ 2008-07-30  0:57 UTC (permalink / raw)
  To: Pieter de Bie; +Cc: Git List, Junio C Hamano
In-Reply-To: <F11D5504-5FA1-4FF7-9D53-032D5027EBB9@ai.rug.nl>


On Jul 29, 2008, at 8:44 PM, Pieter de Bie wrote:

> On Jul 30, 2008, at 2:38 AM, Brian Gernhardt wrote:
>
>> +respository.  'git-svn' is especially useful when it comes to  
>> tracking
>> +repositories not organized in the way Subversion developers  
>> recommend
>> +(trunk, branches, tags directories).
>
> This is of course not true. Git svn is especially useful when  
> branches _are_ organized
> that way, but other configurations are somewhat supported. This  
> comment still refers
> to the comparison with svnimport.

The original statement was in a paragraph by itself, so I assumed it  
was a not a comparison.  Reading it again, I can see your point.

~~ Brian

^ permalink raw reply

* Re: Bizarre missing changes (git bug?)
From: Linus Torvalds @ 2008-07-30  0:48 UTC (permalink / raw)
  To: Roman Zippel; +Cc: Martin Langhoff, Tim Harper, git
In-Reply-To: <alpine.LFD.1.10.0807291716060.3334@nehalem.linux-foundation.org>



On Tue, 29 Jul 2008, Linus Torvalds wrote:
> 
>  - But the other huge mistake you make is EVEN MORE STUPID, because it's 
>    so ironic. That magical output you want, and claim is so perfect, and 
>    point out "thus you can still apply as much simplification as you want 
>    on top of it"? You know what? It already _exists_! It's exactly that 
>    --full-history case.

Put in other terms: what you ask for can be fairly trivially done as a 
filter on the _current_ git output (preferably merged into the tool that 
shows it graphically in the first place), with absolutely no downside.

In contrast, if somebody was really so _stupid_ as to go with your output 
format, then yes, he could further simplify it down to the current default 
format, but with a huge performance/interactivity downside.

See? Your preferred format is not actually the "best" format. Not at all. 
Quite the reverse. Your preferred format is much better off being a 
secondary post-processing format exactly because it can be generated from 
one of the primary formats easily enough.

But the reverse isn't true: the current primary formats cannot be 
generated from your preferred format without losing something important 
(performance).

But I'll make you a deal: if you actually write the filter in C form, I 
can pretty much guarantee that we can easily add it as a new flag. It 
really should be pretty easy to integrate it into the revision parsing 
machinery alongside --topo-order, since it's really the same kind of 
operation.

In fact, it's possible that the current --topo-order sorting could 
possibly be made to just do the simplification (conditionally, of course, 
since it has the latency problem). See the function

	void sort_in_topological_order(struct commit_list ** list, int lifo)

in commit.c - that's where it would hook in.

		Linus

^ permalink raw reply

* Re: [PATCH] Documentation: Remove mentions of git-svnimport.
From: Pieter de Bie @ 2008-07-30  0:44 UTC (permalink / raw)
  To: Brian Gernhardt; +Cc: Git List, Junio C Hamano
In-Reply-To: <1217378299-733-1-git-send-email-benji@silverinsanity.com>


On Jul 30, 2008, at 2:38 AM, Brian Gernhardt wrote:

> +respository.  'git-svn' is especially useful when it comes to  
> tracking
> +repositories not organized in the way Subversion developers recommend
> +(trunk, branches, tags directories).

This is of course not true. Git svn is especially useful when branches  
_are_ organized
that way, but other configurations are somewhat supported. This  
comment still refers
to the comparison with svnimport.

^ permalink raw reply

* Re: git svn documentation
From: Brian Gernhardt @ 2008-07-30  0:39 UTC (permalink / raw)
  To: Jurko Gospodnetić; +Cc: git
In-Reply-To: <g6o6j6$k7$1@ger.gmane.org>


On Jul 29, 2008, at 6:45 PM, Jurko Gospodnetić wrote:

>  git svn documentation should be updated so it no longer references  
> the no longer supported 'git svnimport' command. Currently this  
> causes an invalid link to be added to the git svn html documentation.

Thanks for the report.  I wrote a patch for this and sent it to the  
list, but forgot to send it as a reply to your message.

~~ Brian

^ permalink raw reply

* [PATCH] Documentation: Remove mentions of git-svnimport.
From: Brian Gernhardt @ 2008-07-30  0:38 UTC (permalink / raw)
  To: Git List; +Cc: Junio C Hamano

git-svnimport is no longer supported, so don't mention it in the
documentation.  This also updates the description, removing the
historical discussion, since it mostly dealt with how it differed from
svnimport.

Noticed by Jurko Gospodnetić <jurko.gospodnetic@docte.hr>

Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
---
 Documentation/git-svn.txt |   22 ++++++----------------
 1 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index e7c0f1c..4d82a08 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -12,18 +12,10 @@ SYNOPSIS
 DESCRIPTION
 -----------
 'git-svn' is a simple conduit for changesets between Subversion and git.
-It is not to be confused with linkgit:git-svnimport[1], which is
-read-only.
-
-'git-svn' was originally designed for an individual developer who wants a
-bidirectional flow of changesets between a single branch in Subversion
-and an arbitrary number of branches in git.  Since its inception,
-'git-svn' has gained the ability to track multiple branches in a manner
-similar to 'git-svnimport'.
-
-'git-svn' is especially useful when it comes to tracking repositories
-not organized in the way Subversion developers recommend (trunk,
-branches, tags directories).
+It provides a bidirectional flow of changes between a Subversion and a git
+respository.  'git-svn' is especially useful when it comes to tracking
+repositories not organized in the way Subversion developers recommend
+(trunk, branches, tags directories).
 
 COMMANDS
 --------
@@ -218,8 +210,7 @@ Any other arguments are passed directly to 'git-log'
 
 'commit-diff'::
 	Commits the diff of two tree-ish arguments from the
-	command-line.  This command is intended for interoperability with
-	'git-svnimport' and does not rely on being inside an `git-svn
+	command-line.  This command does not rely on being inside an `git-svn
 	init`-ed repository.  This command takes three arguments, (a) the
 	original tree to diff against, (b) the new tree result, (c) the
 	URL of the target Subversion repository.  The final argument
@@ -317,8 +308,7 @@ config key: svn.findcopiesharder
 -A<filename>::
 --authors-file=<filename>::
 
-Syntax is compatible with the files used by 'git-svnimport' and
-'git-cvsimport':
+Syntax is compatible with the file used by 'git-cvsimport':
 
 ------------------------------------------------------------------------
 	loginname = Joe User <user@example.com>
-- 
1.6.0.rc1.154.ge3fc

^ permalink raw reply related

* Re: Bizarre missing changes (git bug?)
From: Linus Torvalds @ 2008-07-30  0:32 UTC (permalink / raw)
  To: Roman Zippel; +Cc: Martin Langhoff, Tim Harper, git
In-Reply-To: <Pine.LNX.4.64.0807291433430.6791@localhost.localdomain>



On Wed, 30 Jul 2008, Roman Zippel wrote:
> 
> What the short simplified history is more pure laziness

No.

Roman, you're an idiot who doesn't even _understand_ what you are talking 
about. Sadly, you then _think_ you are so smart that you then refuse to 
even consider the fact that others disagree, so you don't even read what 
they write.

Go to my previous email in this thread. Do the example. Look at the 
simplified version. Ponder.

It's not "pure lazyness" when you get the simplified version. It's 
actually a MORE USEFUL RESULT! The simplified version shows the minimal 
explanation of how things ended up the way they are, and that is damn 
useful. What you want is extra _clutter_ most of the time.

It's really sad how you cannot get over your own prejudices here. 

So Roman. Go back, read my previous email in this thread. It's message ID 
is

	<alpine.LFD.1.10.0807291006070.3334@nehalem.linux-foundation.org>

in case it helps you find it.

Read it twice, or three times. Read it with the notion that maybe you 
didn't know best after all. Read it with the possibility that maybe there 
are smarter people than you, and people who have actually worked with git 
for several years.

And if you can't do that, at least stop cc'ing me with your idiocy.

To get to the meat of your email:

> The point I'm trying to make is that the compact history graph has the 
> potential to completely replace the simplified history. The only problem 
> is that it needs a bit of cached extra information, then it can be as fast 
> the short simplified history for the common case and it still can produce 
> as much information as the full simplified history, thus you can still 
> apply as much simplification as you want on top of it.

You're simply full of sh*t. You make two huge mistakes, and I'll spend 
another few minutes of my life trying to educate you one final more time, 
even though from every single indication I have so far, you are unable to 
learn simply because you think you already know the answer.

Your two mistakes are:

 - your "only" problem is fundamental.  It's unsolvable. Git history 
   simplification isn't per-file or even per-directory.  It's 
   per-any-random-set-of-pathnames. You can't "cache" the simplified 
   information, and it's not "a bit" of cached extra info. It's 
   fundamentally a metric truckload of info.

   With a cache, you can make the performance of a repeated query go fast, 
   but that's totally uninteresting.

 - But the other huge mistake you make is EVEN MORE STUPID, because it's 
   so ironic. That magical output you want, and claim is so perfect, and 
   point out "thus you can still apply as much simplification as you want 
   on top of it"? You know what? It already _exists_! It's exactly that 
   --full-history case.

   Can you not see that? That's exactly that --full-history --parents 
   cases. It gives you the full information. You can simplify it to what 
   you want, exactly because it did _not_ simplify things for you. I've 
   even told you so, multiple times, when I suggested you try to do that 
   simplification in "gitk".

In other words, git has the two cases you want: the "extreme simplified 
history" (that is nice to see what really _mattered_, with no extra 
unnecessary duplicate history that didn't actually affect the end result), 
and the "full" history (ooh, I know, we could make a command line called 
"--full-history" to get the latter, so that people who wanted to see it 
all and perhaps distill it to something else could do so).

And I've told you over and over what you should look at, and I've told you 
over and over that the default is actually the _useful_ case, and why. But 
you seem to refuse to listen. You just close your ears and repeat your 
mantra, even though people smarter than you have told you why it's done 
the way it's done. 

Stop stuffing your ears. Listen to what people tell you.

		Linus

^ permalink raw reply

* Re: Bizarre missing changes (git bug?)
From: Martin Langhoff @ 2008-07-30  0:25 UTC (permalink / raw)
  To: Roman Zippel; +Cc: Linus Torvalds, Tim Harper, git
In-Reply-To: <Pine.LNX.4.64.0807291433430.6791@localhost.localdomain>

On Wed, Jul 30, 2008 at 12:16 PM, Roman Zippel <zippel@linux-m68k.org> wrote:
> I already did the prototype

afaict people around here are only interested if it can be done
without losing the early-output niceness of current git-log. That it
can be worked out in a "put it all in memory and work it in there"
model is _not_ interesting for git.

cheers,



m
-- 
 martin.langhoff@gmail.com
 martin@laptop.org -- School Server Architect
 - ask interesting questions
 - don't get distracted with shiny stuff - working code first
 - http://wiki.laptop.org/go/User:Martinlanghoff

^ permalink raw reply

* Re: Bizarre missing changes (git bug?)
From: Roman Zippel @ 2008-07-30  0:16 UTC (permalink / raw)
  To: Martin Langhoff; +Cc: Linus Torvalds, Tim Harper, git
In-Reply-To: <46a038f90807282015m7ce3da10h71dfee221c960332@mail.gmail.com>

Hi,

On Tue, 29 Jul 2008, Martin Langhoff wrote:

> On Tue, Jul 29, 2008 at 2:59 PM, Roman Zippel <zippel@linux-m68k.org> wrote:
> > Can we please get past this and look at what is required to produce the
> > correct history?
> 
> Roman - correct is --full-history -- any simplification that makes it
> easy on your eyes *is* a simplification. And consumers that want to do
> nice user-friendly simplification like gitk does can hang off the data
> stream.

I don't quite understand what you're trying to say.
To avoid further confusion it maybe helps to specify a few of the terms:

- full history graph: produced by "git-log --full-history --parents"
- compact history graph: the full history graph without without any 
  repeated merges, this is what my example script produces.
- full simplified history: output of "git-log --full-history"
- short simplified history: standard output of "git-log"

The important part about the history graphs is that all commits are 
properly connected in it (i.e. all except the head commit have a child), 
This is needed to know if you don't just what want to know what happened, 
but also how it got merged, also any graphical interface needs it to 
produce a useful history graph.

What the short simplified history is more pure laziness, it's fast and 
gets the most common cases right, but in order to do this it has to ignore 
part of the history. The full simplified history at least produces 
produces the full change history, but it lacks part of the merge history 
and it stills takes longer to generate.

The point I'm trying to make is that the compact history graph has the 
potential to completely replace the simplified history. The only problem 
is that it needs a bit of cached extra information, then it can be as fast 
the short simplified history for the common case and it still can produce 
as much information as the full simplified history, thus you can still 
apply as much simplification as you want on top of it.

Keep in mind that e.g. git-web is using the full simplified history, so 
what I'm offering also has the potential to improve git-web performance...

> > it's also possible to update it when merging/pulling new data.
> 
> If that's what you want to do, you can prototype it with a hook on
> fetch and commit. That is definitely an area that hasn't been explored
> - what nicer (but expensive) views on the history we have can be
> afforded by pre-computing things on fetch and commit hooks.

I already did the prototype, I know how to generate that information, the 
problem is to get that information to the various graphical interfaces.

bye, Roman

^ permalink raw reply

* [PATCH] format-patch: Produce better output with --inline or --attach
From: Kevin Ballard @ 2008-07-29 23:49 UTC (permalink / raw)
  To: git; +Cc: Kevin Ballard, Junio C Hamano

This patch makes two small changes to improve the output of --inline and --attach.

The first is to write a newline preceding the boundary. This is needed because
MIME defines the encapsulation boundary as including the preceding CRLF (or in
this case, just LF), so we should be writing one. Without this, the last newline
in the pre-diff content is consumed instead.

The second change is to always write the line termination character (default: newline)
even when using --inline or --attach. This is simply to improve the aesthetics of
the resulting message. When using --inline an email client should render the
resulting message identically to the non-inline version. And when using --attach
this adds a blank line preceding the attachment in the email, which is visually
attractive.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---
 diff.c                                             |    3 +--
 log-tree.c                                         |    2 +-
 ....format-patch_--attach_--stdout_initial..master |    6 ++++++
 ...format-patch_--attach_--stdout_initial..master^ |    4 ++++
 ...ff.format-patch_--attach_--stdout_initial..side |    2 ++
 ...tdout_--subject-prefix=TESTCASE_initial..master |    6 ++++++
 ....format-patch_--inline_--stdout_initial..master |    6 ++++++
 ...format-patch_--inline_--stdout_initial..master^ |    4 ++++
 ...ormat-patch_--inline_--stdout_initial..master^^ |    2 ++
 ...ff.format-patch_--inline_--stdout_initial..side |    2 ++
 10 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index a07812c..cbf2547 100644
--- a/diff.c
+++ b/diff.c
@@ -3223,11 +3223,10 @@ void diff_flush(struct diff_options *options)
 
 	if (output_format & DIFF_FORMAT_PATCH) {
 		if (separator) {
+			putc(options->line_termination, options->file);
 			if (options->stat_sep) {
 				/* attach patch instead of inline */
 				fputs(options->stat_sep, options->file);
-			} else {
-				putc(options->line_termination, options->file);
 			}
 		}
 
diff --git a/log-tree.c b/log-tree.c
index 5505606..bd8b9e4 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -198,7 +198,7 @@ void log_write_email_headers(struct rev_info *opt, const char *name,
 		extra_headers = subject_buffer;
 
 		snprintf(buffer, sizeof(buffer) - 1,
-			 "--%s%s\n"
+			 "\n--%s%s\n"
 			 "Content-Type: text/x-patch;"
 			 " name=\"%s.diff\"\n"
 			 "Content-Transfer-Encoding: 8bit\n"
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_initial..master b/t/t4013/diff.format-patch_--attach_--stdout_initial..master
index cf6891f..43346b9 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_initial..master
+++ b/t/t4013/diff.format-patch_--attach_--stdout_initial..master
@@ -19,6 +19,8 @@ This is the second commit.
  file2   |    3 ---
  3 files changed, 5 insertions(+), 3 deletions(-)
  delete mode 100644 file2
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
 Content-Transfer-Encoding: 8bit
@@ -75,6 +77,8 @@ Content-Transfer-Encoding: 8bit
  file1   |    3 +++
  2 files changed, 5 insertions(+), 0 deletions(-)
  create mode 100644 file1
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
 Content-Transfer-Encoding: 8bit
@@ -122,6 +126,8 @@ Content-Transfer-Encoding: 8bit
  file3   |    4 ++++
  3 files changed, 9 insertions(+), 0 deletions(-)
  create mode 100644 file3
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
 Content-Transfer-Encoding: 8bit
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_initial..master^ b/t/t4013/diff.format-patch_--attach_--stdout_initial..master^
index fe02587..d7490a9 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_initial..master^
+++ b/t/t4013/diff.format-patch_--attach_--stdout_initial..master^
@@ -19,6 +19,8 @@ This is the second commit.
  file2   |    3 ---
  3 files changed, 5 insertions(+), 3 deletions(-)
  delete mode 100644 file2
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
 Content-Transfer-Encoding: 8bit
@@ -75,6 +77,8 @@ Content-Transfer-Encoding: 8bit
  file1   |    3 +++
  2 files changed, 5 insertions(+), 0 deletions(-)
  create mode 100644 file1
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
 Content-Transfer-Encoding: 8bit
diff --git a/t/t4013/diff.format-patch_--attach_--stdout_initial..side b/t/t4013/diff.format-patch_--attach_--stdout_initial..side
index 9ff828e..38f7902 100644
--- a/t/t4013/diff.format-patch_--attach_--stdout_initial..side
+++ b/t/t4013/diff.format-patch_--attach_--stdout_initial..side
@@ -17,6 +17,8 @@ Content-Transfer-Encoding: 8bit
  file3   |    4 ++++
  3 files changed, 9 insertions(+), 0 deletions(-)
  create mode 100644 file3
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
 Content-Transfer-Encoding: 8bit
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master b/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
index a8093be..fca5cce 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
+++ b/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
@@ -19,6 +19,8 @@ This is the second commit.
  file2   |    3 ---
  3 files changed, 5 insertions(+), 3 deletions(-)
  delete mode 100644 file2
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
 Content-Transfer-Encoding: 8bit
@@ -75,6 +77,8 @@ Content-Transfer-Encoding: 8bit
  file1   |    3 +++
  2 files changed, 5 insertions(+), 0 deletions(-)
  create mode 100644 file1
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
 Content-Transfer-Encoding: 8bit
@@ -122,6 +126,8 @@ Content-Transfer-Encoding: 8bit
  file3   |    4 ++++
  3 files changed, 9 insertions(+), 0 deletions(-)
  create mode 100644 file3
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
 Content-Transfer-Encoding: 8bit
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..master b/t/t4013/diff.format-patch_--inline_--stdout_initial..master
index aa110c0..6d6fac3 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..master
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master
@@ -19,6 +19,8 @@ This is the second commit.
  file2   |    3 ---
  3 files changed, 5 insertions(+), 3 deletions(-)
  delete mode 100644 file2
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
 Content-Transfer-Encoding: 8bit
@@ -75,6 +77,8 @@ Content-Transfer-Encoding: 8bit
  file1   |    3 +++
  2 files changed, 5 insertions(+), 0 deletions(-)
  create mode 100644 file1
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
 Content-Transfer-Encoding: 8bit
@@ -122,6 +126,8 @@ Content-Transfer-Encoding: 8bit
  file3   |    4 ++++
  3 files changed, 9 insertions(+), 0 deletions(-)
  create mode 100644 file3
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
 Content-Transfer-Encoding: 8bit
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^
index 95e9ea4..18a1110 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^
@@ -19,6 +19,8 @@ This is the second commit.
  file2   |    3 ---
  3 files changed, 5 insertions(+), 3 deletions(-)
  delete mode 100644 file2
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
 Content-Transfer-Encoding: 8bit
@@ -75,6 +77,8 @@ Content-Transfer-Encoding: 8bit
  file1   |    3 +++
  2 files changed, 5 insertions(+), 0 deletions(-)
  create mode 100644 file1
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0.diff"
 Content-Transfer-Encoding: 8bit
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^
index b8e81e1..4f258b8 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..master^^
@@ -19,6 +19,8 @@ This is the second commit.
  file2   |    3 ---
  3 files changed, 5 insertions(+), 3 deletions(-)
  delete mode 100644 file2
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44.diff"
 Content-Transfer-Encoding: 8bit
diff --git a/t/t4013/diff.format-patch_--inline_--stdout_initial..side b/t/t4013/diff.format-patch_--inline_--stdout_initial..side
index 86ae923..e86dce6 100644
--- a/t/t4013/diff.format-patch_--inline_--stdout_initial..side
+++ b/t/t4013/diff.format-patch_--inline_--stdout_initial..side
@@ -17,6 +17,8 @@ Content-Transfer-Encoding: 8bit
  file3   |    4 ++++
  3 files changed, 9 insertions(+), 0 deletions(-)
  create mode 100644 file3
+
+
 --------------g-i-t--v-e-r-s-i-o-n
 Content-Type: text/x-patch; name="c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a.diff"
 Content-Transfer-Encoding: 8bit
-- 
1.6.0.rc1.166.gbbfa8.dirty

^ permalink raw reply related

* Re: What is 'git BRANCH'?
From: Eric Raible @ 2008-07-29 23:45 UTC (permalink / raw)
  To: git
In-Reply-To: <7v63qob9hz.fsf@gitster.siamese.dyndns.org>

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

> diff --git a/git.c b/git.c
> index 37b1d76..c99e769 100644
> --- a/git.c
> +++ b/git.c
> @@ -448,7 +448,7 @@ int main(int argc, const char **argv)
>  		cmd += 4;
>  		argv[0] = cmd;
>  		handle_internal_command(argc, argv);
> -		die("cannot handle %s internally", cmd);
> +		help_unknown_cmd(cmd);
>  	}

Which on windows leads to the less-than-friendly:

git BRANCH =>
git: 'BRANCH.exe' is not a git-command. See 'git --help'.

I wonder if it wouldn't be better to simply to a case-insensitive
comparison when comparing against the builtin array.  Alternatively,
at least the the extension (if any) ought to be stripped.

I'd be glad to come up with the patch for either given the word...

- Eric

^ permalink raw reply

* Re: [PATCH 3/4] Add a new test for using a custom merge strategy
From: Junio C Hamano @ 2008-07-29 23:43 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <ee13073cd83ff4a3cffb926b21cc54583e67f869.1217372486.git.vmiklos@frugalware.org>

Miklos Vajna <vmiklos@frugalware.org> writes:

> +cat > git-merge-theirs << EOF
> +#!/bin/sh
> +eval git read-tree --reset -u \\\$\$#
> +EOF

This should be $SHELL_PATH, instead of hardcoded /bin/sh, to be easy on
people on Solaris and other systems.

Other than that, the patch is good and there is no need to resend it.
Thanks.

By the way, this eval shows why "theirs" cannot be a symmetric operation
of "ours".  You are taking the last remote HEAD even when you are merging
more than one remote into the current branch at once.  "ours" can be
sensibly defined for an octopus, but "theirs" has this "which theirs"
problem ;-)

^ permalink raw reply

* Re: [PATCH 4/4] Add a second testcase for handling invalid strategies in git-merge
From: Junio C Hamano @ 2008-07-29 23:42 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <f8e5afe347c194013ffd104b196a19a37d762f9b.1217372486.git.vmiklos@frugalware.org>

Miklos Vajna <vmiklos@frugalware.org> writes:

> This one tests '-s index' which is interesting because git-merge-index
> is an existing git command but it is not a valid strategy.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>  t/t7600-merge.sh |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
> index 5eeb6c2..0329aee 100755
> --- a/t/t7600-merge.sh
> +++ b/t/t7600-merge.sh
> @@ -230,6 +230,10 @@ test_expect_success 'test option parsing' '
>  	test_must_fail git merge
>  '
>  
> +test_expect_success 'reject non-strategy with a git-merge-foo name' '
> +	test_must_fail git merge -s index c1
> +'
> +

True, but with the old code that might blindly have executed
git-merge-index the test would also have failed, and you would want to
tell two cases apart, wouldn't you?

^ permalink raw reply

* Re: [PATCH] Make it clear that push can take multiple refspecs
From: Junio C Hamano @ 2008-07-29 23:20 UTC (permalink / raw)
  To: Abhijit Menon-Sen; +Cc: git
In-Reply-To: <20080729231338.GA30717@toroid.org>

Abhijit Menon-Sen <ams@toroid.org> writes:

> At 2008-07-29 15:06:57 -0700, gitster@pobox.com wrote:
>>
>> If you saw the question asked many times in the real world, that
>> would be a good indication that this patch falls into the "helpful"
>> category, not "noise". Will queue.
>
> Thanks.
>
> You may find the appended patch less noisy still. I certainly like being
> able to get rid of the "any number of..." sentence; and I also think the
> example is clearer, though I merged it into an existing one.
> ...
> -<refspec>::
> -	The canonical format of a <refspec> parameter is
> +<refspec>...::
> +	The canonical format of each <refspec> parameter is
>  	`+?<src>:<dst>`; that is, an optional plus `+`, followed

After re-reading this, prompted by your comment about "both synopsis
syntax and regex in the same sentence, I actually do not see a problem
with the original one.  The synopsis talks about the ability for you to
specify zero or more of whatever is called <refspec>, and here we define
what _A_ refspec is.

Somehow I thought the issue was that we said "<refspec>...::" in the
heading of the description without saying zero-or-more, but that is not
the case here.

^ permalink raw reply

* [PATCH 1/4] builtin-help: make some internal functions available to other builtins
From: Miklos Vajna @ 2008-07-29 23:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <cover.1217372486.git.vmiklos@frugalware.org>

Make load_command_list() capable of filtering for a given prefix and
loading into a pair of "struct cmdnames" supplied by the caller.

Make the static add_cmdname(), exclude_cmds() and is_in_cmdlist()
functions non-static.

Make list_commands() accept a custom title, and work from a pair of
"struct cmdnames" supplied by the caller.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 Makefile |    1 +
 help.c   |   77 +++++++++++++++++++++++++++++++------------------------------
 help.h   |   23 ++++++++++++++++++
 3 files changed, 63 insertions(+), 38 deletions(-)
 create mode 100644 help.h

diff --git a/Makefile b/Makefile
index 52c67c1..83d79af 100644
--- a/Makefile
+++ b/Makefile
@@ -355,6 +355,7 @@ LIB_H += git-compat-util.h
 LIB_H += graph.h
 LIB_H += grep.h
 LIB_H += hash.h
+LIB_H += help.h
 LIB_H += list-objects.h
 LIB_H += ll-merge.h
 LIB_H += log-tree.h
diff --git a/help.c b/help.c
index 3cb1962..88c0d5b 100644
--- a/help.c
+++ b/help.c
@@ -9,6 +9,7 @@
 #include "common-cmds.h"
 #include "parse-options.h"
 #include "run-command.h"
+#include "help.h"
 
 static struct man_viewer_list {
 	struct man_viewer_list *next;
@@ -300,18 +301,11 @@ static inline void mput_char(char c, unsigned int num)
 		putchar(c);
 }
 
-static struct cmdnames {
-	int alloc;
-	int cnt;
-	struct cmdname {
-		size_t len;
-		char name[1];
-	} **names;
-} main_cmds, other_cmds;
+struct cmdnames main_cmds, other_cmds;
 
-static void add_cmdname(struct cmdnames *cmds, const char *name, int len)
+void add_cmdname(struct cmdnames *cmds, const char *name, int len)
 {
-	struct cmdname *ent = xmalloc(sizeof(*ent) + len);
+	struct cmdname *ent = xmalloc(sizeof(*ent) + len + 1);
 
 	ent->len = len;
 	memcpy(ent->name, name, len);
@@ -342,7 +336,7 @@ static void uniq(struct cmdnames *cmds)
 	cmds->cnt = j;
 }
 
-static void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
+void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
 {
 	int ci, cj, ei;
 	int cmp;
@@ -418,11 +412,11 @@ static int is_executable(const char *name)
 }
 
 static unsigned int list_commands_in_dir(struct cmdnames *cmds,
-					 const char *path)
+					 const char *path,
+					 const char *prefix)
 {
 	unsigned int longest = 0;
-	const char *prefix = "git-";
-	int prefix_len = strlen(prefix);
+	int prefix_len;
 	DIR *dir = opendir(path);
 	struct dirent *de;
 	struct strbuf buf = STRBUF_INIT;
@@ -430,6 +424,9 @@ static unsigned int list_commands_in_dir(struct cmdnames *cmds,
 
 	if (!dir)
 		return 0;
+	if (!prefix)
+		prefix = "git-";
+	prefix_len = strlen(prefix);
 
 	strbuf_addf(&buf, "%s/", path);
 	len = buf.len;
@@ -460,7 +457,9 @@ static unsigned int list_commands_in_dir(struct cmdnames *cmds,
 	return longest;
 }
 
-static unsigned int load_command_list(void)
+unsigned int load_command_list(const char *prefix,
+		struct cmdnames *main_cmds,
+		struct cmdnames *other_cmds)
 {
 	unsigned int longest = 0;
 	unsigned int len;
@@ -469,7 +468,7 @@ static unsigned int load_command_list(void)
 	const char *exec_path = git_exec_path();
 
 	if (exec_path)
-		longest = list_commands_in_dir(&main_cmds, exec_path);
+		longest = list_commands_in_dir(main_cmds, exec_path, prefix);
 
 	if (!env_path) {
 		fprintf(stderr, "PATH not set\n");
@@ -481,7 +480,7 @@ static unsigned int load_command_list(void)
 		if ((colon = strchr(path, PATH_SEP)))
 			*colon = 0;
 
-		len = list_commands_in_dir(&other_cmds, path);
+		len = list_commands_in_dir(other_cmds, path, prefix);
 		if (len > longest)
 			longest = len;
 
@@ -491,36 +490,38 @@ static unsigned int load_command_list(void)
 	}
 	free(paths);
 
-	qsort(main_cmds.names, main_cmds.cnt,
-	      sizeof(*main_cmds.names), cmdname_compare);
-	uniq(&main_cmds);
+	qsort(main_cmds->names, main_cmds->cnt,
+	      sizeof(*main_cmds->names), cmdname_compare);
+	uniq(main_cmds);
 
-	qsort(other_cmds.names, other_cmds.cnt,
-	      sizeof(*other_cmds.names), cmdname_compare);
-	uniq(&other_cmds);
-	exclude_cmds(&other_cmds, &main_cmds);
+	qsort(other_cmds->names, other_cmds->cnt,
+	      sizeof(*other_cmds->names), cmdname_compare);
+	uniq(other_cmds);
+	exclude_cmds(other_cmds, main_cmds);
 
 	return longest;
 }
 
-static void list_commands(void)
+void list_commands(const char *title, unsigned int longest,
+		struct cmdnames *main_cmds, struct cmdnames *other_cmds)
 {
-	unsigned int longest = load_command_list();
 	const char *exec_path = git_exec_path();
 
-	if (main_cmds.cnt) {
-		printf("available git commands in '%s'\n", exec_path);
-		printf("----------------------------");
-		mput_char('-', strlen(exec_path));
+	if (main_cmds->cnt) {
+		printf("available %s in '%s'\n", title, exec_path);
+		printf("----------------");
+		mput_char('-', strlen(title) + strlen(exec_path));
 		putchar('\n');
-		pretty_print_string_list(&main_cmds, longest);
+		pretty_print_string_list(main_cmds, longest);
 		putchar('\n');
 	}
 
-	if (other_cmds.cnt) {
-		printf("git commands available from elsewhere on your $PATH\n");
-		printf("---------------------------------------------------\n");
-		pretty_print_string_list(&other_cmds, longest);
+	if (other_cmds->cnt) {
+		printf("%s available from elsewhere on your $PATH\n", title);
+		printf("---------------------------------------");
+		mput_char('-', strlen(title));
+		putchar('\n');
+		pretty_print_string_list(other_cmds, longest);
 		putchar('\n');
 	}
 }
@@ -542,7 +543,7 @@ void list_common_cmds_help(void)
 	}
 }
 
-static int is_in_cmdlist(struct cmdnames *c, const char *s)
+int is_in_cmdlist(struct cmdnames *c, const char *s)
 {
 	int i;
 	for (i = 0; i < c->cnt; i++)
@@ -553,7 +554,6 @@ static int is_in_cmdlist(struct cmdnames *c, const char *s)
 
 static int is_git_command(const char *s)
 {
-	load_command_list();
 	return is_in_cmdlist(&main_cmds, s) ||
 		is_in_cmdlist(&other_cmds, s);
 }
@@ -698,8 +698,9 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 			builtin_help_usage, 0);
 
 	if (show_all) {
+		unsigned int longest = load_command_list("git-", &main_cmds, &other_cmds);
 		printf("usage: %s\n\n", git_usage_string);
-		list_commands();
+		list_commands("git commands", longest, &main_cmds, &other_cmds);
 		printf("%s\n", git_more_info_string);
 		return 0;
 	}
diff --git a/help.h b/help.h
new file mode 100644
index 0000000..d614e54
--- /dev/null
+++ b/help.h
@@ -0,0 +1,23 @@
+#ifndef HELP_H
+#define HELP_H
+
+struct cmdnames {
+	int alloc;
+	int cnt;
+	struct cmdname {
+		size_t len;
+		char name[FLEX_ARRAY];
+	} **names;
+};
+
+unsigned int load_command_list(const char *prefix,
+		struct cmdnames *main_cmds,
+		struct cmdnames *other_cmds);
+void add_cmdname(struct cmdnames *cmds, const char *name, int len);
+/* Here we require that excludes is a sorted list. */
+void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes);
+int is_in_cmdlist(struct cmdnames *c, const char *s);
+void list_commands(const char *title, unsigned int longest,
+		struct cmdnames *main_cmds, struct cmdnames *other_cmds);
+
+#endif /* HELP_H */
-- 
1.6.0.rc0.14.g95f8.dirty

^ permalink raw reply related

* [PATCH 2/4] builtin-merge: allow using a custom strategy
From: Miklos Vajna @ 2008-07-29 23:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <cover.1217372486.git.vmiklos@frugalware.org>

Allow using a custom strategy, as long as it's named git-merge-foo. The
error handling is now done using is_git_command(). The list of available
strategies is now shown by list_commands().

If an invalid strategy is supplied, like -s foobar, then git-merge would
list all git-merge-* commands. This is not perfect, since for example
git-merge-index is not a valid strategy.

These are removed from the output by scanning the list of main commands;
if the git-merge-foo command is listed in the all_strategy list, then
it's shown, otherwise excluded. This does not exclude commands somewhere
else in the PATH, where custom strategies are expected.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 builtin-merge.c |   42 +++++++++++++++++++++++++++++++++++-------
 1 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index e78fa18..99b307a 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -22,6 +22,7 @@
 #include "log-tree.h"
 #include "color.h"
 #include "rerere.h"
+#include "help.h"
 
 #define DEFAULT_TWOHEAD (1<<0)
 #define DEFAULT_OCTOPUS (1<<1)
@@ -77,7 +78,9 @@ static int option_parse_message(const struct option *opt,
 static struct strategy *get_strategy(const char *name)
 {
 	int i;
-	struct strbuf err;
+	struct strategy *ret;
+	static struct cmdnames main_cmds, other_cmds;
+	static int longest = 0;
 
 	if (!name)
 		return NULL;
@@ -86,12 +89,37 @@ static struct strategy *get_strategy(const char *name)
 		if (!strcmp(name, all_strategy[i].name))
 			return &all_strategy[i];
 
-	strbuf_init(&err, 0);
-	for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
-		strbuf_addf(&err, " %s", all_strategy[i].name);
-	fprintf(stderr, "Could not find merge strategy '%s'.\n", name);
-	fprintf(stderr, "Available strategies are:%s.\n", err.buf);
-	exit(1);
+	if (!longest) {
+		struct cmdnames not_strategies;
+
+		memset(&main_cmds, 0, sizeof(struct cmdnames));
+		memset(&other_cmds, 0, sizeof(struct cmdnames));
+		memset(&not_strategies, 0, sizeof(struct cmdnames));
+		longest = load_command_list("git-merge-", &main_cmds,
+				&other_cmds);
+		for (i = 0; i < main_cmds.cnt; i++) {
+			int j, found = 0;
+			struct cmdname *ent = main_cmds.names[i];
+			for (j = 0; j < ARRAY_SIZE(all_strategy); j++)
+				if (!strncmp(ent->name, all_strategy[j].name, ent->len)
+						&& !all_strategy[j].name[ent->len])
+					found = 1;
+			if (!found)
+				add_cmdname(&not_strategies, ent->name, ent->len);
+			exclude_cmds(&main_cmds, &not_strategies);
+		}
+	}
+	if (!is_in_cmdlist(&main_cmds, name) && !is_in_cmdlist(&other_cmds, name)) {
+
+		fprintf(stderr, "Could not find merge strategy '%s'.\n\n", name);
+		list_commands("strategies", longest, &main_cmds, &other_cmds);
+		exit(1);
+	}
+
+	ret = xmalloc(sizeof(struct strategy));
+	memset(ret, 0, sizeof(struct strategy));
+	ret->name = xstrdup(name);
+	return ret;
 }
 
 static void append_strategy(struct strategy *s)
-- 
1.6.0.rc0.14.g95f8.dirty

^ permalink raw reply related

* [PATCH 0/4] Allow custom merge strategies, take 4
From: Miklos Vajna @ 2008-07-29 23:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4p68eame.fsf@gitster.siamese.dyndns.org>

On Tue, Jul 29, 2008 at 12:47:05PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> This feels overly wasteful.  Granted, this is not a performance critical
> codepath, but you list all commands that begin with "git-merge-" in
> is_git_command(), only to discard it and then iterate over 140+ main_cmds
> list only to cull the ones whose name do not appear in the strategies
> list.
>
> Perhaps this shows that changing the function is_git_command() is a wrong
> approach (for one thing, with the custom prefix, it is not about "Is it a
> git command" anymore).  Wouldn't it be easier to read if you did this part
> like this instead?
>
>  * make load_command_list capable of loading into a "struct cmdnames" (or
>    pair if you want) supplied by the caller;
>
>  * use it to grab all commands whose name begin with "git-merge-" here;
>
>  * Check if name appears in that list; if it doesn't, you already have the
>    list of commands that could be merge strategy for error reporting.
>
> If you also update list_commands() not to run load_command_list() itself
> but take caller-supplied list of commands, then the API would become much
> cleaner.  The caller would not be limited to "filter with prefix" anymore.
>
> Hmm?

I like the idea. ;-)

Here is what I did:

- I changed only load_command_list() and list_commands().
  load_command_list() still supports filtering, but the result is loaded
  to a pair of "struct cmdnames", supplied by the caller.

- list_commands() works from this pair, additionally supporting the
  custom title.

- export add_cmdname(), exclude_cmds() and is_in_cmdlist() without any
  modification in help.h

The rest is done in builtin-merge.c, as you suggested.

The nice thing is that before this change builtin-merge accepted for
example '-s index' (because git-merge-index was a valid command), but
now this is no longer true.

Miklos Vajna (4):
  builtin-help: make some internal functions available to other
    builtins
  builtin-merge: allow using a custom strategy
  Add a new test for using a custom merge strategy
  Add a second testcase for handling invalid strategies in git-merge

 Makefile                |    1 +
 builtin-merge.c         |   42 +++++++++++++++++++++----
 help.c                  |   77 ++++++++++++++++++++++++-----------------------
 help.h                  |   23 ++++++++++++++
 t/t7600-merge.sh        |    5 +++
 t/t7606-merge-custom.sh |   46 ++++++++++++++++++++++++++++
 6 files changed, 149 insertions(+), 45 deletions(-)
 create mode 100644 help.h
 create mode 100755 t/t7606-merge-custom.sh

^ permalink raw reply

* [PATCH 4/4] Add a second testcase for handling invalid strategies in git-merge
From: Miklos Vajna @ 2008-07-29 23:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <cover.1217372486.git.vmiklos@frugalware.org>

This one tests '-s index' which is interesting because git-merge-index
is an existing git command but it is not a valid strategy.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 t/t7600-merge.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 5eeb6c2..0329aee 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -230,6 +230,10 @@ test_expect_success 'test option parsing' '
 	test_must_fail git merge
 '
 
+test_expect_success 'reject non-strategy with a git-merge-foo name' '
+	test_must_fail git merge -s index c1
+'
+
 test_expect_success 'merge c0 with c1' '
 	git reset --hard c0 &&
 	git merge c1 &&
-- 
1.6.0.rc0.14.g95f8.dirty

^ permalink raw reply related

* [PATCH 3/4] Add a new test for using a custom merge strategy
From: Miklos Vajna @ 2008-07-29 23:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <cover.1217372486.git.vmiklos@frugalware.org>

Testing is done by creating a simple git-merge-theirs strategy which is
the opposite of ours. Using this in real merges is not recommended but
it's perfect for our testing needs.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 t/t7606-merge-custom.sh |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)
 create mode 100755 t/t7606-merge-custom.sh

diff --git a/t/t7606-merge-custom.sh b/t/t7606-merge-custom.sh
new file mode 100755
index 0000000..13e8ff5
--- /dev/null
+++ b/t/t7606-merge-custom.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description='git-merge
+
+Testing a custom strategy.'
+
+. ./test-lib.sh
+
+cat > git-merge-theirs << EOF
+#!/bin/sh
+eval git read-tree --reset -u \\\$\$#
+EOF
+chmod +x git-merge-theirs
+PATH=.:$PATH
+export PATH
+
+test_expect_success 'setup' '
+	echo c0 > c0.c &&
+	git add c0.c &&
+	git commit -m c0 &&
+	git tag c0 &&
+	echo c1 > c1.c &&
+	git add c1.c &&
+	git commit -m c1 &&
+	git tag c1 &&
+	git reset --hard c0 &&
+	echo c2 > c2.c &&
+	git add c2.c &&
+	git commit -m c2 &&
+	git tag c2
+'
+
+test_expect_success 'merge c2 with a custom strategy' '
+	git reset --hard c1 &&
+	git merge -s theirs c2 &&
+	test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
+	test "$(git rev-parse c1)" = "$(git rev-parse HEAD^1)" &&
+	test "$(git rev-parse c2)" = "$(git rev-parse HEAD^2)" &&
+	test "$(git rev-parse c2^{tree})" = "$(git rev-parse HEAD^{tree})" &&
+	git diff --exit-code &&
+	test -f c0.c &&
+	test ! -f c1.c &&
+	test -f c2.c
+'
+
+test_done
-- 
1.6.0.rc0.14.g95f8.dirty

^ permalink raw reply related

* Re: [PATCH] Make it clear that push can take multiple refspecs
From: Abhijit Menon-Sen @ 2008-07-29 23:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7viquobb0e.fsf@gitster.siamese.dyndns.org>

At 2008-07-29 15:06:57 -0700, gitster@pobox.com wrote:
>
> If you saw the question asked many times in the real world, that
> would be a good indication that this patch falls into the "helpful"
> category, not "noise". Will queue.

Thanks.

You may find the appended patch less noisy still. I certainly like being
able to get rid of the "any number of..." sentence; and I also think the
example is clearer, though I merged it into an existing one.

(I would also change the '+?<src>:<dst>' to '[+]<src>:<dst>'. It's a bit
weird to use BOTH regex format and SYNOPSIS format in the same string.)

> I wonder if there are other manual pages with <thing>... notation that
> benefits from similar changes, though.

Yes, I see now that there are several that could use minor changes.
Often nothing more than the following is needed:

-<committish>::
-       The object name of the committish.
+<committish>...::
+       A list of committish object names to describe.

(For git-describe, whose SYNOPSIS already mentions '<committish>...')

I'll submit another patch for those later.

-- ams

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 94d07ab..7f28ea6 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 --------
 [verse]
 'git push' [--all] [--dry-run] [--tags] [--receive-pack=<git-receive-pack>]
-           [--repo=all] [-f | --force] [-v | --verbose] [<repository> <refspec>...]
+           [--repo=all] [-f | --force] [-v | --verbose]
+           [<repository> <refspec>...]
 
 DESCRIPTION
 -----------
@@ -29,8 +30,8 @@ OPTIONS
 	The "remote" repository that is destination of a push
 	operation.  See the section <<URLS,GIT URLS>> below.
 
-<refspec>::
-	The canonical format of a <refspec> parameter is
+<refspec>...::
+	The canonical format of each <refspec> parameter is
 	`+?<src>:<dst>`; that is, an optional plus `+`, followed
 	by the source ref, followed by a colon `:`, followed by
 	the destination ref.
@@ -180,11 +181,11 @@ git push origin :experimental::
 	Find a ref that matches `experimental` in the `origin` repository
 	(e.g. `refs/heads/experimental`), and delete it.
 
-git push origin master:satellite/master::
-	Find a ref that matches `master` in the source repository
-	(most likely, it would find `refs/heads/master`), and update
-	the ref that matches `satellite/master` (most likely, it would
-	be `refs/remotes/satellite/master`) in `origin` repository with it.
+git push origin master:satellite/master dev:satellite/dev::
+	Use the source ref that matches `master` (e.g. `refs/heads/master`)
+	to update the ref that matches `satellite/master` (most probably
+	`refs/remotes/satellite/master`) in the `origin` repository, then
+	do the same for `dev` and `satellite/dev`.
 
 git push origin master:refs/heads/experimental::
 	Create the branch `experimental` in the `origin` repository

^ permalink raw reply related

* git svn documentation
From: Jurko Gospodnetić @ 2008-07-29 22:45 UTC (permalink / raw)
  To: git

   Hi.

   git svn documentation should be updated so it no longer references 
the no longer supported 'git svnimport' command. Currently this causes 
an invalid link to be added to the git svn html documentation.

   Hope this helps.

   Best regards,
     Jurko Gospodnetić

^ permalink raw reply

* Re: What is 'git BRANCH'?
From: Sverre Rabbelier @ 2008-07-29 22:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jurko Gospodnetić, git
In-Reply-To: <7v63qob9hz.fsf@gitster.siamese.dyndns.org>

On Wed, Jul 30, 2008 at 00:39, Junio C Hamano <gitster@pobox.com> wrote:
> diff --git a/git.c b/git.c
> index 37b1d76..c99e769 100644
> --- a/git.c
> +++ b/git.c
> @@ -448,7 +448,7 @@ int main(int argc, const char **argv)
>                cmd += 4;
>                argv[0] = cmd;
>                handle_internal_command(argc, argv);
> -               die("cannot handle %s internally", cmd);
> +               help_unknown_cmd(cmd);
>        }
>
>        /* Look for flags.. */

Why does handle_internal_command not complain after the "	for (i = 0;
i < ARRAY_SIZE(commands); i++) {" that no matching commands were
found? Is that not an implicit assertion that would do well with being
asserted here?

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: Git Community Book
From: Junio C Hamano @ 2008-07-29 22:47 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Scott Chacon, git list
In-Reply-To: <alpine.LNX.1.00.0807291716460.19665@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> In particular, I think it's really useful to show a commit graph with 
> branching and merging, and introduce refs as movable pointers to commits 
> in the graph, and local branches as refs that you move and tracking refs 
> as refs that copy values in other repositories.

I'd very strongly second this.  If somebody is really into screencasts
(and especially from the Ruby circle, I would guess), this may be worth
a look:

    http://excess.org/article/2008/07/ogre-git-tutorial/

I saw a couple of technical inaccuracies in the presentation (I do not
expect any presentation or screencast to be perfect; I've never seen one
without any technical error anyway, perhaps other than my own at OLS a few
years ago), but otherwise it was very well done.  Espcially the part that
builds the commit ancestry chains I was very happy to see it taught like
so.

^ permalink raw reply

* Re: What is 'git BRANCH'?
From: Jurko Gospodnetić @ 2008-07-29 22:43 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <DEBF93FC-BA8D-4513-B4EE-A8648DA1D591@sb.org>

   Hi all.

> On Jul 29, 2008, at 3:24 PM, Junio C Hamano wrote:
> 
>> Jurko Gospodnetić <jurko.gospodnetic@docte.hr> writes:
>>
>>>  Hi.
>>>
>>>  I typed in "git BRANCH" by accident and got the error message:
>>> "fatal: cannot handle BRANCH internally".
>>>
>>>  What does that mean?
>>>
>>>  It is different from the usual "git: 'yada-yada' is not a
>>> git-command. See 'git --help'." message you get when you type in an
>>> incorrect command name.
>>
>> Just a guess; your git is installed on a case-challenged filesystem?

   Thank you all for explaining this, and yes - this was detected on 
Windows with a NTFS drive set to case-insensitive.

   But, if something is running git-branch here... why does this 
script/executable/whatever try to check the name it got called with? Why 
does it not simply do its work no matter the name it got called with?

   If I'm asking something to obvious here - feel free to send me back 
to read the code... :-)

   Best regards,
     Jurko Gospodnetić

^ 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