Git development
 help / color / mirror / Atom feed
* Re: HowTo distribute a hook with the reposity.
From: John P. Hartmann @ 2016-12-28  8:42 UTC (permalink / raw)
  To: Jeff King, Jacob Keller; +Cc: Git mailing list
In-Reply-To: <20161228060840.gelgcs2hd33id56j@sigill.intra.peff.net>

Thanks; your point is taken.  One final wrinkle:

This project is hosted on github.  If I put the hook into the repository 
manually (if I can; I don't know that), is it true that the hook would 
be distributed on a clone action, but not on a pull?

	j.

On 28/12/16 06:08, Jeff King wrote:
> On Tue, Dec 27, 2016 at 09:32:22PM -0800, Jacob Keller wrote:
>
>> On Tue, Dec 27, 2016 at 5:34 PM, John P. Hartmann <jphartmann@gmail.com> wrote:
>>> I would like a hook in .got/hooks to be made available to all who clone or
>>> pull a particular project.  I'd also like the hook to be under git control
>>> (changes committed &c).  I added a hook, but git status does not show it.
>>> Presumably git excludes its files in .git/ from version control lest there
>>> be a chiken-and-egg situation.
>>>
>>> Is there a way to achieve this?  Or a better way to do it?
>>>
>>> Thanks,  j.
>>
>> Best way I found, was add a script with an "installme" shell script or
>> similar that you tell all users of the repository that they are
>> expected to run this to install the scripts. You can' make it happen
>> automatically.
>
> I agree that is the best way to do it.
>
> I didn't dig up previous discussions, but the gist is usually:
>
>   1. Cloning a repository should not run arbitrary code from the remote
>      without the user on the cloning side taking some further affirmative
>      action.
>
>      This is for security reasons. Obviously the next step is quite often
>      to run "make" which does run arbitrary code, but that counts as an
>      action.
>
>   2. We could write a feature in git that manages hooks (or config, etc).
>      But ultimately you would still be running "git clone
>      --trust-remote-hooks" or something to satisfy point (1).
>
>   3. There's not much point in doing point (2), because you can just
>      spell it as "git clone && cd clone && ./install-hooks" and then git
>      does not have to care at all about your scripts.
>
>   4. A hook (or config) management system could do fancy things like
>      merging your custom local config, picking up changes from the
>      remote, etc. But all of that can happen outside of Git totally (and
>      quite often you want to manage things in contributors setups
>      _besides_ Git data anyway).
>
>      An example system is:
>
>        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. :)
>
> -Peff
>

^ permalink raw reply

* Re: HowTo distribute a hook with the reposity.
From: Jeff King @ 2016-12-28  8:52 UTC (permalink / raw)
  To: John P. Hartmann; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <6801f971-418d-18c9-8002-9c2f7b8c8008@gmail.com>

On Wed, Dec 28, 2016 at 08:42:25AM +0000, John P. Hartmann wrote:

> This project is hosted on github.  If I put the hook into the repository
> manually (if I can; I don't know that), is it true that the hook would be
> distributed on a clone action, but not on a pull?

I'm not sure what you mean by "into the repository". If you mean "into
the .git directory", then no, you can't do that. Git will not add ".git"
directory contents to a repository, you cannot manipulate the contents
of ".git" directories on GitHub, and a client wouldn't ever look at them
on clone or fetch anyway.

Did you mean something else?

-Peff

^ permalink raw reply

* Re: [PATCH v9 15/20] ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
From: Karthik Nayak @ 2016-12-28  8:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List, Jacob Keller, Ramsay Jones
In-Reply-To: <xmqqo9zx6phw.fsf@gitster.mtv.corp.google.com>

On Wed, Dec 28, 2016 at 2:41 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Currently the 'lstrip=<N>' option only takes a positive value '<N>'
>> and strips '<N>' slash-separated path components from the left. Modify
>> the 'lstrip' option to also take a negative number '<N>' which would
>> only _leave_ behind 'N' slash-separated path components from the left.
>
> "would only leave behind N components from the left" sounds as if
> the result is A/B, when you are given A/B/C/D/E and asked to
> lstrip:-2.  Given these two tests added by the patch ...
>
>> +test_atom head refname:lstrip=-1 master
>> +test_atom head refname:lstrip=-2 heads/master
>
> ... I somehow think that is not what you wanted to say.  Instead,
> you strip from the left as many as necessary and leave -N
> components that appear at the right-most end, no?
>

Yup, you're correct it should be 'leave -N components from the right-most
end'. Will change that.

>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -98,7 +98,8 @@ refname::
>>       abbreviation mode. If `lstrip=<N>` is appended, strips `<N>`
>>       slash-separated path components from the front of the refname
>>       (e.g., `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo`.
>> -     `<N>` must be a positive integer.
>> +     if `<N>` is a negative number, then only `<N>` path components
>> +     are left behind.
>
> I think positive <N> is so obvious not to require an example but it
> is good that you have one.  The negative <N> case needs illustration
> more than the positive case.  Perhaps something like:
>
>     (e.g. %(refname:lstrip=-1) strips components of refs/tags/frotz
>     from the left to leave only one component, i.e. 'frotz').

Good point, but i'll be using N = -2 rather than -1 since N=-1 can
also be obtained
by using N=2 as shown in the existing documentation. With N=-2 we differentiate
the use cases of N= positive and negative numbers.

diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 9123c6f..814d77a 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -99,7 +99,8 @@ refname::
        slash-separated path components from the front of the refname
        (e.g., `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo`.
        if `<N>` is a negative number, then only `<N>` path components
-       are left behind.
+       are left behind. (e.g., `%(refname:lstrip=-2)` turns
+       `refs/tags/foo` into `tags/foo`).


>
> Would %(refname:lstrip=-4) attempt to strip components of
> refs/tags/frotz from the left to leave only four components, and
> because the original does not have that many components, it ends
> with refs/tags/frotz?
>

It ends up with 'refs/tags/frotz' since there are not enough components.

> I am debating myself if we need something like "When the ref does
> not have enough components, the result becomes an empty string if
> stripping with positive <N>, or it becomes the full refname if
> stripping with negative <N>.  Neither is an error." is necessary
> here.  Or is it too obvious?

I had the same self-debate, and dropped it for being too obvious.

On Wed, Dec 28, 2016 at 8:38 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>
> I do not think it hurts to have, and makes this obvious.
>

But as Jacob mentioned, it doesn't hurt to mention the obvious
sometimes. So i'll
add that in :)

-- 
Regards,
Karthik Nayak

^ permalink raw reply related

* Re: HowTo distribute a hook with the reposity.
From: John P. Hartmann @ 2016-12-28  9:09 UTC (permalink / raw)
  To: Jeff King; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <20161228085248.tu54e6ug5fvpr26l@sigill.intra.peff.net>

Thanks, Peff, for your lucid answer to my question and much more.  All 
is now clear to me.

The problem I am grappling with is how to obtain the latest git commit 
hash and enter it into the generated code.  Configure/make would appear 
to the the time, but by then the user may not have git installed (e.g., 
extracted the project as .zip from github), so it needs to be done "back 
at the farm".

I hear that the best I can do is create a normal script in the repro and 
add to the developer handbook that "btw, here is a git hook that will 
run it automatically if you so choose".

Thank you both for your prompt and exhaustive answers.

	j.

On 28/12/16 08:52, Jeff King wrote:
> On Wed, Dec 28, 2016 at 08:42:25AM +0000, John P. Hartmann wrote:
>
>> This project is hosted on github.  If I put the hook into the repository
>> manually (if I can; I don't know that), is it true that the hook would be
>> distributed on a clone action, but not on a pull?
>
> I'm not sure what you mean by "into the repository". If you mean "into
> the .git directory", then no, you can't do that. Git will not add ".git"
> directory contents to a repository, you cannot manipulate the contents
> of ".git" directories on GitHub, and a client wouldn't ever look at them
> on clone or fetch anyway.
>
> Did you mean something else?
>
> -Peff
>

^ permalink raw reply

* Re: HowTo distribute a hook with the reposity.
From: Jeff King @ 2016-12-28  9:20 UTC (permalink / raw)
  To: John P. Hartmann; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <39886f48-c48f-6800-8aa4-20e0b2ab0e6d@gmail.com>

On Wed, Dec 28, 2016 at 09:09:04AM +0000, John P. Hartmann wrote:

> The problem I am grappling with is how to obtain the latest git commit hash
> and enter it into the generated code.  Configure/make would appear to the
> the time, but by then the user may not have git installed (e.g., extracted
> the project as .zip from github), so it needs to be done "back at the farm".
> 
> I hear that the best I can do is create a normal script in the repro and add
> to the developer handbook that "btw, here is a git hook that will run it
> automatically if you so choose".

Yes, if you want the information to be conveyed in a .zip from GitHub,
then you'll have to commit the value to the repository. For example,
the script in Git itself looks like this:

  https://github.com/git/git/blob/master/GIT-VERSION-GEN

We pull the value from the Git repository if we have one, then fallback
to a .version file which is generated when release tarballs are
generated, and then finally fall back to a baked-in DEF_VER variable
that is updated manually after each release (and that last is what you'd
get if you had, say, a .zip file from GitHub).

I'm not sure you would want to update that DEF_VER for _every_ commit,
though, even if you could do so with a hook. It would mean every commit
would update the version field. And of course it could not have the git
commit id, because that is generated from a hash of the contents that
includes the contents of that version field.

If you're mostly interested in building straight from GitHub .zip files,
those do include the commit id (or tag name) in the directory name, so
your Makefile could deduce it at runtime from that information.

-Peff

^ permalink raw reply

* Re: [PATCH v9 19/20] branch: use ref-filter printing APIs
From: Karthik Nayak @ 2016-12-28  9:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List, Jacob Keller, Ramsay Jones
In-Reply-To: <xmqqinq56phu.fsf@gitster.mtv.corp.google.com>

On Wed, Dec 28, 2016 at 2:47 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>  static char branch_colors[][COLOR_MAXLEN] = {
>> -     GIT_COLOR_RESET,
>> -     GIT_COLOR_NORMAL,       /* PLAIN */
>> -     GIT_COLOR_RED,          /* REMOTE */
>> -     GIT_COLOR_NORMAL,       /* LOCAL */
>> -     GIT_COLOR_GREEN,        /* CURRENT */
>> -     GIT_COLOR_BLUE,         /* UPSTREAM */
>> +     "%(color:reset)",
>> +     "%(color:reset)",       /* PLAIN */
>> +     "%(color:red)",         /* REMOTE */
>> +     "%(color:reset)",       /* LOCAL */
>> +     "%(color:green)",       /* CURRENT */
>> +     "%(color:blue)",        /* UPSTREAM */
>>  };
>
> The contents of this table is no longer tied to COLOR_MAXLEN.  The
> above entries used by default happen to be shorter, but it is
> misleading.
>
>         static const char *branch_colors[] = {
>         "%(color:reset)",
>         ...
>         };
>
> perhaps?
>
> More importantly, does this re-definition of the branch_colors[]
> work with custom colors filled in git_branch_config() by calling
> e.g. color_parse("red", branch_colors[BRANCH_COLOR_REMOTE]), where
> "red" and "remote" come from an existing configuration file?
>
>         [color "branch"]
>                 remote = red
>
> It obviously would not work if you changed the type of branch_colors[]
> because the color_parse() wants the caller to have allocated space
> of COLOR_MAXLEN.
>
> But if filling these ANSI escape sequence into the format works OK,
> then doesn't it tell us that we do not need to have this change to
> use "%(color:reset)" etc. as the new default values?

Good point, this would overwrite the existing configuration based setup
existing in builtin/branch.c.

I think it'd make sense to use the existing branch_colors[] definition without
any changes. That's mean that instead of using %(color:<color>). We hard
code the colors by calling  branch_get_color(). This is ok with me since,
users who which to have their own formats will anyways use --format option.

-- 
Regards,
Karthik Nayak

^ permalink raw reply

* git mergetool Segmentation fault
From: KES @ 2016-12-28 13:09 UTC (permalink / raw)
  To: git

I suppose this is some bug. Because `Segmentation fault` is not expected in any case

http://stackoverflow.com/questions/41362676/how-to-resolve-merge-conflict-while-rebasing

^ permalink raw reply

* [PATCHv2] pathspec: give better message for submodule related pathspec error
From: Stefan Beller @ 2016-12-28 17:17 UTC (permalink / raw)
  To: peff; +Cc: git, bmwill, Stefan Beller
In-Reply-To: <20161228055826.xu2gclwkvisbft6o@sigill.intra.peff.net>

Every once in a while someone complains to the mailing list to have
run into this weird assertion[1].

The usual response from the mailing list is link to old discussions[2],
and acknowledging the problem stating it is known.

For now just improve the user visible error message.

[1] https://www.google.com/search?q=item-%3Enowildcard_len
[2] http://git.661346.n2.nabble.com/assert-failed-in-submodule-edge-case-td7628687.html
    https://www.spinics.net/lists/git/msg249473.html

Signed-off-by: Stefan Beller <sbeller@google.com>
---

Peff wrote:
> Don't you need to flip the logic here? An assert() triggers when the
> condition is not true, but an "if" does the opposite. So "assert(X)"
> should always become "if (!X) die(...)".

Duh! and it should compile as well. 

Thanks,
Stefan

 pathspec.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/pathspec.c b/pathspec.c
index 22ca74a126..4724d522f2 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -313,8 +313,11 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
 	}
 
 	/* sanity checks, pathspec matchers assume these are sane */
-	assert(item->nowildcard_len <= item->len &&
-	       item->prefix         <= item->len);
+	if (item->nowildcard_len > item->len ||
+	    item->prefix         > item->len)
+		die (_("Path leads inside submodule '%s', but the submodule "
+		       "was not recognized, i.e. not initialized or deleted"),
+		       item->original);
 	return magic;
 }
 
-- 
2.11.0.196.gee862f456e.dirty


^ permalink raw reply related

* Re: [RFH] gpg --import entropy while running tests
From: Theodore Ts'o @ 2016-12-28 16:05 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20161228083930.5li6cb6yplusc26m@sigill.intra.peff.net>

On Wed, Dec 28, 2016 at 03:39:30AM -0500, Jeff King wrote:
> >   https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=commit;h=4473db1ef24031ff4e26c9a9de95dbe898ed2b97
> > 
> > So this does seem like a gpg bug.
> 
> I've submitted a bug report to gpg:
> 
>   https://bugs.gnupg.org/gnupg/issue2897
> 
> so we'll see what they say.

Yeah, they are definitely doing something very.... hard to explain.

Pid 8348 is the gpg-agent process which the main gpg program (pid
8344) connected to.  It starts trying to get randomness in response to
a KEYWRAP command:

8348  10:58:57.882909 access("/dev/random", R_OK) = 0
8348  10:58:57.883205 access("/dev/urandom", R_OK) = 0
8348  10:58:57.883472 open("/dev/urandom", O_RDONLY) = 9
8348  10:58:57.883729 fcntl(9, F_GETFD) = 0
8348  10:58:57.883914 fcntl(9, F_SETFD, FD_CLOEXEC) = 0

It opens /dev/urandom, but then never uses fd 9 ever again.  Instead,
it uses getrandom, but in a pretty silly fashion, with lots of sleeps
in between, and not between each progress report, either:

8348  10:58:57.884129 write(8, "S PROGRESS need_entropy X 30 120", 32 <unfinished ...>
8344  10:58:57.884338 <... read resumed> "S PROGRESS need_entropy X 30 120", 1002) = 32
8348  10:58:57.884424 <... write resumed> ) = 32
8344  10:58:57.884488 read(5,  <unfinished ...>
8348  10:58:57.884550 write(8, "\n", 1 <unfinished ...>
8344  10:58:57.884715 <... read resumed> "\n", 970) = 1
8348  10:58:57.884800 <... write resumed> ) = 1
8344  10:58:57.884883 read(5,  <unfinished ...>
8348  10:58:57.884951 nanosleep({0, 100000000}, NULL) = 0
8348  10:58:57.985363 select(10, [9], NULL, NULL, {0, 100000}) = 1 (in [9], left {0, 99994})
8348  10:58:57.985593 getrandom("&\275\354^\256\320\3w\21:R]`eJ\t\t\350\245\202>\255\237\324\324\340\24^c\323\210\376"..., 90, 0) = 90
8348  10:58:57.985751 write(8, "S PROGRESS need_entropy X 120 12"..., 33) = 33
8344  10:58:57.985885 <... read resumed> "S PROGRESS need_entropy X 120 12"..., 1002) = 33
8348  10:58:57.985934 write(8, "\n", 1 <unfinished ...>
8344  10:58:57.985982 read(5,  <unfinished ...>
8348  10:58:57.986015 <... write resumed> ) = 1
8344  10:58:57.986048 <... read resumed> "\n", 969) = 1
8348  10:58:57.986090 nanosleep({0, 100000000},  <unfinished ...>
8344  10:58:57.986142 read(5,  <unfinished ...>
8348  10:58:58.086253 <... nanosleep resumed> NULL) = 0
8348  10:58:58.086370 write(8, "S PROGRESS need_entropy X 30 120", 32) = 32
8344  10:58:58.086502 <... read resumed> "S PROGRESS need_entropy X 30 120", 1002) = 32
8348  10:58:58.086541 write(8, "\n", 1 <unfinished ...>
8344  10:58:58.086579 read(5,  <unfinished ...>
8348  10:58:58.086604 <... write resumed> ) = 1
8344  10:58:58.086630 <... read resumed> "\n", 970) = 1
8348  10:58:58.086661 nanosleep({0, 100000000},  <unfinished ...>
8344  10:58:58.086703 read(5,  <unfinished ...>
8348  10:58:58.186815 <... nanosleep resumed> NULL) = 0
8348  10:58:58.186894 select(10, [9], NULL, NULL, {0, 100000}) = 1 (in [9], left {0, 99995})
8348  10:58:58.187038 getrandom("\365\221\374m\360\235\27\330\264\223\365\363<6\302\324F\5\354Q|,\366\253\337u\226\265\345\250CA"..., 90, 0) = 90

The worst part of this is that the commit description claims this is a
workaround for libgcrypt using /dev/random, but it's not using
/dev/random --- it's using getrandom, and it pointlessly opened
/dev/urandom first (having never opened /dev/random).

This looks like a classic case of Lotus Notes / Websphere disease ---
to many d*mned layers of abstraction....

						- Ted

^ permalink raw reply

* [PATCH] contrib: remove gitview
From: Stefan Beller @ 2016-12-28 17:28 UTC (permalink / raw)
  To: peff; +Cc: git, jvoss, Stefan Beller
In-Reply-To: <20161228064255.f4akjdsq24r2hqn7@sigill.intra.peff.net>

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 related

* [PATCH] am: add am.signoff add config variable
From: Eduardo Habkost @ 2016-12-28 17:40 UTC (permalink / raw)
  To: git; +Cc: Paul Tan

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>
---
 builtin/am.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/am.c b/builtin/am.c
index 31fb605..d2e0233 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);
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH] am: add am.signoff add config variable
From: Stefan Beller @ 2016-12-28 17:45 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: git@vger.kernel.org, Paul Tan
In-Reply-To: <1482946838-28779-1-git-send-email-ehabkost@redhat.com>

On Wed, Dec 28, 2016 at 9:40 AM, 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 think this is a good idea (from a design standpoint and what the user needs).

Just like e97a5e765d (git-am: add am.threeWay config variable), we'd
prefer if you'd
also update Documentation/config.txt as well as a new test. :)

Thanks,
Stefan

^ permalink raw reply

* Re: [PATCH] am: add am.signoff add config variable
From: Eduardo Habkost @ 2016-12-28 17:50 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git@vger.kernel.org, Paul Tan
In-Reply-To: <CAGZ79kbRMYyaOmuqymx9dsLGdvX+iM9OMMQtQGS=uA+dO6_MVQ@mail.gmail.com>

On Wed, Dec 28, 2016 at 09:45:24AM -0800, Stefan Beller wrote:
> On Wed, Dec 28, 2016 at 9:40 AM, 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 think this is a good idea (from a design standpoint and what the user needs).
> 
> Just like e97a5e765d (git-am: add am.threeWay config variable), we'd
> prefer if you'd
> also update Documentation/config.txt as well as a new test. :)

Sorry, I was using commit e97a5e765d as reference when adding the
new option, but I was looking at "git log -p builtin/am.c" and
didn't see the rest of commit. :)

I will send a new version with the appropriate documentation and
test code.

-- 
Eduardo

^ permalink raw reply

* [PATCH] contrib: remove git-convert-objects
From: Stefan Beller @ 2016-12-28 18:02 UTC (permalink / raw)
  To: gitster; +Cc: git, torvalds, Stefan Beller

git-convert-objects, originally named git-convert-cache was used in
early 2005 to convert to a new repository format, e.g. adding an author
date.

By now the need for conversion of the very early repositories is less
relevant, we no longer need to keep it in contrib; remove it.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 contrib/convert-objects/convert-objects.c       | 329 ------------------------
 contrib/convert-objects/git-convert-objects.txt |  29 ---
 2 files changed, 358 deletions(-)
 delete mode 100644 contrib/convert-objects/convert-objects.c
 delete mode 100644 contrib/convert-objects/git-convert-objects.txt

diff --git a/contrib/convert-objects/convert-objects.c b/contrib/convert-objects/convert-objects.c
deleted file mode 100644
index f3b57bf1d2..0000000000
--- a/contrib/convert-objects/convert-objects.c
+++ /dev/null
@@ -1,329 +0,0 @@
-#include "cache.h"
-#include "blob.h"
-#include "commit.h"
-#include "tree.h"
-
-struct entry {
-	unsigned char old_sha1[20];
-	unsigned char new_sha1[20];
-	int converted;
-};
-
-#define MAXOBJECTS (1000000)
-
-static struct entry *convert[MAXOBJECTS];
-static int nr_convert;
-
-static struct entry * convert_entry(unsigned char *sha1);
-
-static struct entry *insert_new(unsigned char *sha1, int pos)
-{
-	struct entry *new = xcalloc(1, sizeof(struct entry));
-	hashcpy(new->old_sha1, sha1);
-	memmove(convert + pos + 1, convert + pos, (nr_convert - pos) * sizeof(struct entry *));
-	convert[pos] = new;
-	nr_convert++;
-	if (nr_convert == MAXOBJECTS)
-		die("you're kidding me - hit maximum object limit");
-	return new;
-}
-
-static struct entry *lookup_entry(unsigned char *sha1)
-{
-	int low = 0, high = nr_convert;
-
-	while (low < high) {
-		int next = (low + high) / 2;
-		struct entry *n = convert[next];
-		int cmp = hashcmp(sha1, n->old_sha1);
-		if (!cmp)
-			return n;
-		if (cmp < 0) {
-			high = next;
-			continue;
-		}
-		low = next+1;
-	}
-	return insert_new(sha1, low);
-}
-
-static void convert_binary_sha1(void *buffer)
-{
-	struct entry *entry = convert_entry(buffer);
-	hashcpy(buffer, entry->new_sha1);
-}
-
-static void convert_ascii_sha1(void *buffer)
-{
-	unsigned char sha1[20];
-	struct entry *entry;
-
-	if (get_sha1_hex(buffer, sha1))
-		die("expected sha1, got '%s'", (char *) buffer);
-	entry = convert_entry(sha1);
-	memcpy(buffer, sha1_to_hex(entry->new_sha1), 40);
-}
-
-static unsigned int convert_mode(unsigned int mode)
-{
-	unsigned int newmode;
-
-	newmode = mode & S_IFMT;
-	if (S_ISREG(mode))
-		newmode |= (mode & 0100) ? 0755 : 0644;
-	return newmode;
-}
-
-static int write_subdirectory(void *buffer, unsigned long size, const char *base, int baselen, unsigned char *result_sha1)
-{
-	char *new = xmalloc(size);
-	unsigned long newlen = 0;
-	unsigned long used;
-
-	used = 0;
-	while (size) {
-		int len = 21 + strlen(buffer);
-		char *path = strchr(buffer, ' ');
-		unsigned char *sha1;
-		unsigned int mode;
-		char *slash, *origpath;
-
-		if (!path || strtoul_ui(buffer, 8, &mode))
-			die("bad tree conversion");
-		mode = convert_mode(mode);
-		path++;
-		if (memcmp(path, base, baselen))
-			break;
-		origpath = path;
-		path += baselen;
-		slash = strchr(path, '/');
-		if (!slash) {
-			newlen += sprintf(new + newlen, "%o %s", mode, path);
-			new[newlen++] = '\0';
-			hashcpy((unsigned char *)new + newlen, (unsigned char *) buffer + len - 20);
-			newlen += 20;
-
-			used += len;
-			size -= len;
-			buffer = (char *) buffer + len;
-			continue;
-		}
-
-		newlen += sprintf(new + newlen, "%o %.*s", S_IFDIR, (int)(slash - path), path);
-		new[newlen++] = 0;
-		sha1 = (unsigned char *)(new + newlen);
-		newlen += 20;
-
-		len = write_subdirectory(buffer, size, origpath, slash-origpath+1, sha1);
-
-		used += len;
-		size -= len;
-		buffer = (char *) buffer + len;
-	}
-
-	write_sha1_file(new, newlen, tree_type, result_sha1);
-	free(new);
-	return used;
-}
-
-static void convert_tree(void *buffer, unsigned long size, unsigned char *result_sha1)
-{
-	void *orig_buffer = buffer;
-	unsigned long orig_size = size;
-
-	while (size) {
-		size_t len = 1+strlen(buffer);
-
-		convert_binary_sha1((char *) buffer + len);
-
-		len += 20;
-		if (len > size)
-			die("corrupt tree object");
-		size -= len;
-		buffer = (char *) buffer + len;
-	}
-
-	write_subdirectory(orig_buffer, orig_size, "", 0, result_sha1);
-}
-
-static unsigned long parse_oldstyle_date(const char *buf)
-{
-	char c, *p;
-	char buffer[100];
-	struct tm tm;
-	const char *formats[] = {
-		"%c",
-		"%a %b %d %T",
-		"%Z",
-		"%Y",
-		" %Y",
-		NULL
-	};
-	/* We only ever did two timezones in the bad old format .. */
-	const char *timezones[] = {
-		"PDT", "PST", "CEST", NULL
-	};
-	const char **fmt = formats;
-
-	p = buffer;
-	while (isspace(c = *buf))
-		buf++;
-	while ((c = *buf++) != '\n')
-		*p++ = c;
-	*p++ = 0;
-	buf = buffer;
-	memset(&tm, 0, sizeof(tm));
-	do {
-		const char *next = strptime(buf, *fmt, &tm);
-		if (next) {
-			if (!*next)
-				return mktime(&tm);
-			buf = next;
-		} else {
-			const char **p = timezones;
-			while (isspace(*buf))
-				buf++;
-			while (*p) {
-				if (!memcmp(buf, *p, strlen(*p))) {
-					buf += strlen(*p);
-					break;
-				}
-				p++;
-			}
-		}
-		fmt++;
-	} while (*buf && *fmt);
-	printf("left: %s\n", buf);
-	return mktime(&tm);
-}
-
-static int convert_date_line(char *dst, void **buf, unsigned long *sp)
-{
-	unsigned long size = *sp;
-	char *line = *buf;
-	char *next = strchr(line, '\n');
-	char *date = strchr(line, '>');
-	int len;
-
-	if (!next || !date)
-		die("missing or bad author/committer line %s", line);
-	next++; date += 2;
-
-	*buf = next;
-	*sp = size - (next - line);
-
-	len = date - line;
-	memcpy(dst, line, len);
-	dst += len;
-
-	/* Is it already in new format? */
-	if (isdigit(*date)) {
-		int datelen = next - date;
-		memcpy(dst, date, datelen);
-		return len + datelen;
-	}
-
-	/*
-	 * Hacky hacky: one of the sparse old-style commits does not have
-	 * any date at all, but we can fake it by using the committer date.
-	 */
-	if (*date == '\n' && strchr(next, '>'))
-		date = strchr(next, '>')+2;
-
-	return len + sprintf(dst, "%lu -0700\n", parse_oldstyle_date(date));
-}
-
-static void convert_date(void *buffer, unsigned long size, unsigned char *result_sha1)
-{
-	char *new = xmalloc(size + 100);
-	unsigned long newlen = 0;
-
-	/* "tree <sha1>\n" */
-	memcpy(new + newlen, buffer, 46);
-	newlen += 46;
-	buffer = (char *) buffer + 46;
-	size -= 46;
-
-	/* "parent <sha1>\n" */
-	while (!memcmp(buffer, "parent ", 7)) {
-		memcpy(new + newlen, buffer, 48);
-		newlen += 48;
-		buffer = (char *) buffer + 48;
-		size -= 48;
-	}
-
-	/* "author xyz <xyz> date" */
-	newlen += convert_date_line(new + newlen, &buffer, &size);
-	/* "committer xyz <xyz> date" */
-	newlen += convert_date_line(new + newlen, &buffer, &size);
-
-	/* Rest */
-	memcpy(new + newlen, buffer, size);
-	newlen += size;
-
-	write_sha1_file(new, newlen, commit_type, result_sha1);
-	free(new);
-}
-
-static void convert_commit(void *buffer, unsigned long size, unsigned char *result_sha1)
-{
-	void *orig_buffer = buffer;
-	unsigned long orig_size = size;
-
-	if (memcmp(buffer, "tree ", 5))
-		die("Bad commit '%s'", (char *) buffer);
-	convert_ascii_sha1((char *) buffer + 5);
-	buffer = (char *) buffer + 46;    /* "tree " + "hex sha1" + "\n" */
-	while (!memcmp(buffer, "parent ", 7)) {
-		convert_ascii_sha1((char *) buffer + 7);
-		buffer = (char *) buffer + 48;
-	}
-	convert_date(orig_buffer, orig_size, result_sha1);
-}
-
-static struct entry * convert_entry(unsigned char *sha1)
-{
-	struct entry *entry = lookup_entry(sha1);
-	enum object_type type;
-	void *buffer, *data;
-	unsigned long size;
-
-	if (entry->converted)
-		return entry;
-	data = read_sha1_file(sha1, &type, &size);
-	if (!data)
-		die("unable to read object %s", sha1_to_hex(sha1));
-
-	buffer = xmalloc(size);
-	memcpy(buffer, data, size);
-
-	if (type == OBJ_BLOB) {
-		write_sha1_file(buffer, size, blob_type, entry->new_sha1);
-	} else if (type == OBJ_TREE)
-		convert_tree(buffer, size, entry->new_sha1);
-	else if (type == OBJ_COMMIT)
-		convert_commit(buffer, size, entry->new_sha1);
-	else
-		die("unknown object type %d in %s", type, sha1_to_hex(sha1));
-	entry->converted = 1;
-	free(buffer);
-	free(data);
-	return entry;
-}
-
-int main(int argc, char **argv)
-{
-	unsigned char sha1[20];
-	struct entry *entry;
-
-	setup_git_directory();
-
-	if (argc != 2)
-		usage("git-convert-objects <sha1>");
-	if (get_sha1(argv[1], sha1))
-		die("Not a valid object name %s", argv[1]);
-
-	entry = convert_entry(sha1);
-	printf("new sha1: %s\n", sha1_to_hex(entry->new_sha1));
-	return 0;
-}
diff --git a/contrib/convert-objects/git-convert-objects.txt b/contrib/convert-objects/git-convert-objects.txt
deleted file mode 100644
index f871880cfb..0000000000
--- a/contrib/convert-objects/git-convert-objects.txt
+++ /dev/null
@@ -1,29 +0,0 @@
-git-convert-objects(1)
-======================
-
-NAME
-----
-git-convert-objects - Converts old-style git repository
-
-
-SYNOPSIS
---------
-[verse]
-'git-convert-objects'
-
-DESCRIPTION
------------
-Converts old-style git repository to the latest format
-
-
-Author
-------
-Written by Linus Torvalds <torvalds@osdl.org>
-
-Documentation
---------------
-Documentation by David Greaves, Junio C Hamano and the git-list <git@vger.kernel.org>.
-
-GIT
----
-Part of the linkgit:git[7] suite
-- 
2.11.0.259.ga95e92af08


^ permalink raw reply related

* [PATCH v4 2/2] repack: die on incremental + write-bitmap-index
From: David Turner @ 2016-12-28 18:12 UTC (permalink / raw)
  To: git, peff; +Cc: David Turner
In-Reply-To: <1482948720-21488-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 +++-----
 t/t6500-gc.sh           | 8 ++++----
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index 80dd06b..9c3dd09 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' '
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index def2aca..1762dfa 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -54,15 +54,15 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre
 	test_commit 410 &&
 	# Our first gc will create a pack; our second will create a second pack
 	git gc --auto &&
-	ls .git/objects/pack | grep -v bitmap | sort >existing_packs &&
+	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/ | grep -v bitmap | sort >post_packs &&
-	comm --output-delimiter , -1 -3 existing_packs post_packs >new &&
-	comm --output-delimiter , -2 -3 existing_packs post_packs >del &&
+	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
 '
-- 
2.8.0.rc4.22.g8ae061a


^ permalink raw reply related

* [PATCH v4 1/2] auto gc: don't write bitmaps for incremental repacks
From: David Turner @ 2016-12-28 18:11 UTC (permalink / raw)
  To: git, peff; +Cc: David Turner

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..def2aca 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 | grep -v bitmap | sort >existing_packs &&
+	test_commit 523 &&
+	test_commit 790 &&
+
+	git gc --auto 2>err &&
+	test_i18ngrep ! "^warning:" err &&
+	ls .git/objects/pack/ | grep -v bitmap | sort >post_packs &&
+	comm --output-delimiter , -1 -3 existing_packs post_packs >new &&
+	comm --output-delimiter , -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

* Re: [PATCH] pathspec: give better message for submodule related pathspec error
From: Brandon Williams @ 2016-12-28 18:13 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git
In-Reply-To: <20161228000559.17842-1-sbeller@google.com>

On 12/27, Stefan Beller wrote:
> Every once in a while someone complains to the mailing list to have
> run into this weird assertion[1].
> 
> The usual response from the mailing list is link to old discussions[2],
> and acknowledging the problem stating it is known.
> 
> For now just improve the user visible error message.
> 
> [1] https://www.google.com/search?q=item-%3Enowildcard_len
> [2] http://git.661346.n2.nabble.com/assert-failed-in-submodule-edge-case-td7628687.html
>     https://www.spinics.net/lists/git/msg249473.html
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> 
> If you were following the mailing list closely today, you may sense
> that I am cleaning up stalled branches. :)
> 
> I think such a hot fix is warranted given how often we had reports
> on the mailing list.
> 
> Thanks,
> Stefan
> 
>  pathspec.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/pathspec.c b/pathspec.c
> index 22ca74a126..d522f43331 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -313,8 +313,11 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>  	}
>  
>  	/* sanity checks, pathspec matchers assume these are sane */
> -	assert(item->nowildcard_len <= item->len &&
> -	       item->prefix         <= item->len);
> +	if (item->nowildcard_len <= item->len &&
> +	    item->prefix         <= item->len)
> +		die (_("Path leads inside submodule '%s', but the submodule "
> +		       "was not recognized, i.e. not initialized or deleted"),
> +		       ce->name);
>  	return magic;

I haven't been following everything on the list these past couple days,
but are we sure this is caused by submodules?  Also variable 'ce'
shouldn't be in scope here.

-- 
Brandon Williams

^ permalink raw reply

* Re: [PATCHv2] pathspec: give better message for submodule related pathspec error
From: Brandon Williams @ 2016-12-28 18:15 UTC (permalink / raw)
  To: Stefan Beller; +Cc: peff, git
In-Reply-To: <20161228171746.22859-1-sbeller@google.com>

On 12/28, Stefan Beller wrote:
> Every once in a while someone complains to the mailing list to have
> run into this weird assertion[1].
> 
> The usual response from the mailing list is link to old discussions[2],
> and acknowledging the problem stating it is known.
> 
> For now just improve the user visible error message.
> 
> [1] https://www.google.com/search?q=item-%3Enowildcard_len
> [2] http://git.661346.n2.nabble.com/assert-failed-in-submodule-edge-case-td7628687.html
>     https://www.spinics.net/lists/git/msg249473.html
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> 
> Peff wrote:
> > Don't you need to flip the logic here? An assert() triggers when the
> > condition is not true, but an "if" does the opposite. So "assert(X)"
> > should always become "if (!X) die(...)".
> 
> Duh! and it should compile as well. 
> 
> Thanks,
> Stefan
> 
>  pathspec.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/pathspec.c b/pathspec.c
> index 22ca74a126..4724d522f2 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -313,8 +313,11 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
>  	}
>  
>  	/* sanity checks, pathspec matchers assume these are sane */
> -	assert(item->nowildcard_len <= item->len &&
> -	       item->prefix         <= item->len);
> +	if (item->nowildcard_len > item->len ||
> +	    item->prefix         > item->len)
> +		die (_("Path leads inside submodule '%s', but the submodule "
> +		       "was not recognized, i.e. not initialized or deleted"),
> +		       item->original);
>  	return magic;
>  }

Turns out I should comment on the most recent version of the patch :P
This looks better to me. (It resolves the issue with using a variable
not in scope).

-- 
Brandon Williams

^ permalink raw reply

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

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?

-- 
Brandon Williams

^ permalink raw reply

* [PATCH v2] am: add am.signoff add config variable
From: Eduardo Habkost @ 2016-12-28 18:35 UTC (permalink / raw)
  To: git; +Cc: Paul Tan, 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
---
 Documentation/config.txt |  5 +++++
 Documentation/git-am.txt |  6 ++++--
 builtin/am.c             |  2 ++
 t/t4150-am.sh            | 24 ++++++++++++++++++++++++
 4 files changed, 35 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..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::
 	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..41b5481c9 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -479,6 +479,30 @@ 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 $(git cat-file commit HEAD | grep -c "Signed-off-by:") -eq 0
+'
+
+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

* Re: [PATCH v2] am: add am.signoff add config variable
From: Stefan Beller @ 2016-12-28 18:51 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: git@vger.kernel.org, Paul Tan
In-Reply-To: <20161228183501.15068-1-ehabkost@redhat.com>

On Wed, Dec 28, 2016 at 10:35 AM, 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.
>
> 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

Thanks!
Documentation and code looks good to me, for the test a small nit below.

> +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 &&

"expected" is never read in this test, so we can omit this line?

> +       git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&

So we check if the previous commit is not tampered with,

> +       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

I would have suggested to grep for $signoff instead of "Signed-off-by:",
but it turns out being fuzzy here is better and would also catch e.g.
a broken sign off.

> +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
> +'

This test looks good to me,

Thanks,
Stefan

^ permalink raw reply

* Re: HowTo distribute a hook with the reposity.
From: Jacob Keller @ 2016-12-28 18:53 UTC (permalink / raw)
  To: Jeff King; +Cc: John P. Hartmann, Git mailing list
In-Reply-To: <20161228060840.gelgcs2hd33id56j@sigill.intra.peff.net>

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!

Regards,
Jake

> -Peff

^ permalink raw reply

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

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.

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: Eduardo Habkost @ 2016-12-28 19:11 UTC (permalink / raw)
  To: Stefan Beller; +Cc: git@vger.kernel.org, Paul Tan
In-Reply-To: <CAGZ79kaBpC5ym2N_fMZHDmL4gGpU8pFAsupCE4aTdENh+=z72g@mail.gmail.com>

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:
> > 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
> 
> Thanks!
> Documentation and code looks good to me, for the test a small nit below.
> 
> > +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 &&
> 
> "expected" is never read in this test, so we can omit this line?
> 

Oops, I have deleted a "test_cmp expected actual" line by mistake.

> > +       git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
> 
> So we check if the previous commit is not tampered with,

We do, but only if we have a "test_cmp expected actual" line
here.

Fixed by:

diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 41b5481c9..d4b6a832f 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -486,6 +486,7 @@ test_expect_success '--no-signoff overrides am.signoff' '
 	git am --no-signoff <patch2 &&
 	printf "%s\n" "$signoff" >expected &&
 	git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
+	test_cmp expected actual &&
 	test $(git cat-file commit HEAD | grep -c "Signed-off-by:") -eq 0
 '
 
> 
> > +       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 would have suggested to grep for $signoff instead of "Signed-off-by:",
> but it turns out being fuzzy here is better and would also catch e.g.
> a broken sign off.

Yes, I want to ensure no extra Signed-off-by line is present
except for $signof (that is already present in the original
e-mail).

> 
> > +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
> > +'
> 
> This test looks good to me,
> 
> Thanks,
> Stefan

-- 
Eduardo

^ permalink raw reply related

* 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


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