Re: [PATCH v3] am: add am.signoff add config variable

[Andreas Schwab]

On Dez 28 2016, Eduardo Habkost  wrote:

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

That's one dash too much.

Andreas.

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

Re: [PATCH v5 0/2] repack (oops)

[Jeff King]

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

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

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

-Peff

Re: [PATCH] contrib: remove gitview

[Jeff King]

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

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

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

-Peff

[PATCH] unpack-trees: move checkout state into check_updates

[Stefan Beller]

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

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

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

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

[PATCH v3] am: add am.signoff add config variable

[Eduardo Habkost]

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 
---
Changes v1 -> v2:
* Added documentation to Documentation/git-am.txt and
  Documentation/config.txt
* Added test cases to t4150-am.sh

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

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 30cb94610..6b2990203 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -822,6 +822,11 @@ am.keepcr::
by giving `--no-keep-cr` from the command line.
See linkgit:git-am[1], linkgit:git-mailsplit[1].
 
+am.signoff::
+   If true, git-am will add a `Signed-off-by:` line to the commit
+   message. See the signoff option in linkgit:git-commit[1] for
+   more information.
+
 am.threeWay::
By default, `git am` will fail if the patch does not apply cleanly. When
set to true, this setting tells `git am` to fall back on 3-way merge if
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 12879e402..1f14986c7 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -9,7 +9,7 @@ git-am - Apply a series of patches from a mailbox
 SYNOPSIS
 
 [verse]
-'git am' [--signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8]
+'git am' [--[no-]signoff] [--keep] [--[no-]keep-cr] [--[no-]utf8]
 [--[no-]3way] [--interactive] [--committer-date-is-author-date]
 [--ignore-date] [--ignore-space-change | --ignore-whitespace]
 [--whitespace=] [-C] [-p] [--directory=]
@@ -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", >message_id);
 
+   git_config_get_bool("am.signoff", >signoff);
+
state->scissors = SCISSORS_UNSET;
 
argv_array_init(>git_apply_opts);
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 89a5bacac..d65c8e5c4 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -479,6 +479,32 @@ test_expect_success 'am --signoff adds Signed-off-by: 
line' '
test_cmp expected actual
 '
 
+test_expect_success '--no-signoff overrides am.signoff' '
+   rm -fr .git/rebase-apply &&
+   git reset --hard first &&
+   test_config am.signoff true &&
+   git am --no-signoff expected &&
+   git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
+   test_cmp expected actual &&
+   git cat-file commit HEAD > actual &&
+   ! grep -q "Signed-off-by:" actual
+'
+
+test_expect_success 'am.signoff adds Signed-off-by: line' '
+   rm -fr .git/rebase-apply &&
+   git reset --hard first &&
+   test_config am.signoff true &&
+   git am 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

[PATCH v5 2/2] repack: die on incremental + write-bitmap-index

[David Turner]

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

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

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

[PATCH v5 1/2] auto gc: don't write bitmaps for incremental repacks

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

[PATCH v5 0/2] repack (oops)

[David Turner]

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

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

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

-- 
2.8.0.rc4.22.g8ae061a

Re: [PATCH v4 2/2] repack: die on incremental + write-bitmap-index

[Johannes Sixt]

Am 28.12.2016 um 19:12 schrieb David Turner:

+static const char incremental_bitmap_conflict_error[] = N_(
+"Incremental repacks are incompatible with bitmap indexes.  Use \n"


The SP before LF could be removed.


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


The string is marked for translation here...


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

+   if (write_bitmaps && !(pack_everything & ALL_INTO_ONE))
+   die(incremental_bitmap_conflict_error);


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

die(_(incremental_bitmap_conflict_error));

-- Hannes

Re: [PATCH] contrib: remove gitview

[Javantea]

Dear Stefan,

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

Regards,
Javantea

On Wed, 28 Dec 2016 09:28:37 -0800, Stefan Beller  wrote:
>gitview did not have meaningful contributions since 2007, which gives the
>impression it is either a mature or dead project.
>
>In both cases we should not carry it in git.git as the README for contrib
>states we only want to carry experimental things to give early exposure.
>
>Recently a security vulnerability was reported by Javantea, so the decision
>to either fix the issue or remove the code in question becomes a bit
>more urgent.
>
>Reported-by: Javantea 
>Signed-off-by: Stefan Beller 
>---
> 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..00
>--- 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 
>-"""
>-__copyright__ = "Copyright (C) 2006 Hewlett-Packard Development Company, L.P."
>-__copyright__ = "Copyright (C) 2007 Aneesh Kumar K.V -__author__= "Aneesh Kumar K.V "
>-
>-
>-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.*) (?P\d+) 
>(?P[+-]\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",
>-

Re: [PATCH v2] am: add am.signoff add config variable

[Stefan Beller]

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

Uh, no please.

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

For grep just use !

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

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

Thanks,
Stefan

Re: [PATCH v2] am: add am.signoff add config variable

[Eduardo Habkost]

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

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

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

-- 
Eduardo

Re: [PATCH v2] am: add am.signoff add config variable

[Eduardo Habkost]

On Wed, Dec 28, 2016 at 08:07:54PM +0100, Andreas Schwab wrote:
> On Dez 28 2016, Eduardo Habkost  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=] [-C] [-p] [--directory=]
> > @@ -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

Re: [PATCH v2] am: add am.signoff add config variable

[Eduardo Habkost]

On Wed, Dec 28, 2016 at 10:51:28AM -0800, Stefan Beller wrote:
> On Wed, Dec 28, 2016 at 10:35 AM, Eduardo Habkost  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 
> > ---
> > 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  > +   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 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 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  > +   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

Re: [PATCH v2] am: add am.signoff add config variable

[Andreas Schwab]

On Dez 28 2016, Eduardo Habkost  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=] [-C] [-p] [--directory=]
> @@ -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, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: HowTo distribute a hook with the reposity.

[Jacob Keller]

On Tue, Dec 27, 2016 at 10:08 PM, Jeff King  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

Re: [PATCH v2] am: add am.signoff add config variable

[Stefan Beller]

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

[PATCH v2] am: add am.signoff add config variable

[Eduardo Habkost]

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 
---
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=] [-C] [-p] [--directory=]
@@ -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", >message_id);
 
+   git_config_get_bool("am.signoff", >signoff);
+
state->scissors = SCISSORS_UNSET;
 
argv_array_init(>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 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 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

Re: What's cooking in git.git (Dec 2016, #08; Tue, 27)

[Brandon Williams]

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

Re: [PATCHv2] pathspec: give better message for submodule related pathspec error

[Brandon Williams]

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

Re: [PATCH] pathspec: give better message for submodule related pathspec error

[Brandon Williams]

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

[PATCH v4 1/2] auto gc: don't write bitmaps for incremental repacks

[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 
Signed-off-by: Jeff King 
---
 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(, "--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

[PATCH v4 2/2] repack: die on incremental + write-bitmap-index

[David Turner]

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

Signed-off-by: David Turner 
---
 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

[PATCH] contrib: remove git-convert-objects

[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 
---
 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..00
--- 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 (100)
-
-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, ))
-   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 

Re: [PATCH] am: add am.signoff add config variable

[Eduardo Habkost]

On Wed, Dec 28, 2016 at 09:45:24AM -0800, Stefan Beller wrote:
> On Wed, Dec 28, 2016 at 9:40 AM, Eduardo Habkost  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

Re: [PATCH] am: add am.signoff add config variable

[Stefan Beller]

On Wed, Dec 28, 2016 at 9:40 AM, Eduardo Habkost  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

[PATCH] am: add am.signoff add config variable

[Eduardo Habkost]

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 
---
 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", >message_id);
 
+   git_config_get_bool("am.signoff", >signoff);
+
state->scissors = SCISSORS_UNSET;
 
argv_array_init(>git_apply_opts);
-- 
2.7.4

[PATCH] contrib: remove gitview

[Stefan Beller]

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 
Signed-off-by: Stefan Beller 
---
 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..00
--- 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 
-"""
-__copyright__ = "Copyright (C) 2006 Hewlett-Packard Development Company, L.P."
-__copyright__ = "Copyright (C) 2007 Aneesh Kumar K.V 

Re: [RFH] gpg --import entropy while running tests

[Theodore Ts'o]

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 

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,  
8348  10:58:57.884550 write(8, "\n", 1 
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,  
8348  10:58:57.884951 nanosleep({0, 1}, NULL) = 0
8348  10:58:57.985363 select(10, [9], NULL, NULL, {0, 10}) = 1 (in [9], 
left {0, 4})
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 
8344  10:58:57.985982 read(5,  
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, 1},  
8344  10:58:57.986142 read(5,  
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 
8344  10:58:58.086579 read(5,  
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, 1},  
8344  10:58:58.086703 read(5,  
8348  10:58:58.186815 <... nanosleep resumed> NULL) = 0
8348  10:58:58.186894 select(10, [9], NULL, NULL, {0, 10}) = 1 (in [9], 
left {0, 5})
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

[PATCHv2] pathspec: give better message for submodule related pathspec error

[Stefan Beller]

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

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

git mergetool Segmentation fault

[KES]

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

Re: [PATCH v9 19/20] branch: use ref-filter printing APIs

[Karthik Nayak]

On Wed, Dec 28, 2016 at 2:47 AM, Junio C Hamano  wrote:
> Karthik Nayak  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:). 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

Re: HowTo distribute a hook with the reposity.

[Jeff King]

On Wed, Dec 28, 2016 at 09:09:04AM +, 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

Re: HowTo distribute a hook with the reposity.

[John P. Hartmann]

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

Re: [PATCH v9 15/20] ref-filter: modify the 'lstrip=' option to work with negative ''

[Karthik Nayak]

On Wed, Dec 28, 2016 at 2:41 AM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> Currently the 'lstrip=' option only takes a positive value ''
>> and strips '' slash-separated path components from the left. Modify
>> the 'lstrip' option to also take a negative number '' 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=` is appended, strips ``
>>   slash-separated path components from the front of the refname
>>   (e.g., `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo`.
>> - `` must be a positive integer.
>> + if `` is a negative number, then only `` path components
>> + are left behind.
>
> I think positive  is so obvious not to require an example but it
> is good that you have one.  The negative  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 `` is a negative number, then only `` 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 , or it becomes the full refname if
> stripping with negative .  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  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

Re: HowTo distribute a hook with the reposity.

[Jeff King]

On Wed, Dec 28, 2016 at 08:42:25AM +, 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

Re: HowTo distribute a hook with the reposity.

[John P. Hartmann]

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  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 ).  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

Re: [RFH] gpg --import entropy while running tests

[Jeff King]

On Wed, Dec 28, 2016 at 03:02:30AM -0500, Jeff King wrote:

> > Is it a bug in gpg (oddly, the kernel reports lots of entropy available,
> > and generating the signatures themselves is quite fast)? Or is the new
> > version doing something special in the import process that we need to
> > work around or disable?
> 
> Answering my own question (somewhat): this is bisectable in the gnupg
> repository, and it turns out to be caused by their 4473db1ef (agent:
> Kludge to mitigate blocking calls in Libgcrypt., 2016-11-11), which
> introduces a 100ms sleep (yuck) that is presumably triggering way more
> than it needs to. More details at:
> 
>   
> 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.

-Peff

Re: [PATCH v9 11/20] ref-filter: introduce refname_atom_parser()

[Karthik Nayak]

On Wed, Dec 28, 2016 at 2:34 AM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> +symref::
>> + The ref which the given symbolic ref refers to. If not a
>> + symbolic ref, nothing is printed. Respects the `:short` and
>> + `:strip` options in the same way as `refname` above.
>> +
>
> I am slightly unhappy with this name.  If we had an atom that lets
> you ask "Is this a symref?" and yields "" or "->", it could also be
> called symref, and we would name it "is_symref" or something to
> disambiguate it.  Then it is only fair to give this one that lets
> you ask "What does this symref point at?" a bit more specific name,
> like "symref_target" or something.
>
> But probably I am worried too much.  "is_symref", if necessary, can
> be written as "%(if:notequals=)%(symref)%(then)...%(else)...%(end)"
> and it is not likely that it would be used often, so let's keep it
> as-is.

You're probably right about it not being the right name, since symref doesn't
indicate that the atom will print the ref being pointed to, but the
name 'symref'
is short and I guess its easily understandable.

-- 
Regards,
Karthik Nayak

Re: [RFH] gpg --import entropy while running tests

[Jeff King]

On Wed, Dec 28, 2016 at 02:23:03AM -0500, Jeff King wrote:

> That's a lot of time not using any CPU. What's going on? Running with
> "sh -x" shows that we spend most of the time in this line from
> lib-gpg.sh:
> 
>   gpg --homedir "${GNUPGHOME}" 2>/dev/null --import \
> "$TEST_DIRECTORY"/lib-gpg/keyring.gpg
> 
> And running gpg with "--debug-level guru" shows that we are blocking
> while waiting for entropy. Has anybody else seen this? I feel like I
> noticed it starting a few weeks ago, and indeed dropping back to gpg
> 2.0.26 (from 2.1.17) makes the problem go away.
> 
> Is it a bug in gpg (oddly, the kernel reports lots of entropy available,
> and generating the signatures themselves is quite fast)? Or is the new
> version doing something special in the import process that we need to
> work around or disable?

Answering my own question (somewhat): this is bisectable in the gnupg
repository, and it turns out to be caused by their 4473db1ef (agent:
Kludge to mitigate blocking calls in Libgcrypt., 2016-11-11), which
introduces a 100ms sleep (yuck) that is presumably triggering way more
than it needs to. More details at:

  
https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=commit;h=4473db1ef24031ff4e26c9a9de95dbe898ed2b97

So this does seem like a gpg bug.

-Peff