Git development
 help / color / mirror / Atom feed
* Re: What's in git.git (stable)
From: Junio C Hamano @ 2006-12-22 21:49 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <200612222156.21215.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> Junio C Hamano wrote:
> ...
>>>> Possibilities:
>>>> 
>>>>  (1) Forget about that "protection" business.  If you do not
>>>>      want mistakes, use 'branch.*.merge' but otherwise we will
>>>>      continue to follow the good old "first set of branches"
>>>>      rule.
>>>
>>> What about marking default branch to merge explicitely using
>>> "Merge:" in remotes/<repo>, or remote.<name>.merge?
>> 
>> Sorry, how is that an improvement over the current branch.*.merge?
>> and how would that help not breaking existing setups?
>
> I meant that in addition to forgetting about "protection" business.
> This would be intermediate improvement over old behavior.

I do not think so.  It does not talk about "when on my local
branch X do this", and applies to all pulls from the named
remote.  Then longstanding rule of merging the first set of
branches is just fine and as expressive.  You see them the first
in the list, and you already know they somehow matter more.

On the other hand, I think Santi's branch.*.merge (done in
commit 5372806a) _was_ a real improvement.

> Perhaps make "protection" business optional, default to on for
> new users?

Now the question is how you would tell "new users".

The possibility (2) is not even good enough, because even old
timers work in a newly cloned repositories.

^ permalink raw reply

* Re: What's in git.git (stable)
From: Junio C Hamano @ 2006-12-22 21:44 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: git, Randal L. Schwartz, Josef Weidendorfer, Luben Tuikov
In-Reply-To: <Pine.LNX.4.63.0612222141060.19693@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> So, if I understand your option (2) correctly, it complains _only_ if 
> there is at least one branch.*.merge in the config, but not for the 
> current branch?
>
> I think that would safeguard the existing repositories _and_ the new ones, 
> because git-clone sets them up with such an entry to begin with.
>
> If that behaviour was meant by (2), I am all for it.

I am not quite sure about that.  An old timer would work in a
newly cloned repository after all, and what this "newbie
protection" is breaking is not existing repositories but
expectation from existing users.

In any case, here is a patch for discussion.

diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index f163821..b4d071b 100755
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -145,10 +145,22 @@ canon_refs_list_for_fetch () {
 			merge_branches=$(git-repo-config \
 			    --get-all "branch.${curr_branch}.merge")
 		fi
-		# If we are fetching only one branch, then first branch
-		# is the only thing that makes sense to merge anyway,
-		# so there is no point refusing that traditional rule.
-		if test $# != 1 && test "z$merge_branches" = z
+		if test "z$merge_branches" = z &&
+			# If we are fetching only one branch, then
+			# first branch is the only thing that makes
+			# sense to merge anyway, so there is no point
+			# refusing that traditional rule.
+			test $# != 1 &&
+
+			# Also, old timers have been happily working
+			# with the first branch rule without having
+			# any branch.*.merge configuration, so if
+			# there is none, do not bother with this
+			# "newbie protection".  A newly cloned
+			# repository would have branch.master.merge
+			# set for it.
+			git repo-config --get-regexp \
+				'^branch\..*\.merge$' >/dev/null
 		then
 			merge_branches=..this..would..never..match..
 		fi

^ permalink raw reply related

* Re: [PATCH 1/2] libgit.a: add some UTF-8 handling functions
From: Johannes Schindelin @ 2006-12-22 21:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, Uwe Kleine-König, git
In-Reply-To: <7virg3y624.fsf@assigned-by-dhcp.cox.net>

Hi,

On Fri, 22 Dec 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > This adds utf8_byte_count(), utf8_strlen() and print_wrapped_text().
> >
> > The most important is probably utf8_strlen(), which returns the length
> > of the text, if it is in UTF-8, otherwise -1.
> >
> > Note that we do not go the full nine yards: we could also check that
> > the character is encoded with the minimum amount of bytes, as pointed
> > out by Uwe Kleine-Koenig.
> >
> > The function print_wrapped_text() can be used to wrap text to a certain
> > line length.
> 
> If you do wrapped_text, I think you do not _want_ strlen (the
> definition to me of strlen is "number of characters in the
> string").  What you want is a function that returns the number
> of columns consumed when displayed on monospace terminal.

To me, characters are the symbols occupying one "column" each. Bytes are 
the 8-bit thingies that you usually use to encode the characters.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 1/2] libgit.a: add some UTF-8 handling functions
From: Junio C Hamano @ 2006-12-22 21:27 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Nicolas Pitre, Uwe Kleine-König, git
In-Reply-To: <Pine.LNX.4.63.0612222201200.19693@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> This adds utf8_byte_count(), utf8_strlen() and print_wrapped_text().
>
> The most important is probably utf8_strlen(), which returns the length
> of the text, if it is in UTF-8, otherwise -1.
>
> Note that we do not go the full nine yards: we could also check that
> the character is encoded with the minimum amount of bytes, as pointed
> out by Uwe Kleine-Koenig.
>
> The function print_wrapped_text() can be used to wrap text to a certain
> line length.

If you do wrapped_text, I think you do not _want_ strlen (the
definition to me of strlen is "number of characters in the
string").  What you want is a function that returns the number
of columns consumed when displayed on monospace terminal.

^ permalink raw reply

* Re: confusion over the new branch and merge config
From: Nicolas Pitre @ 2006-12-22 21:20 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <emhh4k$u4q$1@sea.gmane.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1490 bytes --]


Could you at least keep me in CC when replying to me please?

On Fri, 22 Dec 2006, Jakub Narebski wrote:

> <opublikowany i wysłany>

?

> Nicolas Pitre wrote:
> > On Thu, 21 Dec 2006, Junio C Hamano wrote:
> 
> >> No, the message says "any REMOTE branch" -- refs/heads/next is
> >> what it is called at the remote, and that is how the value is
> >> expected to be spelled; I think somebody added an example to
> >> config.txt recently to stress this.  The above error messasge
> >> obviously was not clear enough.  Rewording appreciated.
> > 
> > But wouldn't it be much less confusing if it used the local name for 
> > that remote branch instead?  After all it is what should be used with 
> > git-merge if performed manually, it is what diff, log, and al must use 
> > as well.  Why would this need a remote name for something that is a 
> > local operation after all?  I think "refs/heads/master" is really 
> > ambigous since you might be confused between the local and remote 
> > meaning of it, whereas "origin/master" carries no confusion at all.
> 
> Perhaps less confusing, but also less powerfull. Current notation
> allows for pulling _without need for tracking branches_.

Is this really a killer feature worth the confusion?

If you put the repo to pull from on the command line then sure you might 
not want a tracking branch, but if you go to the trouble of adding a 
branch.blah.merge config entry then you certainly don't mind having a 
tracking branch?


Nicolas

^ permalink raw reply

* [RFC/PATCH 3/2] Wrap lines in shortlog
From: Johannes Schindelin @ 2006-12-22 21:15 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, Uwe Kleine-König, git
In-Reply-To: <7vslf7zrdp.fsf@assigned-by-dhcp.cox.net>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 253 bytes --]

Hi,

It is nicer to wrap the lines of too long oneline descriptions. This patch 
even works in UTF-8.

The patch is attached, since I cannot find the setting in pine to make it 
a UTF-8 one. Besides, I deliberately fscked up one test case.

Ciao,
Dscho

[-- Attachment #2: shortlog.patch --]
[-- Type: TEXT/PLAIN, Size: 2978 bytes --]

[PATCH] Use print_wrapped_text() in shortlog

Some oneline descriptions are just too long. In shortlog, it looks much
nicer when they are wrapped. Since print_wrapped_text() is UTF-8 aware,
it also works with those descriptions.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	Probably this should check i18n.commitencoding, too...

 builtin-shortlog.c  |    4 +++-
 t/t4201-shortlog.sh |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index edb4042..30e7cb5 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -4,6 +4,7 @@
 #include "diff.h"
 #include "path-list.h"
 #include "revision.h"
+#include "utf8.h"
 
 static const char shortlog_usage[] =
 "git-shortlog [-n] [-s] [<commit-id>... ]";
@@ -321,7 +322,8 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 		} else {
 			printf("%s (%d):\n", list.items[i].path, onelines->nr);
 			for (j = onelines->nr - 1; j >= 0; j--)
-				printf("      %s\n", onelines->items[j].path);
+				print_wrapped_text(onelines->items[j].path,
+					6, 9, 76);
 			printf("\n");
 		}
 
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh
new file mode 100644
index 0000000..e4085f9
--- /dev/null
+++ b/t/t4201-shortlog.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Johannes E. Schindelin
+#
+
+test_description='git-shortlog
+'
+
+. ./test-lib.sh
+
+echo 1 > a1
+git add a1
+tree=$(git write-tree)
+commit=$((echo "Test"; echo) | git commit-tree $tree)
+git update-ref HEAD $commit 
+
+echo 2 > a1
+git commit -m "This is a very, very long first line for the commit message to see if it is wrapped correctly" a1
+
+# test if the wrapping is still valid when replacing all i's by treble clefs.
+echo 3 > a1
+git commit -m "$(echo "This is a very, very long first line for the commit message to see if it is wrapped correctly" | sed "s/i/1234/g" | tr 1234 '\360\235\204\236')" a1
+
+# now fsck up the utf8
+git repo-config i18n.commitencoding non-utf-8
+echo 4 > a1
+git commit -m "$(echo "This is a very, very long first line for the commit message to see if it is wrapped correctly" | sed "s/i/1234/g" | tr 1234 '\370\235\204\236')" a1
+
+echo 5 > a1
+git commit -m "a								12	34	56	78" a1
+
+git shortlog HEAD > out
+
+cat > expect << EOF
+A U Thor (5):
+      Test
+      This is a very, very long first line for the commit message to see if
+         it is wrapped correctly
+      Th𝄞s 𝄞s a very, very long f𝄞rst l𝄞ne for the comm𝄞t message to see 𝄞f
+         𝄞t 𝄞s wrapped correctly
+      Thø„žs ø„žs a very, very long fø„žrst lø„žne for the commø„žt
+         message to see ø„žf ø„žt ø„žs wrapped correctly
+      a								12	34
+         56	78
+
+EOF
+
+test_expect_success 'shortlog wrapping' 'diff -u expect out'
+
+test_done

^ permalink raw reply related

* Re: warning merge message
From: Junio C Hamano @ 2006-12-22 21:12 UTC (permalink / raw)
  To: Josef Weidendorfer; +Cc: Luben Tuikov, git
In-Reply-To: <200612222142.15489.Josef.Weidendorfer@gmx.de>

Josef Weidendorfer <Josef.Weidendorfer@gmx.de> writes:

> On Friday 22 December 2006 07:00, Junio C Hamano wrote:
>> Junio C Hamano <junkio@cox.net> writes:
>> 
>> > The above message was meant only for "git pull", but was leaked
>> > even when you did "git fetch"; it was a bug and was corrected
>> > already (hopefully).
>> 
>> Gaah... it turns out that it was not fixed properly.
>
> Sorry, I am missing something.

You are not missing anything -- I CC'ed you not because I meant
to point fingers at you but I hoped you had better ideas since
you touched the related logic recently.

> What is the exact problem that goes wrong here?

The problem is the same as on another thread where Merlyn got
his scripts broken.  It is not _issuing_ the warning that is
wrong anymore, but is about deciding how to decide that no merge
candidate should exist.

We used to always merge with the first set of branches (either
the first "Pull: " line in remotes/$origin or the first instance
of "remotes.$origin.fetch" configuration).  Santi then added
"branch.$current.merge" to override that depending on what
branch we are currently on, which was backward compatible --
without such a configuration, we still used the "first set of
branches" rule.

People for a long time observed "the first set of branches" rule
was often a wrong thing to do while on a branch other than
'master' (but we do not want to add hardcoded 'master'
unnecessarily), and I recently screwed up by changing the logic
in such a way that everything is marked as not-for-merge unless
branches.*.merge is not set when pulling from the default
remote, which was completely bogus and bitten Luben.

^ permalink raw reply

* [PATCH 2/2] git-commit-tree: if i18n.commitencoding is utf-8 (default), check it
From: Johannes Schindelin @ 2006-12-22 21:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, Uwe Kleine-König, git
In-Reply-To: <7vslf7zrdp.fsf@assigned-by-dhcp.cox.net>


Now, git-commit-tree refuses to commit when i18n.commitencoding is
either unset, or set to "utf-8", and the commit message does not
minimally conform to the UTF-8 encoding.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	Unfortunately, I could not think of a shorter oneline description. 
	But my next patch fixes at least the output in shortlog.

 builtin-commit-tree.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
index 856f3cd..810b440 100644
--- a/builtin-commit-tree.c
+++ b/builtin-commit-tree.c
@@ -7,6 +7,7 @@
 #include "commit.h"
 #include "tree.h"
 #include "builtin.h"
+#include "utf8.h"
 
 #define BLOCKING (1ul << 14)
 
@@ -32,7 +33,7 @@ static void add_buffer(char **bufp, unsigned int *sizep, const char *fmt, ...)
 	len = vsnprintf(one_line, sizeof(one_line), fmt, args);
 	va_end(args);
 	size = *sizep;
-	newsize = size + len;
+	newsize = size + len + 1;
 	alloc = (size + 32767) & ~32767;
 	buf = *bufp;
 	if (newsize > alloc) {
@@ -40,7 +41,7 @@ static void add_buffer(char **bufp, unsigned int *sizep, const char *fmt, ...)
 		buf = xrealloc(buf, alloc);
 		*bufp = buf;
 	}
-	*sizep = newsize;
+	*sizep = newsize - 1;
 	memcpy(buf + size, one_line, len);
 }
 
@@ -127,6 +128,15 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 	while (fgets(comment, sizeof(comment), stdin) != NULL)
 		add_buffer(&buffer, &size, "%s", comment);
 
+	/* And check the encoding */
+	buffer[size] = '\0';
+	if (!strcmp(git_commit_encoding, "utf-8") && utf8_strlen(buffer) < 0) {
+		fprintf(stderr, "Commit message does not conform to UTF-8.\n"
+			"Please fix the message,"
+			" or set the config variable i18n.commitencoding.\n");
+		return 1;
+	}
+
 	if (!write_sha1_file(buffer, size, commit_type, commit_sha1)) {
 		printf("%s\n", sha1_to_hex(commit_sha1));
 		return 0;
-- 
1.4.4.3.ge5f98-dirty

^ permalink raw reply related

* [PATCH 1/2] libgit.a: add some UTF-8 handling functions
From: Johannes Schindelin @ 2006-12-22 21:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, Uwe Kleine-König, git
In-Reply-To: <7vslf7zrdp.fsf@assigned-by-dhcp.cox.net>


This adds utf8_byte_count(), utf8_strlen() and print_wrapped_text().

The most important is probably utf8_strlen(), which returns the length
of the text, if it is in UTF-8, otherwise -1.

Note that we do not go the full nine yards: we could also check that
the character is encoded with the minimum amount of bytes, as pointed
out by Uwe Kleine-Koenig.

The function print_wrapped_text() can be used to wrap text to a certain
line length.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---

	On Fri, 22 Dec 2006, Junio C Hamano wrote:

	> Nicolas Pitre <nico@cam.org> writes:
	> 
	> > On Fri, 22 Dec 2006, Johannes Schindelin wrote:
	> >> 
	> >> On Thu, 21 Dec 2006, Junio C Hamano wrote:
	> >> 
	> >> >  (2) update commit-tree to reject non utf-8 log messages and
	> >> >      author/committer names when i18n.commitEncoding is _NOT_
	> >> >      set, or set to utf-8.
	> >> 
	> >> The problem is: you cannot easily recognize if it is UTF8 or 
	> >> not, programatically. There is a good indicator _against_ 
	> >> UTF8, namely the first byte can _only_ be 0xxxxxxx, 110xxxxx, 
	> >> 1110xxxx, 11110xxx. But there is no _positive_ sign that it 
	> >> is UTF8. For example, many umlauts and other special 
	> >> modifications to letters, stay in the range 0x7f-0xff.
	> >
	> > Still... that would be a good enough thing to have in the 
	> > majority of cases, wouldn't it?
	> 
	> I think that would be very sane thing to do.

	Well, this patch together with the next one implements that.

 Makefile |    6 ++-
 utf8.c   |   93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 utf8.h   |    8 +++++
 3 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 29c4662..b4ca48b 100644
--- a/Makefile
+++ b/Makefile
@@ -237,7 +237,8 @@ LIB_H = \
 	archive.h blob.h cache.h commit.h csum-file.h delta.h grep.h \
 	diff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
-	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h
+	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
+	utf8.h
 
 DIFF_OBJS = \
 	diff.o diff-lib.o diffcore-break.o diffcore-order.o \
@@ -256,7 +257,8 @@ LIB_OBJS = \
 	revision.o pager.o tree-walk.o xdiff-interface.o \
 	write_or_die.o trace.o list-objects.o grep.o \
 	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
-	color.o wt-status.o archive-zip.o archive-tar.o shallow.o
+	color.o wt-status.o archive-zip.o archive-tar.o shallow.o \
+	utf8.o
 
 BUILTIN_OBJS = \
 	builtin-add.o \
diff --git a/utf8.c b/utf8.c
new file mode 100644
index 0000000..06a66c7
--- /dev/null
+++ b/utf8.c
@@ -0,0 +1,93 @@
+#include "git-compat-util.h"
+#include "utf8.h"
+
+/*
+ * This function returns the number of bytes occupied by the character
+ * pointed to by the variable start. If it is not valid UTF-8, it
+ * returns -1.
+ */
+int utf8_byte_count(const char *start)
+{
+	unsigned char c = *(unsigned char *)start;
+	int i, count = 0;
+
+	if (!(c & 0x80))
+		count = 1;
+	else if ((c & 0xe0) == 0xc0)
+		count = 2;
+	else if ((c & 0xf0) == 0xe0)
+		count = 3;
+	else if ((c & 0xf8) == 0xf0)
+		count = 4;
+	else
+		return -1;
+
+	for (i = 1; i < count; i++)
+		if ((start[i] & 0xc0) != 0x80)
+			return -1;
+	return count;
+}
+
+int utf8_strlen(const char *text)
+{
+	int len = 0;
+	while (*text) {
+		int count = utf8_byte_count(text);
+		if (count < 0)
+			return -1;
+		len += count;
+		text += count;
+	}
+	return len;
+}
+
+static void print_spaces(int count)
+{
+	static const char s[] = "                    ";
+	while (count >= sizeof(s)) {
+		fwrite(s, sizeof(s) - 1, 1, stdout);
+		count -= sizeof(s) - 1;
+	}
+	fwrite(s, count, 1, stdout);
+}
+
+/*
+ * Wrap the text, if necessary. The variable indent is the indent for the
+ * first line, indent2 is the indent for all other lines.
+ */
+void print_wrapped_text(const char *text, int indent, int indent2, int len)
+{
+	int count = 0, space = -1;
+	int l = utf8_strlen(text), assume_utf8 = (l >= 0);
+
+	l = indent;
+
+	for (;;) {
+		char c = text[count];
+		if (!c || isspace(c)) {
+			if (l < len || space < 0) {
+				const char *start = text;
+				if (space >= 0)
+					start += space;
+				else
+					print_spaces(indent);
+				fwrite(start, text + count - start, 1, stdout);
+				if (!c) {
+					putchar('\n');
+					return;
+				} else if (c == '\t')
+					l |= 0x07;
+				space = count;
+			} else {
+				putchar('\n');
+				text += space + 1;
+				indent = indent2;
+				space = -1;
+				count = l = 0;
+				continue;
+			}
+		}
+		count += assume_utf8 ? utf8_byte_count(text + count) : 1;
+		l++;
+	}
+}
diff --git a/utf8.h b/utf8.h
new file mode 100644
index 0000000..96dded9
--- /dev/null
+++ b/utf8.h
@@ -0,0 +1,8 @@
+#ifndef GIT_UTF8_H
+#define GIT_UTF8_H
+
+int utf8_byte_count(const char *start);
+int utf8_strlen(const char *text);
+void print_wrapped_text(const char *text, int indent, int indent2, int len);
+
+#endif
-- 
1.4.4.3.ge5f98-dirty

^ permalink raw reply related

* Re: confusion over the new branch and merge config
From: Jakub Narebski @ 2006-12-22 21:04 UTC (permalink / raw)
  To: git
In-Reply-To: <Pine.LNX.4.64.0612221539100.18171@xanadu.home>

<opublikowany i wysłany>

Nicolas Pitre wrote:
> On Thu, 21 Dec 2006, Junio C Hamano wrote:

>> No, the message says "any REMOTE branch" -- refs/heads/next is
>> what it is called at the remote, and that is how the value is
>> expected to be spelled; I think somebody added an example to
>> config.txt recently to stress this.  The above error messasge
>> obviously was not clear enough.  Rewording appreciated.
> 
> But wouldn't it be much less confusing if it used the local name for 
> that remote branch instead?  After all it is what should be used with 
> git-merge if performed manually, it is what diff, log, and al must use 
> as well.  Why would this need a remote name for something that is a 
> local operation after all?  I think "refs/heads/master" is really 
> ambigous since you might be confused between the local and remote 
> meaning of it, whereas "origin/master" carries no confusion at all.

Perhaps less confusing, but also less powerfull. Current notation
allows for pulling _without need for tracking branches_.

Although I wonder if it is possible to have multiple branch.<name>.remote,
and branch.<name>.merge always refering to latest remote (octopus if there
are more than one branch.<name>.merge for remote)...
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: What's in git.git (stable)
From: Jakub Narebski @ 2006-12-22 20:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejqrznwo.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> <opublikowany i wysłany>
>>
>> Junio C Hamano wrote:
>>
>>> Possibilities:
>>> 
>>>  (1) Forget about that "protection" business.  If you do not
>>>      want mistakes, use 'branch.*.merge' but otherwise we will
>>>      continue to follow the good old "first set of branches"
>>>      rule.
>>
>> What about marking default branch to merge explicitely using
>> "Merge:" in remotes/<repo>, or remote.<name>.merge?
> 
> Sorry, how is that an improvement over the current branch.*.merge?
> and how would that help not breaking existing setups?

I meant that in addition to forgetting about "protection" business.
This would be intermediate improvement over old behavior, marking
clearly which is default branch to merge (with first branch still
being default, and perhaps error out if there is _only_ wildcard
Pull:/fetch line/variable; the branches marked with + are ineligible
as candidate for merge with "first branch being default branch to
merge" default).

Perhaps make "protection" business optional, default to on for
new users?
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: confusion over the new branch and merge config
From: Nicolas Pitre @ 2006-12-22 20:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd56cam66.fsf@assigned-by-dhcp.cox.net>

On Thu, 21 Dec 2006, Junio C Hamano wrote:

> Nicolas Pitre <nico@cam.org> writes:
> 
> > $ git pull origin/next
> > fatal: The remote end hung up unexpectedly
> > Cannot get the repository state from git://git.kernel.org/pub/scm/git/git.git/next
> >
> > [ WTF?  Where that ...pub/scm/git/git.git/next comes from?  Hmmm... ]
> 
> This comes from ancient request by Linus to allow:
> 
> 	$ cat .git/remotes/jgarzik
> 	URL: master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/
> 	$ git pull jgarzik/misc-2.6
> 
> See http://article.gmane.org/gmane.comp.version-control.git/6181
> for the full text.
> 
> Personally I thought this was confusing when I implemented it
> the first time, and I still find it confusing.
> 
> I suspect nobody uses it.  I am all for removing this "URL
> prefix shorthand" feature in v1.5.0.

Please do.  I'm sure Linus can find a better way now.

> > $ git pull . remotes/origin/next
> > Already up-to-date.
> >
> > [ Woooh!  But since I always hated this syntax let's try merge instead. ]
> >
> > $ git merge origin/next
> > Already up-to-date.
> 
> Yes, that is one of the reasons that you would prefer 'merge'
> when you are working locally.

Sure.  But why doesn't pull accept "origin/next"?

> > $ git pull origin
> > Warning: No merge candidate found because value of config option
> >          "branch.local_next.merge" does not match any remote branch fetched.
> > No changes.
> >
> > So this means that branch.local_next.merge should be set to origin/next?  
> 
> No, the message says "any REMOTE branch" -- refs/heads/next is
> what it is called at the remote, and that is how the value is
> expected to be spelled; I think somebody added an example to
> config.txt recently to stress this.  The above error messasge
> obviously was not clear enough.  Rewording appreciated.

But wouldn't it be much less confusing if it used the local name for 
that remote branch instead?  After all it is what should be used with 
git-merge if performed manually, it is what diff, log, and al must use 
as well.  Why would this need a remote name for something that is a 
local operation after all?  I think "refs/heads/master" is really 
ambigous since you might be confused between the local and remote 
meaning of it, whereas "origin/master" carries no confusion at all.


Nicolas

^ permalink raw reply

* Re: What's in git.git (stable)
From: Johannes Schindelin @ 2006-12-22 20:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Randal L. Schwartz
In-Reply-To: <7virg3zo1k.fsf@assigned-by-dhcp.cox.net>

Hi,

On Fri, 22 Dec 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > HOWEVER, it has been in "What's cooking in git (topics)".
> >
> > Having said that, we could (at least for some time) print a big red 
> > warning for the specific case of "git pull" meaning "git pull origin 
> > <whateveristhefirstfetchedhead>" that this will GO AWAY SOON.
> >
> > Of course, you would not see it. Only your script.
> >
> > BTW I would _never_ allow a script to rely on such a DWIM feature.
> 
> That's too strong a statement.  It is not DWIM but has been a
> longstanding rule to use "the first set of branches" for the
> merge.  We've merged the first set of branches since day one,
> and even after branch.*.merge we've done so unless you do not
> have such configuration.

Okay, I have been wrong. My sincere apologies.

So, if I understand your option (2) correctly, it complains _only_ if 
there is at least one branch.*.merge in the config, but not for the 
current branch?

I think that would safeguard the existing repositories _and_ the new ones, 
because git-clone sets them up with such an entry to begin with.

If that behaviour was meant by (2), I am all for it.

Ciao,
Dscho

^ permalink raw reply

* Re: newbie question - git-pull and local branch merge
From: Junio C Hamano @ 2006-12-22 20:43 UTC (permalink / raw)
  To: Pelle Svensson; +Cc: git
In-Reply-To: <6bb9c1030612220326w6ce88d0ar1ca5ca49b306c71e@mail.gmail.com>

"Pelle Svensson" <pelle2004@gmail.com> writes:

> Problem is that git-pull failed to merge closed to what you describe in case 2
> and I really like to know hat has been done between the 2 last versions
> on the Linus side.

Let me have a set of illustrations (I cannot think without one
X-<).  You forked off of Linus's tip some time ago (time flows
from left to right):

      x---x---x---x Pelle
     /
 ---o Linus

in the meantime Linus's tip progressed:

      x---x---x---x   Pelle
     /
 ---o---*---*---*---* Linus

and you are trying to create a merge 'M' by pulling from him:


      x---x---x---x...M    Pelle
     /               / 
 ---o---*---*---*---*      Linus

The 'pull' conflicted so you do not have 'M' commit yet.

In such a situation, a command that helps to view what happened
to the files that got conflicted is:

	$ git log -p --merge

By default it uses the "unmreged" files (in your case,
arch/arm/mach-ep93xx/core.c and include/asm-arm/system.h).  You
can limit the output by giving specific paths:

	$ git log -p --merge include/asm-arm/system.h

This would show changes from both sides (i.e. commits marked
with 'x' and '*').

> The problematic file is include/asm-arm/system.h
>
> I attached some output below. Last (D) is a file diff between the time when
> I first pulled the Linus tree and a copy of that which I'm now working in.
> I think the resolve/edit part is easy because Linus tree seem to added
> an extern declaration of 'adjust_cr' closed to line 152 and the other
> conflict code (++<<<...) should be removed.

> But is it not possible to show changes/diff version by version of
> what has been done in the Linus tree?

I think "log --merge" is what you want, but if you do not want
to see your own changes, you can:

	$ git log -p ..MERGE_HEAD -- include/asm-arm/system.h

The notation "..MERGE_HEAD" in the above is a short-hand for
"HEAD..MERGE_HEAD", and "A..B" means "show commits reachable
from B but I am not interested in the ones reachable from A", so
it shows what are in MERGE_HEAD but not in your HEAD, which I
think is exactly what you want.  And paths on the command line
limits the commits considered by the command to only those that
touch the specified paths.

> A. git-status output
> B. git-diff output
> C. And if I do git-pull again
> D. diff output between file stamp 12-dec and 21-dec
>
> A. git-status output
> ============
>
> # On branch refs/heads/ep93xx-pmp
> #
> # Updated but not checked in:
> #   (will commit)
> #
> #	modified: Documentation/block/biodoc.txt
> #     ...
> #	modified: sound/usb/usbaudio.c

These are list of paths that got changed on Linus's side and
either you did not change at all, or had changes that were
cleanly merged.

> #
> #
> # Changed but not updated:
> #   (use git-update-index to mark for commit)
> #
> #	unmerged: arch/arm/mach-ep93xx/core.c
> #	modified: arch/arm/mach-ep93xx/ep93xx_devices.c
> #	modified: drivers/net/arm/ep93xx_eth.c
> #	modified: drivers/video/ep93xxfb.c
> #	unmerged: include/asm-arm/system.h
> #	modified: include/asm-arm/system.h

I thought you had local modification to two paths, but this
shows four 'modified:' files.  Please make sure these four
'modified' files were not modified on the Linus's side
(e.g. "git log -p ..MERGE_HEAD -- drivers/video/ep93xxfb.c" as
shown above), and only contains your own changes.

Two 'unmerged' paths are what you changed and committed in some
of the 'x' commits in the above picture and also had
modifications on Linus's side.

> B. git-diff output
> ==========
>
> diff --cc include/asm-arm/system.h
> index 04d6d2c,aa223fc..0000000
> --- a/include/asm-arm/system.h
> +++ b/include/asm-arm/system.h
> @@@ -170,31 -178,6 +178,34 @@@
>  	  : : "r" (val) : "cc");
>  }
>
> ++<<<<<<< HEAD/include/asm-arm/system.h
> +extern unsigned long cr_no_alignment;	/* defined in entry-armv.S */
> +extern unsigned long cr_alignment;	/* defined in entry-armv.S */
> ...

You added this section and Linus's side did not have it.

> ...
> +	raw_local_irq_restore(flags);
> +}
> +#endif
> +
> ++=======
> ++>>>>>>> 9bfb18392ef586467277fa25d8f3a7a93611f6df/include/asm-arm/system.h
>  #define UDBG_UNDEFINED	(1 << 0)
>  #define UDBG_SYSCALL	(1 << 1)
>  #define UDBG_BADABORT	(1 << 2)

So as you said, it seems appropriate to just keep your changes.
by removing the conflict markers.

> C. And if I do git-pull again
> =================
>
> Trying really trivial in-index merge...
> arch/arm/mach-ep93xx/core.c: needs merge
> arch/arm/mach-ep93xx/ep93xx_devices.c: needs update
> drivers/net/arm/ep93xx_eth.c: needs update
> drivers/video/ep93xxfb.c: needs update
> include/asm-arm/system.h: needs merge
> Nope.
> Merging HEAD with 3e67c0987d7567ad666641164a153dca9a43b11d
> Merging:
> 87d627b097b52c47610037bfb890940752fbf4c8 added .orig to gitignore
> 3e67c0987d7567ad666641164a153dca9a43b11d [PATCH] truncate: clear page
> dirtiness before running try_to_free_buffers()
> found 1 common ancestor(s):
> f238085415c56618e042252894f2fcc971add645 Merge branch 'for-linus' of
> git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid

Was this the end of the output?  I hope you hit this message:

	die("Entry '%s' not uptodate. Cannot merge.", ce->name);

and the second pull did not make any change.  At least that is
how it ought to work.  You might have spotted a bug that was
uncovered only because no git old timers would even imagine
running 'pull' again after getting conflicts from the first
attempt, but I suspect that the above quoted output is not
showing the end of the output.

^ permalink raw reply

* Re: warning merge message
From: Josef Weidendorfer @ 2006-12-22 20:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Luben Tuikov, git
In-Reply-To: <7v1wms79mg.fsf@assigned-by-dhcp.cox.net>

On Friday 22 December 2006 07:00, Junio C Hamano wrote:
> Junio C Hamano <junkio@cox.net> writes:
> 
> > Luben Tuikov <ltuikov@yahoo.com> writes:
> >
> >> Can we please eliminate this message:
> >> Warning: No merge candidate found because value of config option
> >>          "branch.master.merge" does not match any remote branch fetched.
> >
> > The above message was meant only for "git pull", but was leaked
> > even when you did "git fetch"; it was a bug and was corrected
> > already (hopefully).
> 
> Gaah... it turns out that it was not fixed properly.

Sorry, I am missing something.
What is the exact problem that goes wrong here?

The message is printed when "git pull [args]" does not result
in any merge with current branch. What is wrong about this?
But a "git pull ..." always should lead to some merge
in the end in every possible work flow, or should it not?

Josef

^ permalink raw reply

* Re: Updated Kernel Hacker's guide to git
From: Francis Moreau @ 2006-12-22 20:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jay Cliburn, git, Jeff Garzik, Linus Torvalds
In-Reply-To: <7vvek41b29.fsf@assigned-by-dhcp.cox.net>

On 12/22/06, Junio C Hamano <junkio@cox.net> wrote:
> "Francis Moreau" <francis.moro@gmail.com> writes:
>
> > I think this part is really confusing. For a new comer, saying that:
> >
> >       git diff a b == git diff a..b
> >
> > is really not intuitive. Maybe just because adding a new symbol ".."
> > in git diff command line means (for me) that we're doing a special
> > diff. I would never thought by my own that ".." means a simple "to".
>
> We did not originally have "A B"; you were supposed to always
> say "A..B".  But all other SCM had "A B" notation, so we added
> support for both because doing so was trivial and there is no
> risk of confusion (because diff is about two points while log is
> about a set).  These two notations are interchangeable for
> "diff".  If it confuses you, you can stick to the "git diff A B"
> notation.  Of if you are like Linus, stick to "A..B" notation.
> Either way, you can pretend that the other notation does not
> even exist and be happy ;-).
>

no, no, I think you miss my point here. What is confusing, and it
seems I'm not the only one to find it confusing,  is the fact that
"git diff A B == git diff A..B", not the "A..B" notation per se.

git diff A B, is really intuitive and easy to understand because we
all use to using the diff "A B" notation and it always has worked with
2 endpoints. So I would bet that all newbies who use for the first
time the git diff command will use "A B" notation, not "A..B" one.

> Yes, users often wondered why "git diff" accepts "A B", "git
> log" wants "A..B" and "git log A B" is a disaster.  But the root
> cause of the confusion was not about notation but about the
> conceptual difference (two points vs a set).
>
> I do not think changing the meaning of "diff A..B" to what "diff
> A...B" means is a good thing.  The notation "..." even _looks_
> like a magic, and in diff context, what it does _is_ magic (it
> is magic in log context, too).  You are giving two points, but

But ".." is magic too ! I would really expect something magic to
happen when using the "A..B" notation because I'm a new comer in git
and I never used the "A..B" notation before.

And since I already used this notation when using git log command, I
would expect "git diff A..B" to give the current result given by "git
diff A...B".

But yes, now I have understand that git diff works with end points
unlike git log, I can stick with two notations:

        git diff A B
        git diff A...B (3 dots)

and I'm happy ;)

thanks
-- 
Francis

^ permalink raw reply

* Re: What's in git.git (stable)
From: Quy Tonthat @ 2006-12-22 20:21 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vodpw46zj.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> * The 'master' branch has these since the last announcement.
[...]
> 
>    Quy Tonthat (2):
>       ...
>       Documentation/git-branch: new -r to delete remote-tracking branches.
> 
This one is not there on the 'master' branch.
Quy

^ permalink raw reply

* Re: What's in git.git (stable)
From: Junio C Hamano @ 2006-12-22 20:16 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <emhdir$he9$1@sea.gmane.org>

Jakub Narebski <jnareb@gmail.com> writes:

> <opublikowany i wysłany>
>
> Junio C Hamano wrote:
>
>> Possibilities:
>> 
>>  (1) Forget about that "protection" business.  If you do not
>>      want mistakes, use 'branch.*.merge' but otherwise we will
>>      continue to follow the good old "first set of branches"
>>      rule.
>
> What about marking default branch to merge explicitely using
> "Merge:" in remotes/<repo>, or remote.<name>.merge?

Sorry, how is that an improvement over the current branch.*.merge?
and how would that help not breaking existing setups?

^ permalink raw reply

* Re: [PATCH 1/4] gitweb: Add missing show '...' links change.
From: Jakub Narebski @ 2006-12-22 20:09 UTC (permalink / raw)
  To: git
In-Reply-To: <11668162952375-git-send-email-robfitz@273k.net>

Robert Fitzsimons wrote:

> I'm also resubmiting the original optimizations patches with a
> few changes.

Nice series of patches. Ack (FWIW).

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: What's in git.git (stable)
From: Junio C Hamano @ 2006-12-22 20:13 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, Randal L. Schwartz
In-Reply-To: <Pine.LNX.4.63.0612221949490.19693@wbgn013.biozentrum.uni-wuerzburg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> HOWEVER, it has been in "What's cooking in git (topics)".
>
> Having said that, we could (at least for some time) print a big red 
> warning for the specific case of "git pull" meaning "git pull origin 
> <whateveristhefirstfetchedhead>" that this will GO AWAY SOON.
>
> Of course, you would not see it. Only your script.
>
> BTW I would _never_ allow a script to rely on such a DWIM feature.

That's too strong a statement.  It is not DWIM but has been a
longstanding rule to use "the first set of branches" for the
merge.  We've merged the first set of branches since day one,
and even after branch.*.merge we've done so unless you do not
have such configuration.

The complaint (or "newbie protection") was a backward
incompatible change, and it was executed poorly.  So my
apologies go to Merlyn.

While many "new features" can be done in backward compatible
way, any "newbie protection" can break existing practices --
because the whole point of "protection" is to forbid what once
was allowed only because we did not use to check.  And when we
have something that we check now that we did not check before,
that by definition will not be backward compatible.

Some backward incompatibilities are more justfiable than others.
For example, we recently changed "git add" without argument not
to do anything.  It used to have the same effect as "git add .",
but (1) adding all files in a working tree is a rare event, (2)
just a single dot is not hard to type, (3) without argument you
will get a response "not doing anything".  On the other hand,
doing "git add" without argument by mistake and ending up with
the index with many unwanted files is more painful and more
likely to hapen -- it is more valuable to protect users against
it than saving the two keysrokes.  That's why I think that
change is a justified improvement.

I do not think this "merge complaint", at least in the form that
lived on 'master', is justifiable.  The first one that was
merged from the topic made it a requirement to have the
branch.*.merge configuration when you do not give explicit
refspecs on the command line, which was just outright WRONG.
The one currently on the public repository says if the fetch
ended up getting only one branch then not having branch.*.merge
is Ok, which arguably is an improvement, but I do not think is
good enough.  I mentioned other possibilities in my earlier
message.

I also personally think that it is not as clear cut as the "git
add" case that "the first set of branches" rule is wrong, but
the trouble I caused to Merlyn and Luben primarily comes from
the check to trigger "newbie protection", not from what the
protection is trying to do.

^ permalink raw reply

* Re: [PATCH 2/4] gitweb: optimize git_get_last_activity.
From: Jakub Narebski @ 2006-12-22 20:07 UTC (permalink / raw)
  To: git
In-Reply-To: <11668162963671-git-send-email-robfitz@273k.net>

Robert Fitzsimons wrote:

> Only return one line of output and we don't need the refname value.

Refname doesn't hurt and we have it "for free" (meaning: we have to
calculate it anyway to get commiterepoch). Although we do not use it.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* [PATCH] make the message about the need for a new branch a bit clearer
From: Nicolas Pitre @ 2006-12-22 20:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Signed-off-by: Nicolas Pitre <nico@cam.org>
---

diff --git a/git-checkout.sh b/git-checkout.sh
index c877440..923d81c 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -146,8 +146,11 @@ fi
 
 [ -z "$branch$newbranch" ] &&
 	[ "$new" != "$old" ] &&
-	die "git checkout: to checkout the requested commit you need to specify 
-              a name for a new branch which is created and switched to"
+	die "git checkout: provided reference cannot be checked out directly
+
+  You need -b to associate a new branch with the wanted checkout. Example:
+  git checkout -b <new_branch_name> $arg
+"
 
 if [ "X$old" = X ]
 then

^ permalink raw reply related

* Re: What's in git.git (stable)
From: Jakub Narebski @ 2006-12-22 20:04 UTC (permalink / raw)
  To: git
In-Reply-To: <7vwt4jzrjb.fsf@assigned-by-dhcp.cox.net>

<opublikowany i wysłany>

Junio C Hamano wrote:

> Possibilities:
> 
>  (1) Forget about that "protection" business.  If you do not
>      want mistakes, use 'branch.*.merge' but otherwise we will
>      continue to follow the good old "first set of branches"
>      rule.

What about marking default branch to merge explicitely using
"Merge:" in remotes/<repo>, or remote.<name>.merge?

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* [PATCH 3/4] gitweb: optimize git_shortlog_body.
From: Robert Fitzsimons @ 2006-12-22 19:38 UTC (permalink / raw)
  To: git; +Cc: Robert Fitzsimons
In-Reply-To: <11668162963671-git-send-email-robfitz@273k.net>

Don't call gitweb_have_snapshot from within the loop.

Signed-off-by: Robert Fitzsimons <robfitz@273k.net>
---
 gitweb/gitweb.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 01e3a8a..d2ddac8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2636,6 +2636,8 @@ sub git_shortlog_body {
 	# uses global variable $project
 	my ($revlist, $from, $to, $refs, $extra) = @_;
 
+	my $have_snapshot = gitweb_have_snapshot();
+
 	$from = 0 unless defined $from;
 	$to = $#{$revlist} if (!defined $to || $#{$revlist} < $to);
 
@@ -2663,7 +2665,7 @@ sub git_shortlog_body {
 		      $cgi->a({-href => href(action=>"commit", hash=>$commit)}, "commit") . " | " .
 		      $cgi->a({-href => href(action=>"commitdiff", hash=>$commit)}, "commitdiff") . " | " .
 		      $cgi->a({-href => href(action=>"tree", hash=>$commit, hash_base=>$commit)}, "tree");
-		if (gitweb_have_snapshot()) {
+		if ($have_snapshot) {
 			print " | " . $cgi->a({-href => href(action=>"snapshot", hash=>$commit)}, "snapshot");
 		}
 		print "</td>\n" .
-- 
1.4.4.3.gc902c

^ permalink raw reply related

* [PATCH 1/4] gitweb: Add missing show '...' links change.
From: Robert Fitzsimons @ 2006-12-22 19:38 UTC (permalink / raw)
  To: git; +Cc: Robert Fitzsimons

Part of the patch for "gitweb: Show '...' links in "summary" view only
if there are more items" (313ce8cee665447e4476d7e8985b270346a8e5a1) is
missing.  Add it back in.

Signed-off-by: Robert Fitzsimons <robfitz@273k.net>
---


With all the tooing and frowing this part of the original patch got
lost.  I'm also resubmiting the original optimizations patches with a
few changes.

Robert


 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ebbc397..80c04b8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2983,6 +2983,7 @@ sub git_summary {
 	if (@forklist) {
 		git_print_header_div('forks');
 		git_project_list_body(\@forklist, undef, 0, 15,
+		                      $#forklist <= 15 ? undef :
 		                      $cgi->a({-href => href(action=>"forks")}, "..."),
 				      'noheader');
 	}
-- 
1.4.4.3.gc902c

^ permalink raw reply related


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