Git development
 help / color / mirror / Atom feed
* Re: git-am: "Patch fragment without a header"
From: H. Peter Anvin @ 2006-02-07  3:32 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwtg73ld7.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> 
> Linus would have said "Say No to MIME".  I am a bit too busy
> right now so if you are inclined to take a look, the problem is
> in mailinfo.
> 

Unfortunately git-mailinfo is in C, otherwise I'd have suggested using 
the Perl MIME-tools, which seems to have all this stuff in it.

It might just be easier to try to rewrite git-mailinfo in Perl...

	-hpa

^ permalink raw reply

* Re: gitk changing line color for no reason after merge
From: Pavel Roskin @ 2006-02-07  5:18 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: git
In-Reply-To: <1138900897.28967.18.camel@dv>

Hello!

Sorry for replying to myself, but there is nobody else to reply to.

> I think it would be much better if line colors only change at
> non-trivial nodes, i.e. those with more than one child or parent.

I didn't realize that the colors correspond to nodes, not branches.
Every node has one color that is used for lines to all of its children.
It would be much better to assign colors to "branches" consisting of
individual lines connecting nodes, but changing that would require many
changes in gitk.

> diff --git a/gitk b/gitk
> index f12b3ce..14ff570 100755
> --- a/gitk
> +++ b/gitk
> @@ -770,7 +770,7 @@ proc assigncolor {id} {
>  
>      if [info exists colormap($id)] return
>      set ncolors [llength $colors]
> -    if {$nparents($id) <= 1 && $nchildren($id) == 1} {
> +    if {$nchildren($id) == 1} {
>  	set child [lindex $children($id) 0]
>  	if {[info exists colormap($child)]
>  	    && $nparents($child) == 1} {
> 
> 

I still stand behind this patch because it eliminates color changes on
the nodes that have exactly one child and parent.  $nparents($id) is
irrelevant here, because it characterizes the current node, but the code
decides whether the line should change color at the child node.

However, further changes to reduce color changes didn't produce nice
results for me.  If I try to keep one color running as long as possible,
I get branches of the same color because, as I said, gitk uses the same
color for connections to all children.  So, every node on the branch
spurs branching lines of the same color, which can intersect or run
side-by-side.

I can submit this patch formally, but I hope to get some comments first.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* Re: git-am: "Patch fragment without a header"
From: Junio C Hamano @ 2006-02-07  5:35 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: git
In-Reply-To: <43E814C2.6090104@zytor.com>

"H. Peter Anvin" <hpa@zytor.com> writes:

> Unfortunately git-mailinfo is in C, otherwise I'd have suggested using
> the Perl MIME-tools, which seems to have all this stuff in it.

Heh, spawn Perl for every message?  I'd be ****ed by Linus if I
did so ;-).

This should fix it and I'd appreciate if you try it on other
messages.

I tried it on the message you quoted with:

    git-mailinfo -u /var/tmp/msg /var/tmp/patch <./+hpa.eml >/var/tmp/info

The resulting 'msg' and 'info' looks reasonable utf8 and patch
was not corrupt.

-- >8 --
[PATCH] mailinfo: reset CTE after each multipart

If the first part uses quoted-printable to protect iso8859-1
name in the commit log, and the second part was plain ascii text
patchfile without even Content-Transfer-Encoding subheader, we
incorrectly tried to decode the patch as quoted printable.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/mailinfo.c b/mailinfo.c
index 0265a29..ff2d4d4 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -707,6 +707,9 @@ static void handle_multipart_body(void)
 		if (!len) {
 			if (handle_multipart_one_part() < 0)
 				return;
+			/* Reset per part headers */
+			transfer_encoding = TE_DONTCARE;
+			charset[0] = 0;
 		}
 		else
 			check_subheader_line(line, len);

^ permalink raw reply related

* Heads up, git.git approaches 1.2.0
From: Junio C Hamano @ 2006-02-07  8:40 UTC (permalink / raw)
  To: git
In-Reply-To: <7vbqxkapr2.fsf@assigned-by-dhcp.cox.net>

I've done a bit more work to round the corners diff-tree -c/--cc
had.  Most notably, earlier the algorithm punted if the merge
removed or added files.  These should raise red flag, because
diff-tree -c/--cc deal with case where the result is different
from all parents, so removal of a file means all parents had it
in some shape but the file does not appear in the result, and
addition of a file means no parents had it but merge made it
magically appear.  These cases are now shown as interesting.

BTW, I was pleasantly surprised that "git show 2950411" showed
exactly what the commit log message said.  Now I agree with
Linus that --cc is a wonderful thing ;-).

Tonight I have pushed out all I said I would want to have in the
next feature release 1.2.0 in the master branch, so that people
can beat it more easily and help us shaking out potential bugs.
Most of the things are something I've been using myself on git
itself and also during my day-job, so there hopefully won't be
any major data eating problems.  Admittedly I almost never do
"git commit paths...", but for the last couple of days I tried
to use it whenever possible to make sure it works as intended.
It defaults to --include behaviour which is the traditional one.

"git commit --only paths..." and running "git commit" from a
subdirectory are also implemented and I've tried them with toy
commits.  If you are one of the people who missed them, please
try it out and see if they satisfy your expectations.

I might receive some more HTTP fixes from Mark, documentation
updates from J. Bruce etc. and I'd merge them before 1.2.0. But
otherwise I am pretty happy with what's there in the "master"
branch right now.  The tip of the "master" is this commit:

    commit 5a798fb57f788692467b1a11416dd5ebff0d31ae
    Author: Junio C Hamano <junkio@cox.net>

        git-commit: finishing touches.

Please consider it a 1.2.0-rc1, and treat it the same way you
would treat -rc1 from the kernel project.  If you are going to
test only one -rc version, this is THE one to test, and all the
rest are bugfixes.

^ permalink raw reply

* [PATCH] format-patch: Remove last vestiges of --mbox option
From: Andreas Ericsson @ 2006-02-07  9:37 UTC (permalink / raw)
  To: git

Don't mention it in docs or --help output.
Remove mbox, date and author variables from git-format-patch.sh.

Use DESCRIPTION text from man-page to update LONG_USAGE output. It's
a bit silly to have two texts saying the same thing in different words,
and I'm too lazy to update both.

Signed-off-by: Andreas Ericsson <ae@op5.se>

---

 Documentation/git-format-patch.txt |   32 ++++++++++----------------------
 git-format-patch.sh                |   36 ++++++++++++++++++------------------
 2 files changed, 28 insertions(+), 40 deletions(-)

6fc31fee29cfea12a6213a775231ac6c136989d3
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 47705de..9ac0636 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -9,28 +9,27 @@ git-format-patch - Prepare patches for e
 SYNOPSIS
 --------
 [verse]
-'git-format-patch' [-n | -k] [-o <dir> | --stdout] [-s] [-c] [--mbox]
+'git-format-patch' [-n | -k] [-o <dir> | --stdout] [-s] [-c]
 		 [--diff-options] <his> [<mine>]
 
 DESCRIPTION
 -----------
 Prepare each commit with its patch since <mine> head forked from
-<his> head, one file per patch, for e-mail submission.  Each
-output file is numbered sequentially from 1, and uses the first
-line of the commit message (massaged for pathname safety) as the
-filename.
+<his> head, one file per patch formatted to resemble UNIX mailbox
+format, for e-mail submission or use with gitlink:git-am[1].
 
-When -o is specified, output files are created in that
-directory; otherwise in the current working directory.
+Each output file is numbered sequentially from 1, and uses the
+first line of the commit message (massaged for pathname safety)
+as the filename.
+
+When -o is specified, output files are created in <dir>; otherwise
+they are created in the current working directory.  This option
+is ignored if --stdout is specified.
 
 When -n is specified, instead of "[PATCH] Subject", the first
 line is formatted as "[PATCH N/M] Subject", unless you have only
 one patch.
 
-When --mbox is specified, the output is formatted to resemble
-UNIX mailbox format, and can be concatenated together for
-processing with applymbox.
-
 
 OPTIONS
 -------
@@ -45,11 +44,6 @@ OPTIONS
 	Do not strip/add '[PATCH]' from the first line of the
 	commit log message.
 
--a|--author, -d|--date::
-	Output From: and Date: headers for commits made by
-	yourself as well.  Usually these are output only for
-	commits made by people other than yourself.
-
 -s|--signoff::
 	Add `Signed-off-by:` line to the commit message, using
 	the committer identity of yourself.
@@ -61,12 +55,6 @@ OPTIONS
         has a SP character immediately followed by a TAB
         character.
 
--m|--mbox::
-	Format the output files for closer to mbox format by
-	adding a phony Unix "From " line, so they can be
-	concatenated together and fed to `git-applymbox`.
-	Implies --author and --date.
-
 --stdout::
 	This flag generates the mbox formatted output to the
 	standard output, instead of saving them into a file per
diff --git a/git-format-patch.sh b/git-format-patch.sh
index 5fb8ce1..e54c9e4 100755
--- a/git-format-patch.sh
+++ b/git-format-patch.sh
@@ -3,23 +3,23 @@
 # Copyright (c) 2005 Junio C Hamano
 #
 
-USAGE='[-n | -k] [-o <dir> | --stdout] [--signoff] [--check] [--mbox] [--diff-options] <upstream> [<our-head>]'
-LONG_USAGE='Prepare each commit with its patch since our-head forked from upstream,
-one file per patch, for e-mail submission.  Each output file is
-numbered sequentially from 1, and uses the first line of the commit
-message (massaged for pathname safety) as the filename.
-
-There are three output modes.  By default, output files are created in
-the current working directory; when -o is specified, they are created
-in that directory instead; when --stdout is specified, they are spit
-on standard output, and can be piped to git-am.
-
-When -n is specified, instead of "[PATCH] Subject", the first line is formatted
-as "[PATCH N/M] Subject", unless you have only one patch.
-
-When --mbox is specified, the output is formatted to resemble
-UNIX mailbox format, and can be concatenated together for processing
-with applymbox.'
+USAGE='[-n | -k] [-o <dir> | --stdout] [--signoff] [--check] [--diff-options] <his> [<mine>]'
+LONG_USAGE='Prepare each commit with its patch since <mine> head forked from
+<his> head, one file per patch formatted to resemble UNIX mailbox
+format, for e-mail submission or use with git-am.
+
+Each output file is numbered sequentially from 1, and uses the
+first line of the commit message (massaged for pathname safety)
+as the filename.
+
+When -o is specified, output files are created in <dir>; otherwise
+they are created in the current working directory.  This option
+is ignored if --stdout is specified.
+
+When -n is specified, instead of "[PATCH] Subject", the first
+line is formatted as "[PATCH N/M] Subject", unless you have only
+one patch.'
+
 . git-sh-setup
 
 # Force diff to run in C locale.
@@ -48,7 +48,7 @@ do
     -s|--s|--si|--sig|--sign|--signo|--signof|--signoff)
     signoff=t ;;
     --st|--std|--stdo|--stdou|--stdout)
-    stdout=t mbox=t date=t author=t ;;
+    stdout=t ;;
     -o=*|--o=*|--ou=*|--out=*|--outp=*|--outpu=*|--output=*|--output-=*|\
     --output-d=*|--output-di=*|--output-dir=*|--output-dire=*|\
     --output-direc=*|--output-direct=*|--output-directo=*|\
-- 
1.1.6.ga2c6

^ permalink raw reply related

* Re: gitk changing line color for no reason after merge
From: Junio C Hamano @ 2006-02-07 10:04 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: Paul Mackerras, git
In-Reply-To: <1139289517.15955.23.camel@dv>

Pavel Roskin <proski@gnu.org> writes:

> I still stand behind this patch because it eliminates color changes on
> the nodes that have exactly one child and parent.  $nparents($id) is
> irrelevant here, because it characterizes the current node, but the code
> decides whether the line should change color at the child node.

It all depends on what you are trying to achieve with colours.
Being a bit colour-challenged, I am not in a good position to
comment on how much gitk's use of different colours is helping
the readability, but I suspect not very much.  Use of too many
colours just makes things distracting.  Especially weaker
colours like light yellow are very hard to see on a white
background, at least to me.

Trying to differenciate "trunk" with "side branches" may be
sometimes useful in a small one-man project, but it quickly
breaks down once you start merging from all over the place,
since merges in distributed development do not have inherent
distinction between "trunk" and "side branches".

One thing it _could_ do is to assign different colours to edges
coming into a merge node, and match the colour of the edge that
leads to a parent to the colour in which the diff text from
that parent is shown.  Then the colouring becomes somewhat
meaningful.

Maybe gitk is already doing it, but it makes me suspect it
doesn't, to read the way the code initialises "$ctext tag conf
m$N" and uses the m$N tag for each diff output line, which is
done pretty much independently from the procedure that paint
edges (the one you touch in your patch).

^ permalink raw reply

* [PATCH] http-fetch: Abort requests for objects which arrived in packs
From: Mark Wooding @ 2006-02-07 10:07 UTC (permalink / raw)
  To: git

From: Mark Wooding <mdw@distorted.org.uk>

In fetch_object, there's a call to release an object request if the
object mysteriously arrived, say in a pack.  Unfortunately, the fetch
attempt for this object might already be in progress, and we'll leak the
descriptor.  Instead, try to tidy away the request.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
---

 http-fetch.c |   16 +++++++++++++++-
 http.c       |   18 +++++++++++++++++-
 http.h       |    1 +
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/http-fetch.c b/http-fetch.c
index bddbd6b..ce3df5f 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -773,6 +773,20 @@ static int fetch_pack(struct alt_base *r
 	return 0;
 }
 
+static void abort_object_request(struct object_request *obj_req)
+{
+	if (obj_req->local >= 0) {
+		close(obj_req->local);
+		obj_req->local = -1;
+	}
+	unlink(obj_req->tmpfile);
+	if (obj_req->slot) {
+ 		release_active_slot(obj_req->slot);
+		obj_req->slot = NULL;
+	}
+	release_object_request(obj_req);
+}
+
 static int fetch_object(struct alt_base *repo, unsigned char *sha1)
 {
 	char *hex = sha1_to_hex(sha1);
@@ -785,7 +799,7 @@ static int fetch_object(struct alt_base 
 		return error("Couldn't find request for %s in the queue", hex);
 
 	if (has_sha1_file(obj_req->sha1)) {
-		release_object_request(obj_req);
+		abort_object_request(obj_req);
 		return 0;
 	}
 
diff --git a/http.c b/http.c
index 632c2c5..14a7669 100644
--- a/http.c
+++ b/http.c
@@ -420,10 +420,26 @@ void run_active_slot(struct active_reque
 #endif
 }
 
-static void finish_active_slot(struct active_request_slot *slot)
+static void closedown_active_slot(struct active_request_slot *slot)
 {
         active_requests--;
         slot->in_use = 0;
+}
+
+void release_active_slot(struct active_request_slot *slot)
+{
+	closedown_active_slot(slot);
+	if (slot->curl) {
+		curl_multi_remove_handle(curlm, slot->curl);
+		curl_easy_cleanup(slot->curl);
+		slot->curl = NULL;
+	}
+	fill_active_slots();
+}
+
+static void finish_active_slot(struct active_request_slot *slot)
+{
+	closedown_active_slot(slot);
         curl_easy_getinfo(slot->curl, CURLINFO_HTTP_CODE, &slot->http_code);
 
 	/* Store slot results so they can be read after the slot is reused */
diff --git a/http.h b/http.h
index d6dc9d8..36fa154 100644
--- a/http.h
+++ b/http.h
@@ -61,6 +61,7 @@ extern struct active_request_slot *get_a
 extern int start_active_slot(struct active_request_slot *slot);
 extern void run_active_slot(struct active_request_slot *slot);
 extern void finish_all_active_slots(void);
+extern void release_active_slot(struct active_request_slot *slot);
 
 #ifdef USE_CURL_MULTI
 extern void fill_active_slots(void);

^ permalink raw reply related

* gitk now uses git-diff-tree --cc
From: Paul Mackerras @ 2006-02-07 10:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Junio,

I have pushed out the changes to gitk to make it use git-diff-tree
--cc, and display its output with colorization.  It's now 400 lines
shorter. :)  Perhaps you could pull the new version into your git
repository?

Thanks,
Paul.

^ permalink raw reply

* Re: gitk changing line color for no reason after merge
From: Paul Mackerras @ 2006-02-07  8:56 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: git
In-Reply-To: <1139289517.15955.23.camel@dv>

Pavel Roskin writes:
> Hello!

> I didn't realize that the colors correspond to nodes, not branches.
> Every node has one color that is used for lines to all of its children.
> It would be much better to assign colors to "branches" consisting of
> individual lines connecting nodes, but changing that would require many

Why would that be better?

> > -    if {$nparents($id) <= 1 && $nchildren($id) == 1} {
> > +    if {$nchildren($id) == 1} {
> >  	set child [lindex $children($id) 0]
> >  	if {[info exists colormap($child)]
> >  	    && $nparents($child) == 1} {
> > 
> > 
> 
> I still stand behind this patch because it eliminates color changes on
> the nodes that have exactly one child and parent.  $nparents($id) is

Ummm... I don't see that you have changed anything for nodes that have
exactly one parent?

> However, further changes to reduce color changes didn't produce nice
> results for me.  If I try to keep one color running as long as possible,
> I get branches of the same color because, as I said, gitk uses the same
> color for connections to all children.  So, every node on the branch
> spurs branching lines of the same color, which can intersect or run
> side-by-side.

The colors are really just to make the lines visually distinct.  They
(the colors) have no semantic meaning.  (I did try coloring the lines
according to the committer, but I just ended up with an awful lot of
lines being the same color - corresponding to one Linus Torvalds. :)

Paul.

^ permalink raw reply

* [PATCH 1/2] Document git-diff-tree --always
From: Petr Baudis @ 2006-02-07 11:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Documentation/git-diff-tree.txt |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
index 674ee61..9153e4c 100644
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -95,6 +95,10 @@ separated with a single space are given.
 	hunks disappear, the commit itself and the commit log
 	message is not shown, just like any other "empty diff" cases.
 
+--always::
+	Show the commit itself and the commit log message even
+	if the diff itself is empty.
+
 
 Limiting Output
 ---------------

^ permalink raw reply related

* [PATCH 2/2] Basic documentation for git-show
From: Petr Baudis @ 2006-02-07 11:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20060207114744.27532.21094.stgit@machine.or.cz>

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Documentation/git-show.txt |   49 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-show.txt b/Documentation/git-show.txt
new file mode 100644
index 0000000..9c359a4
--- /dev/null
+++ b/Documentation/git-show.txt
@@ -0,0 +1,49 @@
+git-show(1)
+===========
+
+NAME
+----
+git-show - Show one commit with difference it introduces.
+
+
+SYNOPSIS
+--------
+'git-show' <option>...
+
+DESCRIPTION
+-----------
+Shows commit log and textual diff for a single commit.  The
+command internally invokes 'git-rev-list' piped to
+'git-diff-tree', and takes command line options for both of
+these commands. It also presents the merge commit in a special
+format as produced by 'git-diff-tree --cc'.
+
+This manual page describes only the most frequently used options.
+
+
+OPTIONS
+-------
+<commitid>::
+	ID of the commit to show.
+
+--pretty=<format>::
+	Controls the output format for the commit logs.
+	<format> can be one of 'raw', 'medium', 'short', 'full',
+	and 'oneline'.
+
+Author
+------
+Written by Linus Torvalds <torvalds@osdl.org> and
+Junio C Hamano <junkio@cox.net>
+
+
+Documentation
+-------------
+Documentation by David Greaves, Petr Baudis and the git-list <git@vger.kernel.org>.
+
+This manual page is a stub. You can help the git documentation by expanding it.
+
+GIT
+---
+Part of the gitlink:git[7] suite
+

^ permalink raw reply related

* Re: [Cogito] Various bugs
From: Jonas Fonseca @ 2006-02-07 13:55 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060207003643.GJ31278@pasky.or.cz>

Petr Baudis <pasky@suse.cz> wrote Tue, Feb 07, 2006:
> Dear diary, on Tue, Jan 31, 2006 at 05:13:18AM CET, I got a letter
> where Jonas Fonseca <fonseca@diku.dk> said that...
> > A few Cogito bugs I found yesterday.
> > 
> >  - cg-fetch between local repos fails when the cloned branch URL does
> >    not point to a .git directory and a needed object from the repository
> >    being cloned is packed. git-local-fetch expects a .git directory.
> 
> Can't reproduce and I don't buy it. The very first line of local fetch
> handler is
> 
> 	[ -d "$uri/.git" ] && uri="$uri/.git"

I cannot reproduce it either and my strace file is long gone thanks to
cg-clean. Come to think of it it might have been caused by a pack file
not following the more strict name rules which recently was introduced
in GIT.

> >  - cg-status reports a deleted file both as deleted and as unknown:
> > 
> > 	fonseca@antimatter:~/src/elinks/0.12 > git --version
> > 	git version 1.1.6.g1506
> > 	fonseca@antimatter:~/src/elinks/0.12 > cg --version
> > 	cogito-0.17pre.GIT (d3aa9a2b3375e36c774ea477492db76baa1db03e)
> > 	fonseca@antimatter:~/src/elinks/0.12 > cg rm AUTHORS
> > 	Removing file AUTHORS
> > 	fonseca@antimatter:~/src/elinks/0.12 > cg status | grep AUTHORS
> > 	? AUTHORS
> > 	D AUTHORS
> 
> This is fine, I'd say. The file was not deleted from the tree, either do
> that manually by rm or say cg-rm -f.

Yes, I guess you are right, this is a special case. But I think it needs
to be noted that this is 'normal'. Something like this vague patch
signed-off-by me.

diff --git a/cg-status b/cg-status
index 6abc52f..d38c61f 100755
--- a/cg-status
+++ b/cg-status
@@ -21,12 +21,12 @@
 # D::
 #	'<file>' has been deleted.
 # !::
-#	'<file>' is gone from your working copy but not deleted by cg-rm.
+#	'<file>' is gone from your working copy but not deleted by `cg-rm`.
 # M::
 #	'<file>' has been touched or modified.
 # m::
 #	'<file>' has been touched or modified, but will not be automatically
-#	committed the next time you call cg-commit. This is used during a
+#	committed the next time you call `cg-commit`. This is used during a
 #	merge to mark files which contained local changes before the merge.
 #
 # OPTIONS
@@ -55,6 +55,14 @@
 #	Path to the directory to use as the base for the working tree
 #	file list (instead of the current directory).
 #
+# NOTES
+# -----
+# If a file has been removed with `cg-rm` without using the `-f` option
+# to remove it physically from the tree it will be reported as both being
+# deleted and unknown. The reason for this is that the file is internally
+# marked as deleted and thus also untracked. After next commit it will only
+# be reported as being untracked.
+#
 # FILES
 # -----
 # $GIT_DIR/info/exclude::

-- 
Jonas Fonseca

^ permalink raw reply related

* Re: git-am: "Patch fragment without a header"
From: Linus Torvalds @ 2006-02-07 15:46 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: Junio C Hamano, git
In-Reply-To: <43E814C2.6090104@zytor.com>



On Mon, 6 Feb 2006, H. Peter Anvin wrote:
> 
> Unfortunately git-mailinfo is in C, otherwise I'd have suggested using the
> Perl MIME-tools, which seems to have all this stuff in it.
> 
> It might just be easier to try to rewrite git-mailinfo in Perl...

It might just be easier to make the mime checking go the hell away.

I do _not_ want to see some random perl code for the stuff I use (hundreds 
of times) a day. I can apply 5 patches a second right now, and more 
importantly, I know what it does.

Partly because I always edit out the stupid MIME stuff. I think it was a 
mistake to accept MIME patches in the first place.

		Linus

^ permalink raw reply

* Re: [Cogito] Various bugs
From: Linus Torvalds @ 2006-02-07 15:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Petr Baudis, git
In-Reply-To: <7v8xsn50rf.fsf@assigned-by-dhcp.cox.net>



On Mon, 6 Feb 2006, Junio C Hamano wrote:
> 
> It depends on what you expect, but it meets _my_ expectation:
> 
>     $ GIT_AUTHOR_NAME='' git-commit-tree $(git-write-tree) </dev/null
>     Committing initial tree a2b59c3848164a2c9c3c75fbaadccaed9485da92
>     ef90563fa278735af367e7606ea7eb2559121ca7
>     $ git-cat-file commit ef90563fa278735af367e7606ea7eb2559121ca7
>     tree a2b59c3848164a2c9c3c75fbaadccaed9485da92
>     author  <junkio@cox.net> 1139281078 -0800
>     committer Junio C Hamano <junkio@cox.net> 1139281078 -0800
> 
> That is, the user said GIT_AUTHOR_NAME is empty, so he gets a
> commit with an empty author name.

Yes. That said, we should probably disallow that in git-commit-tree (and 
let the user fix it up some way).

> get_ident() in ident.c does this. getenv("GIT_AUTHOR_NAME") and
> friends are passed to it, and git_default_* are takenfrom gecos.
> It might match some peoples' expectation (but not mine) if we
> did this instead.

No, don't use the default name.

An empty GIT_AUTHOR_NAME should _not_ mean that we use the default name 
(which is usually the committer), because rather than meaning "default", 
it most likely means "buggy import script". 

I'd rather have an email import of mine say that it cannot commit, than 
have it put "Linus Torvalds" in the author line (and some random email).

			Linus

^ permalink raw reply

* Re: [Cogito] Various bugs
From: Petr Baudis @ 2006-02-07 16:49 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0602070751410.3854@g5.osdl.org>

Dear diary, on Tue, Feb 07, 2006 at 04:53:57PM CET, I got a letter
where Linus Torvalds <torvalds@osdl.org> said that...
> 
> 
> On Mon, 6 Feb 2006, Junio C Hamano wrote:
> > 
> > It depends on what you expect, but it meets _my_ expectation:
> > 
> >     $ GIT_AUTHOR_NAME='' git-commit-tree $(git-write-tree) </dev/null
> >     Committing initial tree a2b59c3848164a2c9c3c75fbaadccaed9485da92
> >     ef90563fa278735af367e7606ea7eb2559121ca7
> >     $ git-cat-file commit ef90563fa278735af367e7606ea7eb2559121ca7
> >     tree a2b59c3848164a2c9c3c75fbaadccaed9485da92
> >     author  <junkio@cox.net> 1139281078 -0800
> >     committer Junio C Hamano <junkio@cox.net> 1139281078 -0800
> > 
> > That is, the user said GIT_AUTHOR_NAME is empty, so he gets a
> > commit with an empty author name.

That is what I expect and I must have done something wrong the last
night since it works for me now as well. Sorry. So I'm all alone at
this bug again. ;-)

> Yes. That said, we should probably disallow that in git-commit-tree (and 
> let the user fix it up some way).

What way? Sometimes you just receive mails from people who have only
email addy in the from line, or you can be importing from some other VCS
where the mapping does not exist and the importers may not deem it
necessary to have it in GIT. Sure, it may be kernel policy to disallow
this, but I wouldn't enforce this for all projects in GIT.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Of the 3 great composers Mozart tells us what it's like to be human,
Beethoven tells us what it's like to be Beethoven and Bach tells us
what it's like to be the universe.  -- Douglas Adams

^ permalink raw reply

* Re: [Cogito] Various bugs
From: Linus Torvalds @ 2006-02-07 16:56 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, git
In-Reply-To: <20060207164946.GN31278@pasky.or.cz>



On Tue, 7 Feb 2006, Petr Baudis wrote:
> 
> > Yes. That said, we should probably disallow that in git-commit-tree (and 
> > let the user fix it up some way).
> 
> What way? Sometimes you just receive mails from people who have only
> email addy in the from line, or you can be importing from some other VCS
> where the mapping does not exist and the importers may not deem it
> necessary to have it in GIT. Sure, it may be kernel policy to disallow
> this, but I wouldn't enforce this for all projects in GIT.

Umm. Having an empty name is _wrong_. It makes things like the shortlogs 
break.

Yes, it has happened in the kernel a few times, but those were bugs, and 
I'd have been very happy if git-write-tree had just aborted on me.

If you don't have any better name than the email/user-name, just use that 
(but at least for the kernel, I'd much prefer a round of "google" first to 
see if something better is available). An _empty_ name is never 
acceptable.

		Linus

^ permalink raw reply

* Re: [PATCH] git-commit: revamp the git-commit semantics.
From: Nicolas Pitre @ 2006-02-07 16:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Marco Costalba, git mailing list
In-Reply-To: <7voe1le71b.fsf@assigned-by-dhcp.cox.net>

On Sun, 5 Feb 2006, Junio C Hamano wrote:

> I was hoping that, once people get used to --include and --only,
> they start to "revel in the index" (as Linus puts it), and
> realize that defaulting to --only is not such a good idea after
> all.  When that happens, I could leave --include as the
> default without getting complaints from them ;-).

I know this is like fighting old habits, but at some point one as to 
realize that there was a mistake in the interface and the best way to 
solve that mistake is to go with the objectively most intuitive 
solution.

People can get used to the index concept like you did yourself of 
course, but that is still dismissing the very reason for doing that 
change in the first place, which is that to a git beginner it is counter 
intuitive to have side effects when explicit paths are specified 
_alone_.

> The net effect, if we end up doing so, would be that we gained
> an extra flag --only that the user can use to explicitly ask for
> a partial commit when they want one, without disrupting the
> established workflow of old timers.

It is a bit deconcerting to see that git has already reached the 
software stone age and that so called "old timers" already have more 
weight than clean interfaces... even after less than a year.

Please don't make it another CVS and fix those inconsistencies clean and 
quick while the user base is not yet inapt to change.

> The --only semantics is a useful thing in many situations, while
> the --include semantics is also a useful thing in many other
> cases.  The latter might be a better match to the way the git
> internal works, but both are equally useful options that support
> different workflows.  I do not see an inherent reason to declare
> one over the other to be "the default".

What has happened to the "least surprise" principle?  That clearly 
favorize the former IMHO.

> So we could instead
> have no defaults at all (i.e. you have to explicitly say which
> kind you want if you use paths...).

And that would be just a nice usability annoyance.

Maybe being too intimate with git doesn't make it obvious to you, but if 
you do a survey of the number of tools that allows accepting a list of 
paths for their only arguments it should be pretty obvious that the vast 
majority operate only on those explicit paths without any hidden side 
effect.

If "git commit" is meant to be a user helper script to hide the low 
level git plumbing then it better do it with the least surprise 
principle in mind, otherwise it misses its very purpose.  And the faster 
this is corrected the better, especially since this is not such a 
disturbing change after all.


Nicolas

^ permalink raw reply

* [PATCH] .gitignore git-rerere and config.mak
From: Andreas Ericsson @ 2006-02-07 17:23 UTC (permalink / raw)
  To: git

Signed-off-by: Andreas Ericsson <ae@op5.se>

---

 .gitignore |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

0a04811b6d4a6673869cd078471cd1ee6db31e5a
diff --git a/.gitignore b/.gitignore
index 513f22e..d7e8d2a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -78,6 +78,7 @@ git-relink
 git-repack
 git-repo-config
 git-request-pull
+git-rerere
 git-reset
 git-resolve
 git-rev-list
@@ -123,3 +124,4 @@ git-core.spec
 libgit.a
 *.o
 *.py[co]
+config.mak
-- 
1.1.6.g53f3

^ permalink raw reply related

* Re: [PATCH] git-commit: revamp the git-commit semantics.
From: Junio C Hamano @ 2006-02-07 17:50 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0602071135110.5397@localhost.localdomain>

Nicolas Pitre <nico@cam.org> writes:

> What has happened to the "least surprise" principle?  That clearly 
> favorize the former IMHO.

Please see below.

> Maybe being too intimate with git doesn't make it obvious to you,...

I think you are looking at this with a bit shortsighted view.
We are aiming for the same end results.

Let's say you have a git-commit that does --only semantics by
default "some day", and everybody knows that is going to happen
on that "some day" for a long time (say, a month), because we
are discussing about it right now in this thread.

And a newcomer comes.  He's never used git before.  When he
types "git commit paths...", the new --only semantics happens.
By that time (a month is a long time in git timescale) everybody
knows "git commit paths..." mean --only, so if the newcomer has
trouble with "git commit paths...", people can help him with
shared knowledge on what that command means.  And what the
command does is less surprising than the current behaviour.

It is a Good Change.  I am agreeing with you that if --only were
the default from before I inherited git, things might have been
better.

But let's say you make that "some day" today, or in the 1.2.0
that is hopefully imminent.  That newcomer is not using git yet.
Many people and their scripts are already using git, expecting
"git commit paths..."  to mean --include semantics.  What kind
of surprise are you giving them?

That is this '--only/--include are available but currently the
traditional --include is the default' is all about.  It is about
not breaking current users during transition.  And People's
Scripts.  If a tool wants traditional semantics, it can hardcode
"--include" (or --only if not), and does not have to worry about
the default getting switched "some day".

Once things are prepared that way, we can switch the default any
day, as long as we give users enough advance notice.

It is possible that you are well aware of all of the above, and
are arguing that "enough advance notice" period should be zero
days.  But I don't think that is nice.  When was this temporary
thing formerly proposed in this round of discussion to make
everybody realize that is what is happening?  4 days ago?  And
the actual code that claims to do --only hit the "master" branch
only 8 hours or so ago.

I was hoping that before 1.2.0 I can take a final "just in case"
quick poll to see everybody agrees making --only the default is
the way to go, then state it clearly that 1.3.0 will have the
default switched to --only in the release announcement.

^ permalink raw reply

* Re: [PATCH] git-commit: revamp the git-commit semantics.
From: Junio C Hamano @ 2006-02-07 18:18 UTC (permalink / raw)
  To: git; +Cc: Nicolas Pitre
In-Reply-To: <7vfymvvz1r.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> I was hoping that before 1.2.0 I can take a final "just in case"
> quick poll to see everybody agrees making --only the default is
> the way to go, then state it clearly that 1.3.0 will have the
> default switched to --only in the release announcement.

Clarification.  Here is what I want to do.

I've said enough times that we _may_ switch the default from
traditional --include semantics to new --only semantics.  Nico
already even objected to the "may" part, and I am hoping it is
shared by everybody on the list that "may" should be "will
soon".  Unless somebody comes up with a convincing argument
otherwise, I'll write "this will switch to --only by the time
1.3.0 release is made, so start converting your scripts to
explicitly say --include or --only if you want a specific
behaviour, and start training your fingers in the meantime as
well" in the 1.2.0 announcement, probably done sometime late
this week.

That means 1.2.0 (and its maintenance series 1.2.X) will ship
with --include semantics for "git commit paths...", and will
allow explicit --only/--include.  After 1.2.0, at some point,
the "master" branch will start shipping with --only semantics as
default.  No script should break when 1.3.0 happens.

People who want to use --include semantics will acquire a habit
of explicitly askign for --include during 1.3.0 development
period.  They do not need to unlearn anything when 1.3.0 happens.

People who learned to type --only can unlearn it to reduce
typing when 1.3.0 happens, but unlearning is not a requirement.
Being explicit should always work.

New people that come after 1.3.0 will get the --only semantics
by default, the intuitiveness of which has been argued to death,
without using any flags.

^ permalink raw reply

* Re: [PATCH] .gitignore git-rerere and config.mak
From: Junio C Hamano @ 2006-02-07 18:20 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git
In-Reply-To: <20060207172234.41A975BF02@nox.op5.se>

Andreas Ericsson <ae@op5.se> writes:

> --- a/.gitignore
> +++ b/.gitignore
> @@ -123,3 +124,4 @@ git-core.spec
>  libgit.a
>  *.o
>  *.py[co]
> +config.mak

I am not sure about this part.  It is plausible that somebody
who privately uses config.mak has it in _his_ repository under
version control.  Should we list it in .gitignore?

^ permalink raw reply

* "git diff a..b" broken
From: Linus Torvalds @ 2006-02-07 18:23 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


There's a funny breakage from the recent "--cc" changes.

If you do 

	git-diff-tree --cc version version2

and "version2" is a merge, it will do something really strange. I haven't 
figured out quite _what_ it does, but it's definitely the wrong thing.

Just a heads up. I'll try to track down what the breakage is.

		Linus

^ permalink raw reply

* Re: "git diff a..b" broken
From: Linus Torvalds @ 2006-02-07 18:26 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0602071020120.3854@g5.osdl.org>



On Tue, 7 Feb 2006, Linus Torvalds wrote:
> 
> Just a heads up. I'll try to track down what the breakage is.

Ahh. Very simple.

The "--cc" implies "-p", but without the recursive part.

		Linus

----
diff --git a/diff-tree.c b/diff-tree.c
index f3280a1..7148323 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -284,14 +284,15 @@ int main(int argc, const char **argv)
 		}
 		usage(diff_tree_usage);
 	}
-	if (diff_options.output_format == DIFF_FORMAT_PATCH)
-		diff_options.recursive = 1;
 
 	if (combine_merges) {
 		diff_options.output_format = DIFF_FORMAT_PATCH;
 		ignore_merges = 0;
 	}
 
+	if (diff_options.output_format == DIFF_FORMAT_PATCH)
+		diff_options.recursive = 1;
+
 	diff_tree_setup_paths(get_pathspec(prefix, argv));
 	diff_setup_done(&diff_options);
 

^ permalink raw reply related

* Re: [PATCH] git-commit: revamp the git-commit semantics.
From: Nicolas Pitre @ 2006-02-07 18:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vfymvvz1r.fsf@assigned-by-dhcp.cox.net>

On Tue, 7 Feb 2006, Junio C Hamano wrote:

> That is this '--only/--include are available but currently the
> traditional --include is the default' is all about.  It is about
> not breaking current users during transition.  And People's
> Scripts.  If a tool wants traditional semantics, it can hardcode
> "--include" (or --only if not), and does not have to worry about
> the default getting switched "some day".

I'm completely with you here.  However is there so many tools that 
relies on the current behavior of "git commit"?

> Once things are prepared that way, we can switch the default any
> day, as long as we give users enough advance notice.

Good.  It just seemed to me that you weren't convinced that any default 
switching was needed.  Sorry if I misread you.

What prompted my initial reply to you was this:

|The --only semantics is a useful thing in many situations, while
|the --include semantics is also a useful thing in many other
|cases.  The latter might be a better match to the way the git
|internal works, but both are equally useful options that support
|different workflows.  I do not see an inherent reason to declare
|one over the other to be "the default".  So we could instead
|have no defaults at all (i.e. you have to explicitly say which
|kind you want if you use paths...).

to which I disagreed.

> It is possible that you are well aware of all of the above, and
> are arguing that "enough advance notice" period should be zero
> days.  But I don't think that is nice.

Certainly not zero days.  I'm just trying to counter-balance the 
"without disrupting the established workflow of old timers" argument 
which is the worst enemy of healthy software evolution.  If such an 
argument was to prevail in the Linux world then we would have had those 
evil "stable driver ABI" for a long time already.  If that argument was 
to prevail for git then that would be the beginning of its stagnation.

As for the advance notice, it might be a good idea to simply switch the 
default in 1.3.0 giving any script author relying on the --include 
behavior (if any) proper insentive to fix them. The 1.3.x branch being 
the so called "unstable" branch makes it the appropriate place to do 
it at the earliest, providing script authors enough time to test their 
modifications on the real thing before it becomes official 1.4.0.

But you should consider all above rambling as part of the feedback you 
asked for.  I don't pretend to always be right.


Nicolas

^ permalink raw reply

* Re: [PATCH] .gitignore git-rerere and config.mak
From: Johannes Schindelin @ 2006-02-07 18:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andreas Ericsson, git
In-Reply-To: <7vk6c7uj21.fsf@assigned-by-dhcp.cox.net>

Hi,

On Tue, 7 Feb 2006, Junio C Hamano wrote:

> Andreas Ericsson <ae@op5.se> writes:
> 
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -123,3 +124,4 @@ git-core.spec
> >  libgit.a
> >  *.o
> >  *.py[co]
> > +config.mak
> 
> I am not sure about this part.  It is plausible that somebody
> who privately uses config.mak has it in _his_ repository under
> version control.  Should we list it in .gitignore?

I have it in .gitignore. If there were changes to config.mak which I had 
in all my copies of git, I'd have them directly in the Makefile.

Hth,
Dscho

^ 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