Git development
 help / color / mirror / Atom feed
* [PATCH] git-pull: don't complain about branch merge config if only fetching tags
From: Gerrit Pape @ 2007-12-21 12:44 UTC (permalink / raw)
  To: git, Junio C Hamano

When running git pull with the -t switch, it properly fetches tags, but
complains about missing information on how to merge.  Since there's
nothing to merge, make git-pull simply exit after fetching the tags.

The problem has been reported by Joey Hess through
 http://bugs.debian.org/456035

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 git-pull.sh |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 698e82b..43be0bd 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -112,6 +112,11 @@ case "$merge_head" in
 	     exit 1;;
 	  *) exit $?;;
 	esac
+	# exit if only tags have been fetched
+	not_for_merge=$(sed -e '/	not-for-merge	tag/d' \
+			"$GIT_DIR"/FETCH_HEAD)
+	test "$not_for_merge" != '' || exit 0
+
 	curr_branch=${curr_branch#refs/heads/}
 
 	echo >&2 "You asked me to pull without telling me which branch you"
-- 
1.5.3.7

^ permalink raw reply related

* [PATCH v2] Make git send-email accept $EDITOR with arguments
From: Gustaf Hendeby @ 2007-12-21 11:36 UTC (permalink / raw)
  To: luciano; +Cc: git, gitster, Gustaf Hendeby
In-Reply-To: <20071220203211.GA12296@bit.office.eurotux.com>

Currently git send-email does not accept $EDITOR with arguments, eg,
emacs -nw, when starting an editor to produce a cover letter.  This
fix uses perl's implicit splitting to perform the task and that should
hopefully cover most interesting cases.

Signed-off-by:  Gustaf Hendeby <hendeby@isy.liu.se>
---

Thanks to Luciano for the tip to use the internal splitting in perl,
that should be a better solution than to split on all spaces.  I don't
think it is necessary, though, to add an extra error message if the
system call fails, system in it self already produces something that
should be clear enough.  If anyone got a strong oppinion for another
error message I'll fix that.

Junio, even if this is technically not a bug fix, it would be nice to
get this fix into the 1.5.4 so that the usage of $EDITOR becomes more
consistent throughout git.

/Gustaf

 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 248d035..5764668 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -400,7 +400,7 @@ EOT
 	close(C);
 
 	my $editor = $ENV{GIT_EDITOR} || $repo->config("core.editor") || $ENV{VISUAL} || $ENV{EDITOR} || "vi";
-	system($editor, $compose_filename);
+	system("$editor $compose_filename");
 
 	open(C2,">",$compose_filename . ".final")
 		or die "Failed to open $compose_filename.final : " . $!;
-- 
1.5.4.rc1.4.gb8173-dirty

^ permalink raw reply related

* [PATCH] parse-options: Add a gitcli(5) man page.
From: Pierre Habouzit @ 2007-12-21 11:19 UTC (permalink / raw)
  To: Junio C Hamano, git
In-Reply-To: <20071221111356.GA25953@artemis.madism.org>

This page should hold every information about the git ways to parse command
lines, and best practices to be used for scripting.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
    > > Unfortunately I've been busy lately and have already discarded
    > > the whole series that had the {} stuff.  Could you resend
    > > cleaned up patch please, so that I can take a look over the
    > > weekend?
    > 
    >   sure, this is the patch from
    > http://article.gmane.org/gmane.comp.version-control.git/68140 actually,
    > I will resend a proper patch when I've been able to get that one back :)

    in fact that is not the proper one, and I had one kept here. I just
    pushed the 3 patches on my ph/parseopt branch at
    git://git.madism.org/git.git, rebased on the current next.

    Cheers,

 Documentation/Makefile   |    2 +-
 Documentation/gitcli.txt |  113 ++++++++++++++++++++++++++++++++++++++++++++++
 Makefile                 |    1 +
 3 files changed, 115 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/gitcli.txt

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 76df06c..c4486d3 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -2,7 +2,7 @@ MAN1_TXT= \
 	$(filter-out $(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \
 		$(wildcard git-*.txt)) \
 	gitk.txt
-MAN5_TXT=gitattributes.txt gitignore.txt gitmodules.txt
+MAN5_TXT=gitattributes.txt gitignore.txt gitcli.txt gitmodules.txt
 MAN7_TXT=git.txt
 
 MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
new file mode 100644
index 0000000..b7dcf9c
--- /dev/null
+++ b/Documentation/gitcli.txt
@@ -0,0 +1,113 @@
+gitcli(5)
+=========
+
+NAME
+----
+gitcli - git command line interface and conventions
+
+SYNOPSIS
+--------
+gitcli
+
+
+DESCRIPTION
+-----------
+
+This manual describes best practice in how to use git CLI.  Here are
+the rules that you should follow when you are scripting git:
+
+ * it's preferred to use the non dashed form of git commands, which means that
+   you should prefer `"git foo"` to `"git-foo"`.
+
+ * splitting short options to separate words (prefer `"git foo -a -b"`
+   to `"git foo -ab"`, the latter may not even work).
+
+ * when a command line option takes an argument, use the 'sticked' form.  In
+   other words, write `"git foo -oArg"` instead of `"git foo -o Arg"` for short
+   options, and `"git foo --long-opt=Arg"` instead of `"git foo --long-opt Arg"`
+   for long options.  An option that takes optional option-argument must be
+   written in the 'sticked' form.
+
+ * when you give a revision parameter to a command, make sure the parameter is
+   not ambiguous with a name of a file in the work tree.  E.g. do not write
+   `"git log -1 HEAD"` but write `"git log -1 HEAD --"`; the former will not work
+   if you happen to have a file called `HEAD` in the work tree.
+
+
+ENHANCED CLI
+------------
+From the git 1.5.4 series and further, many git commands (not all of them at the
+time of the writing though) come with an enhanced option parser.
+
+Here is an exhaustive list of the facilities provided by this option parser.
+
+
+Magic Options
+~~~~~~~~~~~~~
+Commands which have the enhanced option parser activated all understand a
+couple of magic command line options:
+
+-h::
+	gives a pretty printed usage of the command.
++
+---------------------------------------------
+$ git describe -h
+usage: git-describe [options] <committish>*
+
+    --contains            find the tag that comes after the commit
+    --debug               debug search strategy on stderr
+    --all                 use any ref in .git/refs
+    --tags                use any tag in .git/refs/tags
+    --abbrev [<n>]        use <n> digits to display SHA-1s
+    --candidates <n>      consider <n> most recent tags (default: 10)
+---------------------------------------------
+
+--help-all::
+	Some git commands take options that are only used for plumbing or that
+	are deprecated, and such options are hidden from the default usage. This
+	option gives the full list of options.
+
+
+Negating options
+~~~~~~~~~~~~~~~~
+Options with long option names can be negated by prefixing `"--no-"`. For
+example, `"git branch"` has the option `"--track"` which is 'on' by default. You
+can use `"--no-track"` to override that behaviour. The same goes for `"--color"`
+and `"--no-color"`.
+
+
+Aggregating short options
+~~~~~~~~~~~~~~~~~~~~~~~~~
+Commands that support the enhanced option parser allow you to aggregate short
+options. This means that you can for example use `"git rm -rf"` or
+`"git clean -fdx"`.
+
+
+Separating argument from the option
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+You can write the mandatory option parameter to an option as a separate
+word on the command line.  That means that all the following uses work:
+
+----------------------------
+$ git foo --long-opt=Arg
+$ git foo --long-opt Arg
+$ git foo -oArg
+$ git foo -o Arg
+----------------------------
+
+However, this is *NOT* allowed for switches with an optionnal value, where the
+'sticked' form must be used:
+----------------------------
+$ git describe --abbrev HEAD     # correct
+$ git describe --abbrev=10 HEAD  # correct
+$ git describe --abbrev 10 HEAD  # NOT WHAT YOU MEANT
+----------------------------
+
+
+Documentation
+-------------
+Documentation by Pierre Habouzit.
+
+GIT
+---
+Part of the gitlink:git[7] suite
diff --git a/Makefile b/Makefile
index b635be8..dd6c0d6 100644
--- a/Makefile
+++ b/Makefile
@@ -1173,6 +1173,7 @@ check-docs::
 		documented,gitattributes | \
 		documented,gitignore | \
 		documented,gitmodules | \
+		documented,gitcli | \
 		documented,git-tools | \
 		sentinel,not,matching,is,ok ) continue ;; \
 		esac; \
-- 
1.5.4.rc1.1096.g37c7b

^ permalink raw reply related

* Re: [PATCH] git-send-email: Add --suppress-all-from option.
From: Brian Swetland @ 2007-12-21 11:14 UTC (permalink / raw)
  To: David Brown; +Cc: git
In-Reply-To: <1198216860-487-1-git-send-email-git@davidb.org>

[David Brown <git@davidb.org>]
> Sometimes, it is useful to be able to send a patch to a third party
> without the author of the patch being copied on the message.  An
> common example would be an internal discussion at a company to ask if
> a particular patch should be applied.  Some environments may even have
> policy against such mail being sent outside of the company.
> 
> Add the --suppress-all-from/--no-suppress-all-from options to avoid
> sending patches to the patch author, even if different from the
> sender.  Add the sendemail.suppressallfrom config option to allow this
> to have a different default.
> 
> Signed-off-by: David Brown <git@davidb.org>

Yes-Please: Brian Swetland <swetland@google.com>

This has caused me some pain previously, and just earlier today I 
received an internal patch mail from somebody at another company who
almost certainly did not intend to cc me.

Brian

^ permalink raw reply

* Re: 1.5.4-rc2 plans
From: Pierre Habouzit @ 2007-12-21 11:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk5n8b92w.fsf@gitster.siamese.dyndns.org>

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

On Fri, Dec 21, 2007 at 11:06:47AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@artemis.madism.org> writes:
> 
> > Hmm now I'm confused, I believed we settled for the: --abbrev 10 HEAD is
> > forbidden, --abbrev=10 HEAD works, and --abbrev HEAD too.
> 
> http://thread.gmane.org/gmane.comp.version-control.git/68121/focus=68659 ?

  okay, then it's what I remembered, I was confused because the patch
for that was around for quite some time, but I just resent it so this
one is okay.

> > I'd also like to see any kind of form of gitcli(5) be merged for 1.5.4
> > too, I believe the first version I ever sent. In the thread where I
> > posted the proposal using `{}` the patch introducing it is the version
> > formed using:
> 
> Unfortunately I've been busy lately and have already discarded
> the whole series that had the {} stuff.  Could you resend
> cleaned up patch please, so that I can take a look over the
> weekend?

  sure, this is the patch from
http://article.gmane.org/gmane.comp.version-control.git/68140 actually,
I will resend a proper patch when I've been able to get that one back :)

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: 1.5.4-rc2 plans
From: Junio C Hamano @ 2007-12-21 11:06 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <20071221104704.GC17701@artemis.madism.org>

Pierre Habouzit <madcoder@artemis.madism.org> writes:

> Hmm now I'm confused, I believed we settled for the: --abbrev 10 HEAD is
> forbidden, --abbrev=10 HEAD works, and --abbrev HEAD too.

http://thread.gmane.org/gmane.comp.version-control.git/68121/focus=68659 ?

> I'd also like to see any kind of form of gitcli(5) be merged for 1.5.4
> too, I believe the first version I ever sent. In the thread where I
> posted the proposal using `{}` the patch introducing it is the version
> formed using:

Unfortunately I've been busy lately and have already discarded
the whole series that had the {} stuff.  Could you resend
cleaned up patch please, so that I can take a look over the
weekend?

^ permalink raw reply

* Re: [PATCH] git-send-email: Add --suppress-all-from option.
From: Joel Becker @ 2007-12-21 11:05 UTC (permalink / raw)
  To: David Brown; +Cc: git
In-Reply-To: <1198216860-487-1-git-send-email-git@davidb.org>

On Thu, Dec 20, 2007 at 10:01:00PM -0800, David Brown wrote:
> Sometimes, it is useful to be able to send a patch to a third party
s/Sometimes/Most of the time/g

> without the author of the patch being copied on the message.  An
> common example would be an internal discussion at a company to ask if
> a particular patch should be applied.  Some environments may even have
> policy against such mail being sent outside of the company.
> 
> Add the --suppress-all-from/--no-suppress-all-from options to avoid
> sending patches to the patch author, even if different from the
> sender.  Add the sendemail.suppressallfrom config option to allow this
> to have a different default.
> 
> Signed-off-by: David Brown <git@davidb.org>
Please-dog-yes: Joel Becker <jlbec@evilplan.org>

> ---
>  Documentation/git-send-email.txt |    7 +++++++
>  git-send-email.perl              |    9 ++++++++-
>  2 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index f0bd285..5d06264 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -117,6 +117,13 @@ The --cc option must be repeated for each user you want on the cc list.
>          Default is the value of 'sendemail.suppressfrom' configuration value;
>          if that is unspecified, default to --no-suppress-from.
>  
> +--suppress-all-from, --no-suppress-all-from::
> +        If this is set, do not add the From: address to the cc: list,
> +        even if it is different than the person sending the email.
> +        Default is the value of the 'sendemail.suppressallfrom'
> +        configuration value; if that is unspecified, default to
> +        -no-suppress-all-from.
> +
>  --thread, --no-thread::
>  	If this is set, the In-Reply-To header will be set on each email sent.
>  	If disabled with "--no-thread", no emails will have the In-Reply-To
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 248d035..80265b5 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -90,6 +90,9 @@ Options:
>  
>     --suppress-from Suppress sending emails to yourself. Defaults to off.
>  
> +   --suppress-all-from Never automatically send to a patch author.
> +                  Defaults to off.
> +
>     --thread       Specify that the "In-Reply-To:" header should be set on all
>                    emails. Defaults to on.
>  
> @@ -174,7 +177,8 @@ if ($@) {
>  my ($quiet, $dry_run) = (0, 0);
>  
>  # Variables with corresponding config settings
> -my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc, $cc_cmd);
> +my ($thread, $chain_reply_to, $suppress_from, $suppress_all_from);
> +my ($signed_off_cc, $cc_cmd);
>  my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_authpass, $smtp_ssl);
>  my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
>  
> @@ -182,6 +186,7 @@ my %config_bool_settings = (
>      "thread" => [\$thread, 1],
>      "chainreplyto" => [\$chain_reply_to, 1],
>      "suppressfrom" => [\$suppress_from, 0],
> +    "suppressallfrom" => [\$suppress_all_from, 0],
>      "signedoffcc" => [\$signed_off_cc, 1],
>      "smtpssl" => [\$smtp_ssl, 0],
>  );
> @@ -218,6 +223,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
>  		    "quiet" => \$quiet,
>  		    "cc-cmd=s" => \$cc_cmd,
>  		    "suppress-from!" => \$suppress_from,
> +		    "suppress-all-from!" => \$suppress_all_from,
>  		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_cc,
>  		    "dry-run" => \$dry_run,
>  		    "envelope-sender=s" => \$envelope_sender,
> @@ -700,6 +706,7 @@ foreach my $t (@files) {
>  					$subject = $1;
>  
>  				} elsif (/^(Cc|From):\s+(.*)$/) {
> +					next if ($suppress_all_from);
>  					if (unquote_rfc2047($2) eq $sender) {
>  						next if ($suppress_from);
>  					}
> -- 
> 1.5.3.7
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 

Life's Little Instruction Book #456

	"Send your loved one flowers.  Think of a reason later."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

^ permalink raw reply

* [PATCH] git-tag: fix -l switch handling regression.
From: Pierre Habouzit @ 2007-12-21 10:50 UTC (permalink / raw)
  To: Junio C Hamano, git
In-Reply-To: <20071221104704.GC17701@artemis.madism.org>

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

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

It seems that you didnt took that patch either, that IMHO gives a better
semantics to git tag -l than yours, while keeping backward
compatibility.


 builtin-tag.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index 274901a..020ee1c 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -16,7 +16,7 @@
 static const char * const git_tag_usage[] = {
 	"git-tag [-a|-s|-u <key-id>] [-f] [-m <msg>|-F <file>] <tagname> [<head>]",
 	"git-tag -d <tagname>...",
-	"git-tag [-n [<num>]] -l [<pattern>]",
+	"git-tag -l [-n [<num>]] [<pattern>]",
 	"git-tag -v <tagname>...",
 	NULL
 };
@@ -370,13 +370,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct ref_lock *lock;
 
 	int annotate = 0, sign = 0, force = 0, lines = 0,
-					delete = 0, verify = 0;
-	char *list = NULL, *msgfile = NULL, *keyid = NULL;
-	const char *no_pattern = "NO_PATTERN";
+		list = 0, delete = 0, verify = 0;
+	char *msgfile = NULL, *keyid = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct option options[] = {
-		{ OPTION_STRING, 'l', NULL, &list, "pattern", "list tag names",
-			PARSE_OPT_OPTARG, NULL, (intptr_t) no_pattern },
+		OPT_BOOLEAN('l', NULL, &list, "list tag names"),
 		{ OPTION_INTEGER, 'n', NULL, &lines, NULL,
 				"print n lines of each tag message",
 				PARSE_OPT_OPTARG, NULL, 1 },
@@ -408,7 +406,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 		annotate = 1;
 
 	if (list)
-		return list_tags(list == no_pattern ? NULL : list, lines);
+		return list_tags(argv[0], lines);
 	if (delete)
 		return for_each_tag_name(argv, delete_tag);
 	if (verify)
-- 
1.5.4.rc1.1096.g37c7b


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply related

* Re: [PATCH 4/9] gitk i18n: Initial German translation.
From: Christian Stimming @ 2007-12-21 10:50 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git

Dear Paul,

thanks a lot for applying the i18n patches to the gitk master at 
git.kernel.org. This looks all fine, except for one mistake that slipped in: 
The character encoding of the newly added po/de.po translation was messed up. 
The file must be in UTF-8 encoding (and I submitted it this way), but the one 
that ended up in the repository is in ISO-8859 encoding.

Could you please do 

  recode latin1..utf8 po/de.po

and commit the result? Otherwise a simple "make" will fail, because when 
msgfmt tries to "compile" the de.po it rightfully complains about the file 
not being in valid utf8, although the po file header claims "Content-Type: 
text/plain; charset=UTF-8".

Thank you very much.

Christian

^ permalink raw reply

* Re: Serious bug with pretty format strings & empty bodies?
From: Jonathan del Strother @ 2007-12-21 10:47 UTC (permalink / raw)
  To: git
In-Reply-To: <57518fd10712200508x4b15f9acy10aed83a3cebeba@mail.gmail.com>

Has anyone actually managed to reproduce my problem?  I've got
multiple repos here that show the problem in several commits, made by
different people.  However, I can't actually come up with a way to
reproduce it at will...

I'd really like to see René's patch accepted as it seems to magically
fix all my problems, but it's a bit hard to justify if I'm the only
one that's seeing these broken commit messages.

^ permalink raw reply

* Re: 1.5.4-rc2 plans
From: Pierre Habouzit @ 2007-12-21 10:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsr8lwf7.fsf@gitster.siamese.dyndns.org>

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

On Fri, Dec 21, 2007 at 12:32:28AM +0000, Junio C Hamano wrote:
> I've tagged -rc1 last night.  The changes are mostly fixes.  There are
> some remaining issues I'd like to see fixed/decided before 1.5.4.
> 
> One important issue is to identify and fix regressions since 1.5.3
> series.  No "rewrite scripted git-foo completely in C" can be regression
> free, and we had quite a few internal changes during 1.5.4 cycle (not
> just rewrite to C, but C level uses new and improved API such as strbuf
> and parse-options).  Currently I am aware of these regressions:
> 
>  * handling of options, "--abbrev 10 HEAD", "--abbrev=10 HEAD" and
>    "--abbrev HEAD".  The last one does not work for commands that use
>    parse-options.  Pierre is on top of this, I hope.

Hmm now I'm confused, I believed we settled for the: --abbrev 10 HEAD is
forbidden, --abbrev=10 HEAD works, and --abbrev HEAD too. This would
introduce no regressions _yet_ as none of the commands that use
parse-options and take --abbrev accepted the --abbrev 10 form before. I
already sent this once, and assumed you took it, hence me being silent
the last days. Here is it again then.  Of course this does not affects
other long options for which `--long-option arg` still works (if they do
take an argument).

I'd also like to see any kind of form of gitcli(5) be merged for 1.5.4
too, I believe the first version I ever sent. In the thread where I
posted the proposal using `{}` the patch introducing it is the version
formed using:
  * my first proposal for it ;
  * your english fixes squashed on top of it ;
  * a fix wrt the '--no-' prefix and Boolean options that we discussed.
This version describes the current state of things properly IMHO. I can
find the message id if you need, but I don't have the commit locally
anymore atm.

Cheers,

From 37c7baaa82f36d16697fa190635f5d3efbe2a83d Mon Sep 17 00:00:00 2001
From: Pierre Habouzit <madcoder@debian.org>
Date: Fri, 21 Dec 2007 11:41:41 +0100
Subject: [PATCH] Force the sticked form for options with optional arguments.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 574ed31..4f5c55e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -89,7 +89,7 @@ static int get_value(struct optparse_t *p,
 			*(const char **)opt->value = NULL;
 			return 0;
 		}
-		if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-')) {
+		if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
 			*(const char **)opt->value = (const char *)opt->defval;
 			return 0;
 		}
@@ -103,7 +103,7 @@ static int get_value(struct optparse_t *p,
 			return (*opt->callback)(opt, NULL, 1);
 		if (opt->flags & PARSE_OPT_NOARG)
 			return (*opt->callback)(opt, NULL, 0);
-		if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-'))
+		if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
 			return (*opt->callback)(opt, NULL, 0);
 		if (!arg)
 			return opterror(opt, "requires a value", flags);
@@ -114,7 +114,7 @@ static int get_value(struct optparse_t *p,
 			*(int *)opt->value = 0;
 			return 0;
 		}
-		if (opt->flags & PARSE_OPT_OPTARG && (!arg || !isdigit(*arg))) {
+		if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
 			*(int *)opt->value = opt->defval;
 			return 0;
 		}
-- 
1.5.4.rc1.1096.g37c7b


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply related

* Re: unexpected git-cherry-pick conflict
From: Gerrit Pape @ 2007-12-21 10:37 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0707071949440.4093@racer.site>

On Sat, Jul 07, 2007 at 09:58:08PM +0100, Johannes Schindelin wrote:
> On Mon, 25 Jun 2007, Gerrit Pape wrote:
> > Hi, did you get to this yet?, not to stress you, just to make sure we 
> > don't forget about it.
> 
> Okay. Since now both you and Junio asked for it, and I made today a Git 
> day for me, I looked into this.

Hi, the discussion on this topic unfortunately didn't result in a patch.
The problem is still true with current master, here's again how to
reproduce it

 $ mkdir repo && cd repo
 $ git init
 Initialized empty Git repository in .git/
 $ echo foo >file
 $ ln -s dangling link
 $ git add .
 $ git commit -mfoo
 Created initial commit c6a9189: foo
  2 files changed, 2 insertions(+), 0 deletions(-)
  create mode 100644 file
  create mode 120000 link
 $ git checkout -b branch
 Switched to a new branch "branch"
 $ git rm link
 rm 'link'
 $ git commit -mremovelink
 Created commit 2c60f15: removelink
  1 files changed, 0 insertions(+), 1 deletions(-)
  delete mode 120000 link
 $ mkdir link
 $ echo bar >link/file
 $ git add link
 $ git commit -m adddir
 Created commit d3b30b5: adddir
  1 files changed, 1 insertions(+), 0 deletions(-)
  create mode 100644 link/file
 $ echo bar >>file
 $ git commit -mfile file
 Created commit 8ddc4d5: file
  1 files changed, 1 insertions(+), 0 deletions(-)
 $ git checkout master
 Switched to branch "master"
 $ git cherry-pick 8ddc4d5
 CONFLICT (file/directory): There is a directory with name link in
 8ddc4d5... file. Added link as link~HEAD
 Automatic cherry-pick failed.  After resolving the conflicts,
 mark the corrected paths with 'git-add <paths>'
 and commit the result.
 When commiting, use the option '-c 8ddc4d5' to retain authorship and
 message.
 $ 

Thanks, Gerrit.

^ permalink raw reply

* [PATCH] Make "git stash" configurable
From: しらいしななこ @ 2007-12-21 10:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

"git stash" without argument originally created an unnamed
stash, but some people felt this can be confusing to new
users.  This introduces per-user config variable stash.quick to
control this behavior.

The variable can take one of three values: true, false, ask.

When set to "true", the command allows to create a quick
stash without any user interaction.  When set to "false",
the command shows the list of stash instead.  When set to
"ask", the command asks the user.

For the first time users, when the variable is not set,
the command helps the user to set it interactively.

Signed-off-by: Nanako Shiraishi <nanako3@bluebottle.com>
---
 Documentation/git-stash.txt |   17 +++++++++-
 git-stash.sh                |   72 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index c0147b9..5d39da2 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -22,11 +22,24 @@ and reverts the working directory to match the `HEAD` commit.
 The modifications stashed away by this command can be listed with
 `git-stash list`, inspected with `git-stash show`, and restored
 (potentially on top of a different commit) with `git-stash apply`.
-Calling git-stash without any arguments is equivalent to `git-stash
-save`.  A stash is by default listed as "WIP on 'branchname' ...", but
+A stash is by default listed as "WIP on 'branchname' ...", but
 you can give a more descriptive message on the command line when
 you create one.
 
+Calling git-stash without any arguments is equivalent to `git-stash
+save` when the command is run non-interactively.  When running
+interactively, the value of the `stash.quick` configuration
+variable in `$HOME/.gitconfig` determines what happens.
+
+ - If `stash.quick` is set to `true`, a stash without message is
+   created.
+
+ - If `stash.quick` is set to `false`, it shows the list of
+   stash, without creating a new one.
+
+ - If `stash.quick` is set to `ask`, the user is asked which
+   behavior is desired.
+
 The latest stash you created is stored in `$GIT_DIR/refs/stash`; older
 stashes are found in the reflog of this reference and can be named using
 the usual reflog syntax (e.g. `stash@\{0}` is the most recently
diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..b6223bd 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -192,6 +192,69 @@ apply_stash () {
 	fi
 }
 
+allow_quick_stash () {
+
+	quick=$(git config --global stash.quick)
+	if test $? != 0
+	then
+		if ! test -t 0 || ! test -t 1
+		then
+			return 0
+		fi
+
+		echo '
+*** First time users ***
+
+"git stash" can create an unnamed stash entry without user interaction.
+This is a quick way to save away your work in progress.  Some people
+find this behaviour confusing or dangerous to new users.  You can
+configure the command to list the existing stash entries instead.'
+
+		while :
+		do
+			echo '
+Do you want the command without argument to always...
+
+1. Ask for confirmation
+2. Create an unnamed stash
+3. List existing stash entries
+'
+			printf 'Which one? [1/2/3] '
+			read reply
+			quick=
+			case "$reply" in
+			1|A*)	quick=ask ;;
+			2|C*)	quick=true ;;
+			3|L*)	quick=false ;;
+			*)	continue ;;
+			esac
+			break
+		done
+		git config --global stash.quick $quick
+		echo '
+You can reconfigure this by editing your $HOME/.gitconfig file'
+
+	fi
+
+	case "$quick" in
+	true)	return 0 ;;
+	false)	return 1 ;;
+	ask)	: do not return ;;
+	esac
+
+	if ! test -t 0 || ! test -t 1
+	then
+		return 0
+	fi
+
+	printf 'Do you want to create an unnamed stash? [Y/n] '
+	read reply
+	case "$reply" in
+	[nN]*)	return 1 ;;
+	*)	return 0 ;;
+	esac
+}
+
 # Main command set
 case "$1" in
 list)
@@ -226,11 +289,16 @@ create)
 	create_stash "$*" && echo "$w_commit"
 	;;
 *)
-	if test $# -eq 0
+	if test $# -ne 0
+	then
+		usage
+	fi
+	if allow_quick_stash
 	then
 		save_stash && git-reset --hard
 	else
-		usage
+		echo "*** Stash List ***"
+		list_stash
 	fi
 	;;
 esac
-- 
1.5.3.7

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

----------------------------------------------------------------------
Free pop3 email with a spam filter.
http://www.bluebottle.com/tag/5

^ permalink raw reply related

* msysgit Git-1.5.4-rc1 available
From: Steffen Prohaska @ 2007-12-21 10:05 UTC (permalink / raw)
  To: Git Mailing List, msysGit


for download from

http://msysgit.googlecode.com/files/Git-1.5.4-rc1-preview20071221.exe

	Steffen

^ permalink raw reply

* git consistency and --help
From: Mike Frysinger @ 2007-12-21  9:37 UTC (permalink / raw)
  To: git

along the lines of the different aspects of git being consistent,
shouldnt the shell scripts all be changed to call `man` ?

these call `man`:
git diff --help
git log --help
git stash --help
git rebase --help
git-diff --help
git-log --help

these show a short usage:
git-stash --help
git-rebase --help

looking at the code, it comes down to the git-sh-setup.sh file mostly ...
-mike

^ permalink raw reply

* Re: 1.5.4-rc2 plans
From: Mike Frysinger @ 2007-12-21  9:09 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <476B6ABB.6040009@viscovery.net>

On Dec 21, 2007 2:26 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Junio C Hamano schrieb:
> >  * Should "git stash" stop doing anything useful?  I think the patch
> >    from Nana today may be a reasonable compromise, although I still
> >    think fundamentally different behaviour for the same command
> >    configurable per-user is not very nice (we have precedent in "git
> >    clean" already, though, but "git clean" is inherently dangerous
> >    command, and "git stash" is much more useful and the issue impacts
> >    more people).
>
> IMO we should give in and play the safe game. For those who don't like to
> type "git stash save" can always
>
>     git config --global alias.shelve "stash save"
>     git config --global alias.unshelve "stash apply"
>
> and retrain the fingers.

in the past, i used git merely to checkout code and send diffs to
maintainers ... never for my own work.  ive started to transition from
using svn everywhere to trying out git, and saw reference to this
"stash" command on another list.  i wanted to learn more about it, so
i started off with `git-stash` to get some info, and wondered what
just happened.  then i typoed the --help option and wondered even more
what just happened :).

after flipping through the git mailing list for a while, it's good to
see that `git stash <random crap>` will be fixed in the next release,
and yes the default behavior of saving is confusing.  the argument
that newbies can easily recover their work really only works if the
newbie knows what's going on.  if they knew from the start, then they
wouldnt be newbies eh.

making the default behavior non-destructive (which is to say, not
changing anything) and allowing people to arbitrarily configure the
default behavior sounds sane to me.  taking it up a level, people
could just as easily write functions in their shell environment to do
the same thing.  which is to say that imho, the argument against this
for fear of different behavior depending on user is over the top.
configuration options are there to change the behavior based on the
user's preference.
-mike

^ permalink raw reply

* Re: 1.5.4-rc2 plans
From: Steven Grimm @ 2007-12-21  8:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsr8lwf7.fsf@gitster.siamese.dyndns.org>

On Dec 20, 2007, at 4:32 PM, Junio C Hamano wrote:
> * handling of EDITOR in git commit and git tag is currently different.
>   It expects "vi" not "vi --some-funny-option".  I sent out a
>   for-discussion patch after seeing Steven's and Luciano's.

I'm perfectly fine with your approach (punt and let the shell figure  
it out). It's certainly a lot less code change than mine. However, if  
you want another rev of mine with Johannes' comments taken into  
account, I should be able to do that in the next day or two.

-Steve

^ permalink raw reply

* Re: git-stash: RFC: Adopt the default behavior to other commands
From: しらいしななこ @ 2007-12-21  8:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jörg Sommer
In-Reply-To: <7v4pedov6c.fsf@gitster.siamese.dyndns.org>

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

> しらいしななこ  <nanako3@bluebottle.com> writes:
>
>> How about making this behavior configurable?
>
> First, as a general principle, I'd like to avoid having commands that
> changes their behaviour drastically depending on who the user is.  It
> makes it harder for people experienced a bit more than totally new to
> help others.  If they are truly experts and are familiar about the
> configuration stash.quick, then they will be fine, but others would say
> "Well, it works for me -- 'git stash' itself won't stash but list.  Why
> isn't it working for you, I don't know" and scratch head.

I see.  I usually am not the person who helps but am the 
person who is helped in such a situation, and did not 
consider this issue.

> Having said that, I reserve rights to change my mind later and start
> liking this approach as a compromise.

I will change the patch as you suggested and resubmit.  Let's 
see if I can change your mind (^_^).

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

----------------------------------------------------------------------
Free pop3 email with a spam filter.
http://www.bluebottle.com/tag/5

^ permalink raw reply

* Re: git-stash: RFC: Adopt the default behavior to other commands
From: Wincent Colaiuta @ 2007-12-21  7:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: しらいしななこ, git,
	Jörg Sommer
In-Reply-To: <7v4pedov6c.fsf@gitster.siamese.dyndns.org>

El 20/12/2007, a las 23:31, Junio C Hamano escribió:

> しらいしななこ  <nanako3@bluebottle.com> writes:
>
>> How about making this behavior configurable?
>
> First, as a general principle, I'd like to avoid having commands that
> changes their behaviour drastically depending on who the user is.  It
> makes it harder for people experienced a bit more than totally new to
> help others.  If they are truly experts and are familiar about the
> configuration stash.quick, then they will be fine, but others would  
> say
> "Well, it works for me -- 'git stash' itself won't stash but list.   
> Why
> isn't it working for you, I don't know" and scratch head.

Although the patch is well-intentioned, I totally agree with Junio on  
this point.

The solution here isn't configuration, but education. Some people have  
already told how they've been burnt by doing an accidental stash, but  
how many times does this have to happen to you before you learn your  
lesson? Once? Twice if you are very unlucky?

And this is not a very painful lesson to learn, seeing as "git stash"  
is not an inherently destructive operation. In 99% of cases there is  
no risk of hard-to-reverse "damage". If you accidentally stash, you  
can just unstash with "git stash apply". I already posted a two-line  
patch which tells the user how to do this:

http://marc.info/?l=git&m=119799257404542&w=2

The 1% of cases in which "git stash apply" won't work is where the  
user has unsaved changes in running editors at the time they do the  
accidental stash. IMO, this is no justification to change the  
behaviour of stash. Exactly the same is true of other commands that  
alter the working tree; for example, what happens if you use "git  
checkout" to switch to another branch when you have unsaved changes in  
running editors? Are people suggesting that we should change the  
behaviour of "git checkout" to warn the user that they should save any  
unsaved changes before continuing and then hit "y"? I think that such  
a thing would be absurd.

Cheers,
Wincent

^ permalink raw reply

* Re: 1.5.4-rc2 plans
From: Johannes Sixt @ 2007-12-21  7:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwsr8lwf7.fsf@gitster.siamese.dyndns.org>

Junio C Hamano schrieb:
>  * Should "git stash" stop doing anything useful?  I think the patch
>    from Nana today may be a reasonable compromise, although I still
>    think fundamentally different behaviour for the same command
>    configurable per-user is not very nice (we have precedent in "git
>    clean" already, though, but "git clean" is inherently dangerous
>    command, and "git stash" is much more useful and the issue impacts
>    more people).

IMO we should give in and play the safe game. For those who don't like to
type "git stash save" can always

    git config --global alias.shelve "stash save"
    git config --global alias.unshelve "stash apply"

and retrain the fingers.

-- Hannes

^ permalink raw reply

* [PATCH] git-send-email: Add --suppress-all-from option.
From: David Brown @ 2007-12-21  6:01 UTC (permalink / raw)
  To: git; +Cc: David Brown

Sometimes, it is useful to be able to send a patch to a third party
without the author of the patch being copied on the message.  An
common example would be an internal discussion at a company to ask if
a particular patch should be applied.  Some environments may even have
policy against such mail being sent outside of the company.

Add the --suppress-all-from/--no-suppress-all-from options to avoid
sending patches to the patch author, even if different from the
sender.  Add the sendemail.suppressallfrom config option to allow this
to have a different default.

Signed-off-by: David Brown <git@davidb.org>
---
 Documentation/git-send-email.txt |    7 +++++++
 git-send-email.perl              |    9 ++++++++-
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index f0bd285..5d06264 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -117,6 +117,13 @@ The --cc option must be repeated for each user you want on the cc list.
         Default is the value of 'sendemail.suppressfrom' configuration value;
         if that is unspecified, default to --no-suppress-from.
 
+--suppress-all-from, --no-suppress-all-from::
+        If this is set, do not add the From: address to the cc: list,
+        even if it is different than the person sending the email.
+        Default is the value of the 'sendemail.suppressallfrom'
+        configuration value; if that is unspecified, default to
+        -no-suppress-all-from.
+
 --thread, --no-thread::
 	If this is set, the In-Reply-To header will be set on each email sent.
 	If disabled with "--no-thread", no emails will have the In-Reply-To
diff --git a/git-send-email.perl b/git-send-email.perl
index 248d035..80265b5 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -90,6 +90,9 @@ Options:
 
    --suppress-from Suppress sending emails to yourself. Defaults to off.
 
+   --suppress-all-from Never automatically send to a patch author.
+                  Defaults to off.
+
    --thread       Specify that the "In-Reply-To:" header should be set on all
                   emails. Defaults to on.
 
@@ -174,7 +177,8 @@ if ($@) {
 my ($quiet, $dry_run) = (0, 0);
 
 # Variables with corresponding config settings
-my ($thread, $chain_reply_to, $suppress_from, $signed_off_cc, $cc_cmd);
+my ($thread, $chain_reply_to, $suppress_from, $suppress_all_from);
+my ($signed_off_cc, $cc_cmd);
 my ($smtp_server, $smtp_server_port, $smtp_authuser, $smtp_authpass, $smtp_ssl);
 my ($identity, $aliasfiletype, @alias_files, @smtp_host_parts);
 
@@ -182,6 +186,7 @@ my %config_bool_settings = (
     "thread" => [\$thread, 1],
     "chainreplyto" => [\$chain_reply_to, 1],
     "suppressfrom" => [\$suppress_from, 0],
+    "suppressallfrom" => [\$suppress_all_from, 0],
     "signedoffcc" => [\$signed_off_cc, 1],
     "smtpssl" => [\$smtp_ssl, 0],
 );
@@ -218,6 +223,7 @@ my $rc = GetOptions("sender|from=s" => \$sender,
 		    "quiet" => \$quiet,
 		    "cc-cmd=s" => \$cc_cmd,
 		    "suppress-from!" => \$suppress_from,
+		    "suppress-all-from!" => \$suppress_all_from,
 		    "signed-off-cc|signed-off-by-cc!" => \$signed_off_cc,
 		    "dry-run" => \$dry_run,
 		    "envelope-sender=s" => \$envelope_sender,
@@ -700,6 +706,7 @@ foreach my $t (@files) {
 					$subject = $1;
 
 				} elsif (/^(Cc|From):\s+(.*)$/) {
+					next if ($suppress_all_from);
 					if (unquote_rfc2047($2) eq $sender) {
 						next if ($suppress_from);
 					}
-- 
1.5.3.7

^ permalink raw reply related

* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21  5:21 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712202054350.21557@woody.linux-foundation.org>



On Thu, 20 Dec 2007, Linus Torvalds wrote:
> 
> Not that it matters in real life, since nobody uses -U0, and "git blame" 
> won't care. But let's get it right anyway ;)

.. and here's a test-case to see the *impact* of this whole issue:

	yes | head -1000 > a
	yes | head -2000 > b
	git diff -U0 a b | grep @@
	diff -U0 a b | grep @@

and notice the differences.

Both will add a thousand lines of "y", but the "git diff" will add it at a 
different point, ie git says:

	@@ -488,0 +489,1000 @@ y

while a non-tail-optimizing diff will say

	@@ -1000,0 +1001,1000 @@

which may be a bit more obvious.

Both answers are *correct*, though. The particular choice of "insert at 
line 489, after line 488" is a bit odd, but is because we don't actually 
search to exactly the beginning of where the differences started, we 
search in blocks of 1kB and then we go forward to the next newline.

(We could also go to exactly where the differences started, and if the 
previous character was a newline or the beginning of the file, we'd not 
move forward at all, and then we'd get a more "logical" diff of inserting 
at the beginning).

Considering that the answer is correct, and this only happens for insane 
cases anyway, I don't really think anybody cares, but it's an interesting 
case nonetheless.

			Linus

^ permalink raw reply

* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21  4:58 UTC (permalink / raw)
  To: Kyle McMartin, Junio C Hamano, Git Mailing List; +Cc: Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712202009290.21557@woody.linux-foundation.org>



On Thu, 20 Dec 2007, Linus Torvalds wrote:
>
> And here's the git patch to avoid this optimization when there is 
> context.

Actually, the code to finding one '\n' is still needed to avoid the 
(pathological) case of getting a "\No newline", so scrap that one which 
was too aggressive, and use this (simpler) one instead.

Not that it matters in real life, since nobody uses -U0, and "git blame" 
won't care. But let's get it right anyway ;)

This whole function has had more bugs than it has lines.

		Linus

---
 xdiff-interface.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/xdiff-interface.c b/xdiff-interface.c
index 9ee877c..711029e 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -115,15 +115,18 @@ static void trim_common_tail(mmfile_t *a, mmfile_t *b, long ctx)
 	char *bp = b->ptr + b->size;
 	long smaller = (a->size < b->size) ? a->size : b->size;
 
+	if (ctx)
+		return;
+
 	while (blk + trimmed <= smaller && !memcmp(ap - blk, bp - blk, blk)) {
 		trimmed += blk;
 		ap -= blk;
 		bp -= blk;
 	}
 
-	while (recovered < trimmed && 0 <= ctx)
+	while (recovered < trimmed)
 		if (ap[recovered++] == '\n')
-			ctx--;
+			break;
 	a->size -= (trimmed - recovered);
 	b->size -= (trimmed - recovered);
 }

^ permalink raw reply related

* Re: Linux 2.6.24-rc6
From: Kyle McMartin @ 2007-12-21  4:50 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Junio C Hamano, Git Mailing List, Raphael Assenat, Andrew Morton,
	Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712202026040.21557@woody.linux-foundation.org>

On Thu, Dec 20, 2007 at 08:40:54PM -0800, Linus Torvalds wrote:
> That was a rather long-winded explanation of what happened, mainly because 
> it was all very unexpected to me, and I had personally mistakenly thought 
> the git optimization was perfectly valid and actually had to go through 
> the end result to see what was going on.
> 
> Anyway, the diff on kernel.org should be all ok now, and mirrored out too.
> 

Thanks again for being so quick to track this down, applies fine and is
out for building in rawhide now.

cheers, Kyle

^ permalink raw reply

* Re: Linux 2.6.24-rc6
From: Linus Torvalds @ 2007-12-21  4:40 UTC (permalink / raw)
  To: Kyle McMartin, Junio C Hamano, Git Mailing List, Raphael Assenat,
	Andrew Morton
  Cc: Linux Kernel Mailing List
In-Reply-To: <alpine.LFD.0.9999.0712202009290.21557@woody.linux-foundation.org>



On Thu, 20 Dec 2007, Linus Torvalds wrote:
> 
> It only happened for a few files that had lots of repeated lines - so that 
> the diff could literally be done multiple different ways - and in fact, 
> the file that caused the problems really had a bogus commit that 
> duplicated *way* too much data, and caused lots of #define's to exist 
> twice.

Here's the example of this kind of behaviour: in the 2.6.26-rc5 tree the 
file drivers/video/mbx/reg_bits.h has the #defines for 

	/* DINTRS - Display Interrupt Status Register */
	/* DINTRE - Display Interrupt Enable Register */

duplicated twice due to commit ba282daa919f89c871780f344a71e5403a70b634 
("mbxfb: Improvements and new features") by Raphael Assenat mistakenly 
adding another copy of the same old set of defines that we already got 
added once before by commit fb137d5b7f2301f2717944322bba38039083c431 
("mbxfb: Add more registers bits access macros").

Now, that was a mistake - and one that probably happened because Rafael or 
more likely Andrew Morton used GNU patch with its insane defaults (which 
is to happily apply the same patch that adds things twice, because it 
doesn't really care if the context matches or not).

But what that kind of thing causes is that when you create a patch of the 
end result, it can show the now new duplicate lines two different (but 
equally valid) ways: it can show it as an addition of the _first_ set of 
lines, or it can show it as an addition of the _second_ set of lines. They 
are the same, after all.

Now, it doesn't really matter which way you choose to show it, although 
because of how "git diff" finds similarities, it tends to prefer to show 
the second set of identical lines as the "new" ones. Which is generally 
reasonable.

However, that interacted really badly with the new git logic that said 
that "if the two files end in the same sequence, just ignore the common 
tail of the file", because the latter copy of the identical lines would 
now show up as _part_ of that common tail, so the lines that the git diff 
machinery would normally like to show up as "new" did in fact end up being 
considered uninteresting, because they were part of an idential tail. 

So now "git diff" would happily pick _earlier_ lines as the new ones, and 
it would still be a conceptually valid diff, but because we had trimmed 
the tail of the file, that conceptually valid diff no longer had the 
expected shared context at the end.

And while it's a bit embarrassing, I'm really rather happy that both GNU 
patch and "git apply" actually refused to apply the patch. It may have 
been "conceptually correct" (ie it did really contain all of the changes!) 
but because it lacked the expected context it really wasn't a good patch. 

That was a rather long-winded explanation of what happened, mainly because 
it was all very unexpected to me, and I had personally mistakenly thought 
the git optimization was perfectly valid and actually had to go through 
the end result to see what was going on.

Anyway, the diff on kernel.org should be all ok now, and mirrored out too.

			Linus

^ 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