Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2] am: add am.signoff add config variable
From: Eduardo Habkost @ 2016-12-28 19:12 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: git, Paul Tan, Stefan Beller
In-Reply-To: <871swrg9dh.fsf@linux-m68k.org>

On Wed, Dec 28, 2016 at 08:07:54PM +0100, Andreas Schwab wrote:
> On Dez 28 2016, Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> > index 12879e402..f22f10d40 100644
> > --- a/Documentation/git-am.txt
> > +++ b/Documentation/git-am.txt
> > @@ -9,7 +9,7 @@ git-am - Apply a series of patches from a mailbox
> >  SYNOPSIS
> >  --------
> >  [verse]
> > -'git am' [--signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8]
> > +'git am' [--[no-]signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8]
> >  	 [--[no-]3way] [--interactive] [--committer-date-is-author-date]
> >  	 [--ignore-date] [--ignore-space-change | --ignore-whitespace]
> >  	 [--whitespace=<option>] [-C<n>] [-p<n>] [--directory=<dir>]
> > @@ -32,10 +32,12 @@ OPTIONS
> >  	If you supply directories, they will be treated as Maildirs.
> >  
> >  -s::
> > ---signoff::
> > +--[no]-signoff::
> 
> That should be --[no-]signoff, as in the synopsis.

Thanks for catching it. I will fix it in v3.

-- 
Eduardo

^ permalink raw reply

* Re: [PATCH v2] am: add am.signoff add config variable
From: Eduardo Habkost @ 2016-12-28 19:19 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git@vger.kernel.org, Paul Tan
In-Reply-To: <20161228191142.GF3441@thinpad.lan.raisama.net>

On Wed, Dec 28, 2016 at 05:11:42PM -0200, Eduardo Habkost wrote:
> On Wed, Dec 28, 2016 at 10:51:28AM -0800, Stefan Beller wrote:
> > On Wed, Dec 28, 2016 at 10:35 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
[...]
> > > +       test $(git cat-file commit HEAD | grep -c "Signed-off-by:") -eq 0
> > 
> > and then we check if the top most commit has zero occurrences
> > for lines grepped for sign off. That certainly works, but took me a
> > while to understand (TIL about -c in grep :).
> > 
> > Another way that to write this check, that Git regulars may be more used to is:
> > 
> >     git cat-file commit HEAD | grep "Signed-off-by:" >actual
> >     test_must_be_empty actual
> 
> test_must_be_empty is what I was looking for. But if I do this:
> 
> test_expect_success '--no-signoff overrides am.signoff' '
> 	rm -fr .git/rebase-apply &&
> 	git reset --hard first &&
> 	test_config am.signoff true &&
> 	git am --no-signoff <patch2 &&
> 	printf "%s\n" "$signoff" >expected &&
> 	git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
> 	test_cmp expected actual &&
> 	git cat-file commit HEAD | grep "Signed-off-by:" >actual &&
> 	test_must_be_empty actual
> '
> 
> The test fails because the second "grep" command returns a
> non-zero exit code. Any suggestions to avoid that problem in a
> more idiomatic way?

I just found out that "test_must_fail grep ..." is a common
idiom, so what about:

test_expect_success '--no-signoff overrides am.signoff' '
	rm -fr .git/rebase-apply &&
	git reset --hard first &&
	test_config am.signoff true &&
	git am --no-signoff <patch2 &&
	printf "%s\n" "$signoff" >expected &&
	git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
	test_cmp expected actual &&
	git cat-file commit HEAD | test_must_fail grep -q "Signed-off-by:"
'

-- 
Eduardo

^ permalink raw reply

* Re: [PATCH v2] am: add am.signoff add config variable
From: Stefan Beller @ 2016-12-28 19:24 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: git@vger.kernel.org, Paul Tan
In-Reply-To: <20161228191928.GH3441@thinpad.lan.raisama.net>

On Wed, Dec 28, 2016 at 11:19 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> On Wed, Dec 28, 2016 at 05:11:42PM -0200, Eduardo Habkost wrote:
>> On Wed, Dec 28, 2016 at 10:51:28AM -0800, Stefan Beller wrote:
>> > On Wed, Dec 28, 2016 at 10:35 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> [...]
>> > > +       test $(git cat-file commit HEAD | grep -c "Signed-off-by:") -eq 0
>> >
>> > and then we check if the top most commit has zero occurrences
>> > for lines grepped for sign off. That certainly works, but took me a
>> > while to understand (TIL about -c in grep :).
>> >
>> > Another way that to write this check, that Git regulars may be more used to is:
>> >
>> >     git cat-file commit HEAD | grep "Signed-off-by:" >actual
>> >     test_must_be_empty actual
>>
>> test_must_be_empty is what I was looking for. But if I do this:
>>
>> test_expect_success '--no-signoff overrides am.signoff' '
>>       rm -fr .git/rebase-apply &&
>>       git reset --hard first &&
>>       test_config am.signoff true &&
>>       git am --no-signoff <patch2 &&
>>       printf "%s\n" "$signoff" >expected &&
>>       git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
>>       test_cmp expected actual &&
>>       git cat-file commit HEAD | grep "Signed-off-by:" >actual &&
>>       test_must_be_empty actual
>> '
>>
>> The test fails because the second "grep" command returns a
>> non-zero exit code. Any suggestions to avoid that problem in a
>> more idiomatic way?
>
> I just found out that "test_must_fail grep ..." is a common
> idiom, so what about:

Uh, no please.

test_must_fail is supposed to be used for commands to be tested, i.e.
git commands as this is the git test suite. :)
test_must_fail checks, e.g. that the failing command "properly" fails
instead of bye-bye-segfault. And grep would never do this. (In this
world we assume everything to be perfect except git itself)

For grep just use !

    git cat-file commit HEAD >actual &&
    ! grep "Signed-off-by:" actual

$ git grep "test_must_fail grep" returns 20 occurrences, so in case you're
that would be a good cleanup patch (if you're interested in such things).

Thanks,
Stefan

^ permalink raw reply

* Re: [PATCH] contrib: remove gitview
From: Javantea @ 2016-12-28 19:28 UTC (permalink / raw)
  To: Stefan Beller; +Cc: peff, git
In-Reply-To: <20161228172837.24395-1-sbeller@google.com>

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

Dear Stefan,

Thank you for commenting on my report and getting the ball rolling on this. 
This response sounds good to me.

Regards,
Javantea

On Wed, 28 Dec 2016 09:28:37 -0800, Stefan Beller <sbeller@google.com> wrote:
>gitview did not have meaningful contributions since 2007, which gives the
>impression it is either a mature or dead project.
>
>In both cases we should not carry it in git.git as the README for contrib
>states we only want to carry experimental things to give early exposure.
>
>Recently a security vulnerability was reported by Javantea, so the decision
>to either fix the issue or remove the code in question becomes a bit
>more urgent.
>
>Reported-by: Javantea <jvoss@altsci.com>
>Signed-off-by: Stefan Beller <sbeller@google.com>
>---
> contrib/gitview/gitview     | 1305 -------------------------------------------
> contrib/gitview/gitview.txt |   57 --
> 2 files changed, 1362 deletions(-)
> delete mode 100755 contrib/gitview/gitview
> delete mode 100644 contrib/gitview/gitview.txt
>
>diff --git a/contrib/gitview/gitview b/contrib/gitview/gitview
>deleted file mode 100755
>index 4e23c650fe..0000000000
>--- a/contrib/gitview/gitview
>+++ /dev/null
>@@ -1,1305 +0,0 @@
>-#! /usr/bin/env python
>-
>-# This program is free software; you can redistribute it and/or modify
>-# it under the terms of the GNU General Public License as published by
>-# the Free Software Foundation; either version 2 of the License, or
>-# (at your option) any later version.
>-
>-""" gitview
>-GUI browser for git repository
>-This program is based on bzrk by Scott James Remnant <scott@ubuntu.com>
>-"""
>-__copyright__ = "Copyright (C) 2006 Hewlett-Packard Development Company, L.P."
>-__copyright__ = "Copyright (C) 2007 Aneesh Kumar K.V <aneesh.kumar@gmail.com"
>-__author__    = "Aneesh Kumar K.V <aneesh.kumar@gmail.com>"
>-
>-
>-import sys
>-import os
>-import gtk
>-import pygtk
>-import pango
>-import re
>-import time
>-import gobject
>-import cairo
>-import math
>-import string
>-import fcntl
>-
>-have_gtksourceview2 = False
>-have_gtksourceview = False
>-try:
>-    import gtksourceview2
>-    have_gtksourceview2 = True
>-except ImportError:
>-    try:
>-        import gtksourceview
>-        have_gtksourceview = True
>-    except ImportError:
>-        print "Running without gtksourceview2 or gtksourceview module"
>-
>-re_ident = re.compile('(author|committer) (?P<ident>.*) (?P<epoch>\d+) (?P<tz>[+-]\d{4})')
>-
>-def list_to_string(args, skip):
>-	count = len(args)
>-	i = skip
>-	str_arg=" "
>-	while (i < count ):
>-		str_arg = str_arg + args[i]
>-		str_arg = str_arg + " "
>-		i = i+1
>-
>-	return str_arg
>-
>-def show_date(epoch, tz):
>-	secs = float(epoch)
>-	tzsecs = float(tz[1:3]) * 3600
>-	tzsecs += float(tz[3:5]) * 60
>-	if (tz[0] == "+"):
>-		secs += tzsecs
>-	else:
>-		secs -= tzsecs
>-
>-	return time.strftime("%Y-%m-%d %H:%M:%S", time.gmtime(secs))
>-
>-def get_source_buffer_and_view():
>-	if have_gtksourceview2:
>-		buffer = gtksourceview2.Buffer()
>-		slm = gtksourceview2.LanguageManager()
>-		gsl = slm.get_language("diff")
>-		buffer.set_highlight_syntax(True)
>-		buffer.set_language(gsl)
>-		view = gtksourceview2.View(buffer)
>-	elif have_gtksourceview:
>-		buffer = gtksourceview.SourceBuffer()
>-		slm = gtksourceview.SourceLanguagesManager()
>-		gsl = slm.get_language_from_mime_type("text/x-patch")
>-		buffer.set_highlight(True)
>-		buffer.set_language(gsl)
>-		view = gtksourceview.SourceView(buffer)
>-	else:
>-		buffer = gtk.TextBuffer()
>-		view = gtk.TextView(buffer)
>-	return (buffer, view)
>-
>-
>-class CellRendererGraph(gtk.GenericCellRenderer):
>-	"""Cell renderer for directed graph.
>-
>-	This module contains the implementation of a custom GtkCellRenderer that
>-	draws part of the directed graph based on the lines suggested by the code
>-	in graph.py.
>-
>-	Because we're shiny, we use Cairo to do this, and because we're naughty
>-	we cheat and draw over the bits of the TreeViewColumn that are supposed to
>-	just be for the background.
>-
>-	Properties:
>-	node              (column, colour, [ names ]) tuple to draw revision node,
>-	in_lines          (start, end, colour) tuple list to draw inward lines,
>-	out_lines         (start, end, colour) tuple list to draw outward lines.
>-	"""
>-
>-	__gproperties__ = {
>-	"node":         ( gobject.TYPE_PYOBJECT, "node",
>-			  "revision node instruction",
>-			  gobject.PARAM_WRITABLE
>-			),
>-	"in-lines":     ( gobject.TYPE_PYOBJECT, "in-lines",
>-			  "instructions to draw lines into the cell",
>-			  gobject.PARAM_WRITABLE
>-			),
>-	"out-lines":    ( gobject.TYPE_PYOBJECT, "out-lines",
>-			  "instructions to draw lines out of the cell",
>-			  gobject.PARAM_WRITABLE
>-			),
>-	}
>-
>-	def do_set_property(self, property, value):
>-		"""Set properties from GObject properties."""
>-		if property.name == "node":
>-			self.node = value
>-		elif property.name == "in-lines":
>-			self.in_lines = value
>-		elif property.name == "out-lines":
>-			self.out_lines = value
>-		else:
>-			raise AttributeError, "no such property: '%s'" % property.name
>-
>-	def box_size(self, widget):
>-		"""Calculate box size based on widget's font.
>-
>-		Cache this as it's probably expensive to get.  It ensures that we
>-		draw the graph at least as large as the text.
>-		"""
>-		try:
>-			return self._box_size
>-		except AttributeError:
>-			pango_ctx = widget.get_pango_context()
>-			font_desc = widget.get_style().font_desc
>-			metrics = pango_ctx.get_metrics(font_desc)
>-
>-			ascent = pango.PIXELS(metrics.get_ascent())
>-			descent = pango.PIXELS(metrics.get_descent())
>-
>-			self._box_size = ascent + descent + 6
>-			return self._box_size
>-
>-	def set_colour(self, ctx, colour, bg, fg):
>-		"""Set the context source colour.
>-
>-		Picks a distinct colour based on an internal wheel; the bg
>-		parameter provides the value that should be assigned to the 'zero'
>-		colours and the fg parameter provides the multiplier that should be
>-		applied to the foreground colours.
>-		"""
>-		colours = [
>-		    ( 1.0, 0.0, 0.0 ),
>-		    ( 1.0, 1.0, 0.0 ),
>-		    ( 0.0, 1.0, 0.0 ),
>-		    ( 0.0, 1.0, 1.0 ),
>-		    ( 0.0, 0.0, 1.0 ),
>-		    ( 1.0, 0.0, 1.0 ),
>-		    ]
>-
>-		colour %= len(colours)
>-		red   = (colours[colour][0] * fg) or bg
>-		green = (colours[colour][1] * fg) or bg
>-		blue  = (colours[colour][2] * fg) or bg
>-
>-		ctx.set_source_rgb(red, green, blue)
>-
>-	def on_get_size(self, widget, cell_area):
>-		"""Return the size we need for this cell.
>-
>-		Each cell is drawn individually and is only as wide as it needs
>-		to be, we let the TreeViewColumn take care of making them all
>-		line up.
>-		"""
>-		box_size = self.box_size(widget)
>-
>-		cols = self.node[0]
>-		for start, end, colour in self.in_lines + self.out_lines:
>-			cols = int(max(cols, start, end))
>-
>-		(column, colour, names) = self.node
>-		names_len = 0
>-		if (len(names) != 0):
>-			for item in names:
>-				names_len += len(item)
>-
>-		width = box_size * (cols + 1 ) + names_len
>-		height = box_size
>-
>-		# FIXME I have no idea how to use cell_area properly
>-		return (0, 0, width, height)
>-
>-	def on_render(self, window, widget, bg_area, cell_area, exp_area, flags):
>-		"""Render an individual cell.
>-
>-		Draws the cell contents using cairo, taking care to clip what we
>-		do to within the background area so we don't draw over other cells.
>-		Note that we're a bit naughty there and should really be drawing
>-		in the cell_area (or even the exposed area), but we explicitly don't
>-		want any gutter.
>-
>-		We try and be a little clever, if the line we need to draw is going
>-		to cross other columns we actually draw it as in the .---' style
>-		instead of a pure diagonal ... this reduces confusion by an
>-		incredible amount.
>-		"""
>-		ctx = window.cairo_create()
>-		ctx.rectangle(bg_area.x, bg_area.y, bg_area.width, bg_area.height)
>-		ctx.clip()
>-
>-		box_size = self.box_size(widget)
>-
>-		ctx.set_line_width(box_size / 8)
>-		ctx.set_line_cap(cairo.LINE_CAP_SQUARE)
>-
>-		# Draw lines into the cell
>-		for start, end, colour in self.in_lines:
>-			ctx.move_to(cell_area.x + box_size * start + box_size / 2,
>-					bg_area.y - bg_area.height / 2)
>-
>-			if start - end > 1:
>-				ctx.line_to(cell_area.x + box_size * start, bg_area.y)
>-				ctx.line_to(cell_area.x + box_size * end + box_size, bg_area.y)
>-			elif start - end < -1:
>-				ctx.line_to(cell_area.x + box_size * start + box_size,
>-						bg_area.y)
>-				ctx.line_to(cell_area.x + box_size * end, bg_area.y)
>-
>-			ctx.line_to(cell_area.x + box_size * end + box_size / 2,
>-					bg_area.y + bg_area.height / 2)
>-
>-			self.set_colour(ctx, colour, 0.0, 0.65)
>-			ctx.stroke()
>-
>-		# Draw lines out of the cell
>-		for start, end, colour in self.out_lines:
>-			ctx.move_to(cell_area.x + box_size * start + box_size / 2,
>-					bg_area.y + bg_area.height / 2)
>-
>-			if start - end > 1:
>-				ctx.line_to(cell_area.x + box_size * start,
>-						bg_area.y + bg_area.height)
>-				ctx.line_to(cell_area.x + box_size * end + box_size,
>-						bg_area.y + bg_area.height)
>-			elif start - end < -1:
>-				ctx.line_to(cell_area.x + box_size * start + box_size,
>-						bg_area.y + bg_area.height)
>-				ctx.line_to(cell_area.x + box_size * end,
>-						bg_area.y + bg_area.height)
>-
>-			ctx.line_to(cell_area.x + box_size * end + box_size / 2,
>-					bg_area.y + bg_area.height / 2 + bg_area.height)
>-
>-			self.set_colour(ctx, colour, 0.0, 0.65)
>-			ctx.stroke()
>-
>-		# Draw the revision node in the right column
>-		(column, colour, names) = self.node
>-		ctx.arc(cell_area.x + box_size * column + box_size / 2,
>-				cell_area.y + cell_area.height / 2,
>-				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.set_font_size(13)
>-			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(object):
>-	""" This represent a commit object obtained after parsing the git-rev-list
>-	output """
>-
>-	__slots__ = ['children_sha1', 'message', 'author', 'date', 'committer',
>-				 'commit_date', 'commit_sha1', 'parent_sha1']
>-
>-	children_sha1 = {}
>-
>-	def __init__(self, commit_lines):
>-		self.message		= ""
>-		self.author		= ""
>-		self.date		= ""
>-		self.committer		= ""
>-		self.commit_date	= ""
>-		self.commit_sha1	= ""
>-		self.parent_sha1	= [ ]
>-		self.parse_commit(commit_lines)
>-
>-
>-	def parse_commit(self, commit_lines):
>-
>-		# First line is the sha1 lines
>-		line = string.strip(commit_lines[0])
>-		sha1 = re.split(" ", line)
>-		self.commit_sha1 = sha1[0]
>-		self.parent_sha1 = sha1[1:]
>-
>-		#build the child list
>-		for parent_id in self.parent_sha1:
>-			try:
>-				Commit.children_sha1[parent_id].append(self.commit_sha1)
>-			except KeyError:
>-				Commit.children_sha1[parent_id] = [self.commit_sha1]
>-
>-		# IF we don't have parent
>-		if (len(self.parent_sha1) == 0):
>-			self.parent_sha1 = [0]
>-
>-		for line in commit_lines[1:]:
>-			m = re.match("^ ", line)
>-			if (m != None):
>-				# First line of the commit message used for short log
>-				if self.message == "":
>-					self.message = string.strip(line)
>-				continue
>-
>-			m = re.match("tree", line)
>-			if (m != None):
>-				continue
>-
>-			m = re.match("parent", line)
>-			if (m != None):
>-				continue
>-
>-			m = re_ident.match(line)
>-			if (m != None):
>-				date = show_date(m.group('epoch'), m.group('tz'))
>-				if m.group(1) == "author":
>-					self.author = m.group('ident')
>-					self.date = date
>-				elif m.group(1) == "committer":
>-					self.committer = m.group('ident')
>-					self.commit_date = date
>-
>-				continue
>-
>-	def get_message(self, with_diff=0):
>-		if (with_diff == 1):
>-			message = self.diff_tree()
>-		else:
>-			fp = os.popen("git cat-file commit " + self.commit_sha1)
>-			message = fp.read()
>-			fp.close()
>-
>-		return message
>-
>-	def diff_tree(self):
>-		fp = os.popen("git diff-tree --pretty --cc  -v -p --always " +  self.commit_sha1)
>-		diff = fp.read()
>-		fp.close()
>-		return diff
>-
>-class AnnotateWindow(object):
>-	"""Annotate window.
>-	This object represents and manages a single window containing the
>-	annotate information of the file
>-	"""
>-
>-	def __init__(self):
>-		self.window = gtk.Window(gtk.WINDOW_TOPLEVEL)
>-		self.window.set_border_width(0)
>-		self.window.set_title("Git repository browser annotation window")
>-		self.prev_read = ""
>-
>-		# Use two thirds of the screen by default
>-		screen = self.window.get_screen()
>-		monitor = screen.get_monitor_geometry(0)
>-		width = int(monitor.width * 0.66)
>-		height = int(monitor.height * 0.66)
>-		self.window.set_default_size(width, height)
>-
>-	def add_file_data(self, filename, commit_sha1, line_num):
>-		fp = os.popen("git cat-file blob " + commit_sha1 +":"+filename)
>-		i = 1;
>-		for line in fp.readlines():
>-			line = string.rstrip(line)
>-			self.model.append(None, ["HEAD", filename, line, i])
>-			i = i+1
>-		fp.close()
>-
>-		# now set the cursor position
>-		self.treeview.set_cursor(line_num-1)
>-		self.treeview.grab_focus()
>-
>-	def _treeview_cursor_cb(self, *args):
>-		"""Callback for when the treeview cursor changes."""
>-		(path, col) = self.treeview.get_cursor()
>-		commit_sha1 = self.model[path][0]
>-		commit_msg = ""
>-		fp = os.popen("git cat-file commit " + commit_sha1)
>-		for line in fp.readlines():
>-			commit_msg =  commit_msg + line
>-		fp.close()
>-
>-		self.commit_buffer.set_text(commit_msg)
>-
>-	def _treeview_row_activated(self, *args):
>-		"""Callback for when the treeview row gets selected."""
>-		(path, col) = self.treeview.get_cursor()
>-		commit_sha1 = self.model[path][0]
>-		filename    = self.model[path][1]
>-		line_num    = self.model[path][3]
>-
>-		window = AnnotateWindow();
>-		fp = os.popen("git rev-parse "+ commit_sha1 + "~1")
>-		commit_sha1 = string.strip(fp.readline())
>-		fp.close()
>-		window.annotate(filename, commit_sha1, line_num)
>-
>-	def data_ready(self, source, condition):
>-		while (1):
>-			try :
>-				# A simple readline doesn't work
>-				# a readline bug ??
>-				buffer = source.read(100)
>-
>-			except:
>-				# resource temporary not available
>-				return True
>-
>-			if (len(buffer) == 0):
>-				gobject.source_remove(self.io_watch_tag)
>-				source.close()
>-				return False
>-
>-			if (self.prev_read != ""):
>-				buffer = self.prev_read + buffer
>-				self.prev_read = ""
>-
>-			if (buffer[len(buffer) -1] != '\n'):
>-				try:
>-					newline_index = buffer.rindex("\n")
>-				except ValueError:
>-					newline_index = 0
>-
>-				self.prev_read = buffer[newline_index:(len(buffer))]
>-				buffer = buffer[0:newline_index]
>-
>-			for buff in buffer.split("\n"):
>-				annotate_line = re.compile('^([0-9a-f]{40}) (.+) (.+) (.+)$')
>-				m = annotate_line.match(buff)
>-				if not m:
>-					annotate_line = re.compile('^(filename) (.+)$')
>-					m = annotate_line.match(buff)
>-					if not m:
>-						continue
>-					filename = m.group(2)
>-				else:
>-					self.commit_sha1 = m.group(1)
>-					self.source_line = int(m.group(2))
>-					self.result_line = int(m.group(3))
>-					self.count	    = int(m.group(4))
>-					#set the details only when we have the file name
>-					continue
>-
>-				while (self.count > 0):
>-					# set at result_line + count-1 the sha1 as commit_sha1
>-					self.count = self.count - 1
>-					iter = self.model.iter_nth_child(None, self.result_line + self.count-1)
>-					self.model.set(iter, 0, self.commit_sha1, 1, filename, 3, self.source_line)
>-
>-
>-	def annotate(self, filename, commit_sha1, line_num):
>-		# verify the commit_sha1 specified has this filename
>-
>-		fp = os.popen("git ls-tree "+ commit_sha1 + " -- " + filename)
>-		line = string.strip(fp.readline())
>-		if line == '':
>-			# pop up the message the file is not there as a part of the commit
>-			fp.close()
>-			dialog = gtk.MessageDialog(parent=None, flags=0,
>-					type=gtk.MESSAGE_WARNING, buttons=gtk.BUTTONS_CLOSE,
>-					message_format=None)
>-			dialog.set_markup("The file %s is not present in the parent commit %s" % (filename, commit_sha1))
>-			dialog.run()
>-			dialog.destroy()
>-			return
>-
>-		fp.close()
>-
>-		vpan = gtk.VPaned();
>-		self.window.add(vpan);
>-		vpan.show()
>-
>-		scrollwin = gtk.ScrolledWindow()
>-		scrollwin.set_policy(gtk.POLICY_AUTOMATIC, gtk.POLICY_AUTOMATIC)
>-		scrollwin.set_shadow_type(gtk.SHADOW_IN)
>-		vpan.pack1(scrollwin, True, True);
>-		scrollwin.show()
>-
>-		self.model = gtk.TreeStore(str, str, str, int)
>-		self.treeview = gtk.TreeView(self.model)
>-		self.treeview.set_rules_hint(True)
>-		self.treeview.set_search_column(0)
>-		self.treeview.connect("cursor-changed", self._treeview_cursor_cb)
>-		self.treeview.connect("row-activated", self._treeview_row_activated)
>-		scrollwin.add(self.treeview)
>-		self.treeview.show()
>-
>-		cell = gtk.CellRendererText()
>-		cell.set_property("width-chars", 10)
>-		cell.set_property("ellipsize", pango.ELLIPSIZE_END)
>-		column = gtk.TreeViewColumn("Commit")
>-		column.set_resizable(True)
>-		column.pack_start(cell, expand=True)
>-		column.add_attribute(cell, "text", 0)
>-		self.treeview.append_column(column)
>-
>-		cell = gtk.CellRendererText()
>-		cell.set_property("width-chars", 20)
>-		cell.set_property("ellipsize", pango.ELLIPSIZE_END)
>-		column = gtk.TreeViewColumn("File Name")
>-		column.set_resizable(True)
>-		column.pack_start(cell, expand=True)
>-		column.add_attribute(cell, "text", 1)
>-		self.treeview.append_column(column)
>-
>-		cell = gtk.CellRendererText()
>-		cell.set_property("width-chars", 20)
>-		cell.set_property("ellipsize", pango.ELLIPSIZE_END)
>-		column = gtk.TreeViewColumn("Data")
>-		column.set_resizable(True)
>-		column.pack_start(cell, expand=True)
>-		column.add_attribute(cell, "text", 2)
>-		self.treeview.append_column(column)
>-
>-		# The commit message window
>-		scrollwin = gtk.ScrolledWindow()
>-		scrollwin.set_policy(gtk.POLICY_AUTOMATIC, gtk.POLICY_AUTOMATIC)
>-		scrollwin.set_shadow_type(gtk.SHADOW_IN)
>-		vpan.pack2(scrollwin, True, True);
>-		scrollwin.show()
>-
>-		commit_text = gtk.TextView()
>-		self.commit_buffer = gtk.TextBuffer()
>-		commit_text.set_buffer(self.commit_buffer)
>-		scrollwin.add(commit_text)
>-		commit_text.show()
>-
>-		self.window.show()
>-
>-		self.add_file_data(filename, commit_sha1, line_num)
>-
>-		fp = os.popen("git blame --incremental -C -C -- " + filename + " " + commit_sha1)
>-		flags = fcntl.fcntl(fp.fileno(), fcntl.F_GETFL)
>-		fcntl.fcntl(fp.fileno(), fcntl.F_SETFL, flags | os.O_NONBLOCK)
>-		self.io_watch_tag = gobject.io_add_watch(fp, gobject.IO_IN, self.data_ready)
>-
>-
>-class DiffWindow(object):
>-	"""Diff window.
>-	This object represents and manages a single window containing the
>-	differences between two revisions on a branch.
>-	"""
>-
>-	def __init__(self):
>-		self.window = gtk.Window(gtk.WINDOW_TOPLEVEL)
>-		self.window.set_border_width(0)
>-		self.window.set_title("Git repository browser diff window")
>-
>-		# Use two thirds of the screen by default
>-		screen = self.window.get_screen()
>-		monitor = screen.get_monitor_geometry(0)
>-		width = int(monitor.width * 0.66)
>-		height = int(monitor.height * 0.66)
>-		self.window.set_default_size(width, height)
>-
>-
>-		self.construct()
>-
>-	def construct(self):
>-		"""Construct the window contents."""
>-		vbox = gtk.VBox()
>-		self.window.add(vbox)
>-		vbox.show()
>-
>-		menu_bar = gtk.MenuBar()
>-		save_menu = gtk.ImageMenuItem(gtk.STOCK_SAVE)
>-		save_menu.connect("activate", self.save_menu_response, "save")
>-		save_menu.show()
>-		menu_bar.append(save_menu)
>-		vbox.pack_start(menu_bar, expand=False, fill=True)
>-		menu_bar.show()
>-
>-		hpan = gtk.HPaned()
>-
>-		scrollwin = gtk.ScrolledWindow()
>-		scrollwin.set_policy(gtk.POLICY_AUTOMATIC, gtk.POLICY_AUTOMATIC)
>-		scrollwin.set_shadow_type(gtk.SHADOW_IN)
>-		hpan.pack1(scrollwin, True, True)
>-		scrollwin.show()
>-
>-		(self.buffer, sourceview) = get_source_buffer_and_view()
>-
>-		sourceview.set_editable(False)
>-		sourceview.modify_font(pango.FontDescription("Monospace"))
>-		scrollwin.add(sourceview)
>-		sourceview.show()
>-
>-		# The file hierarchy: a scrollable treeview
>-		scrollwin = gtk.ScrolledWindow()
>-		scrollwin.set_policy(gtk.POLICY_AUTOMATIC, gtk.POLICY_AUTOMATIC)
>-		scrollwin.set_shadow_type(gtk.SHADOW_IN)
>-		scrollwin.set_size_request(20, -1)
>-		hpan.pack2(scrollwin, True, True)
>-		scrollwin.show()
>-
>-		self.model = gtk.TreeStore(str, str, str)
>-		self.treeview = gtk.TreeView(self.model)
>-		self.treeview.set_search_column(1)
>-		self.treeview.connect("cursor-changed", self._treeview_clicked)
>-		scrollwin.add(self.treeview)
>-		self.treeview.show()
>-
>-		cell = gtk.CellRendererText()
>-		cell.set_property("width-chars", 20)
>-		column = gtk.TreeViewColumn("Select to annotate")
>-		column.pack_start(cell, expand=True)
>-		column.add_attribute(cell, "text", 0)
>-		self.treeview.append_column(column)
>-
>-		vbox.pack_start(hpan, expand=True, fill=True)
>-		hpan.show()
>-
>-	def _treeview_clicked(self, *args):
>-		"""Callback for when the treeview cursor changes."""
>-		(path, col) = self.treeview.get_cursor()
>-		specific_file = self.model[path][1]
>-		commit_sha1 =  self.model[path][2]
>-		if specific_file ==  None :
>-			return
>-		elif specific_file ==  "" :
>-			specific_file =  None
>-
>-		window = AnnotateWindow();
>-		window.annotate(specific_file, commit_sha1, 1)
>-
>-
>-	def commit_files(self, commit_sha1, parent_sha1):
>-		self.model.clear()
>-		add  = self.model.append(None, [ "Added", None, None])
>-		dele = self.model.append(None, [ "Deleted", None, None])
>-		mod  = self.model.append(None, [ "Modified", None, None])
>-		diff_tree = re.compile('^(:.{6}) (.{6}) (.{40}) (.{40}) (A|D|M)\s(.+)$')
>-		fp = os.popen("git diff-tree -r --no-commit-id " + parent_sha1 + " " + commit_sha1)
>-		while 1:
>-			line = string.strip(fp.readline())
>-			if line == '':
>-				break
>-			m = diff_tree.match(line)
>-			if not m:
>-				continue
>-
>-			attr = m.group(5)
>-			filename = m.group(6)
>-			if attr == "A":
>-				self.model.append(add,  [filename, filename, commit_sha1])
>-			elif attr == "D":
>-				self.model.append(dele, [filename, filename, commit_sha1])
>-			elif attr == "M":
>-				self.model.append(mod,  [filename, filename, commit_sha1])
>-		fp.close()
>-
>-		self.treeview.expand_all()
>-
>-	def set_diff(self, commit_sha1, parent_sha1, encoding):
>-		"""Set the differences showed by this window.
>-		Compares the two trees and populates the window with the
>-		differences.
>-		"""
>-		# Diff with the first commit or the last commit shows nothing
>-		if (commit_sha1 == 0 or parent_sha1 == 0 ):
>-			return
>-
>-		fp = os.popen("git diff-tree -p " + parent_sha1 + " " + commit_sha1)
>-		self.buffer.set_text(unicode(fp.read(), encoding).encode('utf-8'))
>-		fp.close()
>-		self.commit_files(commit_sha1, parent_sha1)
>-		self.window.show()
>-
>-	def save_menu_response(self, widget, string):
>-		dialog = gtk.FileChooserDialog("Save..", None, gtk.FILE_CHOOSER_ACTION_SAVE,
>-				(gtk.STOCK_CANCEL, gtk.RESPONSE_CANCEL,
>-					gtk.STOCK_SAVE, gtk.RESPONSE_OK))
>-		dialog.set_default_response(gtk.RESPONSE_OK)
>-		response = dialog.run()
>-		if response == gtk.RESPONSE_OK:
>-			patch_buffer = self.buffer.get_text(self.buffer.get_start_iter(),
>-					self.buffer.get_end_iter())
>-			fp = open(dialog.get_filename(), "w")
>-			fp.write(patch_buffer)
>-			fp.close()
>-		dialog.destroy()
>-
>-class GitView(object):
>-	""" This is the main class
>-	"""
>-	version = "0.9"
>-
>-	def __init__(self, with_diff=0):
>-		self.with_diff = with_diff
>-		self.window =	gtk.Window(gtk.WINDOW_TOPLEVEL)
>-		self.window.set_border_width(0)
>-		self.window.set_title("Git repository browser")
>-
>-		self.get_encoding()
>-		self.get_bt_sha1()
>-
>-		# Use three-quarters of the screen by default
>-		screen = self.window.get_screen()
>-		monitor = screen.get_monitor_geometry(0)
>-		width = int(monitor.width * 0.75)
>-		height = int(monitor.height * 0.75)
>-		self.window.set_default_size(width, height)
>-
>-		# FIXME AndyFitz!
>-		icon = self.window.render_icon(gtk.STOCK_INDEX, gtk.ICON_SIZE_BUTTON)
>-		self.window.set_icon(icon)
>-
>-		self.accel_group = gtk.AccelGroup()
>-		self.window.add_accel_group(self.accel_group)
>-		self.accel_group.connect_group(0xffc2, 0, gtk.ACCEL_LOCKED, self.refresh);
>-		self.accel_group.connect_group(0xffc1, 0, gtk.ACCEL_LOCKED, self.maximize);
>-		self.accel_group.connect_group(0xffc8, 0, gtk.ACCEL_LOCKED, self.fullscreen);
>-		self.accel_group.connect_group(0xffc9, 0, gtk.ACCEL_LOCKED, self.unfullscreen);
>-
>-		self.window.add(self.construct())
>-
>-	def refresh(self, widget, event=None, *arguments, **keywords):
>-		self.get_encoding()
>-		self.get_bt_sha1()
>-		Commit.children_sha1 = {}
>-		self.set_branch(sys.argv[without_diff:])
>-		self.window.show()
>-		return True
>-
>-	def maximize(self, widget, event=None, *arguments, **keywords):
>-		self.window.maximize()
>-		return True
>-
>-	def fullscreen(self, widget, event=None, *arguments, **keywords):
>-		self.window.fullscreen()
>-		return True
>-
>-	def unfullscreen(self, widget, event=None, *arguments, **keywords):
>-		self.window.unfullscreen()
>-		return True
>-
>-	def get_bt_sha1(self):
>-		""" Update the bt_sha1 dictionary with the
>-		respective sha1 details """
>-
>-		self.bt_sha1 = { }
>-		ls_remote = re.compile('^(.{40})\trefs/([^^]+)(?:\\^(..))?$');
>-		fp = os.popen('git ls-remote "${GIT_DIR-.git}"')
>-		while 1:
>-			line = string.strip(fp.readline())
>-			if line == '':
>-				break
>-			m = ls_remote.match(line)
>-			if not m:
>-				continue
>-			(sha1, name) = (m.group(1), m.group(2))
>-			if not self.bt_sha1.has_key(sha1):
>-				self.bt_sha1[sha1] = []
>-			self.bt_sha1[sha1].append(name)
>-		fp.close()
>-
>-	def get_encoding(self):
>-		fp = os.popen("git config --get i18n.commitencoding")
>-		self.encoding=string.strip(fp.readline())
>-		fp.close()
>-		if (self.encoding == ""):
>-			self.encoding = "utf-8"
>-
>-
>-	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)
>-		menu_bar = gtk.MenuBar()
>-		menu_bar.set_pack_direction(gtk.PACK_DIRECTION_RTL)
>-		help_menu = gtk.MenuItem("Help")
>-		menu = gtk.Menu()
>-		about_menu = gtk.MenuItem("About")
>-		menu.append(about_menu)
>-		about_menu.connect("activate", self.about_menu_response, "about")
>-		about_menu.show()
>-		help_menu.set_submenu(menu)
>-		help_menu.show()
>-		menu_bar.append(help_menu)
>-		menu_bar.show()
>-		vbox.pack_start(menu_bar, expand=False, fill=True)
>-		vbox.pack_start(paned, expand=True, fill=True)
>-		paned.show()
>-		vbox.show()
>-		return vbox
>-
>-
>-	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_AUTOMATIC, gtk.POLICY_AUTOMATIC)
>-		scrollwin.set_shadow_type(gtk.SHADOW_IN)
>-		vbox.pack_start(scrollwin, expand=True, fill=True)
>-		scrollwin.show()
>-
>-		self.treeview = gtk.TreeView()
>-		self.treeview.set_rules_hint(True)
>-		self.treeview.set_search_column(4)
>-		self.treeview.connect("cursor-changed", self._treeview_cursor_cb)
>-		scrollwin.add(self.treeview)
>-		self.treeview.show()
>-
>-		cell = CellRendererGraph()
>-		column = gtk.TreeViewColumn()
>-		column.set_resizable(True)
>-		column.pack_start(cell, expand=True)
>-		column.add_attribute(cell, "node", 1)
>-		column.add_attribute(cell, "in-lines", 2)
>-		column.add_attribute(cell, "out-lines", 3)
>-		self.treeview.append_column(column)
>-
>-		cell = gtk.CellRendererText()
>-		cell.set_property("width-chars", 65)
>-		cell.set_property("ellipsize", pango.ELLIPSIZE_END)
>-		column = gtk.TreeViewColumn("Message")
>-		column.set_resizable(True)
>-		column.pack_start(cell, expand=True)
>-		column.add_attribute(cell, "text", 4)
>-		self.treeview.append_column(column)
>-
>-		cell = gtk.CellRendererText()
>-		cell.set_property("width-chars", 40)
>-		cell.set_property("ellipsize", pango.ELLIPSIZE_END)
>-		column = gtk.TreeViewColumn("Author")
>-		column.set_resizable(True)
>-		column.pack_start(cell, expand=True)
>-		column.add_attribute(cell, "text", 5)
>-		self.treeview.append_column(column)
>-
>-		cell = gtk.CellRendererText()
>-		cell.set_property("ellipsize", pango.ELLIPSIZE_END)
>-		column = gtk.TreeViewColumn("Date")
>-		column.set_resizable(True)
>-		column.pack_start(cell, expand=True)
>-		column.add_attribute(cell, "text", 6)
>-		self.treeview.append_column(column)
>-
>-		return vbox
>-
>-	def about_menu_response(self, widget, string):
>-		dialog = gtk.AboutDialog()
>-		dialog.set_name("Gitview")
>-		dialog.set_version(GitView.version)
>-		dialog.set_authors(["Aneesh Kumar K.V <aneesh.kumar@gmail.com>"])
>-		dialog.set_website("http://www.kernel.org/pub/software/scm/git/")
>-		dialog.set_copyright("Use and distribute under the terms of the GNU General Public License")
>-		dialog.set_wrap_license(True)
>-		dialog.run()
>-		dialog.destroy()
>-
>-
>-	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()
>-
>-		self.table = gtk.Table(rows=4, columns=4)
>-		self.table.set_row_spacings(6)
>-		self.table.set_col_spacings(6)
>-		vbox.pack_start(self.table, expand=False, fill=True)
>-		self.table.show()
>-
>-		align = gtk.Alignment(0.0, 0.5)
>-		label = gtk.Label()
>-		label.set_markup("<b>Revision:</b>")
>-		align.add(label)
>-		self.table.attach(align, 0, 1, 0, 1, gtk.FILL, gtk.FILL)
>-		label.show()
>-		align.show()
>-
>-		align = gtk.Alignment(0.0, 0.5)
>-		self.revid_label = gtk.Label()
>-		self.revid_label.set_selectable(True)
>-		align.add(self.revid_label)
>-		self.table.attach(align, 1, 2, 0, 1, gtk.EXPAND | gtk.FILL, gtk.FILL)
>-		self.revid_label.show()
>-		align.show()
>-
>-		align = gtk.Alignment(0.0, 0.5)
>-		label = gtk.Label()
>-		label.set_markup("<b>Committer:</b>")
>-		align.add(label)
>-		self.table.attach(align, 0, 1, 1, 2, gtk.FILL, gtk.FILL)
>-		label.show()
>-		align.show()
>-
>-		align = gtk.Alignment(0.0, 0.5)
>-		self.committer_label = gtk.Label()
>-		self.committer_label.set_selectable(True)
>-		align.add(self.committer_label)
>-		self.table.attach(align, 1, 2, 1, 2, gtk.EXPAND | gtk.FILL, gtk.FILL)
>-		self.committer_label.show()
>-		align.show()
>-
>-		align = gtk.Alignment(0.0, 0.5)
>-		label = gtk.Label()
>-		label.set_markup("<b>Timestamp:</b>")
>-		align.add(label)
>-		self.table.attach(align, 0, 1, 2, 3, gtk.FILL, gtk.FILL)
>-		label.show()
>-		align.show()
>-
>-		align = gtk.Alignment(0.0, 0.5)
>-		self.timestamp_label = gtk.Label()
>-		self.timestamp_label.set_selectable(True)
>-		align.add(self.timestamp_label)
>-		self.table.attach(align, 1, 2, 2, 3, gtk.EXPAND | gtk.FILL, gtk.FILL)
>-		self.timestamp_label.show()
>-		align.show()
>-
>-		align = gtk.Alignment(0.0, 0.5)
>-		label = gtk.Label()
>-		label.set_markup("<b>Parents:</b>")
>-		align.add(label)
>-		self.table.attach(align, 0, 1, 3, 4, gtk.FILL, gtk.FILL)
>-		label.show()
>-		align.show()
>-		self.parents_widgets = []
>-
>-		align = gtk.Alignment(0.0, 0.5)
>-		label = gtk.Label()
>-		label.set_markup("<b>Children:</b>")
>-		align.add(label)
>-		self.table.attach(align, 2, 3, 3, 4, gtk.FILL, gtk.FILL)
>-		label.show()
>-		align.show()
>-		self.children_widgets = []
>-
>-		scrollwin = gtk.ScrolledWindow()
>-		scrollwin.set_policy(gtk.POLICY_AUTOMATIC, gtk.POLICY_AUTOMATIC)
>-		scrollwin.set_shadow_type(gtk.SHADOW_IN)
>-		vbox.pack_start(scrollwin, expand=True, fill=True)
>-		scrollwin.show()
>-
>-		(self.message_buffer, sourceview) = get_source_buffer_and_view()
>-
>-		sourceview.set_editable(False)
>-		sourceview.modify_font(pango.FontDescription("Monospace"))
>-		scrollwin.add(sourceview)
>-		sourceview.show()
>-
>-		return vbox
>-
>-	def _treeview_cursor_cb(self, *args):
>-		"""Callback for when the treeview cursor changes."""
>-		(path, col) = self.treeview.get_cursor()
>-		commit = self.model[path][0]
>-
>-		if commit.committer is not None:
>-			committer = commit.committer
>-			timestamp = commit.commit_date
>-			message   =  commit.get_message(self.with_diff)
>-			revid_label = commit.commit_sha1
>-		else:
>-			committer = ""
>-			timestamp = ""
>-			message = ""
>-			revid_label = ""
>-
>-		self.revid_label.set_text(revid_label)
>-		self.committer_label.set_text(committer)
>-		self.timestamp_label.set_text(timestamp)
>-		self.message_buffer.set_text(unicode(message, self.encoding).encode('utf-8'))
>-
>-		for widget in self.parents_widgets:
>-			self.table.remove(widget)
>-
>-		self.parents_widgets = []
>-		self.table.resize(4 + len(commit.parent_sha1) - 1, 4)
>-		for idx, parent_id in enumerate(commit.parent_sha1):
>-			self.table.set_row_spacing(idx + 3, 0)
>-
>-			align = gtk.Alignment(0.0, 0.0)
>-			self.parents_widgets.append(align)
>-			self.table.attach(align, 1, 2, idx + 3, idx + 4,
>-					gtk.EXPAND | gtk.FILL, gtk.FILL)
>-			align.show()
>-
>-			hbox = gtk.HBox(False, 0)
>-			align.add(hbox)
>-			hbox.show()
>-
>-			label = gtk.Label(parent_id)
>-			label.set_selectable(True)
>-			hbox.pack_start(label, expand=False, fill=True)
>-			label.show()
>-
>-			image = gtk.Image()
>-			image.set_from_stock(gtk.STOCK_JUMP_TO, gtk.ICON_SIZE_MENU)
>-			image.show()
>-
>-			button = gtk.Button()
>-			button.add(image)
>-			button.set_relief(gtk.RELIEF_NONE)
>-			button.connect("clicked", self._go_clicked_cb, parent_id)
>-			hbox.pack_start(button, expand=False, fill=True)
>-			button.show()
>-
>-			image = gtk.Image()
>-			image.set_from_stock(gtk.STOCK_FIND, gtk.ICON_SIZE_MENU)
>-			image.show()
>-
>-			button = gtk.Button()
>-			button.add(image)
>-			button.set_relief(gtk.RELIEF_NONE)
>-			button.set_sensitive(True)
>-			button.connect("clicked", self._show_clicked_cb,
>-					commit.commit_sha1, parent_id, self.encoding)
>-			hbox.pack_start(button, expand=False, fill=True)
>-			button.show()
>-
>-		# Populate with child details
>-		for widget in self.children_widgets:
>-			self.table.remove(widget)
>-
>-		self.children_widgets = []
>-		try:
>-			child_sha1 = Commit.children_sha1[commit.commit_sha1]
>-		except KeyError:
>-			# We don't have child
>-			child_sha1 = [ 0 ]
>-
>-		if ( len(child_sha1) > len(commit.parent_sha1)):
>-			self.table.resize(4 + len(child_sha1) - 1, 4)
>-
>-		for idx, child_id in enumerate(child_sha1):
>-			self.table.set_row_spacing(idx + 3, 0)
>-
>-			align = gtk.Alignment(0.0, 0.0)
>-			self.children_widgets.append(align)
>-			self.table.attach(align, 3, 4, idx + 3, idx + 4,
>-					gtk.EXPAND | gtk.FILL, gtk.FILL)
>-			align.show()
>-
>-			hbox = gtk.HBox(False, 0)
>-			align.add(hbox)
>-			hbox.show()
>-
>-			label = gtk.Label(child_id)
>-			label.set_selectable(True)
>-			hbox.pack_start(label, expand=False, fill=True)
>-			label.show()
>-
>-			image = gtk.Image()
>-			image.set_from_stock(gtk.STOCK_JUMP_TO, gtk.ICON_SIZE_MENU)
>-			image.show()
>-
>-			button = gtk.Button()
>-			button.add(image)
>-			button.set_relief(gtk.RELIEF_NONE)
>-			button.connect("clicked", self._go_clicked_cb, child_id)
>-			hbox.pack_start(button, expand=False, fill=True)
>-			button.show()
>-
>-			image = gtk.Image()
>-			image.set_from_stock(gtk.STOCK_FIND, gtk.ICON_SIZE_MENU)
>-			image.show()
>-
>-			button = gtk.Button()
>-			button.add(image)
>-			button.set_relief(gtk.RELIEF_NONE)
>-			button.set_sensitive(True)
>-			button.connect("clicked", self._show_clicked_cb,
>-					child_id, commit.commit_sha1, self.encoding)
>-			hbox.pack_start(button, expand=False, fill=True)
>-			button.show()
>-
>-	def _destroy_cb(self, widget):
>-		"""Callback for when a window we manage is destroyed."""
>-		self.quit()
>-
>-
>-	def quit(self):
>-		"""Stop the GTK+ main loop."""
>-		gtk.main_quit()
>-
>-	def run(self, args):
>-		self.set_branch(args)
>-		self.window.connect("destroy", self._destroy_cb)
>-		self.window.show()
>-		gtk.main()
>-
>-	def set_branch(self, args):
>-		"""Fill in different windows with info from the reposiroty"""
>-		fp = os.popen("git rev-parse --sq --default HEAD " + list_to_string(args, 1))
>-		git_rev_list_cmd = fp.read()
>-		fp.close()
>-		fp = os.popen("git rev-list  --header --topo-order --parents " + git_rev_list_cmd)
>-		self.update_window(fp)
>-
>-	def update_window(self, fp):
>-		commit_lines = []
>-
>-		self.model = gtk.ListStore(gobject.TYPE_PYOBJECT, gobject.TYPE_PYOBJECT,
>-				gobject.TYPE_PYOBJECT, gobject.TYPE_PYOBJECT, str, str, str)
>-
>-		# used for cursor positioning
>-		self.index = {}
>-
>-		self.colours = {}
>-		self.nodepos = {}
>-		self.incomplete_line = {}
>-		self.commits = []
>-
>-		index = 0
>-		last_colour = 0
>-		last_nodepos = -1
>-		out_line = []
>-		input_line = fp.readline()
>-		while (input_line != ""):
>-			# The commit header ends with '\0'
>-			# This NULL is immediately followed by the sha1 of the
>-			# next commit
>-			if (input_line[0] != '\0'):
>-				commit_lines.append(input_line)
>-				input_line = fp.readline()
>-				continue;
>-
>-			commit = Commit(commit_lines)
>-			if (commit != None ):
>-				self.commits.append(commit)
>-
>-			# Skip the '\0
>-			commit_lines = []
>-			commit_lines.append(input_line[1:])
>-			input_line = fp.readline()
>-
>-		fp.close()
>-
>-		for commit in self.commits:
>-			(out_line, last_colour, last_nodepos) = self.draw_graph(commit,
>-										index, out_line,
>-										last_colour,
>-										last_nodepos)
>-			self.index[commit.commit_sha1] = index
>-			index += 1
>-
>-		self.treeview.set_model(self.model)
>-		self.treeview.show()
>-
>-	def draw_graph(self, commit, index, out_line, last_colour, last_nodepos):
>-		in_line=[]
>-
>-		#   |   -> outline
>-		#   X
>-		#   |\  <- inline
>-
>-		# Reset nodepostion
>-		if (last_nodepos > 5):
>-			last_nodepos = -1
>-
>-		# Add the incomplete lines of the last cell in this
>-		try:
>-			colour = self.colours[commit.commit_sha1]
>-		except KeyError:
>-			self.colours[commit.commit_sha1] = last_colour+1
>-			last_colour = self.colours[commit.commit_sha1]
>-			colour =   self.colours[commit.commit_sha1]
>-
>-		try:
>-			node_pos = self.nodepos[commit.commit_sha1]
>-		except KeyError:
>-			self.nodepos[commit.commit_sha1] = last_nodepos+1
>-			last_nodepos = self.nodepos[commit.commit_sha1]
>-			node_pos =  self.nodepos[commit.commit_sha1]
>-
>-		#The first parent always continue on the same line
>-		try:
>-			# check we already have the value
>-			tmp_node_pos = self.nodepos[commit.parent_sha1[0]]
>-		except KeyError:
>-			self.colours[commit.parent_sha1[0]] = colour
>-			self.nodepos[commit.parent_sha1[0]] = node_pos
>-
>-		for sha1 in self.incomplete_line.keys():
>-			if (sha1 != commit.commit_sha1):
>-				self.draw_incomplete_line(sha1, node_pos,
>-						out_line, in_line, index)
>-			else:
>-				del self.incomplete_line[sha1]
>-
>-
>-		for parent_id in commit.parent_sha1:
>-			try:
>-				tmp_node_pos = self.nodepos[parent_id]
>-			except KeyError:
>-				self.colours[parent_id] = last_colour+1
>-				last_colour = self.colours[parent_id]
>-				self.nodepos[parent_id] = last_nodepos+1
>-				last_nodepos = self.nodepos[parent_id]
>-
>-			in_line.append((node_pos, self.nodepos[parent_id],
>-						self.colours[parent_id]))
>-			self.add_incomplete_line(parent_id)
>-
>-		try:
>-			branch_tag = self.bt_sha1[commit.commit_sha1]
>-		except KeyError:
>-			branch_tag = [ ]
>-
>-
>-		node = (node_pos, colour, branch_tag)
>-
>-		self.model.append([commit, node, out_line, in_line,
>-				commit.message, commit.author, commit.date])
>-
>-		return (in_line, last_colour, last_nodepos)
>-
>-	def add_incomplete_line(self, sha1):
>-		try:
>-			self.incomplete_line[sha1].append(self.nodepos[sha1])
>-		except KeyError:
>-			self.incomplete_line[sha1] = [self.nodepos[sha1]]
>-
>-	def draw_incomplete_line(self, sha1, node_pos, out_line, in_line, index):
>-		for idx, pos in enumerate(self.incomplete_line[sha1]):
>-			if(pos == node_pos):
>-				#remove the straight line and add a slash
>-				if ((pos, pos, self.colours[sha1]) in out_line):
>-					out_line.remove((pos, pos, self.colours[sha1]))
>-				out_line.append((pos, pos+0.5, self.colours[sha1]))
>-				self.incomplete_line[sha1][idx] = pos = pos+0.5
>-			try:
>-				next_commit = self.commits[index+1]
>-				if (next_commit.commit_sha1 == sha1 and pos != int(pos)):
>-				# join the line back to the node point
>-				# This need to be done only if we modified it
>-					in_line.append((pos, pos-0.5, self.colours[sha1]))
>-					continue;
>-			except IndexError:
>-				pass
>-			in_line.append((pos, pos, self.colours[sha1]))
>-
>-
>-	def _go_clicked_cb(self, widget, revid):
>-		"""Callback for when the go button for a parent is clicked."""
>-		try:
>-			self.treeview.set_cursor(self.index[revid])
>-		except KeyError:
>-			dialog = gtk.MessageDialog(parent=None, flags=0,
>-					type=gtk.MESSAGE_WARNING, buttons=gtk.BUTTONS_CLOSE,
>-					message_format=None)
>-			dialog.set_markup("Revision <b>%s</b> not present in the list" % revid)
>-			# revid == 0 is the parent of the first commit
>-			if (revid != 0 ):
>-				dialog.format_secondary_text("Try running gitview without any options")
>-			dialog.run()
>-			dialog.destroy()
>-
>-		self.treeview.grab_focus()
>-
>-	def _show_clicked_cb(self, widget,  commit_sha1, parent_sha1, encoding):
>-		"""Callback for when the show button for a parent is clicked."""
>-		window = DiffWindow()
>-		window.set_diff(commit_sha1, parent_sha1, encoding)
>-		self.treeview.grab_focus()
>-
>-without_diff = 0
>-if __name__ == "__main__":
>-
>-	if (len(sys.argv) > 1 ):
>-		if (sys.argv[1] == "--without-diff"):
>-			without_diff = 1
>-
>-	view = GitView( without_diff != 1)
>-	view.run(sys.argv[without_diff:])
>diff --git a/contrib/gitview/gitview.txt b/contrib/gitview/gitview.txt
>deleted file mode 100644
>index 7b5f9002b9..0000000000
>--- a/contrib/gitview/gitview.txt
>+++ /dev/null
>@@ -1,57 +0,0 @@
>-gitview(1)
>-==========
>-
>-NAME
>-----
>-gitview - A GTK based repository browser for git
>-
>-SYNOPSIS
>---------
>-[verse]
>-'gitview' [options] [args]
>-
>-DESCRIPTION
>----------
>-
>-Dependencies:
>-
>-* Python 2.4
>-* PyGTK 2.8 or later
>-* PyCairo 1.0 or later
>-
>-OPTIONS
>--------
>---without-diff::
>-
>-	If the user doesn't want to list the commit diffs in the main window.
>-	This may speed up the repository browsing.
>-
>-<args>::
>-
>-	All the valid option for linkgit:git-rev-list[1].
>-
>-Key Bindings
>-------------
>-F4::
>-	To maximize the window
>-
>-F5::
>-	To reread references.
>-
>-F11::
>-	Full screen
>-
>-F12::
>-	Leave full screen
>-
>-EXAMPLES
>---------
>-
>-gitview v2.6.12.. include/scsi drivers/scsi::
>-
>-	Show as the changes since version v2.6.12 that changed any file in the
>-	include/scsi or drivers/scsi subdirectories
>-
>-gitview --since=2.weeks.ago::
>-
>-	Show the changes during the last two weeks
>-- 
>2.11.0.259.ga95e92af08
>
>
>

^ permalink raw reply

* Re: [PATCH v4 2/2] repack: die on incremental + write-bitmap-index
From: Johannes Sixt @ 2016-12-28 22:32 UTC (permalink / raw)
  To: David Turner; +Cc: git, peff
In-Reply-To: <1482948720-21488-2-git-send-email-dturner@twosigma.com>

Am 28.12.2016 um 19:12 schrieb David Turner:
> +static const char incremental_bitmap_conflict_error[] = N_(
> +"Incremental repacks are incompatible with bitmap indexes.  Use \n"

The SP before LF could be removed.

> +"--no-write-bitmap-index or disable the pack.writebitmaps configuration."
> +);

The string is marked for translation here...

> +
> +
>  static int repack_config(const char *var, const char *value, void *cb)
>  {
>  	if (!strcmp(var, "repack.usedeltabaseoffset")) {
> @@ -206,6 +212,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  	if (pack_kept_objects < 0)
>  		pack_kept_objects = write_bitmaps;
>
> +	if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
> +		die(incremental_bitmap_conflict_error);

... but then any translations remain unused. This should be

		die(_(incremental_bitmap_conflict_error));

-- Hannes


^ permalink raw reply

* [PATCH v5 0/2] repack (oops)
From: David Turner @ 2016-12-28 22:45 UTC (permalink / raw)
  To: git, peff, j6t; +Cc: David Turner

This version addresses Johannes Sixt's comments on v4.  Also, I
messed up the rebase on v4.

David Turner (2):
  auto gc: don't write bitmaps for incremental repacks
  repack: die on incremental + write-bitmap-index

 builtin/gc.c            |  9 ++++++++-
 builtin/repack.c        |  9 +++++++++
 t/t5310-pack-bitmaps.sh |  8 +++-----
 t/t6500-gc.sh           | 25 +++++++++++++++++++++++++
 4 files changed, 45 insertions(+), 6 deletions(-)

-- 
2.8.0.rc4.22.g8ae061a


^ permalink raw reply

* [PATCH v5 1/2] auto gc: don't write bitmaps for incremental repacks
From: David Turner @ 2016-12-28 22:45 UTC (permalink / raw)
  To: git, peff, j6t; +Cc: David Turner
In-Reply-To: <1482965142-6778-1-git-send-email-dturner@twosigma.com>

When git gc --auto does an incremental repack of loose objects, we do
not expect to be able to write a bitmap; it is very likely that
objects in the new pack will have references to objects outside of the
pack.  So we shouldn't try to write a bitmap, because doing so will
likely issue a warning.

This warning was making its way into gc.log.  When the gc.log was
present, future auto gc runs would refuse to run.

Patch by Jeff King.
Bug report, test, and commit message by David Turner.

Signed-off-by: David Turner <dturner@twosigma.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/gc.c  |  9 ++++++++-
 t/t6500-gc.sh | 25 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 069950d..331f219 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -191,6 +191,11 @@ static void add_repack_all_option(void)
 	}
 }
 
+static void add_repack_incremental_option(void)
+{
+       argv_array_push(&repack, "--no-write-bitmap-index");
+}
+
 static int need_to_gc(void)
 {
 	/*
@@ -208,7 +213,9 @@ static int need_to_gc(void)
 	 */
 	if (too_many_packs())
 		add_repack_all_option();
-	else if (!too_many_loose_objects())
+	else if (too_many_loose_objects())
+		add_repack_incremental_option();
+	else
 		return 0;
 
 	if (run_hook_le(NULL, "pre-auto-gc", NULL))
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 5d7d414..1762dfa 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -43,4 +43,29 @@ test_expect_success 'gc is not aborted due to a stale symref' '
 	)
 '
 
+test_expect_success 'auto gc with too many loose objects does not attempt to create bitmaps' '
+	test_config gc.auto 3 &&
+	test_config gc.autodetach false &&
+	test_config pack.writebitmaps true &&
+	# We need to create two object whose sha1s start with 17
+	# since this is what git gc counts.  As it happens, these
+	# two blobs will do so.
+	test_commit 263 &&
+	test_commit 410 &&
+	# Our first gc will create a pack; our second will create a second pack
+	git gc --auto &&
+	ls .git/objects/pack | sort >existing_packs &&
+	test_commit 523 &&
+	test_commit 790 &&
+
+	git gc --auto 2>err &&
+	test_i18ngrep ! "^warning:" err &&
+	ls .git/objects/pack/ | sort >post_packs &&
+	comm -1 -3 existing_packs post_packs >new &&
+	comm -2 -3 existing_packs post_packs >del &&
+	test_line_count = 0 del && # No packs are deleted
+	test_line_count = 2 new # There is one new pack and its .idx
+'
+
+
 test_done
-- 
2.8.0.rc4.22.g8ae061a


^ permalink raw reply related

* [PATCH v5 2/2] repack: die on incremental + write-bitmap-index
From: David Turner @ 2016-12-28 22:45 UTC (permalink / raw)
  To: git, peff, j6t; +Cc: David Turner
In-Reply-To: <1482965142-6778-1-git-send-email-dturner@twosigma.com>

The bitmap index only works for single packs, so requesting an
incremental repack with bitmap indexes makes no sense.

Signed-off-by: David Turner <dturner@twosigma.com>
---
 builtin/repack.c        | 9 +++++++++
 t/t5310-pack-bitmaps.sh | 8 +++-----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index 80dd06b..677bc7c 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -18,6 +18,12 @@ static const char *const git_repack_usage[] = {
 	NULL
 };
 
+static const char incremental_bitmap_conflict_error[] = N_(
+"Incremental repacks are incompatible with bitmap indexes.  Use\n"
+"--no-write-bitmap-index or disable the pack.writebitmaps configuration."
+);
+
+
 static int repack_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "repack.usedeltabaseoffset")) {
@@ -206,6 +212,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 	if (pack_kept_objects < 0)
 		pack_kept_objects = write_bitmaps;
 
+	if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
+		die(_(incremental_bitmap_conflict_error));
+
 	packdir = mkpathdup("%s/pack", get_object_directory());
 	packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, (int)getpid());
 
diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index b4c7a6f..424bec7 100755
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -118,12 +118,10 @@ test_expect_success 'fetch (partial bitmap)' '
 	test_cmp expect actual
 '
 
-test_expect_success 'incremental repack cannot create bitmaps' '
+test_expect_success 'incremental repack fails when bitmaps are requested' '
 	test_commit more-1 &&
-	find .git/objects/pack -name "*.bitmap" >expect &&
-	git repack -d &&
-	find .git/objects/pack -name "*.bitmap" >actual &&
-	test_cmp expect actual
+	test_must_fail git repack -d 2>err &&
+	test_i18ngrep "Incremental repacks are incompatible with bitmap" err
 '
 
 test_expect_success 'incremental repack can disable bitmaps' '
-- 
2.8.0.rc4.22.g8ae061a


^ permalink raw reply related

* [PATCH v3] am: add am.signoff add config variable
From: Eduardo Habkost @ 2016-12-28 22:55 UTC (permalink / raw)
  To: git; +Cc: Paul Tan, Andreas Schwab, Stefan Beller

git-am has options to enable --message-id and --3way by default,
but no option to enable --signoff by default. Add a "am.signoff"
config option.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Added documentation to Documentation/git-am.txt and
  Documentation/config.txt
* Added test cases to t4150-am.sh

Changes v2 -> v3:
* Fix doc to mention "--[no-]signoff" instead of "--[no]-signoff"
  * Reported-by: Andreas Schwab <schwab@linux-m68k.org>
* Add missing test_cmp line on test code
* Use "! grep" instead of "$(grep -c ...)" -eq 0
  * Suggested-by: Stefan Beller <sbeller@google.com>
---
 Documentation/config.txt |  5 +++++
 Documentation/git-am.txt |  6 ++++--
 builtin/am.c             |  2 ++
 t/t4150-am.sh            | 26 ++++++++++++++++++++++++++
 4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 30cb94610..6b2990203 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -822,6 +822,11 @@ am.keepcr::
 	by giving `--no-keep-cr` from the command line.
 	See linkgit:git-am[1], linkgit:git-mailsplit[1].
 
+am.signoff::
+	If true, git-am will add a `Signed-off-by:` line to the commit
+	message. See the signoff option in linkgit:git-commit[1] for
+	more information.
+
 am.threeWay::
 	By default, `git am` will fail if the patch does not apply cleanly. When
 	set to true, this setting tells `git am` to fall back on 3-way merge if
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 12879e402..1f14986c7 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -9,7 +9,7 @@ git-am - Apply a series of patches from a mailbox
 SYNOPSIS
 --------
 [verse]
-'git am' [--signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8]
+'git am' [--[no-]signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8]
 	 [--[no-]3way] [--interactive] [--committer-date-is-author-date]
 	 [--ignore-date] [--ignore-space-change | --ignore-whitespace]
 	 [--whitespace=<option>] [-C<n>] [-p<n>] [--directory=<dir>]
@@ -32,10 +32,12 @@ OPTIONS
 	If you supply directories, they will be treated as Maildirs.
 
 -s::
---signoff::
+--[no-]-signoff::
 	Add a `Signed-off-by:` line to the commit message, using
 	the committer identity of yourself.
 	See the signoff option in linkgit:git-commit[1] for more information.
+	The `am.signoff` configuration variable can be used to specify the
+	default behaviour.  `--no-signoff` is useful to override `am.signoff`.
 
 -k::
 --keep::
diff --git a/builtin/am.c b/builtin/am.c
index 31fb60578..d2e02334f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -154,6 +154,8 @@ static void am_state_init(struct am_state *state, const char *dir)
 
 	git_config_get_bool("am.messageid", &state->message_id);
 
+	git_config_get_bool("am.signoff", &state->signoff);
+
 	state->scissors = SCISSORS_UNSET;
 
 	argv_array_init(&state->git_apply_opts);
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 89a5bacac..d65c8e5c4 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -479,6 +479,32 @@ test_expect_success 'am --signoff adds Signed-off-by: line' '
 	test_cmp expected actual
 '
 
+test_expect_success '--no-signoff overrides am.signoff' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard first &&
+	test_config am.signoff true &&
+	git am --no-signoff <patch2 &&
+	printf "%s\n" "$signoff" >expected &&
+	git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
+	test_cmp expected actual &&
+	git cat-file commit HEAD > actual &&
+	! grep -q "Signed-off-by:" actual
+'
+
+test_expect_success 'am.signoff adds Signed-off-by: line' '
+	rm -fr .git/rebase-apply &&
+	git reset --hard first &&
+	test_config am.signoff true &&
+	git am <patch2 &&
+	printf "%s\n" "$signoff" >expected &&
+	echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >>expected &&
+	git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
+	test_cmp expected actual &&
+	echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >expected &&
+	git cat-file commit HEAD | grep "Signed-off-by:" >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'am stays in branch' '
 	echo refs/heads/master2 >expected &&
 	git symbolic-ref HEAD >actual &&
-- 
2.11.0.259.g40922b1


^ permalink raw reply related

* [PATCH] unpack-trees: move checkout state into check_updates
From: Stefan Beller @ 2016-12-28 23:26 UTC (permalink / raw)
  To: l.s.r, gitster; +Cc: git, Stefan Beller

The checkout state was introduced via 16da134b1f9
(read-trees: refactor the unpack_trees() part, 2006-07-30). An attempt to
refactor the checkout state was done in b56aa5b268e (unpack-trees: pass
checkout state explicitly to check_updates(), 2016-09-13), but we can
go even further.

The `struct checkout state` is not used in unpack_trees apart from
initializing it, so move it into the function that makes use of it,
which is `check_updates`.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 unpack-trees.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index ea799d37c5..78703af135 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -224,14 +224,18 @@ static void unlink_entry(const struct cache_entry *ce)
 	schedule_dir_for_removal(ce->name, ce_namelen(ce));
 }
 
-static int check_updates(struct unpack_trees_options *o,
-			 const struct checkout *state)
+static int check_updates(struct unpack_trees_options *o)
 {
 	unsigned cnt = 0, total = 0;
 	struct progress *progress = NULL;
 	struct index_state *index = &o->result;
 	int i;
 	int errs = 0;
+	struct checkout state = CHECKOUT_INIT;
+	state.force = 1;
+	state.quiet = 1;
+	state.refresh_cache = 1;
+	state.istate = &o->result;
 
 	if (o->update && o->verbose_update) {
 		for (total = cnt = 0; cnt < index->cache_nr; cnt++) {
@@ -270,7 +274,7 @@ static int check_updates(struct unpack_trees_options *o,
 			display_progress(progress, ++cnt);
 			ce->ce_flags &= ~CE_UPDATE;
 			if (o->update && !o->dry_run) {
-				errs |= checkout_entry(ce, state, NULL);
+				errs |= checkout_entry(ce, &state, NULL);
 			}
 		}
 	}
@@ -1100,14 +1104,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	int i, ret;
 	static struct cache_entry *dfc;
 	struct exclude_list el;
-	struct checkout state = CHECKOUT_INIT;
 
 	if (len > MAX_UNPACK_TREES)
 		die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
-	state.force = 1;
-	state.quiet = 1;
-	state.refresh_cache = 1;
-	state.istate = &o->result;
 
 	memset(&el, 0, sizeof(el));
 	if (!core_apply_sparse_checkout || !o->update)
@@ -1244,7 +1243,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	}
 
 	o->src_index = NULL;
-	ret = check_updates(o, &state) ? (-2) : 0;
+	ret = check_updates(o) ? (-2) : 0;
 	if (o->dst_index) {
 		if (!ret) {
 			if (!o->result.cache_tree)
-- 
2.11.0.rc2.51.g8b63c0e.dirty


^ permalink raw reply related

* Re: [PATCH] contrib: remove gitview
From: Jeff King @ 2016-12-29  1:59 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git, jvoss, Aneesh Kumar K.V
In-Reply-To: <20161228172837.24395-1-sbeller@google.com>

On Wed, Dec 28, 2016 at 09:28:37AM -0800, Stefan Beller wrote:

> gitview did not have meaningful contributions since 2007, which gives the
> impression it is either a mature or dead project.
> 
> In both cases we should not carry it in git.git as the README for contrib
> states we only want to carry experimental things to give early exposure.
> 
> Recently a security vulnerability was reported by Javantea, so the decision
> to either fix the issue or remove the code in question becomes a bit
> more urgent.
> 
> Reported-by: Javantea <jvoss@altsci.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  contrib/gitview/gitview     | 1305 -------------------------------------------
>  contrib/gitview/gitview.txt |   57 --
>  2 files changed, 1362 deletions(-)
>  delete mode 100755 contrib/gitview/gitview
>  delete mode 100644 contrib/gitview/gitview.txt

Thanks for assembling the patch. This seems reasonable to me, though I'd
like to get an Ack from Aneesh if we can.

-Peff

^ permalink raw reply

* Re: [PATCH v5 0/2] repack (oops)
From: Jeff King @ 2016-12-29  2:04 UTC (permalink / raw)
  To: David Turner; +Cc: git, j6t
In-Reply-To: <1482965142-6778-1-git-send-email-dturner@twosigma.com>

On Wed, Dec 28, 2016 at 05:45:40PM -0500, David Turner wrote:

> This version addresses Johannes Sixt's comments on v4.  Also, I
> messed up the rebase on v4.

Thanks. The test logic in this one looks good to me.

-Peff

^ permalink raw reply

* Re: [PATCH v3] am: add am.signoff add config variable
From: Andreas Schwab @ 2016-12-29  7:58 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: git, Paul Tan, Stefan Beller
In-Reply-To: <20161228225544.16388-1-ehabkost@redhat.com>

On Dez 28 2016, Eduardo Habkost <ehabkost@redhat.com> wrote:

> @@ -32,10 +32,12 @@ OPTIONS
>  	If you supply directories, they will be treated as Maildirs.
>  
>  -s::
> ---signoff::
> +--[no-]-signoff::

That's one dash too much.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: [PATCH v2] am: add am.signoff add config variable
From: Pranit Bauva @ 2016-12-29  7:59 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: Stefan Beller, git@vger.kernel.org, Paul Tan
In-Reply-To: <20161228191928.GH3441@thinpad.lan.raisama.net>

Hey Eduardo,

On Thu, Dec 29, 2016 at 12:49 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
>> test_expect_success '--no-signoff overrides am.signoff' '
>>       rm -fr .git/rebase-apply &&
>>       git reset --hard first &&
>>       test_config am.signoff true &&
>>       git am --no-signoff <patch2 &&
>>       printf "%s\n" "$signoff" >expected &&
>>       git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
>>       test_cmp expected actual &&
>>       git cat-file commit HEAD | grep "Signed-off-by:" >actual &&
>>       test_must_be_empty actual
>> '
>>
>> The test fails because the second "grep" command returns a
>> non-zero exit code. Any suggestions to avoid that problem in a
>> more idiomatic way?
>
> I just found out that "test_must_fail grep ..." is a common
> idiom, so what about:

Is there any particular reason to use "grep" instead of "test_cmp"? To
check for non-zero error code, you can always use "! test_cmp".

Regards,
Pranit Bauva

^ permalink raw reply

* Re: [PATCH] am: add am.signoff add config variable
From: Eric Wong @ 2016-12-29  8:47 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: git, Paul Tan
In-Reply-To: <1482946838-28779-1-git-send-email-ehabkost@redhat.com>

Eduardo Habkost <ehabkost@redhat.com> wrote:
> git-am has options to enable --message-id and --3way by default,
> but no option to enable --signoff by default. Add a "am.signoff"
> config option.

I'm not sure this is a good idea.  IANAL, but a sign-off
has some sort of legal meaning for this project (DCO)
and that would be better decided on a patch-by-patch basis
rather than a blanket statement.

I don't add my SoB to patches (either my own or received) until
I'm comfortable with it; and I'd rather err on the side of
forgetting and being prodded to resubmit rather than putting
an SoB on the wrong patch.


(I'm barely online today, no rush needed in responding)

^ permalink raw reply

* [PATCH] git-p4: do not pass '-r 0' to p4 commands
From: Igor Kushnir @ 2016-12-29  9:05 UTC (permalink / raw)
  To: git; +Cc: Lars Schneider, Luke Diamand, Ori Rawlings, Igor Kushnir

git-p4 crashes when used with a very old p4 client version
that does not support the '-r <number>' option in its commands.

Allow making git-p4 work with old p4 clients by setting git-p4.retries to 0.

Alternatively git-p4.retries could be made opt-in.
But since only very old, barely maintained p4 versions don't support
the '-r' option, the setting-retries-to-0 workaround would do.

The "-r retries" option is present in Perforce 2012.2 Command Reference,
but absent from Perforce 2012.1 Command Reference.

Signed-off-by: Igor Kushnir <igorkuo@gmail.com>
---
 Documentation/git-p4.txt | 1 +
 git-p4.py                | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index bae862ddc..f4f1be5be 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -479,6 +479,7 @@ git-p4.client::
 git-p4.retries::
 	Specifies the number of times to retry a p4 command (notably,
 	'p4 sync') if the network times out. The default value is 3.
+	'-r 0' is not passed to p4 commands if this option is set to 0.
 
 Clone and sync variables
 ~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/git-p4.py b/git-p4.py
index 22e3f57e7..e5a9e1cce 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -83,7 +83,9 @@ def p4_build_cmd(cmd):
     if retries is None:
         # Perform 3 retries by default
         retries = 3
-    real_cmd += ["-r", str(retries)]
+    if retries != 0:
+        # Provide a way to not pass this option by setting git-p4.retries to 0
+        real_cmd += ["-r", str(retries)]
 
     if isinstance(cmd,basestring):
         real_cmd = ' '.join(real_cmd) + ' ' + cmd
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH] git-p4: do not pass '-r 0' to p4 commands
From: Luke Diamand @ 2016-12-29  9:20 UTC (permalink / raw)
  To: Igor Kushnir; +Cc: Git Users, Lars Schneider, Ori Rawlings
In-Reply-To: <20161229090533.4717-1-igorkuo@gmail.com>

On 29 December 2016 at 09:05, Igor Kushnir <igorkuo@gmail.com> wrote:
> git-p4 crashes when used with a very old p4 client version
> that does not support the '-r <number>' option in its commands.
>
> Allow making git-p4 work with old p4 clients by setting git-p4.retries to 0.
>
> Alternatively git-p4.retries could be made opt-in.
> But since only very old, barely maintained p4 versions don't support
> the '-r' option, the setting-retries-to-0 workaround would do.
>
> The "-r retries" option is present in Perforce 2012.2 Command Reference,
> but absent from Perforce 2012.1 Command Reference.

That looks like a good fix, thanks.

I feel sad for anyone still using 2012.1.

Luke


>
> Signed-off-by: Igor Kushnir <igorkuo@gmail.com>
> ---
>  Documentation/git-p4.txt | 1 +
>  git-p4.py                | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index bae862ddc..f4f1be5be 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -479,6 +479,7 @@ git-p4.client::
>  git-p4.retries::
>         Specifies the number of times to retry a p4 command (notably,
>         'p4 sync') if the network times out. The default value is 3.
> +       '-r 0' is not passed to p4 commands if this option is set to 0.
>
>  Clone and sync variables
>  ~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/git-p4.py b/git-p4.py
> index 22e3f57e7..e5a9e1cce 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -83,7 +83,9 @@ def p4_build_cmd(cmd):
>      if retries is None:
>          # Perform 3 retries by default
>          retries = 3
> -    real_cmd += ["-r", str(retries)]
> +    if retries != 0:
> +        # Provide a way to not pass this option by setting git-p4.retries to 0
> +        real_cmd += ["-r", str(retries)]
>
>      if isinstance(cmd,basestring):
>          real_cmd = ' '.join(real_cmd) + ' ' + cmd
> --
> 2.11.0
>

^ permalink raw reply

* Re: [PATCH] git-p4: do not pass '-r 0' to p4 commands
From: Lars Schneider @ 2016-12-29 10:02 UTC (permalink / raw)
  To: Igor Kushnir; +Cc: git, Luke Diamand, Ori Rawlings
In-Reply-To: <20161229090533.4717-1-igorkuo@gmail.com>


> On 29 Dec 2016, at 10:05, Igor Kushnir <igorkuo@gmail.com> wrote:
> 
> git-p4 crashes when used with a very old p4 client version
> that does not support the '-r <number>' option in its commands.
> 
> Allow making git-p4 work with old p4 clients by setting git-p4.retries to 0.
> 
> Alternatively git-p4.retries could be made opt-in.
> But since only very old, barely maintained p4 versions don't support
> the '-r' option, the setting-retries-to-0 workaround would do.
> 
> The "-r retries" option is present in Perforce 2012.2 Command Reference,
> but absent from Perforce 2012.1 Command Reference.

Thanks for this workaround!


> Signed-off-by: Igor Kushnir <igorkuo@gmail.com>
> ---
> Documentation/git-p4.txt | 1 +
> git-p4.py                | 4 +++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
> index bae862ddc..f4f1be5be 100644
> --- a/Documentation/git-p4.txt
> +++ b/Documentation/git-p4.txt
> @@ -479,6 +479,7 @@ git-p4.client::
> git-p4.retries::
> 	Specifies the number of times to retry a p4 command (notably,
> 	'p4 sync') if the network times out. The default value is 3.
> +	'-r 0' is not passed to p4 commands if this option is set to 0.

At this point in the docs we have never talked about the "-r" flag and I
would argue it is an "implementation detail". Therefore, I would prefer
something like: "Set the value to 0 if want to disable retries or if 
your p4 version does not support retries (pre 2012.2)."


> 
> Clone and sync variables
> ~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/git-p4.py b/git-p4.py
> index 22e3f57e7..e5a9e1cce 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -83,7 +83,9 @@ def p4_build_cmd(cmd):
>     if retries is None:
>         # Perform 3 retries by default
>         retries = 3
> -    real_cmd += ["-r", str(retries)]
> +    if retries != 0:

How about "retries > 0"?


> +        # Provide a way to not pass this option by setting git-p4.retries to 0
> +        real_cmd += ["-r", str(retries)]
> 
>     if isinstance(cmd,basestring):
>         real_cmd = ' '.join(real_cmd) + ' ' + cmd
> -- 
> 2.11.0
> 


^ permalink raw reply

* Re: What's cooking in git.git (Dec 2016, #08; Tue, 27)
From: Duy Nguyen @ 2016-12-29 10:06 UTC (permalink / raw)
  To: Brandon Williams; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20161228181808.GC33595@google.com>

On Thu, Dec 29, 2016 at 1:18 AM, Brandon Williams <bmwill@google.com> wrote:
> On 12/27, Junio C Hamano wrote:
>> * bw/pathspec-cleanup (2016-12-14) 16 commits
>>  - pathspec: rename prefix_pathspec to init_pathspec_item
>>  - pathspec: small readability changes
>>  - pathspec: create strip submodule slash helpers
>>  - pathspec: create parse_element_magic helper
>>  - pathspec: create parse_long_magic function
>>  - pathspec: create parse_short_magic function
>>  - pathspec: factor global magic into its own function
>>  - pathspec: simpler logic to prefix original pathspec elements
>>  - pathspec: always show mnemonic and name in unsupported_magic
>>  - pathspec: remove unused variable from unsupported_magic
>>  - pathspec: copy and free owned memory
>>  - pathspec: remove the deprecated get_pathspec function
>>  - ls-tree: convert show_recursive to use the pathspec struct interface
>>  - dir: convert fill_directory to use the pathspec struct interface
>>  - dir: remove struct path_simplify
>>  - mv: remove use of deprecated 'get_pathspec()'
>>
>>  Code clean-up in the pathspec API.
>>
>>  Waiting for the (hopefully) final round of review before 'next'.
>
> What more needs to be reviewed for this series?

I wanted to have a look at it but I successfully managed to put if off
so far. Will do soonish.
-- 
Duy

^ permalink raw reply

* Re: What's cooking in git.git (Dec 2016, #08; Tue, 27)
From: Lars Schneider @ 2016-12-29 10:10 UTC (permalink / raw)
  To: Junio C Hamano, bmwill; +Cc: git
In-Reply-To: <xmqqy3z155o1.fsf@gitster.mtv.corp.google.com>


> On 28 Dec 2016, at 00:11, Junio C Hamano <gitster@pobox.com> wrote:
> 
> 
> * bw/realpath-wo-chdir (2016-12-22) 5 commits
>  (merged to 'next' on 2016-12-22 at fea8fa870f)
> + real_path: canonicalize directory separators in root parts
> + real_path: have callers use real_pathdup and strbuf_realpath
> + real_path: create real_pathdup
> + real_path: convert real_path_internal to strbuf_realpath
> + real_path: resolve symlinks by hand
> (this branch is used by bw/grep-recurse-submodules.)
> 
> The implementation of "real_path()" was to go there with chdir(2)
> and call getcwd(3), but this obviously wouldn't be usable in a
> threaded environment.  Rewrite it to manually resolve relative
> paths including symbolic links in path components.

"real_path: resolve symlinks by hand" (05b458c) introduces
"MAXSYMLINKS" which is already defined on macOS in

/usr/include/sys/param.h:197:9:

 * .., MAXSYMLINKS defines the
 * maximum number of symbolic links that may be expanded in a path name.
 * It should be set high enough to allow all legitimate uses, but halt
 * infinite loops reasonably quickly.
 */


Log with JS: https://travis-ci.org/git/git/jobs/187092215
Log without JS: https://s3.amazonaws.com/archive.travis-ci.org/jobs/187092215/log.txt

- Lars


^ permalink raw reply

* [PATCH v2] git-p4: do not pass '-r 0' to p4 commands
From: Igor Kushnir @ 2016-12-29 10:22 UTC (permalink / raw)
  To: git; +Cc: Lars Schneider, Luke Diamand, Ori Rawlings, Igor Kushnir

git-p4 crashes when used with a very old p4 client version
that does not support the '-r <number>' option in its commands.

Allow making git-p4 work with old p4 clients by setting git-p4.retries to 0.

Alternatively git-p4.retries could be made opt-in.
But since only very old, barely maintained p4 versions don't support
the '-r' option, the setting-retries-to-0 workaround would do.

The "-r retries" option is present in Perforce 2012.2 Command Reference,
but absent from Perforce 2012.1 Command Reference.

Signed-off-by: Igor Kushnir <igorkuo@gmail.com>
---
 Documentation/git-p4.txt | 2 ++
 git-p4.py                | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt
index bae862ddc..7436c64a9 100644
--- a/Documentation/git-p4.txt
+++ b/Documentation/git-p4.txt
@@ -479,6 +479,8 @@ git-p4.client::
 git-p4.retries::
 	Specifies the number of times to retry a p4 command (notably,
 	'p4 sync') if the network times out. The default value is 3.
+	Set the value to 0 to disable retries or if your p4 version
+	does not support retries (pre 2012.2).
 
 Clone and sync variables
 ~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/git-p4.py b/git-p4.py
index 22e3f57e7..7bda915bd 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -83,7 +83,9 @@ def p4_build_cmd(cmd):
     if retries is None:
         # Perform 3 retries by default
         retries = 3
-    real_cmd += ["-r", str(retries)]
+    if retries > 0:
+        # Provide a way to not pass this option by setting git-p4.retries to 0
+        real_cmd += ["-r", str(retries)]
 
     if isinstance(cmd,basestring):
         real_cmd = ' '.join(real_cmd) + ' ' + cmd
-- 
2.11.0


^ permalink raw reply related

* Re: HowTo distribute a hook with the reposity.
From: Lars Schneider @ 2016-12-29 10:29 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Jeff King, John P. Hartmann, Git mailing list
In-Reply-To: <CA+P7+xoBpAV1hiZQ0WZHP-kbB9zhddMhtj_CZ1Wejdur3oPXhQ@mail.gmail.com>


> On 28 Dec 2016, at 19:53, Jacob Keller <jacob.keller@gmail.com> wrote:
> 
> On Tue, Dec 27, 2016 at 10:08 PM, Jeff King <peff@peff.net> wrote:
>> 
>>       https://github.com/Autodesk/enterprise-config-for-git
>> 
>>     (with the disclaimer that I've never used it myself, so I have no
>>     idea how good it is).
>> 
>> I think you probably know all that, Jake, but I am laying it out for the
>> benefit of the OP and the list. :)
>> 
> 
> Heh, well I didn't know about this last part so it's still useful to
> me! I knew of the larger description for why not but wasn't exactly
> clear how to word it so I am glad you filled that in. Thanks!

Author of the "enterprise-config-for-git" here. The version in the public
Git repo is a stripped down version of the one we use at my day job for
our engineering workforce. I tried to extract the "generally useful bits".
Unfortunately it cannot be used "as-is". Don't hesitate to ping me if you
want to learn more about it and/or if you have an idea how to make it
better suited for open source collaboration.

Cheers,
Lars


^ permalink raw reply

* Re: [PATCH] unpack-trees: move checkout state into check_updates
From: René Scharfe @ 2016-12-29 13:53 UTC (permalink / raw)
  To: Stefan Beller; +Cc: gitster, git
In-Reply-To: <20161228232616.21109-1-sbeller@google.com>

Am 29.12.2016 um 00:26 schrieb Stefan Beller:
> The checkout state was introduced via 16da134b1f9
> (read-trees: refactor the unpack_trees() part, 2006-07-30). An attempt to
> refactor the checkout state was done in b56aa5b268e (unpack-trees: pass
> checkout state explicitly to check_updates(), 2016-09-13), but we can
> go even further.
>
> The `struct checkout state` is not used in unpack_trees apart from
> initializing it, so move it into the function that makes use of it,
> which is `check_updates`.

Thanks for catching this, the result looks nicer.

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  unpack-trees.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index ea799d37c5..78703af135 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -224,14 +224,18 @@ static void unlink_entry(const struct cache_entry *ce)
>  	schedule_dir_for_removal(ce->name, ce_namelen(ce));
>  }
>
> -static int check_updates(struct unpack_trees_options *o,
> -			 const struct checkout *state)
> +static int check_updates(struct unpack_trees_options *o)
>  {
>  	unsigned cnt = 0, total = 0;
>  	struct progress *progress = NULL;
>  	struct index_state *index = &o->result;
>  	int i;
>  	int errs = 0;
> +	struct checkout state = CHECKOUT_INIT;
> +	state.force = 1;
> +	state.quiet = 1;
> +	state.refresh_cache = 1;
> +	state.istate = &o->result;

And here (as well as in two more places in this function) we could use 
"index" instead of "&o->result".  Not sure if it's worth a patch, though.

>
>  	if (o->update && o->verbose_update) {
>  		for (total = cnt = 0; cnt < index->cache_nr; cnt++) {
> @@ -270,7 +274,7 @@ static int check_updates(struct unpack_trees_options *o,
>  			display_progress(progress, ++cnt);
>  			ce->ce_flags &= ~CE_UPDATE;
>  			if (o->update && !o->dry_run) {
> -				errs |= checkout_entry(ce, state, NULL);
> +				errs |= checkout_entry(ce, &state, NULL);
>  			}
>  		}
>  	}
> @@ -1100,14 +1104,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>  	int i, ret;
>  	static struct cache_entry *dfc;
>  	struct exclude_list el;
> -	struct checkout state = CHECKOUT_INIT;
>
>  	if (len > MAX_UNPACK_TREES)
>  		die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
> -	state.force = 1;
> -	state.quiet = 1;
> -	state.refresh_cache = 1;
> -	state.istate = &o->result;
>
>  	memset(&el, 0, sizeof(el));
>  	if (!core_apply_sparse_checkout || !o->update)
> @@ -1244,7 +1243,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>  	}
>
>  	o->src_index = NULL;
> -	ret = check_updates(o, &state) ? (-2) : 0;
> +	ret = check_updates(o) ? (-2) : 0;
>  	if (o->dst_index) {
>  		if (!ret) {
>  			if (!o->result.cache_tree)
>

^ permalink raw reply

* Re: [PATCH] string-list: make compare function compatible with qsort(3)
From: René Scharfe @ 2016-12-29 13:53 UTC (permalink / raw)
  To: Jeff King; +Cc: Git List, Junio C Hamano, Johannes Schindelin
In-Reply-To: <20161221161220.x3qkcwmuangcdc2l@sigill.intra.peff.net>

Am 21.12.2016 um 17:12 schrieb Jeff King:
> On Wed, Dec 21, 2016 at 10:36:41AM +0100, René Scharfe wrote:
>
>> One shortcoming is that the comparison function is restricted to working
>> with the string members of items; util is inaccessible to it.  Another
>> one is that the value of cmp is passed in a global variable to
>> cmp_items(), making string_list_sort() non-reentrant.
>
> I think this approach is OK for string_list, but it doesn't help the
> general case that wants qsort_s() to actually access global data. I
> don't know how common that is in our codebase, though.
>
> So I'm fine with it, but I think we might eventually need to revisit the
> qsort_s() thing anyway.

I have to admit I didn't even consider the possibility that the pattern 
of accessing global variables in qsort(1) compare functions could have 
spread.

And indeed, at least ref-filter.c::compare_refs() and 
worktree.c::compare_worktree() so that as well.  The latter uses 
fspathcmp(), which is OK as ignore_case is only set once when reading 
the config, but the first one looks, well, interesting.  Perhaps a 
single ref filter per program is enough?

Anyway, that's a good enough argument for me for adding that newfangled 
C11 function after all..

Btw.: Found with

   git grep 'QSORT.*;$' |
   sed 's/.* /int /; s/);//' |
   sort -u |
   git grep -Ww -f-

and

   git grep -A1 'QSORT.*,$'

>> Remove the intermediate layer, i.e. cmp_items(), make the comparison
>> functions compatible with qsort(3) and pass them pointers to full items.
>> This allows comparisons to also take the util member into account, and
>> avoids the need to pass the real comparison function to an intermediate
>> function, removing the need for a global function.
>
> I'm not sure if access to the util field is really of any value, after
> looking at it in:
>
>   http://public-inbox.org/git/20161125171546.fa3zpapbjngjcl26@sigill.intra.peff.net/
>
> Though note that if we do take this patch, there are probably one or two
> spots that could switch from QSORT() to string_list_sort().

Yes, but as you noted in that thread there is not much point in doing 
that; the only net win is that we can pass a list as a single pointer 
instead of as base pointer and element count -- the special compare 
function needs to be specified anyway (once), somehow.

René

^ permalink raw reply

* Re: [PATCH v2] am: add am.signoff add config variable
From: Eduardo Habkost @ 2016-12-29 15:37 UTC (permalink / raw)
  To: Pranit Bauva; +Cc: Stefan Beller, git@vger.kernel.org, Paul Tan
In-Reply-To: <CAFZEwPOPMrCXTc+SMhjGSnPKLmefcde4MgJsz7n5rBApACZOug@mail.gmail.com>

On Thu, Dec 29, 2016 at 01:29:33PM +0530, Pranit Bauva wrote:
> Hey Eduardo,
> 
> On Thu, Dec 29, 2016 at 12:49 AM, Eduardo Habkost <ehabkost@redhat.com> wrote:
> >> test_expect_success '--no-signoff overrides am.signoff' '
> >>       rm -fr .git/rebase-apply &&
> >>       git reset --hard first &&
> >>       test_config am.signoff true &&
> >>       git am --no-signoff <patch2 &&
> >>       printf "%s\n" "$signoff" >expected &&
> >>       git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
> >>       test_cmp expected actual &&
> >>       git cat-file commit HEAD | grep "Signed-off-by:" >actual &&
> >>       test_must_be_empty actual
> >> '
> >>
> >> The test fails because the second "grep" command returns a
> >> non-zero exit code. Any suggestions to avoid that problem in a
> >> more idiomatic way?
> >
> > I just found out that "test_must_fail grep ..." is a common
> > idiom, so what about:
> 
> Is there any particular reason to use "grep" instead of "test_cmp"? To
> check for non-zero error code, you can always use "! test_cmp".

The test code is checking only the "Signed-off-by" lines, not the
whole commit message. "test_cmp" would require recovering the
entire contents of the original commit message, which would add
complexity to the test code.

-- 
Eduardo

^ 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