Re: [PATCH] Allow combined diff to ignore white-spaces

2013-03-02 Thread Junio C Hamano 
Antoine Pelisse  writes:

> Currently, it's not possible to use the space-ignoring options (-b, -w,
> --ignore-space-at-eol) with combined diff. It makes it pretty impossible
> to read a merge between a branch that changed all tabs to spaces, and a
> branch with functional changes.
>
> Pass diff flags to diff engine, so that combined diff behaves as normal
> diff does with spaces.
>
> It also means that a conflict-less merge done using a ignore-* strategy
> option will not show any conflict if shown in combined-diff using the
> same option.
>
> Signed-off-by: Antoine Pelisse 
> ---
> That should be reviewed carefully as I'm not exactly sure that does make
> sense with the way combined-diff works.
> Still it seems natural to me to be able to remove the space in combined
> diff as we do with normal diff. Especially as I unfortunately have to
> deal with many space + feature merges that are very hard to analyze/handle
> if space differences can't be hidden.

Assuming "That" at the beginning of the off-line comment refers to
"this patch", I actually I do not think this patch needs to be
reviewed carefully at all.

The change in your patch to make the internal pairwise diff to
ignore whitespaces is an obvious and a sensible first step for your
stated goal.  With it, a block of lines where the postimage makes no
change other than whitespace changes from one parent and matches
with the other parent will see no hunks in either of the pair-wise
diff, and such a hunk will not appear in the final output, which is
exactly what you want.

What needs to be audited carefully is the part of combine diff logic
that this patch does *not* touch.

An example.  After collecting pairwise diffs between the shared
postimage and individual parents, we align the hunks and coalesce
"identical" preimage lines to form shared "-" (removed) lines.  I
think that step is done with byte-for-byte comparison.  If the
postimage removes a line from one parent and a corresponding line
from another parent, and if these corresponding lines in the parents
differ only by whitespaces in a way the user told us to ignore with
-b/-w/etc., you would need to tweak the logic to coalesce
corresponding "lost" lines in the append_lost() function. Otherwise,
you will see two " -" and "- " lines that remove these corresponding
lines from the parents that are identical when you ignore
whitespaces.

You should add some tests; it would help you think about these
issues through.

For example, in this history:

   git init
   seq 11 >ten
   git add ten
   git commit -m initial

   seq 10 | sed -e 's/6/6 six/' -e 's/2/2 two/' >ten
   echo 11 >>ten
   git commit -m ten -a

   git checkout -b side HEAD^
   seq 10 | sed -e 's/^/  /' | sed -e 's/2/2 dos/' >ten
   echo 11 >>ten
   git commit -m indent -a

   git merge master

   seq 10 | sed -e 's/5/5 go/' -e 's/2/2 dois/' >ten
   git commit -m merged -a

one side indents the original and tweaks some lines, while the other
side tweaks some other lines without reindenting.  Most notably, the
fifth line on one side is "5" while on the other side is "  5", and
this line is rewritten to "5 go" in the final.  The lost lines are
not coalesced to a single "-- 5" (nor "--   5") but shows that two
preimages were different only by whitespaces.  Compare that with
what happens to the final line "11" that gets lost in the result.

Thanks.

> Cheers,
> Antoine
>
>  combine-diff.c |6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/combine-diff.c b/combine-diff.c
> index 35d41cd..7ca0a72 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -215,7 +215,7 @@ static void combine_diff(const unsigned char *parent, 
> unsigned int mode,
>struct sline *sline, unsigned int cnt, int n,
>int num_parent, int result_deleted,
>struct userdiff_driver *textconv,
> -  const char *path)
> +  const char *path, long flags)
>  {
>   unsigned int p_lno, lno;
>   unsigned long nmask = (1UL << n);
> @@ -231,7 +231,7 @@ static void combine_diff(const unsigned char *parent, 
> unsigned int mode,
>   parent_file.ptr = grab_blob(parent, mode, &sz, textconv, path);
>   parent_file.size = sz;
>   memset(&xpp, 0, sizeof(xpp));
> - xpp.flags = 0;
> + xpp.flags = flags;
>   memset(&xecfg, 0, sizeof(xecfg));
>   memset(&state, 0, sizeof(state));
>   state.nmask = nmask;
> @@ -962,7 +962,7 @@ static void show_patch_diff(struct combine_diff_path 
> *elem, int num_parent,
>elem->parent[i].mode,
>&result_file, sline,
>cnt, i, num_parent, result_deleted,
> -  textconv, elem->path);
> +  textconv, elem->path, opt->xdl_opts);
>   }
>
>   show_hunks = make_hunk

Re: [PATCH] Avoid loading commits twice in log with diffs

2013-03-02 Thread Junio C Hamano 
Thomas Rast  writes:

> Test  with patchbefore
> 
> 4000.2: log --raw -3000   0.50(0.43+0.06)   0.54(0.46+0.06) +7.0%***
> 4000.3: log -p -3000  2.34(2.20+0.13)   2.37(2.22+0.13) +1.2%
> 
> Significance hints:  '.' 0.1  '*' 0.05  '**' 0.01  '***' 0.001

It may be a silly question but what is a significance hint?

> Signed-off-by: Thomas Rast 
> ---
>  log-tree.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/log-tree.c b/log-tree.c
> index eb1a1b4..277a38f 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -715,7 +715,7 @@ static int log_tree_diff(struct rev_info *opt, struct 
> commit *commit, struct log
>  {
>   int showed_log;
>   struct commit_list *parents;
> - unsigned const char *sha1 = commit->object.sha1;
> + unsigned const char *sha1 = commit->tree->object.sha1;

Overall I think this goes in the right direction and I can see why
the changes in later two hunks are correct, but I am not sure if we
can safely assume that the caller has parsed the incoming commit and
we have a good commit->tree here already.

Right now, this static function has a single call-site in a public
function log_tree_commit(), whose existing callers may all pass an
already parsed commit, but I feel somewhat uneasy to do the above
without some mechanism in place (either parse it here or in the
caller if unparsed, or document that log_tree_commit() must be
called with a parsed commit and perhaps add an assert there) to
ensure that the invariant is not broken in the future.

Thanks.

>   if (!opt->diff && !DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS))
>   return 0;
> @@ -742,7 +742,8 @@ static int log_tree_diff(struct rev_info *opt, struct 
> commit *commit, struct log
>* parent, showing summary diff of the others
>* we merged _in_.
>*/
> - diff_tree_sha1(parents->item->object.sha1, sha1, "", 
> &opt->diffopt);
> + parse_commit(parents->item);
> + diff_tree_sha1(parents->item->tree->object.sha1, sha1, 
> "", &opt->diffopt);
>   log_tree_diff_flush(opt);
>   return !opt->loginfo;
>   }
> @@ -755,7 +756,8 @@ static int log_tree_diff(struct rev_info *opt, struct 
> commit *commit, struct log
>   for (;;) {
>   struct commit *parent = parents->item;
>  
> - diff_tree_sha1(parent->object.sha1, sha1, "", &opt->diffopt);
> + parse_commit(parent);
> + diff_tree_sha1(parent->tree->object.sha1, sha1, "", 
> &opt->diffopt);
>   log_tree_diff_flush(opt);
>  
>   showed_log |= !opt->loginfo;
--
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: make sure rename pretty print works

2013-03-02 Thread Junio C Hamano 
Antoine Pelisse  writes:

> Add basic use cases and corner cases tests for
> "git diff -M --summary/stat".
>
> Signed-off-by: Antoine Pelisse 
> ---
>  t/t4056-rename-pretty.sh |   54 
> ++
>  1 file changed, 54 insertions(+)
>  create mode 100755 t/t4056-rename-pretty.sh

I wonder if this needs a new test script of its own.

If we anticipate future additions, it would make sense but otherwise
it may be better if we can find an existing one these tests can be
folded into.

> diff --git a/t/t4056-rename-pretty.sh b/t/t4056-rename-pretty.sh
> new file mode 100755
> index 000..806046f
> --- /dev/null
> +++ b/t/t4056-rename-pretty.sh
> @@ -0,0 +1,54 @@
> +#!/bin/sh
> +
> +test_description='Rename pretty print
> +
> +'

A single line would be sufficient...

> +test_expect_success common_prefix '
> + mkdir -p c/d &&
> + git mv c/b/a c/d/e &&
> + git commit -m. &&
> + git show -M --summary >output &&

I guess the unsightly "commit -m." is an attempt to prevent the
later grep from matching log message randomly, but if you test the
output from "git diff -M --stat/summary HEAD^ HEAD" you do not have
to worry about it, no?

Also I wonder if we can verify the filename part in --stat output.

> + test_i18ngrep "c/{b/a => d/e}" output

We would want to make sure that we do not have random cruft around
the paths, and the byte before that 'c' and after that '}' may want
to be verified.
--
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: [bug report] git-am applying maildir patches in reverse

2013-03-02 Thread Jeff King 
On Sat, Mar 02, 2013 at 10:22:46PM -0800, Junio C Hamano wrote:

> Andreas Schwab  writes:
> 
> > You should always cast to unsigned char when determining the order of
> > characters, to be consistent with strcmp/memcmp.
> 
> We treat runs of digits as numbers, so it is not even similar to
> strcmp.  As long as it is internally consistent (i.e. the return
> value inside the loop (*a - *b) must match the last return), it
> should be OK, no?

I almost responded and said something similar, but we also do byte-wise
comparisons for non-numeric elements, and we would want those to match
what other programs may do (and what git used to do).

I highly doubt that it matters in practice, as it would mean:

  1. The sorting of a maildir's filenames are dependent on the sorting
 of non-numeric bits. We can't rule out such a scheme, but I'd guess
 implementations either use numbers, or their sort order is
 meaningless (and that is what I found in the ones I looked at).

  2. The importantly-sorted bits contain non-ascii characters (the
 difference is only seen when we go outside the signed range).

but it doesn't hurt to be thorough (and to set a good example).

-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] Make !pattern in .gitattributes non-fatal

2013-03-02 Thread Junio C Hamano 
Duy Nguyen  writes:

> This "return NULL;" means we ignore "!blah" pattern, which is a
> regression, isn't it? Should we treat '!' as literal here?

Probably not.  Can you point to a project everybody has heard of
that keeps track of a path that begins with an exclamation point?

With clarification to the documentation that says "you cannot use !
to negate" and your "die on such an entry", we have been going in
the direction that forbids the use of such an entry, and making it
mean literal exclamation point is going sideways in the middle of
the road.

Besides, if you want to match a path that begins with an exclam, you
can always say "[!]", no?
--
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: [bug report] git-am applying maildir patches in reverse

2013-03-02 Thread Jeff King 
On Sat, Mar 02, 2013 at 09:44:39AM +0100, Andreas Schwab wrote:

> > +   return *a - *b;
> 
> You should always cast to unsigned char when determining the order of
> characters, to be consistent with strcmp/memcmp.

Thanks, I hadn't heard that advice before, but it makes obvious sense.
Junio, do you want to squash this on top?

diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 1ddbff4..06296d4 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -143,12 +143,12 @@ static int maildir_filename_cmp(const char *a, const char 
*b)
}
else {
if (*a != *b)
-   return *a - *b;
+   return (unsigned char)*a - (unsigned char)*b;
a++;
b++;
}
}
-   return *a - *b;
+   return (unsigned char)*a - (unsigned char)*b;
 }
 
 static int split_maildir(const char *maildir, const char *dir,

-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: [bug report] git-am applying maildir patches in reverse

2013-03-02 Thread Junio C Hamano 
Andreas Schwab  writes:

> You should always cast to unsigned char when determining the order of
> characters, to be consistent with strcmp/memcmp.

We treat runs of digits as numbers, so it is not even similar to
strcmp.  As long as it is internally consistent (i.e. the return
value inside the loop (*a - *b) must match the last return), it
should be OK, no?
--
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] submodule update: when using recursion, show full path

2013-03-02 Thread William Entriken 
Previously when using update with recursion, only the path for the
inner-most module was printed. Now the path is printed relative to
the directory the command was started from. This now matches the
behavior of submodule foreach.

Signed-off-by: William Entriken 
---
 git-submodule.sh | 31 ++-
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2365149..f2c53c9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -588,7 +588,7 @@ cmd_update()
die_if_unmatched "$mode"
if test "$stage" = U
then
-   echo >&2 "Skipping unmerged submodule $sm_path"
+   echo >&2 "Skipping unmerged submodule $prefix$sm_path"
continue
fi
name=$(module_name "$sm_path") || exit
@@ -602,7 +602,7 @@ cmd_update()
 
if test "$update_module" = "none"
then
-   echo "Skipping submodule '$sm_path'"
+   echo "Skipping submodule '$prefix$sm_path'"
continue
fi
 
@@ -611,7 +611,7 @@ cmd_update()
# Only mention uninitialized submodules when its
# path have been specified
test "$#" != "0" &&
-   say "$(eval_gettext "Submodule path '\$sm_path' not 
initialized
+   say "$(eval_gettext "Submodule path '\$prefix\$sm_path' 
not initialized
 Maybe you want to use 'update --init'?")"
continue
fi
@@ -624,7 +624,7 @@ Maybe you want to use 'update --init'?")"
else
subsha1=$(clear_local_git_env; cd "$sm_path" &&
git rev-parse --verify HEAD) ||
-   die "$(eval_gettext "Unable to find current revision in 
submodule path '\$sm_path'")"
+   die "$(eval_gettext "Unable to find current revision in 
submodule path '\$prefix\$sm_path'")"
fi
 
if test "$subsha1" != "$sha1" -o -n "$force"
@@ -643,7 +643,7 @@ Maybe you want to use 'update --init'?")"
(clear_local_git_env; cd "$sm_path" &&
( (rev=$(git rev-list -n 1 $sha1 --not 
--all 2>/dev/null) &&
 test -z "$rev") || git-fetch)) ||
-   die "$(eval_gettext "Unable to fetch in 
submodule path '\$sm_path'")"
+   die "$(eval_gettext "Unable to fetch in 
submodule path '\$prefix\$sm_path'")"
fi
 
# Is this something we just cloned?
@@ -657,20 +657,20 @@ Maybe you want to use 'update --init'?")"
case "$update_module" in
rebase)
command="git rebase"
-   die_msg="$(eval_gettext "Unable to rebase 
'\$sha1' in submodule path '\$sm_path'")"
-   say_msg="$(eval_gettext "Submodule path 
'\$sm_path': rebased into '\$sha1'")"
+   die_msg="$(eval_gettext "Unable to rebase 
'\$sha1' in submodule path '\$prefix\$sm_path'")"
+   say_msg="$(eval_gettext "Submodule path 
'\$prefix\$sm_path': rebased into '\$sha1'")"
must_die_on_failure=yes
;;
merge)
command="git merge"
-   die_msg="$(eval_gettext "Unable to merge 
'\$sha1' in submodule path '\$sm_path'")"
-   say_msg="$(eval_gettext "Submodule path 
'\$sm_path': merged in '\$sha1'")"
+   die_msg="$(eval_gettext "Unable to merge 
'\$sha1' in submodule path '\$prefix\$sm_path'")"
+   say_msg="$(eval_gettext "Submodule path 
'\$prefix\$sm_path': merged in '\$sha1'")"
must_die_on_failure=yes
;;
*)
command="git checkout $subforce -q"
-   die_msg="$(eval_gettext "Unable to checkout 
'\$sha1' in submodule path '\$sm_path'")"
-   say_msg="$(eval_gettext "Submodule path 
'\$sm_path': checked out '\$sha1'")"
+   die_msg="$(eval_gettext "Unable to checkout 
'\$sha1' in submodule path '\$prefix\$sm_path'")"
+   say_msg="$(eval_gettext "Submodule path 
'\$prefix\$sm_path': checked out '\$sha1'")"
;;
esac
 
@@ -688,11 +688,16 @@ Maybe you want to use 'update --init'?")"
 
if test -n "$recursive"
then
-   (clear_lo

Re: [PATCH] Revert "graph.c: mark private file-scope symbols as static"

2013-03-02 Thread Thomas Rast 
John Keeping  writes:

> This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.
>
> CGit uses these symbols to output the correct HTML around graph
> elements.  Making these symbols private means that CGit cannot be
> updated to use Git 1.8.0 or newer, so let's not do that.
>
> Signed-off-by: John Keeping 
> ---
>
> I realise that Git isn't a library so making the API useful for outside
> projects isn't a priority, but making these two methods public makes
> life a lot easier for CGit.
>
> Additionally, it seems that Johan added graph_set_column_colors
> specifically so that CGit should use it - there's no value to having
> that as a method just for its use in graph.c and he was the author of
> CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).

Perhaps you could add a comment in the source to prevent this from
happening again?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
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: Subtree in Git

2013-03-02 Thread Paul Campbell 
On Sat, Mar 2, 2013 at 11:21 AM, David Michael Barr  wrote:
> On Sat, Mar 2, 2013 at 9:05 AM, Paul Campbell  wrote:
>> On Fri, Mar 1, 2013 at 2:28 AM, Kindjal  wrote:
>>> David Michael Barr  rr-dav.id.au> writes:
>>>
 From a quick survey, it appears there are no more than 55 patches
 squashed into the submitted patch.
 As I have an interest in git-subtree for maintaining the out-of-tree
 version of vcs-svn/ and a desire to improve my rebase-fu, I am tempted
 to make some sense of the organic growth that happened on GitHub.
 It doesn't appear that anyone else is willing to do this, so I doubt
 there will be any duplication of effort.

>>>
>>> What is the status of the work on git-subtree described in this thread?
>>> It looks like it's stalled.
>>>
>>
>> I hadn't been aware of that patch. Reading the thread David Michael
>> Barr was going to try picking the patch apart into sensible chunks.
>>
>
> Sorry for not updating the thread. I did end up moving onto other things.
> I quickly realised the reason for globbing all the patches together was
> that the individual patches were not well contained.
> That is single patches with multiple unrelated changes and multiple
> patches changing the same things in different directions.
> To me this means that the first step is to curate the history.
>
>> If this work is still needing done I'd like to volunteer.
>
> You're most welcome. Sorry again for abandoning the thread.
>
> --
> David Michael Barr

Okay, I'll start picking the patch apart this week then feedback when
I have a plan to tackle it all.

-- 
Paul [W] Campbell
--
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] Allow combined diff to ignore white-spaces

2013-03-02 Thread Antoine Pelisse 
Currently, it's not possible to use the space-ignoring options (-b, -w,
--ignore-space-at-eol) with combined diff. It makes it pretty impossible
to read a merge between a branch that changed all tabs to spaces, and a
branch with functional changes.

Pass diff flags to diff engine, so that combined diff behaves as normal
diff does with spaces.

It also means that a conflict-less merge done using a ignore-* strategy
option will not show any conflict if shown in combined-diff using the
same option.

Signed-off-by: Antoine Pelisse 
---
That should be reviewed carefully as I'm not exactly sure that does make
sense with the way combined-diff works.
Still it seems natural to me to be able to remove the space in combined
diff as we do with normal diff. Especially as I unfortunately have to
deal with many space + feature merges that are very hard to analyze/handle
if space differences can't be hidden.

Cheers,
Antoine

 combine-diff.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 35d41cd..7ca0a72 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -215,7 +215,7 @@ static void combine_diff(const unsigned char *parent, 
unsigned int mode,
 struct sline *sline, unsigned int cnt, int n,
 int num_parent, int result_deleted,
 struct userdiff_driver *textconv,
-const char *path)
+const char *path, long flags)
 {
unsigned int p_lno, lno;
unsigned long nmask = (1UL << n);
@@ -231,7 +231,7 @@ static void combine_diff(const unsigned char *parent, 
unsigned int mode,
parent_file.ptr = grab_blob(parent, mode, &sz, textconv, path);
parent_file.size = sz;
memset(&xpp, 0, sizeof(xpp));
-   xpp.flags = 0;
+   xpp.flags = flags;
memset(&xecfg, 0, sizeof(xecfg));
memset(&state, 0, sizeof(state));
state.nmask = nmask;
@@ -962,7 +962,7 @@ static void show_patch_diff(struct combine_diff_path *elem, 
int num_parent,
 elem->parent[i].mode,
 &result_file, sline,
 cnt, i, num_parent, result_deleted,
-textconv, elem->path);
+textconv, elem->path, opt->xdl_opts);
}

show_hunks = make_hunks(sline, cnt, num_parent, dense);
--
1.7.9.5

--
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] Revert "graph.c: mark private file-scope symbols as static"

2013-03-02 Thread Johan Herland 
On Sat, Mar 2, 2013 at 1:46 PM, John Keeping  wrote:
> This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.
>
> CGit uses these symbols to output the correct HTML around graph
> elements.  Making these symbols private means that CGit cannot be
> updated to use Git 1.8.0 or newer, so let's not do that.
>
> Signed-off-by: John Keeping 
> ---
>
> I realise that Git isn't a library so making the API useful for outside
> projects isn't a priority, but making these two methods public makes
> life a lot easier for CGit.
>
> Additionally, it seems that Johan added graph_set_column_colors
> specifically so that CGit should use it - there's no value to having
> that as a method just for its use in graph.c and he was the author of
> CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).

Correct,

Acked-by: Johan Herland 


Have fun! :)

...Johan

-- 
Johan Herland, 
www.herland.net
--
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] tests: make sure rename pretty print works

2013-03-02 Thread Antoine Pelisse 
Add basic use cases and corner cases tests for
"git diff -M --summary/stat".

Signed-off-by: Antoine Pelisse 
---
 t/t4056-rename-pretty.sh |   54 ++
 1 file changed, 54 insertions(+)
 create mode 100755 t/t4056-rename-pretty.sh

diff --git a/t/t4056-rename-pretty.sh b/t/t4056-rename-pretty.sh
new file mode 100755
index 000..806046f
--- /dev/null
+++ b/t/t4056-rename-pretty.sh
@@ -0,0 +1,54 @@
+#!/bin/sh
+
+test_description='Rename pretty print
+
+'
+
+. ./test-lib.sh
+
+test_expect_success nothing_common '
+   mkdir -p a/b/ &&
+   : >a/b/c &&
+   git add a/b/c &&
+   git commit -m. &&
+   mkdir -p c/b/ &&
+   git mv a/b/c c/b/a &&
+   git commit -m. &&
+   git show -M --summary >output &&
+   test_i18ngrep "a/b/c => c/b/a" output
+'
+
+test_expect_success common_prefix '
+   mkdir -p c/d &&
+   git mv c/b/a c/d/e &&
+   git commit -m. &&
+   git show -M --summary >output &&
+   test_i18ngrep "c/{b/a => d/e}" output
+'
+
+test_expect_success common_suffix '
+   mkdir d &&
+   git mv c/d/e d/e &&
+   git commit -m. &&
+   git show -M --summary >output &&
+   test_i18ngrep "{c/d => d}/e" output
+'
+
+test_expect_success common_suffix_prefix '
+   mkdir d/f &&
+   git mv d/e d/f/e &&
+   git commit -m. &&
+   git show -M --summary >output &&
+   test_i18ngrep "d/{ => f}/e" output
+'
+
+test_expect_success common_overlap '
+   mkdir d/f/f &&
+   git mv d/f/e d/f/f/e &&
+   git commit -m. &&
+   git show -M --summary >output &&
+   test_i18ngrep "d/f/{ => f}/e" output
+'
+
+
+test_done
-- 
1.7.9.5

--
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: Debugging a bizarre problem: What can influence 'git fetch'?

2013-03-02 Thread git-users 
On Sat, 2 Mar 2013 18:32:17 +0800
Tay Ray Chuan  wrote:
> 
> It seems that the remote is running the 'dumb' http protocol, you
> might want to try setting the GIT_CURL_VERBOSE environment variable
> for more verbosity.
> 
> Have you tried running git-update-server-info on the remote side?
> Perhaps a push/fetch led to packs being created so the f981a2b object
> isn't available as a loose object but in a pack but the remote still
> indicates otherwise.

Hi.

Yes, git-update-server-info has been used on the remote but to no avail.

I'll try GIT_CURL_VERBOSE. Thanks!

M
--
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] Revert "graph.c: mark private file-scope symbols as static"

2013-03-02 Thread John Keeping 
This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb.

CGit uses these symbols to output the correct HTML around graph
elements.  Making these symbols private means that CGit cannot be
updated to use Git 1.8.0 or newer, so let's not do that.

Signed-off-by: John Keeping 
---

I realise that Git isn't a library so making the API useful for outside
projects isn't a priority, but making these two methods public makes
life a lot easier for CGit.

Additionally, it seems that Johan added graph_set_column_colors
specifically so that CGit should use it - there's no value to having
that as a method just for its use in graph.c and he was the author of
CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15).

 graph.c | 32 ++--
 graph.h | 27 +++
 2 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/graph.c b/graph.c
index 2a3fc5c..b24d04c 100644
--- a/graph.c
+++ b/graph.c
@@ -8,34 +8,6 @@
 /* Internal API */
 
 /*
- * Output the next line for a graph.
- * This formats the next graph line into the specified strbuf.  It is not
- * terminated with a newline.
- *
- * Returns 1 if the line includes the current commit, and 0 otherwise.
- * graph_next_line() will return 1 exactly once for each time
- * graph_update() is called.
- */
-static int graph_next_line(struct git_graph *graph, struct strbuf *sb);
-
-/*
- * Set up a custom scheme for column colors.
- *
- * The default column color scheme inserts ANSI color escapes to colorize
- * the graph. The various color escapes are stored in an array of strings
- * where each entry corresponds to a color, except for the last entry,
- * which denotes the escape for resetting the color back to the default.
- * When generating the graph, strings from this array are inserted before
- * and after the various column characters.
- *
- * This function allows you to enable a custom array of color escapes.
- * The 'colors_max' argument is the index of the last "reset" entry.
- *
- * This functions must be called BEFORE graph_init() is called.
- */
-static void graph_set_column_colors(const char **colors, unsigned short 
colors_max);
-
-/*
  * Output a padding line in the graph.
  * This is similar to graph_next_line().  However, it is guaranteed to
  * never print the current commit line.  Instead, if the commit line is
@@ -90,7 +62,7 @@ enum graph_state {
 static const char **column_colors;
 static unsigned short column_colors_max;
 
-static void graph_set_column_colors(const char **colors, unsigned short 
colors_max)
+void graph_set_column_colors(const char **colors, unsigned short colors_max)
 {
column_colors = colors;
column_colors_max = colors_max;
@@ -1144,7 +1116,7 @@ static void graph_output_collapsing_line(struct git_graph 
*graph, struct strbuf
graph_update_state(graph, GRAPH_PADDING);
 }
 
-static int graph_next_line(struct git_graph *graph, struct strbuf *sb)
+int graph_next_line(struct git_graph *graph, struct strbuf *sb)
 {
switch (graph->state) {
case GRAPH_PADDING:
diff --git a/graph.h b/graph.h
index 19b0f66..aff960c 100644
--- a/graph.h
+++ b/graph.h
@@ -4,6 +4,22 @@
 /* A graph is a pointer to this opaque structure */
 struct git_graph;
 
+/*
+ * Set up a custom scheme for column colors.
+ *
+ * The default column color scheme inserts ANSI color escapes to colorize
+ * the graph. The various color escapes are stored in an array of strings
+ * where each entry corresponds to a color, except for the last entry,
+ * which denotes the escape for resetting the color back to the default.
+ * When generating the graph, strings from this array are inserted before
+ * and after the various column characters.
+ *
+ * This function allows you to enable a custom array of color escapes.
+ * The 'colors_max' argument is the index of the last "reset" entry.
+ *
+ * This functions must be called BEFORE graph_init() is called.
+ */
+void graph_set_column_colors(const char **colors, unsigned short colors_max);
 
 /*
  * Create a new struct git_graph.
@@ -33,6 +49,17 @@ void graph_update(struct git_graph *graph, struct commit 
*commit);
  */
 int graph_is_commit_finished(struct git_graph const *graph);
 
+/*
+ * Output the next line for a graph.
+ * This formats the next graph line into the specified strbuf.  It is not
+ * terminated with a newline.
+ *
+ * Returns 1 if the line includes the current commit, and 0 otherwise.
+ * graph_next_line() will return 1 exactly once for each time
+ * graph_update() is called.
+ */
+int graph_next_line(struct git_graph *graph, struct strbuf *sb);
+
 
 /*
  * graph_show_*: helper functions for printing to stdout
-- 
1.8.2.rc1.339.g93ec2c9

--
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: Subtree in Git

2013-03-02 Thread David Michael Barr 
On Sat, Mar 2, 2013 at 9:05 AM, Paul Campbell  wrote:
> On Fri, Mar 1, 2013 at 2:28 AM, Kindjal  wrote:
>> David Michael Barr  rr-dav.id.au> writes:
>>
>>> From a quick survey, it appears there are no more than 55 patches
>>> squashed into the submitted patch.
>>> As I have an interest in git-subtree for maintaining the out-of-tree
>>> version of vcs-svn/ and a desire to improve my rebase-fu, I am tempted
>>> to make some sense of the organic growth that happened on GitHub.
>>> It doesn't appear that anyone else is willing to do this, so I doubt
>>> there will be any duplication of effort.
>>>
>>
>> What is the status of the work on git-subtree described in this thread?
>> It looks like it's stalled.
>>
>
> I hadn't been aware of that patch. Reading the thread David Michael
> Barr was going to try picking the patch apart into sensible chunks.
>

Sorry for not updating the thread. I did end up moving onto other things.
I quickly realised the reason for globbing all the patches together was
that the individual patches were not well contained.
That is single patches with multiple unrelated changes and multiple
patches changing the same things in different directions.
To me this means that the first step is to curate the history.

> If this work is still needing done I'd like to volunteer.

You're most welcome. Sorry again for abandoning the thread.

--
David Michael Barr
--
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: Debugging a bizarre problem: What can influence 'git fetch'?

2013-03-02 Thread Tay Ray Chuan 
On Fri, Mar 1, 2013 at 10:39 PM,   wrote:
> Is there some way to get 'git fetch'
> to be more verbose?

It seems that the remote is running the 'dumb' http protocol, you
might want to try setting the GIT_CURL_VERBOSE environment variable
for more verbosity.

Have you tried running git-update-server-info on the remote side?
Perhaps a push/fetch led to packs being created so the f981a2b object
isn't available as a loose object but in a pack but the remote still
indicates otherwise.

--
Cheers,
Ray Chuan
--
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] Avoid loading commits twice in log with diffs

2013-03-02 Thread Thomas Rast 
If you run a log with diffs (such as -p, --raw, --stat etc.) the
current code ends up loading many objects twice.  For example, for
'log -3000 -p' my instrumentation said the objects loaded more than
once are distributed as follows:

  2008 blob
  2103 commit
  2678 tree

Fixing blobs and trees will be harder, because those are really used
within the diff engine and need some form of caching.

However, fixing the commits is easy at least at the band-aid level.
They are triggered by log_tree_diff() invoking diff_tree_sha1() on
commits, which duly loads the specified object to dereference it to a
tree.  Since log_tree_diff() knows that it works with commits and they
must have trees, we can simply pass through the trees.

This has a quite dramatic effect on log --raw, though only a
negligible impact on log -p:

Test  with patchbefore

4000.2: log --raw -3000   0.50(0.43+0.06)   0.54(0.46+0.06) +7.0%***
4000.3: log -p -3000  2.34(2.20+0.13)   2.37(2.22+0.13) +1.2%

Significance hints:  '.' 0.1  '*' 0.05  '**' 0.01  '***' 0.001

Signed-off-by: Thomas Rast 
---
 log-tree.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index eb1a1b4..277a38f 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -715,7 +715,7 @@ static int log_tree_diff(struct rev_info *opt, struct 
commit *commit, struct log
 {
int showed_log;
struct commit_list *parents;
-   unsigned const char *sha1 = commit->object.sha1;
+   unsigned const char *sha1 = commit->tree->object.sha1;
 
if (!opt->diff && !DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS))
return 0;
@@ -742,7 +742,8 @@ static int log_tree_diff(struct rev_info *opt, struct 
commit *commit, struct log
 * parent, showing summary diff of the others
 * we merged _in_.
 */
-   diff_tree_sha1(parents->item->object.sha1, sha1, "", 
&opt->diffopt);
+   parse_commit(parents->item);
+   diff_tree_sha1(parents->item->tree->object.sha1, sha1, 
"", &opt->diffopt);
log_tree_diff_flush(opt);
return !opt->loginfo;
}
@@ -755,7 +756,8 @@ static int log_tree_diff(struct rev_info *opt, struct 
commit *commit, struct log
for (;;) {
struct commit *parent = parents->item;
 
-   diff_tree_sha1(parent->object.sha1, sha1, "", &opt->diffopt);
+   parse_commit(parent);
+   diff_tree_sha1(parent->tree->object.sha1, sha1, "", 
&opt->diffopt);
log_tree_diff_flush(opt);
 
showed_log |= !opt->loginfo;
-- 
1.8.2.rc1.393.ga167915

--
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: [bug report] git-am applying maildir patches in reverse

2013-03-02 Thread Andreas Schwab 
Jeff King  writes:

>  static int maildir_filename_cmp(const char *a, const char *b)
>  {
> - while (1) {
> + while (*a && *b) {
>   if (isdigit(*a) && isdigit(*b)) {
>   long int na, nb;
>   na = strtol(a, (char **)&a, 10);
> @@ -148,6 +148,7 @@ static int maildir_filename_cmp(const char *a, const char 
> *b)
>   b++;
>   }
>   }
> + return *a - *b;

You should always cast to unsigned char when determining the order of
characters, to be consistent with strcmp/memcmp.

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."
--
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: Elegant subdirectory checkout of remote-tracking branch?

2013-03-02 Thread Jeff King 
On Fri, Mar 01, 2013 at 10:22:53AM -0500, W. Trevor King wrote:

> These fail because I can't use a remote tracking branch as a
> source for the clone.  It should be possible to do:
> 
>   $ git clone --reference . --single-branch --branch todo 
> git://git.kernel.org/pub/scm/git/git.git Meta
> 
> but that will require (I think) network access during a fetch.

Yes, it will. Junio mentioned already that for him, "Meta" is really a
separate repository, and I think the simplest thing is to just treat it
that way (that's how I handle my personal "meta" branch).

But if you really want to save the extra network round trip during a
fetch, you can either:

  1. Just fetch from the surrounding repository using a custom refspec,
 like:

   git init Meta
   cd Meta
   git config add remote.origin.url ..
   git config add remote.origin.fetch \
 refs/remotes/origin/todo:refs/remotes/origin/todo
   git fetch
   git checkout todo

or

  2. Look into the git-new-workdir script in contrib/workdir, which lets
 you check out an alternate branch in a separate directory.

The latter would probably be the most seamless, but it's also the most
likely to have bugs. :)

-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