Re: [PATCH] Make the global packed_git variable static to sha1_file.c.

[Chris Packham]

Hi,

On 12/02/14 14:57, Stefan Zager wrote:
> From b4796d9d99c03b0b7cddd50808a41413e45f1129 Mon Sep 17 00:00:00 2001
> From: Stefan Zager 
> Date: Mon, 10 Feb 2014 16:55:12 -0800
> Subject: [PATCH] Make the global packed_git variable static to sha1_file.c.
> 
> This is a first step in making the codebase thread-safe.  By and
> large, the operations which might benefit from threading are those
> that work with pack files (e.g., checkout, blame), so the focus of
> this patch is stop leaking the global list of pack files outside of
> sha1_file.c.
> 
> The next step will be to control access to the list of pack files
> with a mutex.  However, that alone is not enough to make pack file
> access thread safe.  Even in a read-only operation, the window list
> associated with each pack file will need to be controlled.
> Additionally, the global counters in sha1_file.c will need to be
> controlled.
> 
> This patch is a pure refactor with no functional changes, so it
> shouldn't require any additional tests.  Adding the actual locks
> will be a functional change, and will require additional tests.
> 
> Signed-off-by: Stefan Zager 
> ---
>  builtin/count-objects.c  |  44 ++-
>  builtin/fsck.c   |  46 +++-
>  builtin/gc.c |  26 +++
>  builtin/pack-objects.c   | 188 
> ---
>  builtin/pack-redundant.c |  37 +++---
>  cache.h  |  16 +++-
>  fast-import.c|   4 +-
>  http-backend.c   |  28 ---
>  http-push.c  |   4 +-
>  http-walker.c|   2 +-
>  pack-revindex.c  |  20 ++---
>  server-info.c|  35 +
>  sha1_file.c  |  35 -
>  sha1_name.c  |  18 -
>  14 files changed, 315 insertions(+), 188 deletions(-)

I'm not really qualified to comment on substance but there are some
basic style issues w.r.t. whitespace namely using 4 spaces for indent
and mixing tabs/spaces. This might seem pedantic for the first round of
a patch but it does put off reviewers.

>From Documentation/CodingGuidelines:

 - We use tabs to indent, and interpret tabs as taking up to
   8 spaces.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

pack bitmap woes on Windows

[Johannes Sixt]

Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with
the following symptoms. I haven't followed the topic. Have there been
patches floating that addressed the problem in one way or another?

(gdb) run
Starting program: D:\Src\mingw-git\t\trash 
directory.t5310-pack-bitmaps/..\..\git.exe rev-list --test-bitmap HEAD
[New thread 3528.0x8d4]
Bitmap v1 test (20 entries loaded)
Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits / 15873b36 
checksum

Breakpoint 1, die (err=0x5939e9 "Out of memory, realloc failed") at usage.c:97
97  if (die_is_recursing()) {
(gdb) bt
#0  die (err=0x5939e9 "Out of memory, realloc failed") at usage.c:97
#1  0x00487c4c in xrealloc (ptr=0x12b10008, size=837107040) at wrapper.c:109
#2  0x0055572b in ewah_to_bitmap (ewah=0xe58c18) at ewah/bitmap.c:105
#3  0x005426fc in test_bitmap_walk (revs=0x22f93c) at pack-bitmap.c:954
#4  0x0046b6ae in cmd_rev_list (argc=2, argv=0x3e263c, prefix=0x0)
at builtin/rev-list.c:329
#5  0x00402048 in run_builtin (p=0x56d41c, argc=3, argv=0x3e263c) at git.c:314
#6  0x0040224f in handle_builtin (argc=3, argv=0x3e263c) at git.c:487
#7  0x00402351 in run_argv (argcp=0x22ff50, argv=0x22ff38) at git.c:533
#8  0x0040257f in mingw_main (argc=3, av=0x3e2638) at git.c:616
#9  0x0040242e in main (argc=4, argv=0x3e2638) at git.c:551
(gdb) up
#1  0x00487c4c in xrealloc (ptr=0x12b10008, size=837107040) at wrapper.c:109
109 die("Out of memory, realloc failed");
(gdb) up
#2  0x0055572b in ewah_to_bitmap (ewah=0xe58c18) at ewah/bitmap.c:105
105 bitmap->words = ewah_realloc(
(gdb) l
100 ewah_iterator_init(&it, ewah);
101
102 while (ewah_iterator_next(&blowup, &it)) {
103 if (i >= bitmap->word_alloc) {
104 bitmap->word_alloc *= 1.5;
105 bitmap->words = ewah_realloc(
106 bitmap->words, bitmap->word_alloc * 
sizeof(eword_t));
107 }
108
109 bitmap->words[i++] = blowup;
(gdb) info locals
bitmap = (struct bitmap *) 0xe58aa0
it = {buffer = 0xe58cd0, buffer_size = 2, pointer = 1, compressed = 52981705,
  literals = 0, rl = 2141159439, lw = 8259520, b = 1}
blowup = 18446744073709551615
i = 69758920
(gdb) info args
ewah = (struct ewah_bitmap *) 0xe58c18
(gdb)

This is after "not ok 3 - rev-list --test-bitmap verifies bitmaps".
Numerous further test cases fail, but I didn't look at them.

-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Error compiling git when docbooc2k is custom compiled.

[Mimiko]

Hello.

I've compiled docbook2x to --prefix=/opt/docbook2X. When building git 
make fails not finding docbook2x-texi, but /opt/docbook2X/bin is in the 
PATH. Investigating I've found that default build of docbook2X from 
sources creates file with the following names:


docbook2man
docbook2texi

So I use option --program-transform-name="s/^docbook2/docbook2x-/" when 
compiling docbook2X.


Please allow in building git detect the name of docbook2X when using 
default options when compiling docbook2X.


--
Mimiko desu.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] l10n updates for 1.9.0 round 2

[Jiang Xin]

Hi Junio,

Update window is still open? Here is l10n for German for your to pull.

The following changes since commit e265f1f7168a262c2f50c68707ff17318ebfdc5a:

  Merge git://github.com/git-l10n/git-po (2014-02-11 11:02:05 -0800)

are available in the git repository at:


  git://github.com/git-l10n/git-po master

for you to fetch changes up to 92cd3e35a0493fed026e5baec97e1e7359bcda96:

  l10n: de.po: correct message when hiding commits by craft
(2014-02-12 07:16:03 +0100)


Ralf Thielow (2):
  l10n: de.po: translate 28 new messages
  l10n: de.po: correct message when hiding commits by craft

 po/de.po | 1810 +-
 1 file changed, 966 insertions(+), 844 deletions(-)

--
Jiang Xin
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-note -C changes commit type?

[Kyle J. McKay]

On Feb 11, 2014, at 16:06, Junio C Hamano wrote:

Johan Herland  writes:


There is currently no way the "git notes" commands will allow you to
store the 3d7de37 commit object directly as a note. There is also
(AFAICS) no easy workaround (git fast-import could've been a
workaround if it did not already require the first N/notemodify
argument to be a blob object). The best alternative, off the top of  
my

head, would be to write your own program using the notes.h API to
manipulate the notes tree directly (or - suboptimally - use other
low-level Git operations to do the same).


Even worse. I do not think such a non-blob object in the notes tree
does not participate in the reachability at all, so you won't be
able to fetch "refs/notes/whatever" and expect to get a useful
result.  I do not think storing the raw bits of commit object as a
blob in the notes tree is useful behaviour, either.  The command
probably should refuse to get anything non-blob via that option.


It would be nice if it let you store a tree or a blob, but I agree  
that it should complain about anything non-blob by default and if tree  
were to be allowed, that should require a special option.


If you do manually construct a notes tree that has a 'tree' entry  
instead of a blob, as soon as you add a new note, that 'tree' gets  
turned back into a blob again.  I was trying to attach a 'tree' as my  
note a while back and decided not to pursue it further after I found  
it got transformed into a 'blob' on the next notes modification.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Make the git codebase thread-safe

[Duy Nguyen]

On Wed, Feb 12, 2014 at 9:02 AM, Robin H. Johnson  wrote:
> On Tue, Feb 11, 2014 at 05:54:51PM -0800,  Stefan Zager wrote:
>> We in the chromium project have a keen interest in adding threading to
>> git in the pursuit of performance for lengthy operations (checkout,
>> status, blame, ...).  Our motivation comes from hitting some
>> performance walls when working with repositories the size of chromium
>> and blink:
> +1 from Gentoo on performance improvements for large repos.
>
> The main repository in the ongoing Git migration project looks to be in
> the 1.5GB range (and for those that want to propose splitting it up, we
> have explored that option and found it lacking), with very deep history
> (but no branches of note, and very few tags).

>From v1.9 shallow clone should work for all push/pull/clone... so
history depth does not matter (on the client side). As for
gentoo-x86's large worktree, using index v4 and avoid full-tree
operations (e.g. "status .", not "status"..) should make all
operations reasonably fast. I plan to make "status" fast even without
path limiting with the help of inotify, but that's not going to be
finished soon. Did I miss anything else?
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Make the git codebase thread-safe

[Duy Nguyen]

On Wed, Feb 12, 2014 at 8:54 AM, Stefan Zager  wrote:
> We in the chromium project have a keen interest in adding threading to
> git in the pursuit of performance for lengthy operations (checkout,
> status, blame, ...).  Our motivation comes from hitting some
> performance walls when working with repositories the size of chromium
> and blink:
>
> https://chromium.googlesource.com/chromium/src
> https://chromium.googlesource.com/chromium/blink

I have no comments about thread safety improvements (well, not yet).
If you have investigated about git performance on chromium
repositories, could you please sum it up? Threading may be an option
to improve performance, but it's probably not the only option.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Make the git codebase thread-safe

[Robin H. Johnson]

On Tue, Feb 11, 2014 at 05:54:51PM -0800,  Stefan Zager wrote:
> We in the chromium project have a keen interest in adding threading to
> git in the pursuit of performance for lengthy operations (checkout,
> status, blame, ...).  Our motivation comes from hitting some
> performance walls when working with repositories the size of chromium
> and blink:
+1 from Gentoo on performance improvements for large repos.

The main repository in the ongoing Git migration project looks to be in
the 1.5GB range (and for those that want to propose splitting it up, we
have explored that option and found it lacking), with very deep history
(but no branches of note, and very few tags).

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Infrastructure Lead
E-Mail : robb...@gentoo.org
GnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] gc: config option for running --auto in background

[Duy Nguyen]

On Tue, Feb 11, 2014 at 2:11 AM, Junio C Hamano  wrote:
> On Mon, Feb 10, 2014 at 10:43 AM, Junio C Hamano  wrote:
>>> If --quiet is set, we should not be printing anyway. If not, I thinkg
>>> we could only print "auto packing in background.." when we actually
>>> can do that, else just print the old message. It means an #ifdef
>>> NO_POSIX_GOODIES here again though..
>>
>> Didn't you change it not to die but return nosys or something?
>
> Ah, the problem is that it is too late to take back "... will do so in
> the background" when you noticed that daemonize() did not succeed, so
> you would need a way to see if we can daemonize() before actually
> doing so if you want to give different messages.
>
> "int can_daemonize(void)" could be an answer that is nicer than
> NO_POSIX_GOODIES, but I am not sure if it is worth it.

Or we could pass the "quiet" flag to daemonize() and let it print
something in the #ifdef NO_POSIX_GOODIES part.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Make the global packed_git variable static to sha1_file.c.

[Stefan Zager]

>From b4796d9d99c03b0b7cddd50808a41413e45f1129 Mon Sep 17 00:00:00 2001
From: Stefan Zager 
Date: Mon, 10 Feb 2014 16:55:12 -0800
Subject: [PATCH] Make the global packed_git variable static to sha1_file.c.

This is a first step in making the codebase thread-safe.  By and
large, the operations which might benefit from threading are those
that work with pack files (e.g., checkout, blame), so the focus of
this patch is stop leaking the global list of pack files outside of
sha1_file.c.

The next step will be to control access to the list of pack files
with a mutex.  However, that alone is not enough to make pack file
access thread safe.  Even in a read-only operation, the window list
associated with each pack file will need to be controlled.
Additionally, the global counters in sha1_file.c will need to be
controlled.

This patch is a pure refactor with no functional changes, so it
shouldn't require any additional tests.  Adding the actual locks
will be a functional change, and will require additional tests.

Signed-off-by: Stefan Zager 
---
 builtin/count-objects.c  |  44 ++-
 builtin/fsck.c   |  46 +++-
 builtin/gc.c |  26 +++
 builtin/pack-objects.c   | 188 ---
 builtin/pack-redundant.c |  37 +++---
 cache.h  |  16 +++-
 fast-import.c|   4 +-
 http-backend.c   |  28 ---
 http-push.c  |   4 +-
 http-walker.c|   2 +-
 pack-revindex.c  |  20 ++---
 server-info.c|  35 +
 sha1_file.c  |  35 -
 sha1_name.c  |  18 -
 14 files changed, 315 insertions(+), 188 deletions(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a7f70cb..5a8e487 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -83,14 +83,32 @@ static char const * const count_objects_usage[] = {
NULL
 };
 
+struct pack_data {
+unsigned long packed;
+off_t size_pack;
+unsigned long num_pack;
+};
+
+int pack_data_fn(struct packed_git *p, void *data)
+{
+struct pack_data *pd = (struct pack_data *) data;
+if (p->pack_local && !open_pack_index(p)) {
+   pd->packed += p->num_objects;
+   pd->size_pack += p->pack_size + p->index_size;
+   pd->num_pack++;
+}
+return 1;
+}
+
 int cmd_count_objects(int argc, const char **argv, const char *prefix)
 {
int i, verbose = 0, human_readable = 0;
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
-   unsigned long loose = 0, packed = 0, packed_loose = 0;
+   unsigned long loose = 0, packed_loose = 0;
off_t loose_size = 0;
+struct pack_data pd = {0,0,0};
struct option opts[] = {
OPT__VERBOSE(&verbose, N_("be verbose")),
OPT_BOOL('H', "human-readable", &human_readable,
@@ -118,41 +136,29 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
closedir(d);
}
if (verbose) {
-   struct packed_git *p;
-   unsigned long num_pack = 0;
-   off_t size_pack = 0;
struct strbuf loose_buf = STRBUF_INIT;
struct strbuf pack_buf = STRBUF_INIT;
struct strbuf garbage_buf = STRBUF_INIT;
-   if (!packed_git)
-   prepare_packed_git();
-   for (p = packed_git; p; p = p->next) {
-   if (!p->pack_local)
-   continue;
-   if (open_pack_index(p))
-   continue;
-   packed += p->num_objects;
-   size_pack += p->pack_size + p->index_size;
-   num_pack++;
-   }
+   prepare_packed_git();
+   foreach_packed_git(pack_data_fn, NULL, &pd);
 
if (human_readable) {
strbuf_humanise_bytes(&loose_buf, loose_size);
-   strbuf_humanise_bytes(&pack_buf, size_pack);
+   strbuf_humanise_bytes(&pack_buf, pd.size_pack);
strbuf_humanise_bytes(&garbage_buf, size_garbage);
} else {
strbuf_addf(&loose_buf, "%lu",
(unsigned long)(loose_size / 1024));
strbuf_addf(&pack_buf, "%lu",
-   (unsigned long)(size_pack / 1024));
+   (unsigned long)(pd.size_pack / 1024));
strbuf_addf(&garbage_buf, "%lu",
(unsigned long)(size_garbage / 1024));
}
 
printf("count: %lu\n", loose);
printf("size: %s\n", loose_buf.buf);
-   printf("in-pack: %lu\n", packed);
-   printf("packs: 

Make the git codebase thread-safe

[Stefan Zager]

We in the chromium project have a keen interest in adding threading to
git in the pursuit of performance for lengthy operations (checkout,
status, blame, ...).  Our motivation comes from hitting some
performance walls when working with repositories the size of chromium
and blink:

https://chromium.googlesource.com/chromium/src
https://chromium.googlesource.com/chromium/blink

We are particularly concerned with the performance of msysgit, and we
have already chalked up a significant performance gain by turning on
the threading code in pack-objects (which was already enabled for
posix platforms, but not on msysgit, owing to the lack of a correct
pread implementation).

To this end, I'd like to start submitting patches that make the code
base generally more thread-safe and thread-friendly.  Right after this
email, I'm going to send the first such patch, which makes the global
list of pack files (packed_git) internal to sha1_file.c.

I realize this may be a contentious topic, and I'd like to get
feedback on the general effort to add more threading to git.  I'd
appreciate any feedback you'd like to give up front.

Thanks!

Stefan Zager
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-note -C changes commit type?

[Junio C Hamano]

Johan Herland  writes:

> There is currently no way the "git notes" commands will allow you to
> store the 3d7de37 commit object directly as a note. There is also
> (AFAICS) no easy workaround (git fast-import could've been a
> workaround if it did not already require the first N/notemodify
> argument to be a blob object). The best alternative, off the top of my
> head, would be to write your own program using the notes.h API to
> manipulate the notes tree directly (or - suboptimally - use other
> low-level Git operations to do the same).

Even worse. I do not think such a non-blob object in the notes tree
does not participate in the reachability at all, so you won't be
able to fetch "refs/notes/whatever" and expect to get a useful
result.  I do not think storing the raw bits of commit object as a
blob in the notes tree is useful behaviour, either.  The command
probably should refuse to get anything non-blob via that option.

Perhaps the notes entry should just note the object name of whatever
commit it wants to refer to in a *blob*?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] tests: turn on network daemon tests by default

[Junio C Hamano]

Jeff King  writes:

> On Tue, Feb 11, 2014 at 11:51:18AM -0800, Junio C Hamano wrote:
>
>> Those who run buildfarms may want to disable the networking test if
>> the buildfarms are not isolated well, for example.  They have to be
>> told somewhere that now they need to explicitly disable these tests
>> and how.
>
> I think they should be OK. The daemons run on the loopback interface, so
> there is hopefully not a security implication. If multiple buildfarms
> are sharing the same loopback space (e.g., running in separate
> directories on the same machine), the "auto" setting should degrade
> gracefully. One daemon will "win" the setup, and the tests will run, and
> on the other, they will be skipped.
>
>> I am in favor of this change but just pointing out possible fallouts
>> might be larger than we think.
>
> Agreed, but I think the only way to know the size of those fallouts is
> to try it and see who complains.  I would not normally be so cavalier
> with git itself, but I think for the test infrastructure, we have a
> small, tech-savvy audience that can help us iterate on it without too
> much pain.

Sure. One immediate complaint is that I would probably need to do
something like this in the build automation:

if testing a branch without this patch
then
: do nothing
else
GIT_TEST_GIT_DAEMON=false
fi

Arguably, it is the fault of the current/original code that treated
*any* non-empty value that is set in the environment variable as
"true"---if it paid attention to GIT_TEST_GIT_DAEMON=NoThanks, we
wouldn't have to have a workaround like this.

I wonder if GIT_TEST_X=$(test_tristate "$GIT_TEST_X") pattern can be
made a bit more friendly, though.  For example, can we behave
differently depending on the reason why $GIT_TEST_X is empty?

 - People who have *not* been opting in to the expensive tests have
   not done anything special; GIT_TEST_X environment variable did
   not exist for them (i.e. unset), and we used to skip when
   "$GIT_TEST_X" is an empty string.

 - We want to encourage people who do not care to run these tests.
   If people do not do anything, their $GIT_TEST_X will continue to
   be an empty string without GIT_TEST_X variable in the
   environment.

If we let people who *do* want to opt out of the expensive tests by
explicitly setting $GIT_TEST_X to an empty string in the new scheme,
wouldn't the transition go a lot smoother?

The caller may have to pass the name of the variable:

GIT_TEST_DAEMON=$(test_tristate GIT_TEST_DAEMON)

and then the callee may become

test_tristate () {
variable=$1
if eval "test x\"\${$variable+isset}\" = xisset"
then
# explicitly set
eval "value=\$$variable"
case "$value" in
"")
echo false ;;
false | auto)
echo "$value" ;;
*)
echo true ;;
esac
else
echo auto
fi
}

so that

 - Any non-empty string other than the magic strings "false" and
   "auto" continue to mean "please I want to test";

 - Setting the variable explicitly to an empty string will continue
   to mean "no I do not want to test";

 - Leaving the variable unset will continue to mean "I don't really
   care; just follow the default the project gives me".

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git-note -C changes commit type?

[Johan Herland]

On Tue, Feb 11, 2014 at 6:23 PM, Joachim Breitner
 wrote:
> Hi,
>
> judging from the documentation I got the impression that I can pass any
> git object has to "git note -C " and it would stored as-is. But it
> seems the objects gets mangled somehow...

...well... the documentation does not say "any object", it actually
explicitly says "blob object"... ;)

> (I want to attach a commit object as a note, to reference the history of
> a feature before the final cleanup rebase. For that I turn the reflog
> into a series of commits, and the final commit is the one I want to
> store there.)
>
> $ mkdir foo
> $ cd foo/
> $ echo foo > a
> $ git init
> Initialisierte leeres Git-Repository in /tmp/foo/.git/
> $ git add a
> $ git commit -m 'A commit'
> [master (Basis-Commit) 3d7de37] A commit
>  1 file changed, 1 insertion(+)
>  create mode 100644 a
> $ echo foo2 > a
> $ git commit -m 'A commit 2' -a
> [master e1bfac4] A commit 2
>  1 file changed, 1 insertion(+), 1 deletion(-)
> $ git notes --ref history add -C 3d7de37 e1bfac4
> $ git ls-tree notes/history
> 100644 blob 5b73d5152e6207e3a2b67e57ca3a2cb94d12061e 
> e1bfac434ebd3135a3784f6fc802f235098eebd0
>
> I was expecting 3d7de37 to be referenced here.
>
> Is that a bug, or is storing commits as notes not supported?

I guess it depends on your POV... The current documentation says "blob
object", and what actually happens (in builtin/notes.c) is that the
given object (3d7de37 in your example) is read into a strbuf (in
parse_reuse_arg()) and stored back into a note object (in
create_note()), without preserving the object type ("blob" type is
hardcoded). This means an incoming blob object (as documented) will be
preserved (i.e. reuse the same SHA-1), but for any other object type,
the object bits will be read, and stored back into a blob object. This
is why your commit (or any other non-blob) ends up with a different
SHA-1 when stored as a note: It is the same bytes, but in a blob
object instead of a commit object.

There is currently no way the "git notes" commands will allow you to
store the 3d7de37 commit object directly as a note. There is also
(AFAICS) no easy workaround (git fast-import could've been a
workaround if it did not already require the first N/notemodify
argument to be a blob object). The best alternative, off the top of my
head, would be to write your own program using the notes.h API to
manipulate the notes tree directly (or - suboptimally - use other
low-level Git operations to do the same).

However before you go there, let's take a step back, and look at what
the result would look like (if you were allowed to store a commit
object directly as a note):

You would have a notes ref "refs/notes/history" whose tree would
contain an entry named e1bfac434ebd3135a3784f6fc802f235098eebd0
pointing to a _commit_ (3d7de37...). Obviously, it would not make
sense to use refs/notes/history while displaying the commit log ("git
log --notes=history"), as the raw commit object would be shown in the
log. However, more fundamentally: a tree referring to a _commit_ is
usually how Git stores _submodule_ links (i.e. which revision of the
named submodule is to be used by this super-repo tree), and I'm (off
the top of my head) not at all sure that such a submodule link in a
notes tree is handled "sanely" by Git - or even that it makes sense at
all. For one, I'm not sure that Git requires (or even expects) the
commit object referenced by a tree to be present in the same object
DB. So if you share your notes, I don't know whether or not the
fetch/push machinery will include the commit object in the shared
notes... These are questions that should be answered before we decide
whether using commits directly as notes makes sense.

If we do figure out that storing commits as note objects is desirable
(and does not have too nasty side-effects), then I am not opposed to
fixing builtin/notes.c to preserve type of the object passed in -C.
Certainly, the current behavior for non-blobs (i.e. copy the object
bytes, but not the object type) is probably not useful to anyone...

That said, there may be other ways to solve your immediate problem:
Instead of storing the commit object directly as a note, you could
store the commit SHA-1 into a blob, and use that as the blob object.
That would also allow you to store multiple commits in the note for
e1bfac4 (in case you had several cleanup rebases leading to the final
commit), or you could store other kinds of metadata in the same note.
Or do you have a requirement that the reflog history (presumably
reachable from 3d7de37) need to be shared (or otherwise kept
reachable)? In that case, you might be better off using an explicit
ref to keep that history alive; e.g. you could create
refs/history/e1bfac4 to point to 3d7de37 ("git update-ref
refs/history/e1bfac4 3d7de37"), and keep everything
alive/reachable/shareable that way...


Hope this helps,

...Johan

-- 
Johan Herland, 
www.herland.net
--
To unsubscribe from this list: send the line "uns

Re: Profiling support?

[David Kastrup]

David Kastrup  writes:

> John Keeping  writes:
>
>> On Tue, Feb 11, 2014 at 03:41:55PM +0100, David Kastrup wrote:
>>> Duy Nguyen  writes:
>>>
>>> > Would perf help? No changes required, and almost no overhead, I think.
>>> 
>>> Not useful.  It would be probably nice for nailing down the performance
>>> gains when the work is finished so that future regressions will be
>>> noticeable.  It's reasonable easy to create a test case that will take
>>> hours with the current git-blame and would finish in seconds with the
>>> improved one.
>>> 
>>> But it's not useful at all for figuring out the hotspots within the
>>> git-blame binary.
>>
>> I would have thought the annotation described at [1] is exactly what
>> you're looking for, isn't it?
>>
>> Alternatively, I've had some success with callgrind and kCachegrind in
>> the past.
>>
>> [1]
>> https://perf.wiki.kernel.org/index.php/Tutorial#Source_level_analysis_with_perf_annotate
>
> Misunderstanding on my part.  I thought this was about the "make perf"
> Makefile target.  I'll have to take a look at what the perf utility
> does.
>
> Thanks for the clarification.

Looks indeed like it could be useful.  In the currently working code
from me (last of the complex patch series), significant time is spent in
moving and allocating memory.  It might make sense not to continue
ignoring the patch I proposed to get rid of the absurd realloc series in
builtin/blame.c.  While it may not be responsible for all of avoidable
allocation churn, it's still large and untypical enough to mask further
culprits.

-- 
David Kastrup
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] tests: turn on network daemon tests by default

[Jeff King]

On Tue, Feb 11, 2014 at 11:51:18AM -0800, Junio C Hamano wrote:

> Those who run buildfarms may want to disable the networking test if
> the buildfarms are not isolated well, for example.  They have to be
> told somewhere that now they need to explicitly disable these tests
> and how.

I think they should be OK. The daemons run on the loopback interface, so
there is hopefully not a security implication. If multiple buildfarms
are sharing the same loopback space (e.g., running in separate
directories on the same machine), the "auto" setting should degrade
gracefully. One daemon will "win" the setup, and the tests will run, and
on the other, they will be skipped.

> I am in favor of this change but just pointing out possible fallouts
> might be larger than we think.

Agreed, but I think the only way to know the size of those fallouts is
to try it and see who complains.  I would not normally be so cavalier
with git itself, but I think for the test infrastructure, we have a
small, tech-savvy audience that can help us iterate on it without too
much pain.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/11] More preparatory work for multiparent tree-walker

[Junio C Hamano]

Kirill Smelkov  writes:

> Sorry for the confusion. Could you please do the following:
>
> Patches should be applied over to ks/tree-diff-walk
> (74aa4a18). Before applying the patches, please cherry-pick
>
> c90483d9(tree-diff: no need to manually verify that there is no
>  mode change for a path)
>
> ef4f0928(tree-diff: no need to pass match to
>  skip_uninteresting())
>
> into that branch, and drop them from ks/combine-diff - we'll have one
> branch reworking the diff tree-walker, and the other taking advantage of
> it for combine-diff.

As long as that does not lose changes to tests and clean-ups, I'm
fine with that direction.  For example, I do not know if you want to
lose e3f62d12 (diffcore-order: export generic ordering interface,
2014-01-20), which is part of the combine-diff topic.




--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] tests: turn on network daemon tests by default

[Junio C Hamano]

Jeff King  writes:

> I dug in the history to see if there was any reasoning given for the
> current "off by default" setting. It looks like Junio asked for it when
> the original http-push tests were added, and everything else just
> followed that. The reasoning there was basically "they're heavyweight
> and we might not be able to run them", but hopefully having the option
> variable will make that OK.

Will queue, thanks.

I may have originally asked for it for one reason, thinking that one
reason would be enough while having another reason not to run them
as well.  But there would be countless silent others who have been
depending on that choice.

Those who run buildfarms may want to disable the networking test if
the buildfarms are not isolated well, for example.  They have to be
told somewhere that now they need to explicitly disable these tests
and how.

I am in favor of this change but just pointing out possible fallouts
might be larger than we think.

>  t/lib-git-daemon.sh |  8 +---
>  t/lib-httpd.sh  | 22 +++---
>  t/test-lib-functions.sh | 44 
>  3 files changed, 60 insertions(+), 14 deletions(-)
>
> diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
> index 1f22de2..e623bef 100644
> --- a/t/lib-git-daemon.sh
> +++ b/t/lib-git-daemon.sh
> @@ -16,9 +16,10 @@
>  #stop_git_daemon
>  #test_done
>  
> -if test -z "$GIT_TEST_GIT_DAEMON"
> +GIT_TEST_GIT_DAEMON=$(test_tristate "$GIT_TEST_GIT_DAEMON")
> +if test "$GIT_TEST_GIT_DAEMON" = false
>  then
> - skip_all="git-daemon testing disabled (define GIT_TEST_GIT_DAEMON to 
> enable)"
> + skip_all="git-daemon testing disabled (unset GIT_TEST_GIT_DAEMON to 
> enable)"
>   test_done
>  fi
>  
> @@ -58,7 +59,8 @@ start_git_daemon() {
>   kill "$GIT_DAEMON_PID"
>   wait "$GIT_DAEMON_PID"
>   trap 'die' EXIT
> - error "git daemon failed to start"
> + test_skip_or_die $GIT_TEST_GIT_DAEMON \
> + "git daemon failed to start"
>   fi
>  }
>  
> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
> index b43162e..bb900ca 100644
> --- a/t/lib-httpd.sh
> +++ b/t/lib-httpd.sh
> @@ -30,9 +30,10 @@
>  # Copyright (c) 2008 Clemens Buchacher 
>  #
>  
> -if test -z "$GIT_TEST_HTTPD"
> +GIT_TEST_HTTPD=$(test_tristate "$GIT_TEST_HTTPD")
> +if test "$GIT_TEST_HTTPD" = false
>  then
> - skip_all="Network testing disabled (define GIT_TEST_HTTPD to enable)"
> + skip_all="Network testing disabled (unset GIT_TEST_HTTPD to enable)"
>   test_done
>  fi
>  
> @@ -76,8 +77,7 @@ GIT_VALGRIND_OPTIONS=$GIT_VALGRIND_OPTIONS; export 
> GIT_VALGRIND_OPTIONS
>  
>  if ! test -x "$LIB_HTTPD_PATH"
>  then
> - skip_all="skipping test, no web server found at '$LIB_HTTPD_PATH'"
> - test_done
> + test_skip_or_die $GIT_TEST_HTTPD "no web server found at 
> '$LIB_HTTPD_PATH'"
>  fi
>  
>  HTTPD_VERSION=`$LIB_HTTPD_PATH -v | \
> @@ -89,19 +89,20 @@ then
>   then
>   if ! test $HTTPD_VERSION -ge 2
>   then
> - skip_all="skipping test, at least Apache version 2 is 
> required"
> - test_done
> + test_skip_or_die $GIT_TEST_HTTPD \
> + "at least Apache version 2 is required"
>   fi
>   if ! test -d "$DEFAULT_HTTPD_MODULE_PATH"
>   then
> - skip_all="Apache module directory not found.  Skipping 
> tests."
> - test_done
> + test_skip_or_die $GIT_TEST_HTTPD \
> + "Apache module directory not found"
>   fi
>  
>   LIB_HTTPD_MODULE_PATH="$DEFAULT_HTTPD_MODULE_PATH"
>   fi
>  else
> - error "Could not identify web server at '$LIB_HTTPD_PATH'"
> + test_skip_or_die $GIT_TEST_HTTPD \
> + "Could not identify web server at '$LIB_HTTPD_PATH'"
>  fi
>  
>  prepare_httpd() {
> @@ -155,9 +156,8 @@ start_httpd() {
>   >&3 2>&4
>   if test $? -ne 0
>   then
> - skip_all="skipping test, web server setup failed"
>   trap 'die' EXIT
> - test_done
> + test_skip_or_die $GIT_TEST_HTTPD "web server setup failed"
>   fi
>  }
>  
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index aeae3ca..3cc09c0 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -716,6 +716,50 @@ perl () {
>   command "$PERL_PATH" "$@"
>  }
>  
> +# Normalize the value given in $1 to one of "true", "false", or "auto". The
> +# result is written to stdout. E.g.:
> +#
> +# GIT_TEST_HTTPD=$(test_tristate "$GIT_TEST_HTTPD")
> +#
> +test_tristate () {
> + case "$1" in
> + ""|auto)
> + echo auto
> + ;;
> + *)
> + # Rely on git-config to handle all the variants of
> + # true/1/on/etc that we allow.
> +

Re: [RFH] hackday and GSoC topic suggestions

[Junio C Hamano]

Junio C Hamano  writes:

> Jeff King  writes:
>
>>  - Branch rename breaks local downstream branches
>>http://article.gmane.org/gmane.comp.version-control.git/241228
>
> If you have a branch B that builds on A, if you are renaming A to C,
> you may want B to automatically set to build on C in some cases, and
> in other cases your renaming A is done explicitly in order to severe
> the tie between A and B and setting the latter to build on C can be
> a bad thing---after all, the user's intention may be to create a
> branch A starting at some commit immediately after this rename so
> that B will keep building on that updated A.
>
> So I am not sure if this is a bug.

Having said that, the current behaviour of leaving B half-configured
to build on a missing branch is undesirable. If we were to change
this so that any branch B that used to build on branch A being
renamed to build on the branch under the new name C, the user may
have to do an extra "--set-upstream-to A" on B after recreating A if
this was done to save away the current state of A to C and then keep
building B on an updated A, so we may have to give _some_ clue what
we are doing behind their back when we rename, e.g.

$ git branch -m A C
warning: branch B is set to build on C now.

or something.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] contrib/diff-highlight: multibyte characters diff

[Junio C Hamano]

Yoshihiro Sugi  writes:

> Signed-off-by: Yoshihiro Sugi 
>
> diff-highlight split each hunks and compare them as byte sequences.
> it causes problems when diff hunks include multibyte characters.
> This change enable to work on such cases by decoding inputs and encoding 
> output as utf8 string.
> ---
>  contrib/diff-highlight/diff-highlight | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/contrib/diff-highlight/diff-highlight 
> b/contrib/diff-highlight/diff-highlight
> index c4404d4..49b4f53 100755
> --- a/contrib/diff-highlight/diff-highlight
> +++ b/contrib/diff-highlight/diff-highlight
> @@ -2,6 +2,7 @@
>  
>  use warnings FATAL => 'all';
>  use strict;
> +use Encode qw(decode_utf8 encode_utf8);
>  
>  # Highlight by reversing foreground and background. You could do
>  # other things like bold or underline if you prefer.
> @@ -15,8 +16,9 @@ my @added;
>  my $in_hunk;
>  
>  while (<>) {
> + $_ = decode_utf8($_);
>   if (!$in_hunk) {
> - print;
> + print encode_utf8($_);
>   $in_hunk = /^$COLOR*\@/;
>   }
>   elsif (/^$COLOR*-/) {
> @@ -30,7 +32,7 @@ while (<>) {
>   @removed = ();
>   @added = ();
>  
> - print;
> + print encode_utf8($_);
>   $in_hunk = /^$COLOR*[\@ ]/;
>   }
>  
> @@ -58,7 +60,8 @@ sub show_hunk {
>  
>   # If one side is empty, then there is nothing to compare or highlight.
>   if (!@$a || !@$b) {
> - print @$a, @$b;
> + print encode_utf8($_) for @$a;
> + print encode_utf8($_) for @$b;
>   return;
>   }
>  
> @@ -67,17 +70,18 @@ sub show_hunk {
>   # stupid, and only handle multi-line hunks that remove and add the same
>   # number of lines.
>   if (@$a != @$b) {
> - print @$a, @$b;
> + print encode_utf8($_) for @$a;
> + print encode_utf8($_) for @$b;
>   return;
>   }
>  
>   my @queue;
>   for (my $i = 0; $i < @$a; $i++) {
>   my ($rm, $add) = highlight_pair($a->[$i], $b->[$i]);
> - print $rm;
> + print encode_utf8($rm);
>   push @queue, $add;
>   }
> - print @queue;
> + print encode_utf8($_) for @queue;
>  }
>  
>  sub highlight_pair {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Introduce experimental remote object access mode

[Junio C Hamano]

Shawn Pearce  writes:

> Why would you do this? Perhaps you need more time in your day
> to consume tea or coffee. Set GIT_RTT and enjoy a beverage.

So the conclusion is that it is not practical to do a lazy fetch if
it is done extremely naively at "we want this object --- wait a bit
and we'll give you" level?

I am wondering if we can do a bit better, like "we want this object
--- wait a bit, ah that's a commit, so it is likely that you may
want the trees and blobs associated with it, too, if not right now
but in a near future, let me push a pack that holds them to you"?

>
> So-not-signed-off-by: this author or anyone else
> ---
>
>   :-)
>
>  sha1_file.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 6e8c05d..9bdcbc3 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -38,6 +38,7 @@ const unsigned char null_sha1[20];
>
>  static const char *no_log_pack_access = "no_log_pack_access";
>  static const char *log_pack_access;
> +static useconds_t rtt;
>
>  /*
>   * This is meant to hold a *small* number of objects that you would
> @@ -436,9 +437,20 @@ void prepare_alt_odb(void)
>   read_info_alternates(get_object_directory(), 0);
>  }
>
> +static void apply_rtt()
> +{
> + if (!rtt) {
> + char *rtt_str = getenv("GIT_RTT");
> + rtt = rtt_str ? strtoul(rtt_str, NULL, 10) * 1000 : 1;
> + }
> + if (rtt > 1)
> + usleep(rtt);
> +}
> +
>  static int has_loose_object_local(const unsigned char *sha1)
>  {
>   char *name = sha1_file_name(sha1);
> + apply_rtt();
>   return !access(name, F_OK);
>  }
>
> @@ -1303,6 +1315,7 @@ void prepare_packed_git(void)
>
>   if (prepare_packed_git_run_once)
>   return;
> +
>   prepare_packed_git_one(get_object_directory(), 1);
>   prepare_alt_odb();
>   for (alt = alt_odb_list; alt; alt = alt->next) {
> @@ -1439,6 +1452,7 @@ static int open_sha1_file(const unsigned char *sha1)
>   struct alternate_object_database *alt;
>
>   fd = git_open_noatime(name);
> + apply_rtt();
>   if (fd >= 0)
>   return fd;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] bash completion: Add --recurse-submodules

[Junio C Hamano]

Keshav Kini  writes:

> Sup Yut Sum  writes:
>
>> Signed-off-by: Sup Yut Sum 
>> ---
>>  contrib/completion/git-completion.bash | 19 ++-
>>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> Aren't you missing a commit message?

The title itself is almost sufficient, I would think.  It may need
to mention that this is only for fetch, pull and push.  I'll
tentatively queue the following.

Stripping the leftmost constant string with ${var##constant} looks
somewhat strange (why wouldn't a single # work?), but that is not a
new problem this patch introduces, and can be cleaned up separately
if/when somebody wants to.

-- >8 --
From: Sup Yut Sum 
Date: Sun, 9 Feb 2014 22:35:31 +0800
Subject: [PATCH] completion: teach --recurse-submodules to fetch, pull and push

Signed-off-by: Sup Yut Sum 
Signed-off-by: Junio C Hamano 
---
 contrib/completion/git-completion.bash | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 8aaf214..c044a68 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1221,14 +1221,20 @@ _git_difftool ()
__git_complete_revlist_file
 }
 
+__git_fetch_recurse_submodules="yes on-demand no"
+
 __git_fetch_options="
--quiet --verbose --append --upload-pack --force --keep --depth=
-   --tags --no-tags --all --prune --dry-run
+   --tags --no-tags --all --prune --dry-run --recurse-submodules=
 "
 
 _git_fetch ()
 {
case "$cur" in
+   --recurse-submodules=*)
+   __gitcomp "$__git_fetch_recurse_submodules" "" 
"${cur##--recurse-submodules=}"
+   return
+   ;;
--*)
__gitcomp "$__git_fetch_options"
return
@@ -1577,6 +1583,10 @@ _git_pull ()
__git_complete_strategy && return
 
case "$cur" in
+   --recurse-submodules=*)
+   __gitcomp "$__git_fetch_recurse_submodules" "" 
"${cur##--recurse-submodules=}"
+   return
+   ;;
--*)
__gitcomp "
--rebase --no-rebase
@@ -1589,6 +1599,8 @@ _git_pull ()
__git_complete_remote_or_refspec
 }
 
+__git_push_recurse_submodules="check on-demand"
+
 _git_push ()
 {
case "$prev" in
@@ -1601,10 +1613,15 @@ _git_push ()
__gitcomp_nl "$(__git_remotes)" "" "${cur##--repo=}"
return
;;
+   --recurse-submodules=*)
+   __gitcomp "$__git_push_recurse_submodules" "" 
"${cur##--recurse-submodules=}"
+   return
+   ;;
--*)
__gitcomp "
--all --mirror --tags --dry-run --force --verbose
--receive-pack= --repo= --set-upstream
+   --recurse-submodules=
"
return
;;
-- 
1.9.0-rc3-244-g3497008

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFH] hackday and GSoC topic suggestions

[Junio C Hamano]

Jeff King  writes:

>  - Branch rename breaks local downstream branches
>http://article.gmane.org/gmane.comp.version-control.git/241228

If you have a branch B that builds on A, if you are renaming A to C,
you may want B to automatically set to build on C in some cases, and
in other cases your renaming A is done explicitly in order to severe
the tie between A and B and setting the latter to build on C can be
a bad thing---after all, the user's intention may be to create a
branch A starting at some commit immediately after this rename so
that B will keep building on that updated A.

So I am not sure if this is a bug.

>  - git stash doesn't use --index as default
>http://article.gmane.org/gmane.comp.version-control.git/235892

I tend to think "git stash" was designed to work this way from day
one.

>  - using git commit-tree with "-F -" adds trailing newlines
>http://article.gmane.org/gmane.comp.version-control.git/236583

According to the initial commit that introduced this option, it
deliberately calls strbuf_complete_line() to make sure we do not
leave an incomplete line at the end.


Many of the other items in your "bugs" section seem to be worth
fixing.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 02/14] trailer: process trailers from file and arguments

[Junio C Hamano]

Christian Couder  writes:

>> Even if we assume, for the sake of discussion, that it *is* a good
>> idea to separate "under this condition" part and "do this" part, I
>> do not think the above is the only or the best way to express
>> "distinct values allowed for the same key".  How do we argue that
>> this from your example
>>
>>   if_exists = add_if_different
>>   if_missing = add
>>
>> is a better design than this, for example?
>>
>>   if_key_value_exists = ignore ; exact matching  exists
>>   if_key_exists = add  ; otherwise
>>   if_missing = add
>
> The question is what happens if we want to say "if the same  value> pair exists but not near where we would add.
>
> With the solution I implemented, that is:
> ...
> With the solution you suggest, should we have:
> ...
>   if_key_value_exists_not_neighbor = add ; exact matching 
> ...
> or:
>   if_key_value_exists = ignore_if_neighbor ; exact matching  exists
> ...
>
> And what happens if we want to say "if key exists and value matches
> ", how do we express that then?
> Or what happens when we want to say "if key exists and  succeeds"?
> ...
> With what I suggest, we can still say:
> ...
> And then people might ask if they can also use something like this:
> ...
> and it will look like we are trying too much to do what should be done
> in only one script.

I think the above illustrates my point very clearly.

These numerous questions you have to ask are indications why
choosing "this condition goes to the left hand side of the equal
sign (e.g. exists)" and "this condition goes to the right hand side
(e.g. do-this-if_neighbor)" is not working well.  The user has to
remember which conditions go to the variable name and which
conditions go to the action part.

And the made-up alternative you listed above is not a solution I
suggest to begin with---it is a strawman "if we assume such a
splitting were a good idea" in the first place ;-).

What I was wondering if it is an better alternative was the below.

>> The latter splits remaining conditional logic from "do this" part
>> (no more "add_if_different" that causes "add" action to see if there
>> is the same key and act differently) and moves it to "under this
>> condition" part.
>> ...
>> I am trying to illustrate that the line to split the "under this
>> condition" and "do this" looks arbitrary and fuzzy here, and because
>> of this arbitrary-ness and fuzziness, it is not very obvious to me
>> why it is a good idea to have "under this condition" as a separate
>> concept from "do this" settings.

That is, not splitting the logic into two parts like you did,
i.e. "if_X = do_Y_if_Z", which invites "why is it not if_true =
do_Y_if_X_and_Z, if_X_and_Z = do_Y, if_Z = do_Y_if_X"?

One possible way to avoid the confusion is to say "do_Y_if_X_and_Z"
without making the rule split into conditions and actions---I am
NOT saying that is _better_, there may be other solutions to avoid
this two-part logic in a cleaner way.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] daemon: move daemonize() to libgit.a

[Junio C Hamano]

Duy Nguyen  writes:

> On Tue, Feb 11, 2014 at 1:46 AM, Junio C Hamano  wrote:
>> Nguyễn Thái Ngọc Duy   writes:
>>
>>> diff --git a/setup.c b/setup.c
>>> index 6c3f85f..b09a412 100644
>>> --- a/setup.c
>>> +++ b/setup.c
>>> @@ -787,3 +787,27 @@ void sanitize_stdfds(void)
>>>   if (fd > 2)
>>>   close(fd);
>>>  }
>>> +
>>> +int daemonize(void)
>>> +{
>>> +#ifdef NO_POSIX_GOODIES
>>> + errno = -ENOSYS;
>>
>> Negated?
>
> Facepalm. I remember I wrote this somewhere but don't remember what
> topic :( Should I resend?

I've already amended it here.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

git-note -C changes commit type?

[Joachim Breitner]

Hi,

judging from the documentation I got the impression that I can pass any
git object has to "git note -C " and it would stored as-is. But it
seems the objects gets mangled somehow...

(I want to attach a commit object as a note, to reference the history of
a feature before the final cleanup rebase. For that I turn the reflog
into a series of commits, and the final commit is the one I want to
store there.)

$ mkdir foo
$ cd foo/
$ echo foo > a
$ git init
Initialisierte leeres Git-Repository in /tmp/foo/.git/
$ git add a
$ git commit -m 'A commit'
[master (Basis-Commit) 3d7de37] A commit
 1 file changed, 1 insertion(+)
 create mode 100644 a
$ echo foo2 > a
$ git commit -m 'A commit 2' -a
[master e1bfac4] A commit 2
 1 file changed, 1 insertion(+), 1 deletion(-)
$ git notes --ref history add -C 3d7de37 e1bfac4
$ git ls-tree notes/history
100644 blob 5b73d5152e6207e3a2b67e57ca3a2cb94d12061e 
e1bfac434ebd3135a3784f6fc802f235098eebd0

I was expecting 3d7de37 to be referenced here.

Is that a bug, or is storing commits as notes not supported?

Thanks,
Joachim


-- 
Joachim “nomeata” Breitner
  m...@joachim-breitner.de • http://www.joachim-breitner.de/
  Jabber: nome...@joachim-breitner.de  • GPG-Key: 0x4743206C
  Debian Developer: nome...@debian.org


signature.asc
Description: This is a digitally signed message part

Re: [PATCH] fast-import.c: always honor the filename case

[Torsten Bögershausen]

On 2014-02-10 15.24, Reuben Hawkins wrote:
>
>
>
> On Sun, Feb 9, 2014 at 2:34 PM, Torsten Bögershausen  > wrote:
>
> On 2014-02-06 12 .24, Reuben Hawkins wrote:
> [snipped away minor interesting stuff]
>
> Reading the answers from Peff and Junio, I am convinced that the 
> fast-import should
> not look at core.ignorecase at all.
>
>
> Agreed, but my patch 0001-fast-import.c-ignorecase-iff-... is working 
> very well (for me anyway), and the ignore-case option may be useful to the 
> git-p4 importer (which I guess is something that should be determined for 
> sure).
>
> If you want, you can turn this into a real patch and send it to the list.
> I think  Pete Wyckoff mailto:p...@padd.com>> is one of 
> the experts about p4.
>
>
> hehe. I thought it was a real patchso I guess I'm not sure what you mean. 
>  What do I need to do to make the patch more real (pardon my ignorance)?
> /Reuben
I have found a patch which was send as an attachment,
and that's why it got the file name 0001-fast-import.c-ignorecase-iff here
So yes, it was a real patch. Please re-send it inline, not as an attachment, as 
you did in
"[PATCH] fast-import.c: always honor the filename case"
>
>
> And the same is for fast-export fixes you have made: If you have the time,
> convert it into a patch and send it to the list.
>
>  
>
>
> Do you mean the fast-export fixes where I pushed all the delete operations to 
> the front of the list?
Yes
/Torsten

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Profiling support?

[David Kastrup]

John Keeping  writes:

> On Tue, Feb 11, 2014 at 03:41:55PM +0100, David Kastrup wrote:
>> Duy Nguyen  writes:
>>
>> > Would perf help? No changes required, and almost no overhead, I think.
>> 
>> Not useful.  It would be probably nice for nailing down the performance
>> gains when the work is finished so that future regressions will be
>> noticeable.  It's reasonable easy to create a test case that will take
>> hours with the current git-blame and would finish in seconds with the
>> improved one.
>> 
>> But it's not useful at all for figuring out the hotspots within the
>> git-blame binary.
>
> I would have thought the annotation described at [1] is exactly what
> you're looking for, isn't it?
>
> Alternatively, I've had some success with callgrind and kCachegrind in
> the past.
>
> [1]
> https://perf.wiki.kernel.org/index.php/Tutorial#Source_level_analysis_with_perf_annotate

Misunderstanding on my part.  I thought this was about the "make perf"
Makefile target.  I'll have to take a look at what the perf utility
does.

Thanks for the clarification.

-- 
David Kastrup
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Profiling support?

[John Keeping]

On Tue, Feb 11, 2014 at 03:41:55PM +0100, David Kastrup wrote:
> Duy Nguyen  writes:
> 
> > On Tue, Feb 11, 2014 at 6:17 PM, David Kastrup  wrote:
> >>
> >> Looking in the Makefile, I just find support for coverage reports using
> >> gcov.  Whatever is there with "profile" in it seems to be for
> >> profile-based compilation rather than using gprof.
> >>
> >> Now since I've managed to push most of the runtime for basic git-blame
> >> operation out of blame.c proper, it becomes important to figure out
> >> where most of the remaining runtime (a sizable part of that being system
> >> time) is being spent.  Loop counts like that provided by gcov (or am I
> >> missing something here?) are not helpful for that, I think I rather need
> >> the kind of per-function breakdown that gprof provides.
> >>
> >> Is there a reason there are no prewired recipes or advice for using
> >> gprof on git?  Is there a way to get the work done, namely seeing the
> >> actual distribution of call times (rather than iterations) using gcov so
> >> that this is not necessary?
> >
> > Would perf help? No changes required, and almost no overhead, I think.
> 
> Not useful.  It would be probably nice for nailing down the performance
> gains when the work is finished so that future regressions will be
> noticeable.  It's reasonable easy to create a test case that will take
> hours with the current git-blame and would finish in seconds with the
> improved one.
> 
> But it's not useful at all for figuring out the hotspots within the
> git-blame binary.

I would have thought the annotation described at [1] is exactly what
you're looking for, isn't it?

Alternatively, I've had some success with callgrind and kCachegrind in
the past.

[1] 
https://perf.wiki.kernel.org/index.php/Tutorial#Source_level_analysis_with_perf_annotate
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 02/14] trailer: process trailers from file and arguments

[Christian Couder]

On Mon, Feb 10, 2014 at 9:51 PM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>> Many entries with the same key but distinct values can be configured
>> using:
>>
>> if_exists = add_if_different
>> if_missing = add
>>
>> Many entries with the same key but values that can be the same can be
>> configured using:
>>
>> if_exists = add
>> if_missing = add
>
> While the above certainly can express the combinations, I am still
> puzzled about the value of having "under this condition" (i.e.
> if-exists/if-missing) and "do this" (i.e. add/add-if-different) as
> two separate concepts.

It is because there are many possible conditions under which the user
might want to do different things, and the syntax of our configuration
files is not a programming language, so it is not well suited to
nicely express what the user might want under different conditions.

That's what I tried to show in my last message in the thread I sent
the link to in my previous message.

(http://thread.gmane.org/gmane.comp.version-control.git/237278/)

So we have to choose a few conditions to avoid possible combinatorial
explosion in the number of possible values.

> If you do not have an existing entry with the same key, no new entry
> can be the same as an existing one---therefore, the former "allow
> only distinct values for the same key" can just say
>
>   trailer."Fixes".action = add_if_different
>
> or something, without any if_exists/if_missing.  In a similar way,
> the latter "duplicated values allowed for the same key" can say
>
>   trailer."Sob".action = add
>
> You can throw into the mix other actions like "add_if_missing", you
> can also express "only one value allowed for the same key---the
> first one wins", "replace" to mean "replace if there is one with the
> same key", and "replace_or_add" to mean "same as 'replace', but add
> if there is no existing entries with the same key".  Will we lose
> expressiveness by simplifying the semantics, getting rid of this
> "under this condition" part and instead making "do this" part more
> intelligent?

I don't think we will lose expressiveness, but I think there might be
a combinatorial explosion in the number of choices.

Right now there are 2 choices for "if_missing" and 5 choices for
"if_exists". This means that if we use only one "action" config
variable and want to have the same expressiveness, we need 10 choices.

It doesn't seem a big difference now, but, if we add more choices,
then the difference will increase a lot.

> Even if we assume, for the sake of discussion, that it *is* a good
> idea to separate "under this condition" part and "do this" part, I
> do not think the above is the only or the best way to express
> "distinct values allowed for the same key".  How do we argue that
> this from your example
>
>   if_exists = add_if_different
>   if_missing = add
>
> is a better design than this, for example?
>
>   if_key_value_exists = ignore ; exact matching  exists
>   if_key_exists = add  ; otherwise
>   if_missing = add

The question is what happens if we want to say "if the same  pair exists but not near where we would add.

With the solution I implemented, that is:

if_exists = add_if_different_neighbor

With the solution you suggest, should we have:

  if_key_value_exists_not_neighbor = add ; exact matching 
exists but not near where we would add
  if_key_value_exists = ignore ; exact matching  exists
  if_key_exists = add  ; otherwise
  if_missing = add

or:

  if_key_value_exists = ignore_if_neighbor ; exact matching  exists
  if_key_exists = add  ; otherwise
  if_missing = add

And what happens if we want to say "if key exists and value matches
", how do we express that then?
Or what happens when we want to say "if key exists and  succeeds"?

With what I suggest, we can still say:

if_exists = add_if_values_matches 

or

if_exists = add_if_succeeds 

With the solution you suggest, should it be:

  if_key_value_exists = ignore
  if_key_exists = add_if_values_matches 

and

  if_key_value_exists = ignore
  if_key_exists = add_if_succeeds 

?

Doesn't it look like redondant between the 2 lines?

And then people might ask if they can also use something like this:

  if_key_value_exists = add_if_succeeds 
  if_key_exists = add_if_succeeds 
  if_key_missing = add_if_succeeds 

and it will look like we are trying too much to do what should be done
in only one script.

> The latter splits remaining conditional logic from "do this" part
> (no more "add_if_different" that causes "add" action to see if there
> is the same key and act differently) and moves it to "under this
> condition" part.
>
> I am trying to illustrate that the line to split the "under this
> condition" and "do this" looks arbitrary and fuzzy here, and because
> of this arbitrary-ness and fuzziness, it is not very obvious to me
> why it is a good idea to have "under this condition" as a separate
> concept from "do this" settings.
>
> What is the advantage of having such an extra 

feature request: text=input option in .gitattributes

[Cameron Taggart]

After requesting this as
https://github.com/msysgit/msysgit/issues/164, I was told to take it
upstream, so here I am.

I would like a text=input feature added that has the same behavior as
text=auto, except that it defaults to core.autocrlf=input behavior
instead of core.autocrlf=true. This would ensure that the repository
is normalized and the text files in the working directory match what
is in the repository by default. In mysysgit#164, I highlighted the
behavior changes in a spreadsheet. This would still allow users to set
core.autocrlf=true if they want CRLF on their Windows system. Using
text=input instead of text=auto would ensure that Windows build
servers like TFS and TeamCity compile libraries with code that matches
exactly what is in the repository. This in turn, would make it much
much easier to do source indexing using my SourceLink project. :)

thanks,
Cameron
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Profiling support?

[David Kastrup]

Duy Nguyen  writes:

> On Tue, Feb 11, 2014 at 6:17 PM, David Kastrup  wrote:
>>
>> Looking in the Makefile, I just find support for coverage reports using
>> gcov.  Whatever is there with "profile" in it seems to be for
>> profile-based compilation rather than using gprof.
>>
>> Now since I've managed to push most of the runtime for basic git-blame
>> operation out of blame.c proper, it becomes important to figure out
>> where most of the remaining runtime (a sizable part of that being system
>> time) is being spent.  Loop counts like that provided by gcov (or am I
>> missing something here?) are not helpful for that, I think I rather need
>> the kind of per-function breakdown that gprof provides.
>>
>> Is there a reason there are no prewired recipes or advice for using
>> gprof on git?  Is there a way to get the work done, namely seeing the
>> actual distribution of call times (rather than iterations) using gcov so
>> that this is not necessary?
>
> Would perf help? No changes required, and almost no overhead, I think.

Not useful.  It would be probably nice for nailing down the performance
gains when the work is finished so that future regressions will be
noticeable.  It's reasonable easy to create a test case that will take
hours with the current git-blame and would finish in seconds with the
improved one.

But it's not useful at all for figuring out the hotspots within the
git-blame binary.

I made do with something like

make CFLAGS=-pg LDFLAGS=-pg

but it is sort of annoying that the required "make clean" apparently
also cleans out the coverage files: for sensible finding of bad stuff
one needs them as well.

At any rate, I'll probably figure out something eventually.  No idea
whether I'll get around to writing some useful instructions.  At the
current point of time, it would appear that a large part of the
remaining user time (about half) is spent in xdl_hash_record so x86_64
architectures already benefit from XDL_FAST_HASH (which seems to hurt
more than it would help with my i686).  So finding a good fast hash
function would likely help.  The current hash function _and_ the
XDL_FAST_HASH replacement used on x86_64 are a drag here because they
are not easily split into a word-unaligned, a (typically long and thus
most performance-relevant) word-aligned, and another word-unaligned part
in a manner that allows calculating the same hash for different
alignments.  Something like a CRC lends itself much better to that, but
since its basic operation is more complex on a general-purpose CPU, it's
not likely to result in a net win.

In assembly language, add-and-multiply operations modulo 2^32-1 are
pretty easy to do and lend themselves well to considering alignment at
the end, but in C, access to mixed precision multiplications and the
carry flag are rather awkward.

-- 
David Kastrup
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Profiling support?

[Duy Nguyen]

On Tue, Feb 11, 2014 at 6:17 PM, David Kastrup  wrote:
>
> Looking in the Makefile, I just find support for coverage reports using
> gcov.  Whatever is there with "profile" in it seems to be for
> profile-based compilation rather than using gprof.
>
> Now since I've managed to push most of the runtime for basic git-blame
> operation out of blame.c proper, it becomes important to figure out
> where most of the remaining runtime (a sizable part of that being system
> time) is being spent.  Loop counts like that provided by gcov (or am I
> missing something here?) are not helpful for that, I think I rather need
> the kind of per-function breakdown that gprof provides.
>
> Is there a reason there are no prewired recipes or advice for using
> gprof on git?  Is there a way to get the work done, namely seeing the
> actual distribution of call times (rather than iterations) using gcov so
> that this is not necessary?

Would perf help? No changes required, and almost no overhead, I think.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Profiling support?

[David Kastrup]

Looking in the Makefile, I just find support for coverage reports using
gcov.  Whatever is there with "profile" in it seems to be for
profile-based compilation rather than using gprof.

Now since I've managed to push most of the runtime for basic git-blame
operation out of blame.c proper, it becomes important to figure out
where most of the remaining runtime (a sizable part of that being system
time) is being spent.  Loop counts like that provided by gcov (or am I
missing something here?) are not helpful for that, I think I rather need
the kind of per-function breakdown that gprof provides.

Is there a reason there are no prewired recipes or advice for using
gprof on git?  Is there a way to get the work done, namely seeing the
actual distribution of call times (rather than iterations) using gcov so
that this is not necessary?

-- 
David Kastrup
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] contrib/diff-highlight: multibyte characters diff

[Yoshihiro Sugi]

Signed-off-by: Yoshihiro Sugi 

diff-highlight split each hunks and compare them as byte sequences.
it causes problems when diff hunks include multibyte characters.
This change enable to work on such cases by decoding inputs and encoding output 
as utf8 string.
---
 contrib/diff-highlight/diff-highlight | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/contrib/diff-highlight/diff-highlight 
b/contrib/diff-highlight/diff-highlight
index c4404d4..49b4f53 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -2,6 +2,7 @@
 
 use warnings FATAL => 'all';
 use strict;
+use Encode qw(decode_utf8 encode_utf8);
 
 # Highlight by reversing foreground and background. You could do
 # other things like bold or underline if you prefer.
@@ -15,8 +16,9 @@ my @added;
 my $in_hunk;
 
 while (<>) {
+   $_ = decode_utf8($_);
if (!$in_hunk) {
-   print;
+   print encode_utf8($_);
$in_hunk = /^$COLOR*\@/;
}
elsif (/^$COLOR*-/) {
@@ -30,7 +32,7 @@ while (<>) {
@removed = ();
@added = ();
 
-   print;
+   print encode_utf8($_);
$in_hunk = /^$COLOR*[\@ ]/;
}
 
@@ -58,7 +60,8 @@ sub show_hunk {
 
# If one side is empty, then there is nothing to compare or highlight.
if (!@$a || !@$b) {
-   print @$a, @$b;
+   print encode_utf8($_) for @$a;
+   print encode_utf8($_) for @$b;
return;
}
 
@@ -67,17 +70,18 @@ sub show_hunk {
# stupid, and only handle multi-line hunks that remove and add the same
# number of lines.
if (@$a != @$b) {
-   print @$a, @$b;
+   print encode_utf8($_) for @$a;
+   print encode_utf8($_) for @$b;
return;
}
 
my @queue;
for (my $i = 0; $i < @$a; $i++) {
my ($rm, $add) = highlight_pair($a->[$i], $b->[$i]);
-   print $rm;
+   print encode_utf8($rm);
push @queue, $add;
}
-   print @queue;
+   print encode_utf8($_) for @queue;
 }
 
 sub highlight_pair {
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Introduce experimental remote object access mode

[Shawn Pearce]

Make it easy to experiment what remote access to objects would be
like if the network ran at say 1 ms round trip latency to obtain
any object not on the local repository.

  $ time git ls-tree -r HEAD
  real 0m0.059s

  $ time GIT_RTT=1 git ls-tree -r HEAD
  real 0m27.283s

Yes kids, slowing down loose object access by just 1ms if all
objects are remote can take a simple ls-tree from 59ms to more
than enough time to drink tea or coffee.

Why would you do this? Perhaps you need more time in your day
to consume tea or coffee. Set GIT_RTT and enjoy a beverage.

So-not-signed-off-by: this author or anyone else
---

  :-)

 sha1_file.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/sha1_file.c b/sha1_file.c
index 6e8c05d..9bdcbc3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -38,6 +38,7 @@ const unsigned char null_sha1[20];

 static const char *no_log_pack_access = "no_log_pack_access";
 static const char *log_pack_access;
+static useconds_t rtt;

 /*
  * This is meant to hold a *small* number of objects that you would
@@ -436,9 +437,20 @@ void prepare_alt_odb(void)
  read_info_alternates(get_object_directory(), 0);
 }

+static void apply_rtt()
+{
+ if (!rtt) {
+ char *rtt_str = getenv("GIT_RTT");
+ rtt = rtt_str ? strtoul(rtt_str, NULL, 10) * 1000 : 1;
+ }
+ if (rtt > 1)
+ usleep(rtt);
+}
+
 static int has_loose_object_local(const unsigned char *sha1)
 {
  char *name = sha1_file_name(sha1);
+ apply_rtt();
  return !access(name, F_OK);
 }

@@ -1303,6 +1315,7 @@ void prepare_packed_git(void)

  if (prepare_packed_git_run_once)
  return;
+
  prepare_packed_git_one(get_object_directory(), 1);
  prepare_alt_odb();
  for (alt = alt_odb_list; alt; alt = alt->next) {
@@ -1439,6 +1452,7 @@ static int open_sha1_file(const unsigned char *sha1)
  struct alternate_object_database *alt;

  fd = git_open_noatime(name);
+ apply_rtt();
  if (fd >= 0)
  return fd;

-- 
1.9.0.rc1.175.g0b1dcb5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/11] More preparatory work for multiparent tree-walker

[Kirill Smelkov]

On Mon, Feb 10, 2014 at 04:28:33PM -0800, Junio C Hamano wrote:
> Kirill Smelkov  writes:
> 
> > Here I'm preparing tree-diff.c to be ready for the new tree-walker, so that 
> > the
> > final change is tractable and looks good and non noisy. Some small speedups
> > are gained along the way. The final bits are almost ready, but I don't want 
> > to
> > release them in a hurry.
> 
> No worries.  We are not in a hurry to apply non-regression-fix
> changes during a pre-release feature freeze period anyway.

I see.


> This seems to somehow conflict with other topics and does not
> cleanly apply on top of your other tree-diff topic, by the way.

Sorry for the confusion. Could you please do the following:

Patches should be applied over to ks/tree-diff-walk
(74aa4a18). Before applying the patches, please cherry-pick

c90483d9(tree-diff: no need to manually verify that there is no
 mode change for a path)

ef4f0928(tree-diff: no need to pass match to
 skip_uninteresting())

into that branch, and drop them from ks/combine-diff - we'll have one
branch reworking the diff tree-walker, and the other taking advantage of
it for combine-diff.

Then apply sent-here patches, which, as I've verified this time, should
apply cleanly.

Thanks and sorry again,
Kirill
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html