Git development
 help / color / mirror / Atom feed
* Re: fatal: unexpected EOF
From: Linus Torvalds @ 2006-02-28 15:34 UTC (permalink / raw)
  To: Brian Gerst; +Cc: Git Mailing List
In-Reply-To: <440449D7.3010508@didntduck.org>



On Tue, 28 Feb 2006, Brian Gerst wrote:
>
> Lately I've been receiving this error frequently from git.kernel.org:
> 
> Fetching pack (head and objects)...
> fatal: unexpected EOF
> cg-fetch: fetching pack failed
> 
> What is causing this?

Almost any error will cause the pack sending to abort, and the git:// 
protocol only opens a single socket for data, so there is no way for the 
other end to say _what_ failed.

With git.kernel.org, I suspect the reason for the failure is almost always 
the same, though: the mirroring is not complete, so it doesn't have all 
object files. The mirroring from master.kernel.org to the actual public 
machines is just a rsync script, so there's no atomicity guarantees.

That said, it might be a load issue too - I don't know what limits 
Peter & co put on the git daemons, and it might also be that it's set up 
to accept at most <n> connections and will close anything else.

		Linus

^ permalink raw reply

* Re: bug?: stgit creates (unneccessary?) conflicts when pulling
From: Catalin Marinas @ 2006-02-28 15:00 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: git
In-Reply-To: <44037A5C.6080409@gmail.com>

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

On 27/02/06, Catalin Marinas <catalin.marinas@gmail.com> wrote:
> An idea (untested, I don't even know whether it's feasible) would be to
> check which patches were merged by reverse-applying them starting with
> the last. In this situation, all the merged patches should just revert
> their changes. You only need to do a git-diff between the bottom and the
> top of the patch and git-apply the output (maybe without even modifying
> the tree). If this operation succeeds, the patch was integrated and you
> don't even need to push it.

I tried some simple tests with the idea above. I attached a patch if
you'd like to try (I won't push it to the main StGIT repository yet.
For safety reasons, it only skips the merged patches when pushing
them. A future version could simply delete the merged patches.

--
Catalin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: merged-test.diff --]
[-- Type: text/x-patch; name="merged-test.diff", Size: 5189 bytes --]

Add a merged upstream test for pull and push

From: Catalin Marinas <catalin.marinas@gmail.com>

This patch adds the --merged option to both pull and push commands. With
this option, these commands will first try to check which patches were
merged upstream by reverse-applying them in reverse order. This should
solve the situation where several patches modify the same line in a file.

Signed-off-by: Catalin Marinas <catalin.marinas@gmail.com>
---

 stgit/commands/pull.py |   20 +++++++++++++++++++-
 stgit/commands/push.py |   17 ++++++++++++++++-
 stgit/git.py           |   12 +++++++++---
 stgit/stack.py         |   20 ++++++++++++++++++++
 4 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/stgit/commands/pull.py b/stgit/commands/pull.py
index 843b579..5d75530 100644
--- a/stgit/commands/pull.py
+++ b/stgit/commands/pull.py
@@ -39,6 +39,9 @@ format."""
 
 options = [make_option('-n', '--nopush',
                        help = 'do not push the patches back after pulling',
+                       action = 'store_true'),
+           make_option('-m', '--merged',
+                       help = 'check for patches merged upstream (slower)',
                        action = 'store_true')]
 
 def func(parser, options, args):
@@ -77,12 +80,27 @@ def func(parser, options, args):
     # push the patches back
     if options.nopush:
         applied = []
+
+    # check for patches merged upstream
+    if options.merged:
+        merged = crt_series.merged_patches(patches)
+    else:
+        merged = []
+
     for p in applied:
+        if p in merged:
+            print 'Patch "%s" merged upstream' % p
+            continue
+
         print 'Pushing patch "%s"...' % p,
         sys.stdout.flush()
-        crt_series.push_patch(p)
+
+        modified = crt_series.push_patch(p)
+
         if crt_series.empty_patch(p):
             print 'done (empty patch)'
+        elif modified:
+            print 'done (modified)'
         else:
             print 'done'
 
diff --git a/stgit/commands/push.py b/stgit/commands/push.py
index 9924a78..72b2663 100644
--- a/stgit/commands/push.py
+++ b/stgit/commands/push.py
@@ -49,6 +49,9 @@ options = [make_option('-a', '--all',
            make_option('--reverse',
                        help = 'push the patches in reverse order',
                        action = 'store_true'),
+           make_option('-m', '--merged',
+                       help = 'check for patches merged upstream (slower)',
+                       action = 'store_true'),
            make_option('--undo',
                        help = 'undo the last push operation',
                        action = 'store_true')]
@@ -134,9 +137,21 @@ def func(parser, options, args):
     elif forwarded == 1:
         print 'Fast-forwarded patch "%s"' % patches[0]
 
-    for p in patches[forwarded:]:
+    patches = patches[forwarded:]
+
+    # check for patches merged upstream
+    if options.merged:
+        merged = crt_series.merged_patches(patches)
+    else:
+        merged = []
+
+    for p in patches:
         is_patch_appliable(p)
 
+        if p in merged:
+            print 'Patch "%s" merged upstream' % p
+            continue
+
         print 'Pushing patch "%s"...' % p,
         sys.stdout.flush()
 
diff --git a/stgit/git.py b/stgit/git.py
index 016bc3a..66b8612 100644
--- a/stgit/git.py
+++ b/stgit/git.py
@@ -462,14 +462,20 @@ def commit(message, files = None, parent
 
     return commit_id
 
-def apply_diff(rev1, rev2):
+def apply_diff(rev1, rev2, check_index = True):
     """Apply the diff between rev1 and rev2 onto the current
     index. This function doesn't need to raise an exception since it
     is only used for fast-pushing a patch. If this operation fails,
     the pushing would fall back to the three-way merge.
     """
-    return os.system('git-diff-tree -p %s %s | git-apply --index 2> /dev/null'
-                     % (rev1, rev2)) == 0
+    if check_index:
+        index_opt = '--index'
+    else:
+        index_opt = ''
+    cmd = 'git-diff-tree -p %s %s | git-apply %s 2> /dev/null' \
+          % (rev1, rev2, index_opt)
+
+    return os.system(cmd) == 0
 
 def merge(base, head1, head2):
     """Perform a 3-way merge between base, head1 and head2 into the
diff --git a/stgit/stack.py b/stgit/stack.py
index e1c55f0..9d5f043 100644
--- a/stgit/stack.py
+++ b/stgit/stack.py
@@ -780,6 +780,26 @@ class Series:
 
         return forwarded
 
+    def merged_patches(self, names):
+        """Test which patches were merged upstream by reverse-applying
+        them in reverse order. The function returns the list of
+        patches detected to have been applied. The state of the tree
+        is restored to the original one
+        """
+        patches = [Patch(name, self.__patch_dir, self.__refs_dir)
+                   for name in names]
+        patches.reverse()
+
+        merged = []
+        for p in patches:
+            if git.apply_diff(p.get_top(), p.get_bottom(), False):
+                merged.append(p.get_name())
+        merged.reverse()
+
+        git.reset()
+
+        return merged
+
     def push_patch(self, name):
         """Pushes a patch on the stack
         """

^ permalink raw reply related

* gitview: Set the default width  of graph cell
From: Aneesh Kumar K.V @ 2006-02-28 14:40 UTC (permalink / raw)
  To: git, Junio C Hamano

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



[-- Attachment #2: 0003-gitview-Set-the-default-width-of-graph-cell.txt --]
[-- Type: text/plain, Size: 755 bytes --]

Subject: gitview: Set the default width  of graph cell

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@gmail.com>

---

 contrib/gitview/gitview |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

b298aa0ee1d98b263fe3d493f2911164a4488693
diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index 47ecaa3..ea05cd4 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -526,6 +526,9 @@ class GitView:
 		self.treeview.show()
 
 		cell = CellRendererGraph()
+		#  Set the default width to 265
+		#  This make sure that we have nice display with large tag names
+		cell.set_property("width", 265)
 		column = gtk.TreeViewColumn()
 		column.set_resizable(True)
 		column.pack_start(cell, expand=True)
-- 
1.2.3.gc55f-dirty


^ permalink raw reply related

* [PATCH] Darwin: Ignore missing /sw/lib
From: Shawn Pearce @ 2006-02-28 14:03 UTC (permalink / raw)
  To: git

When on Darwin platforms don't include Fink or DarwinPorts
into the link path unless the related library directory
is actually present.  The linker on MacOS 10.4 complains
if it is given a directory which does not exist.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

---
 I don't have Fink installed, consequently the linker is whining
 about /sw/lib not existing every time I link a GIT executable.
 I'd rather not see complaints from the linker unless they are
 important.

 Makefile |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

base f3a4ec48e402a7b49d410bdcb23470e9723788b0
last 84434f9549d56e522a2eb4de370100f0a6e5e041
diff --git a/Makefile b/Makefile
index 6c59cee41490d4bfba0fb43102555d8de3371d01..19578fc93a60cc41c31883ceac37a0f1ec4202d7 100644
--- a/Makefile
+++ b/Makefile
@@ -223,11 +223,15 @@ ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
 	NEEDS_LIBICONV = YesPlease
 	## fink
-	ALL_CFLAGS += -I/sw/include
-	ALL_LDFLAGS += -L/sw/lib
+	ifeq ($(shell test -d /sw/lib && echo y),y)
+		ALL_CFLAGS += -I/sw/include
+		ALL_LDFLAGS += -L/sw/lib
+	endif
 	## darwinports
-	ALL_CFLAGS += -I/opt/local/include
-	ALL_LDFLAGS += -L/opt/local/lib
+	ifeq ($(shell test -d /opt/local/lib && echo y),y)
+		ALL_CFLAGS += -I/opt/local/include
+		ALL_LDFLAGS += -L/opt/local/lib
+	endif
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease
-- 
1.2.3.gf3a4

^ permalink raw reply related

* gitview: Some  window layout changes.
From: Aneesh Kumar K.V @ 2006-02-28 13:42 UTC (permalink / raw)
  To: git, Junio C Hamano

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



[-- Attachment #2: 0002-gitview-Some-window-layout-changes.txt --]
[-- Type: text/plain, Size: 1973 bytes --]

Subject: gitview: Some  window layout changes.

This makes menubar look nice

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@gmail.com>

---

 contrib/gitview/gitview |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

bc288bd1cd9c70e7eb1e8742527553d1c2dea61d
diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index aded7ed..47ecaa3 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -368,7 +368,7 @@ class DiffWindow:
 		save_menu.connect("activate", self.save_menu_response, "save")
 		save_menu.show()
 		menu_bar.append(save_menu)
-		vbox.pack_start(menu_bar, False, False, 2)
+		vbox.pack_start(menu_bar, expand=False, fill=True)
 		menu_bar.show()
 
 		scrollwin = gtk.ScrolledWindow()
@@ -482,19 +482,10 @@ class GitView:
 
 	def construct(self):
 		"""Construct the window contents."""
+		vbox = gtk.VBox()
 		paned = gtk.VPaned()
 		paned.pack1(self.construct_top(), resize=False, shrink=True)
 		paned.pack2(self.construct_bottom(), resize=False, shrink=True)
-		self.window.add(paned)
-		paned.show()
-
-
-	def construct_top(self):
-		"""Construct the top-half of the window."""
-		vbox = gtk.VBox(spacing=6)
-		vbox.set_border_width(12)
-		vbox.show()
-
 		menu_bar = gtk.MenuBar()
 		menu_bar.set_pack_direction(gtk.PACK_DIRECTION_RTL)
 		help_menu = gtk.MenuItem("Help")
@@ -506,8 +497,20 @@ class GitView:
 		help_menu.set_submenu(menu)
 		help_menu.show()
 		menu_bar.append(help_menu)
-		vbox.pack_start(menu_bar, False, False, 2)
 		menu_bar.show()
+		vbox.pack_start(menu_bar, expand=False, fill=True)
+		vbox.pack_start(paned, expand=True, fill=True)
+		self.window.add(vbox)
+		paned.show()
+		vbox.show()
+
+
+	def construct_top(self):
+		"""Construct the top-half of the window."""
+		vbox = gtk.VBox(spacing=6)
+		vbox.set_border_width(12)
+		vbox.show()
+
 
 		scrollwin = gtk.ScrolledWindow()
 		scrollwin.set_policy(gtk.POLICY_NEVER, gtk.POLICY_AUTOMATIC)
-- 
1.2.3.gc55f-dirty


^ permalink raw reply related

* Select the text color based on whether the entry in highlighted.
From: Aneesh Kumar K.V @ 2006-02-28 13:41 UTC (permalink / raw)
  To: git, Junio C Hamano, Pavel Roskin

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



[-- Attachment #2: 0001-Select-the-text-color-based-on-whether-the-entry-in-highlighted.-Use.txt --]
[-- Type: text/plain, Size: 1299 bytes --]

From: Pavel Roskin <proski@gnu.org>
Subject: Select the text color based on whether the entry in highlighted.  Use
standard font.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@gmail.com>

---

 contrib/gitview/gitview |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

21154c68bf8b95e0db49d507ea34e0b8a51308df
diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index 048caf6..aded7ed 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -239,20 +239,23 @@ class CellRendererGraph(gtk.GenericCellR
 				box_size / 4, 0, 2 * math.pi)
 
 
+		self.set_colour(ctx, colour, 0.0, 0.5)
+		ctx.stroke_preserve()
+
+		self.set_colour(ctx, colour, 0.5, 1.0)
+		ctx.fill_preserve()
+
 		if (len(names) != 0):
 			name = " "
 			for item in names:
 				name = name + item + " "
 
-			ctx.select_font_face("Monospace")
 			ctx.set_font_size(13)
-			ctx.text_path(name)
-
-		self.set_colour(ctx, colour, 0.0, 0.5)
-		ctx.stroke_preserve()
-
-		self.set_colour(ctx, colour, 0.5, 1.0)
-		ctx.fill()
+			if (flags & 1):
+				self.set_colour(ctx, colour, 0.5, 1.0)
+			else:
+				self.set_colour(ctx, colour, 0.0, 0.5)
+			ctx.show_text(name)
 
 class Commit:
 	""" This represent a commit object obtained after parsing the git-rev-list
-- 
1.2.3.gc55f-dirty


^ permalink raw reply related

* fatal: unexpected EOF
From: Brian Gerst @ 2006-02-28 13:02 UTC (permalink / raw)
  To: Git Mailing List

Lately I've been receiving this error frequently from git.kernel.org:

Fetching pack (head and objects)...
fatal: unexpected EOF
cg-fetch: fetching pack failed

What is causing this?

--
						Brian Gerst

^ permalink raw reply

* [PATCH 2/2] Speed up history generation
From: Luben Tuikov @ 2006-02-28 12:39 UTC (permalink / raw)
  To: git

Speed up history generation as suggested by Linus.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>

---

 gitweb.cgi |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

69d694fccacb09059731abe4918f8f9aa8969690
diff --git a/gitweb.cgi b/gitweb.cgi
index 452528f..bfea65d 100755
--- a/gitweb.cgi
+++ b/gitweb.cgi
@@ -2124,16 +2124,12 @@ sub git_history {
 	      "</div>\n";
 	print "<div class=\"page_path\"><b>/" . esc_html($file_name) . "</b><br/></div>\n";
 
-	open my $fd, "-|", "$gitbin/git-rev-list $hash | $gitbin/git-diff-tree -r --stdin
\'$file_name\'";
-	my $commit;
+	open my $fd, "-|", "$gitbin/git-rev-list $hash -- \'$file_name\'";
 	print "<table cellspacing=\"0\">\n";
 	my $alternate = 0;
 	while (my $line = <$fd>) {
-		if ($line =~ m/^([0-9a-fA-F]{40})/){
-			$commit = $1;
-			next;
-		}
-		if ($line =~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)\t(.*)$/ &&
(defined $commit)) {
+	        if ($line =~ m/^([0-9a-fA-F]{40})/){
+			my $commit = $1;
 			my %co = git_read_commit($commit);
 			if (!%co) {
 				next;
@@ -2165,7 +2161,6 @@ sub git_history {
 			}
 			print "</td>\n" .
 			      "</tr>\n";
-			undef $commit;
 		}
 	}
 	print "</table>\n";
-- 
1.2.3.g975a

^ permalink raw reply related

* [PATCH 1/2] Enable tree (directory) history display
From: Luben Tuikov @ 2006-02-28 12:38 UTC (permalink / raw)
  To: git

This patch allows history display of whole trees/directories,
a la "git-rev-list HEAD <dir or file>", but somewhat
slower, since exported git repository doesn't have
the files checked out so we have to use
"$gitbin/git-rev-list $hash | $gitbin/git-diff-tree -r --stdin \'$file_name\'"
method.

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>

---

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

5c8ae3db3561238a57201fcb3297f16d7b37f377
diff --git a/gitweb.cgi b/gitweb.cgi
index c1bb624..452528f 100755
--- a/gitweb.cgi
+++ b/gitweb.cgi
@@ -1504,6 +1504,7 @@ sub git_tree {
 			      "</td>\n" .
 			      "<td class=\"link\">" .
 			      $cgi->a({-href => "$my_uri?" .
esc_param("p=$project;a=tree;h=$t_hash$base_key;f=$base$t_name")}, "tree") .
+			      " | " . $cgi->a({-href => "$my_uri?" .
esc_param("p=$project;a=history;h=$hash_base;f=$base$t_name")}, "history") .
 			      "</td>\n";
 		}
 		print "</tr>\n";
-- 
1.2.3.g975a

^ permalink raw reply related

* [ANNOUNCE] quilt2git v0.2
From: Tejun Heo @ 2006-02-28 11:11 UTC (permalink / raw)
  To: linux-kernel, git

Hello, v0.2 of quilt2git available.  New in v0.2.

* handles new git HEAD file format properly (regular file storing ref: ...)

* makes use of mail format header from quilt patch description.  From:
  becomes the author, Subject: the subject of the patch.  All commit
  information should be maintained through git2quilt -> quilt2git now.

* --signoff option added.  This option is simply passed to git-commit.

* little fixes

http://home-tj.org/wiki/index.php/Misc
http://home-tj.org/files/misc/quilt2git-0.2
http://home-tj.org/files/misc/git2quilt-0.1

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 2/3] apply --whitespace: configuration option.
From: Andreas Ericsson @ 2006-02-28  9:46 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzmkbn7qx.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Andreas Ericsson <ae@op5.se> writes:
> 
> 
>>Junio C Hamano wrote:
>>
>>>The new configuration option apply.whitespace can take one of
>>>"warn", "error", "error-all", or "strip".  When git-apply is run
>>>to apply the patch to the index, they are used as the default
>>>value if there is no command line --whitespace option.
>>
>>I would think "warn-all" would be the logical thing, since "error"
>>either breaks out early or prints all warnings before denying the
>>patch anyway.
> 
> 
> Actually there is some thinking behind why I did not do warn-all.
> I did consider it at first but rejected.
> 
>  * If you are a busy top echelon person but cares about tree
>    cleanliness, --whitespace=error is good enough.  The patch is
>    rejected on WS basis whether it introduces one such trailing
>    WS or hundreds.  The patch is returned to the submitter and
>    the tree remains clean.
> 
>  * --whitespace=warn-all, if existed, would apply the patch
>    _anyway_, so if you notice you got warnings, and if that
>    bothers you enough that you would want to do something about
>    it, you will have to rewind the HEAD, fix up .dotest/patch
>    and reapply.  This means you are willing to clean up other
>    peoples' patches.
> 
>  * But if you are that kind of person, --whitespace=error-all is
>    a better choice for you.  Your tree stays clean and you do
>    not have to rewind.  Instead, you get all the errors you can
>    go through with your editor (e.g. Emacs users can use C-x `;
>    I hope vim users have similar macros) and fix things.
> 

Good Thinking. Thanks for explaining.

> 
> The last one is somewhat risky, and the output may need to be
> examined carefully depending on the contents (e.g. programming
> language) the project is dealing with.
> 
> 

echo Makefile >> .git/no-ws-strip
echo '*.[ch]' >> .git/ws-strip

Perhaps not viable, and probably stupid as well. Mixed content repos 
would likely just keep the 'warn' policy.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH 2/3] apply --whitespace: configuration option.
From: Junio C Hamano @ 2006-02-28  9:38 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git
In-Reply-To: <440414D6.8050407@op5.se>

Andreas Ericsson <ae@op5.se> writes:

> Junio C Hamano wrote:
>> The new configuration option apply.whitespace can take one of
>> "warn", "error", "error-all", or "strip".  When git-apply is run
>> to apply the patch to the index, they are used as the default
>> value if there is no command line --whitespace option.
>
> I would think "warn-all" would be the logical thing, since "error"
> either breaks out early or prints all warnings before denying the
> patch anyway.

Actually there is some thinking behind why I did not do warn-all.
I did consider it at first but rejected.

 * If you are a busy top echelon person but cares about tree
   cleanliness, --whitespace=error is good enough.  The patch is
   rejected on WS basis whether it introduces one such trailing
   WS or hundreds.  The patch is returned to the submitter and
   the tree remains clean.

 * --whitespace=warn-all, if existed, would apply the patch
   _anyway_, so if you notice you got warnings, and if that
   bothers you enough that you would want to do something about
   it, you will have to rewind the HEAD, fix up .dotest/patch
   and reapply.  This means you are willing to clean up other
   peoples' patches.

 * But if you are that kind of person, --whitespace=error-all is
   a better choice for you.  Your tree stays clean and you do
   not have to rewind.  Instead, you get all the errors you can
   go through with your editor (e.g. Emacs users can use C-x `;
   I hope vim users have similar macros) and fix things.

 * --whitespace=warn would show some, but not all, so that you
   can continue while making a mental note to scold the patch
   submitter to be careful the next time.  You chose "warn" to
   apply the patch anyway, so there is no point showing the full
   extent of damage -- the damage is already done to your tree.

 * --whitespace=strip is for people who care about cleanliness,
   who wants to be nice to the submitters, but not nice enough
   to educate them.  They do not want to fix things by hand.
   Instead they have the tool to do the fixing for them.

The last one is somewhat risky, and the output may need to be
examined carefully depending on the contents (e.g. programming
language) the project is dealing with.

^ permalink raw reply

* Re: [PATCH 2/3] apply --whitespace: configuration option.
From: Andreas Ericsson @ 2006-02-28  9:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Andrew Morton
In-Reply-To: <7vacccuvxz.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> The new configuration option apply.whitespace can take one of
> "warn", "error", "error-all", or "strip".  When git-apply is run
> to apply the patch to the index, they are used as the default
> value if there is no command line --whitespace option.
> 

I would think "warn-all" would be the logical thing, since "error" 
either breaks out early or prints all warnings before denying the patch 
anyway.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* [PATCH] git-apply: war on whitespace -- finishing touches.
From: Junio C Hamano @ 2006-02-28  9:13 UTC (permalink / raw)
  To: git; +Cc: Andrew Morton
In-Reply-To: <20060227153702.375db751.akpm@osdl.org>

Andrew Morton <akpm@osdl.org> writes:

> I'd suggest that git have options to a) generate trailing-whitespace
> warnings, b) generate trailing-whitespace errors and c) strip trailing
> whitespace while applying.   And that the as-shipped default be a).

I've done this and will be pushing it out to "master" branch on
my next git day (Wednesday, west coast US); "maint" branch will
have the same for v1.2.4 sometime by the end of this week.

There is one thing.  By making --whitespace=warn the default,
the diffstat output people would see after "git pull" would also
show the warning message.  I personally do not think this is a
problem (you will know how dirty a tree you are merging into
your tree), but it might not be a bad idea to explicitly squelch
it by making it not to warn when we are not applying.

-- >8 --

This changes the default --whitespace policy to nowarn when we
are only getting --stat, --summary etc. IOW when not applying
the patch.  When applying the patch, the default is warn (spit
out warning message but apply the patch).

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 apply.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

f21d6726150ec4219e94ea605f27a4cd58eb3d99
diff --git a/apply.c b/apply.c
index c4ff418..9deb206 100644
--- a/apply.c
+++ b/apply.c
@@ -75,6 +75,15 @@ static void parse_whitespace_option(cons
 	die("unrecognized whitespace option '%s'", option);
 }
 
+static void set_default_whitespace_mode(const char *whitespace_option)
+{
+	if (!whitespace_option && !apply_default_whitespace) {
+		new_whitespace = (apply
+				  ? warn_on_whitespace
+				  : nowarn_whitespace);
+	}
+}
+
 /*
  * For "diff-stat" like behaviour, we keep track of the biggest change
  * we've seen, and the longest filename. That allows us to do simple
@@ -1955,9 +1964,11 @@ int main(int argc, char **argv)
 		if (fd < 0)
 			usage(apply_usage);
 		read_stdin = 0;
+		set_default_whitespace_mode(whitespace_option);
 		apply_patch(fd, arg);
 		close(fd);
 	}
+	set_default_whitespace_mode(whitespace_option);
 	if (read_stdin)
 		apply_patch(0, "<stdin>");
 	if (whitespace_error) {
-- 
1.2.3.g1da2

^ permalink raw reply related

* Re: [PATCH] Add git-annotate, a tool for assigning blame.
From: Andreas Ericsson @ 2006-02-28  9:08 UTC (permalink / raw)
  To: Ryan Anderson; +Cc: Fredrik Kuivinen, Junio C Hamano, git
In-Reply-To: <20060228084737.GA13537@mythryan2.michonline.com>

Ryan Anderson wrote:
> On Tue, Feb 28, 2006 at 09:27:36AM +0100, Fredrik Kuivinen wrote:
> 
>>On Mon, Feb 20, 2006 at 04:01:56PM -0800, Junio C Hamano wrote:
>>
>>>Fredrik Kuivinen <freku045@student.liu.se> writes:
>>>
>>>
>>>>I have also been working on a blame program.
>>
>>...
>>
>>
>>>BTW, these days I always compile things with 
>>>
>>>	-Wall -Wdeclaration-after-statement
>>>
>>>which caught quite a many.
>>
>>Just out of curiosity, why do you prefer declarations before
>>statements?
> 
> 
> I won't speak for Junio, but the explanations I've heard in the past are
> basically:
> 
> 1) It keeps all declarations in one spot.
> 2) If your function is complicated enough to not need a variable until
> fairly far into the function, it probably should be two (or more) functions.
> 

3) Not all compilers support it.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH] Add git-annotate, a tool for assigning blame.
From: Ryan Anderson @ 2006-02-28  8:47 UTC (permalink / raw)
  To: Fredrik Kuivinen; +Cc: Junio C Hamano, git
In-Reply-To: <20060228082736.GA4593@c165.ib.student.liu.se>

On Tue, Feb 28, 2006 at 09:27:36AM +0100, Fredrik Kuivinen wrote:
> On Mon, Feb 20, 2006 at 04:01:56PM -0800, Junio C Hamano wrote:
> > Fredrik Kuivinen <freku045@student.liu.se> writes:
> > 
> > > I have also been working on a blame program.
> 
> ...
> 
> > BTW, these days I always compile things with 
> > 
> > 	-Wall -Wdeclaration-after-statement
> > 
> > which caught quite a many.
> 
> Just out of curiosity, why do you prefer declarations before
> statements?

I won't speak for Junio, but the explanations I've heard in the past are
basically:

1) It keeps all declarations in one spot.
2) If your function is complicated enough to not need a variable until
fairly far into the function, it probably should be two (or more) functions.

Basically, I think that there's not wrong with doing it that way, per
se, just that it's sometimes a symptom of other problems, so fi you look
for the symptom, the problem sometimes is more obvious.


-- 

Ryan Anderson
  sometimes Pug Majere

^ permalink raw reply

* Re: [PATCH] Add git-annotate, a tool for assigning blame.
From: Junio C Hamano @ 2006-02-28  8:38 UTC (permalink / raw)
  To: Fredrik Kuivinen; +Cc: git
In-Reply-To: <20060228082736.GA4593@c165.ib.student.liu.se>

Fredrik Kuivinen <freku045@student.liu.se> writes:

> On Mon, Feb 20, 2006 at 04:01:56PM -0800, Junio C Hamano wrote:
>> 
>> BTW, these days I always compile things with 
>> 
>> 	-Wall -Wdeclaration-after-statement
>> 
>> which caught quite a many.
>
> Just out of curiosity, why do you prefer declarations before
> statements?

Inertia, IOW, mostly being used to read code written that way.

^ permalink raw reply

* Re: [PATCH] Add git-annotate, a tool for assigning blame.
From: Fredrik Kuivinen @ 2006-02-28  8:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Fredrik Kuivinen, git, Ryan Anderson
In-Reply-To: <7vlkw57f63.fsf@assigned-by-dhcp.cox.net>

On Mon, Feb 20, 2006 at 04:01:56PM -0800, Junio C Hamano wrote:
> Fredrik Kuivinen <freku045@student.liu.se> writes:
> 
> > I have also been working on a blame program.

...

> BTW, these days I always compile things with 
> 
> 	-Wall -Wdeclaration-after-statement
> 
> which caught quite a many.

Just out of curiosity, why do you prefer declarations before
statements?

- Fredrik

^ permalink raw reply

* Re: [PATCH] diff-delta: bound hash list length to avoid O(m*n) behavior
From: Junio C Hamano @ 2006-02-28  8:10 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <7vhd6kq8lc.fsf@assigned-by-dhcp.cox.net>

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

> Nicolas Pitre <nico@cam.org> writes:
>
>> blob 9af06ba723df75fed49f7ccae5b6c9c34bc5115f ->
>> blob dfc9cd58dc065d17030d875d3fea6e7862ede143
>> size (491102 -> 496045)
>> delta size (16 byte blocks): 248899 in less than 0.1 sec
>> delta size (3 byte blocks): 136000 in 11.8 secs
>> delta size (3 byte blocks + this patch): 171688 in 0.79 sec
>
> These numbers are misleading.
>
> The 36K objects pack I used in my previous tests gives 9971223
> (from "next" branch) vs 9664546 (this patch) final pack size.
> The wallclock time on my machine is 1m35 vs 3m30.  I doubt many
> people are willing to pay 100% more waiting time for 3% tighter
> pack.

I tried an experimental patch to cull collided hash buckets
very aggressively.  I haven't applied your last "reuse index"
patch, though -- I think that is orthogonal and I'd like to
leave that to the next round.

With the same dataset: resulting pack is 9651096 vs 9664546
(your patch) final pack size, with wallclock 2m45s (user 2m31).
Still not good enough, and at the same time I wonder why it gets
_smaller_ results than yours.  But the generated pack unpacked
cleanly in a cloned linux-2.6 repository (having objects and
refs up to v2.6.14) and the result was fsck-objects clean.

I'd appreciate it if you can test it on the 20MB blobs and see
what happens if you have time.

BTW, the benchmark I've been doing is with this dataset:

	git rev-list --objects-edge v2.6.14..v2.6.15-rc1 >RL-N
        time git pack-objects <RL-N --stdout | wc -c

---
diff --git a/diff-delta.c b/diff-delta.c
index 0730b24..52df372 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -88,22 +88,35 @@ static struct index ** delta_index(const
 
 	/*
 	 * Now make sure none of the hash buckets has more entries than
-	 * we're willing to test.  Otherwise we short-circuit the entry
-	 * list uniformly to still preserve a good repartition across
-	 * the reference buffer.
+	 * we're willing to test.  Otherwise we cull the entry list to
+	 * limit identical three byte prefixes to still preserve a good
+	 * repartition across the reference buffer.
 	 */
 	for (i = 0; i < hsize; i++) {
+		struct index **list, *bucket, *remaining;
+		int cnt;
 		if (hash_count[i] < hlimit)
 			continue;
-		entry = hash[i];
-		do {
-			struct index *keep = entry;
-			int skip = hash_count[i] / hlimit / 2;
-			do {
-				entry = entry->next;
-			} while(--skip && entry);
-			keep->next = entry;
-		} while(entry);
+		
+		bucket = NULL;
+		list = &bucket;
+		remaining = hash[i];
+		cnt = 0;
+		while (cnt < hlimit && remaining) {
+			struct index *this = remaining, *that;
+			remaining = remaining->next;
+			for (that = bucket; that; that = that->next) {
+				if (!memcmp(that->ptr, this->ptr, 3))
+					break;
+			}
+			if (that)
+				continue; /* discard */
+			cnt++;
+			*list = this;
+			list = &(this->next);
+			this->next = NULL;
+		}
+		hash[i] = bucket;
 	}
 	free(hash_count);
 

^ permalink raw reply related

* Re: [PATCH] diff-delta: bound hash list length to avoid O(m*n) behavior
From: Junio C Hamano @ 2006-02-28  6:51 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0602272110320.25336@localhost.localdomain>

Nicolas Pitre <nico@cam.org> writes:

> The diff-delta code can exhibit O(m*n) behavior with some patological 
> data set where most hash entries end up in the same hash bucket.

> Other data points from the Linux kernel repository:
>
> blob 9af06ba723df75fed49f7ccae5b6c9c34bc5115f ->
> blob dfc9cd58dc065d17030d875d3fea6e7862ede143
> size (491102 -> 496045)
> delta size (16 byte blocks): 248899 in less than 0.1 sec
> delta size (3 byte blocks): 136000 in 11.8 secs
> delta size (3 byte blocks + this patch): 171688 in 0.79 sec

These numbers are misleading.

The 36K objects pack I used in my previous tests gives 9971223
(from "next" branch) vs 9664546 (this patch) final pack size.
The wallclock time on my machine is 1m35 vs 3m30.  I doubt many
people are willing to pay 100% more waiting time for 3% tighter
pack.

Although I do not mean to rain on your effort and substantial
improvement you made with this patch, we need to admit that
improving pathological corner case has quite a diminishing
effect in the overall picture.

But this definitely is an improvement nevertheless.  I should
try this on my wife's AMD64 (Cygwin).  The same datasets I used
in my previous complaint still seem to take a couple of seconds
(or more) each on my Duron 750 X-<.

A handful additional ideas.

 * Lower the hash limit a bit more (obvious).  That might have
   detrimental effect in the general case.

 * Study the hash bucket distribution for the pathological case
   and see if we can cheaply detect a pattern.  I suspect these
   cases have relatively few but heavily collided buckets with
   mostly sparse other entries.  If there is such an easily
   detectable pattern, maybe we can look for such a pattern at
   runtime, and cull hash entries more aggressively in such a
   case?

 * Try checking the target buffer to see if it would have many
   hits on the heavily collided hash entries from the source
   (maybe hash_count the target buffer as well).

 * Have pack-object detect a pathological blob (the test patch I
   sent you previously uses the eye-candy timer for this
   purpose, but we could getrusage() if we want to be more
   precise) by observing how much time is spent for a single
   round, and mark the blob as such, so we do not delta against
   it with other blobs in find_deltas, when we are packing many
   objects.  It does not really matter in the big picture if we
   choose not to delta the pathological ones tightly, as long as
   they are relatively few.

 * Also in pack-object, have an optional backing-store to write
   out deltified representations for results that took more than
   certain amount of time to produce in find_deltas(), and reuse
   them in write_object().

If anybody is interested, here are some other interesting pairs
from the Linux 2.6 repositories.

        2645d6239b74 cb968e7a0fcd
        357980942d73 5bb837052ef1
        5412dcb4b6d0 ac07e18abb1d
        5bb837052ef1 1f8744ad9cde
        63d827d7da07 5bb837052ef1
        9af06ba723df 1896a0eea7e5
        cb968e7a0fcd 97b5578ae9b1
        d8c5a8dbd086 97b5578ae9b1
        d8c5a8dbd086 cb968e7a0fcd
        dfc9cd58dc06 1896a0eea7e5
        fa0f82ec9fa0 b611e27470e1

^ permalink raw reply

* Re: someone changed the contents of my HEAD.
From: Dave Jones @ 2006-02-28  5:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vlkvwt5v9.fsf@assigned-by-dhcp.cox.net>

On Mon, Feb 27, 2006 at 09:22:18PM -0800, Junio C Hamano wrote:
 > > #!/bin/sh
 > > export GIT_AUTHOR_NAME="$1"
 > > export GIT_AUTHOR_EMAIL="$2"
 > > tree=$(git-write-tree) || exit 1
 > > commit=$(git-commit-tree $tree -p HEAD) || exit 1
 > > echo $commit > .git/HEAD
 > 
 > This has been deprecated for a looong time, but perhaps I should
 > have been louder.  "git commit --author" should be fine -- I do
 > not think you do not even need such a wrapper.

I guess I wasn't paying attention :)
It's worked just fine up until a few days ago.
I can change my habits to use the preferred approach though.

 > > For my newly created repos, this isn't a problem, as I can fudge my
 > > commit-as script to write to .git/refs/heads/master instead, but
 > > my concern now is the unpulled changes in the existing repos
 > > I have on master.  Will Linus be able to pull those into his tree
 > > with git 1.2.3, or will I have to recreate those repos with the
 > > new-style .git/HEAD ?
 >
 > A .git/HEAD symlink pointing at refs/heads/master _is_ still
 > (and will be) supported, so either symlink or symref is fine.
 > Bare SHA1 object name in HEAD is not -- git would not know which
 > branch you are on, so "checkin" would not work after that.
 > 
 > Your refs/heads/master might be stale (one rev or more older
 > than what you replaced .git/HEAD with) but still should be a
 > proper ancestor.  After making sure your refs/heads/master has
 > the right commit (you may have it there already if you have been
 > updating them using "git-push"), running "git-symbolic-ref HEAD
 > refs/heads/master" would fix things.

One scary thing fell out of this.  Andrew did a pull on my trees
and found bogons in both of them.  Both agpgart and cpufreq trees
contained partial (1 file) reverts of earlier commits in my last commit
to the head of each repo.  I can't explain how that happened, and
I'm surprised my head-munging would be the reason.

I've just rebuilt those two repos as there was only 3-4 patches
in both, so it wasn't a big deal. Something to watch out for
though if anyone else has been doing something similar to how
I was doing checkins.

		Dave

^ permalink raw reply

* Re: [PATCH 2/2] Don't use excessive non-standard border width
From: Aneesh Kumar K.V @ 2006-02-28  5:53 UTC (permalink / raw)
  To: Pavel Roskin
In-Reply-To: <1141104931.22029.5.camel@dv>

On Tue, Feb 28, 2006 at 12:35:31AM -0500, Pavel Roskin wrote:
> Hi, Aneesh!
> 
> On Tue, 2006-02-28 at 10:46 +0530, Aneesh Kumar K.V wrote:
> > I am not sure about this. I guess it looks much better with a border.
> > 
> > Not applied.
> 
> I think what really annoyed me was the menu with lone "Help" on the
> right hand side, and excessive borders only made it look worse.  Maybe
> the border should not be used on the menu?  It looks wrong to me that
> the menu is appended to the top pane rather than to the whole window.
> 

how about this

diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
index aded7ed..e57c53b 100755
--- a/contrib/gitview/gitview
+++ b/contrib/gitview/gitview
@@ -482,19 +482,10 @@ class GitView:
 
 	def construct(self):
 		"""Construct the window contents."""
+		vbox = gtk.VBox()
 		paned = gtk.VPaned()
 		paned.pack1(self.construct_top(), resize=False, shrink=True)
 		paned.pack2(self.construct_bottom(), resize=False, shrink=True)
-		self.window.add(paned)
-		paned.show()
-
-
-	def construct_top(self):
-		"""Construct the top-half of the window."""
-		vbox = gtk.VBox(spacing=6)
-		vbox.set_border_width(12)
-		vbox.show()
-
 		menu_bar = gtk.MenuBar()
 		menu_bar.set_pack_direction(gtk.PACK_DIRECTION_RTL)
 		help_menu = gtk.MenuItem("Help")
@@ -506,8 +497,20 @@ class GitView:
 		help_menu.set_submenu(menu)
 		help_menu.show()
 		menu_bar.append(help_menu)
-		vbox.pack_start(menu_bar, False, False, 2)
 		menu_bar.show()
+		vbox.pack_start(menu_bar, False, False, 2)
+		vbox.pack_start(paned, True, True, 2)
+		self.window.add(vbox)
+		paned.show()
+		vbox.show()
+
+
+	def construct_top(self):
+		"""Construct the top-half of the window."""
+		vbox = gtk.VBox(spacing=6)
+		vbox.set_border_width(12)
+		vbox.show()
+
 
 		scrollwin = gtk.ScrolledWindow()
 		scrollwin.set_policy(gtk.POLICY_NEVER, gtk.POLICY_AUTOMATIC)

^ permalink raw reply related

* Re: [PATCH 2/2] Don't use excessive non-standard border width
From: Pavel Roskin @ 2006-02-28  5:35 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: git
In-Reply-To: <20060228051611.GB5934@satan.india.hp.com>

Hi, Aneesh!

On Tue, 2006-02-28 at 10:46 +0530, Aneesh Kumar K.V wrote:
> I am not sure about this. I guess it looks much better with a border.
> 
> Not applied.

I think what really annoyed me was the menu with lone "Help" on the
right hand side, and excessive borders only made it look worse.  Maybe
the border should not be used on the menu?  It looks wrong to me that
the menu is appended to the top pane rather than to the whole window.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* Re: someone changed the contents of my HEAD.
From: Junio C Hamano @ 2006-02-28  5:22 UTC (permalink / raw)
  To: Dave Jones; +Cc: git
In-Reply-To: <20060228030446.GA23490@redhat.com>

Dave Jones <davej@redhat.com> writes:

> When checking in changes previously, I used this..
>
> #!/bin/sh
> export GIT_AUTHOR_NAME="$1"
> export GIT_AUTHOR_EMAIL="$2"
> tree=$(git-write-tree) || exit 1
> commit=$(git-commit-tree $tree -p HEAD) || exit 1
> echo $commit > .git/HEAD

This has been deprecated for a looong time, but perhaps I should
have been louder.  "git commit --author" should be fine -- I do
not think you do not even need such a wrapper.

> For my newly created repos, this isn't a problem, as I can fudge my
> commit-as script to write to .git/refs/heads/master instead, but
> my concern now is the unpulled changes in the existing repos
> I have on master.  Will Linus be able to pull those into his tree
> with git 1.2.3, or will I have to recreate those repos with the
> new-style .git/HEAD ?

A .git/HEAD symlink pointing at refs/heads/master _is_ still
(and will be) supported, so either symlink or symref is fine.
Bare SHA1 object name in HEAD is not -- git would not know which
branch you are on, so "checkin" would not work after that.

Your refs/heads/master might be stale (one rev or more older
than what you replaced .git/HEAD with) but still should be a
proper ancestor.  After making sure your refs/heads/master has
the right commit (you may have it there already if you have been
updating them using "git-push"), running "git-symbolic-ref HEAD
refs/heads/master" would fix things.

^ permalink raw reply

* Re: [PATCH 2/2] Don't use excessive non-standard border width
From: Aneesh Kumar K.V @ 2006-02-28  5:16 UTC (permalink / raw)
  To: Pavel Roskin; +Cc: git
In-Reply-To: <20060228045631.21880.27670.stgit@dv.roinet.com>

On Mon, Feb 27, 2006 at 11:56:31PM -0500, Pavel Roskin wrote:
> From: Pavel Roskin <proski@gnu.org>
> 
> 
> ---
> 
>  contrib/gitview/gitview |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
> index aded7ed..00cb8ee 100755
> --- a/contrib/gitview/gitview
> +++ b/contrib/gitview/gitview
> @@ -492,7 +492,6 @@ class GitView:
>  	def construct_top(self):
>  		"""Construct the top-half of the window."""
>  		vbox = gtk.VBox(spacing=6)
> -		vbox.set_border_width(12)
>  		vbox.show()
>  
>  		menu_bar = gtk.MenuBar()
> @@ -574,7 +573,6 @@ class GitView:
>  	def construct_bottom(self):
>  		"""Construct the bottom half of the window."""
>  		vbox = gtk.VBox(False, spacing=6)
> -		vbox.set_border_width(12)
>  		(width, height) = self.window.get_size()
>  		vbox.set_size_request(width, int(height / 2.5))
>  		vbox.show()


I am not sure about this. I guess it looks much better with a border.

Not applied.

-aneesh

^ 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