Re: Configuring the location of ~/.gitconfig

2012-09-27 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 I'd like to configure the location of ~/.gitconfig through an
 environment variable.  My usecase is a simple enough: I have a
 repository with all my dotfiles, and I don't want to symlink
 ~/dotfiles/.gitconfig from $HOME after cloning it.  Does anyone else
 think the feature will be useful?

Not me. For that particular use case, my approach (long before I
switched the vcs that controls my dotfiles to git) have always been
to have ~/src that is version controlled, with a Makefile that
builds/adjusts dotfiles appropriately for each box and installs them
in the proper place.

Of course, if .gitconfig _is_ the same across all boxes I use, that
Makefile is likely to install by making a symlink ~/.gitconfig
that points at src/dot/gitconfig without building.
--
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: CRLF, LF ... CR ?

2012-09-27 Thread Junio C Hamano
David Aguilar dav...@gmail.com writes:

 That said, perhaps the autocrlf code is simple enough that it
 could be easily tweaked to also handle this special case,...

I wouldn't be surprised if it is quite simple.

We (actually Linus, IIRC) simply declared from the get-go that it is
not worth spending any line of code only to worry about pre OSX
Macintosh when we did the end-of-line stuff, and nobody so far
showed any need.

--
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 3/3] completion: improve shell expansion of items

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 02:37:51AM +0200, SZEDER Gábor wrote:

  +# Quotes each element of an IFS-delimited list for shell reuse
  +__git_quote()
  +{
  +   local i
  +   local delim
  +   for i in $1; do
  +   local quoted=${i//\'/\'\\\'\'}
  +   printf ${delim:+$IFS}'%s' $quoted
  +   delim=t
  +   done
  +}
 [...]

 Iterating through all possible completion words undermines the main
 reason for __gitcomp_nl()'s existence: to handle potentially large
 number of possible completion words faster than the old __gitcomp().
 If we really have to iterate in a subshell, then it would perhaps be
 better to drop __gitcomp_nl(), go back to using __gitcomp(), and
 modify that instead.

Thanks for reminding me to time. I noticed your a31e626 while digging in
the history, but forgot that I wanted to do a timing test. Sadly, the
results are very discouraging. Doing a similar test to your 10,000-refs,
I get:

  $ refs=$(seq 1 1)

  $ . git-completion.bash.old
  $ time __gitcomp_nl $refs
  real0m0.065s
  user0m0.064s
  sys 0m0.004s

  $ . git-completion.bash.new
  $ time __gitcomp_nl $refs
  real0m1.799s
  user0m1.828s
  sys 0m0.036s

So, a 2700% slowdown. Yuck.

I also tried running it through sed instead of iterating in bash. I know
that some people will not like the fork, but curiously enough, it was
not that much faster. Which makes me wonder if part of the slowdown is
actually bash unquoting the result in compgen.

 After all, anyone could drop a file called git-cmd-${meta} on his
 $PATH, and then get cmd- offered, because completion of git commands
 still goes through __gitcomp().

Yeah. I wasn't sure if __gitcomp() actually got used on any arbitrary
data, but that's a good example.

I'm not sure where to go next. I guess we could try pre-quoting via git
when generating the list of refs (or files, or whatever) and hope that
is faster.

With this patch as it is, I'm not sure the slowdown is worth it. Yes,
it's more correct in the face of metacharacters, but those are the
uncommon case by a large margin. I'd hate to have a performance
regression in the common case just for that.

-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: A generalization of git blame

2012-09-27 Thread Junio C Hamano
xm...@cs.wisc.edu writes:

 It largely depends on how the user would interact with your program,
 which is totally unclear as we haven't seen any part of it.  I do
 not think we have enough information to answer the question at this
 point.

 Do you mean it largely depends on the diversity of options on input and
 output formats? ...
 ... I know this is not enough for a tool. So this is case, does how the user
 would interact with your program mean that I should add ...

I am not saying anything about what you should or should not do. It
is your program, and we haven't seen anything about it, other than
handwaving, what good it will do to its users, so I am not qualified
to make such a comment on it yet.  What I meant by how the users
would interact with... are things like this:

 - Why users would want to use it in the first place?  What are
   missing from existing tool set?

 - What kind of questions do users ask to the program and how do
   they ask them?

 - How are the answers to these questions presented by the program
   to the users?

 - How do users interpret and use these answers in what way?

Notice that I didn't include How do you compute the answers?  When
we are initially evaluating a feature at how do they interact level,
we are not interested in the implementation at all.
--
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] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test suite for detecting heap corruption

2012-09-27 Thread Junio C Hamano
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: Configuring the location of ~/.gitconfig

2012-09-27 Thread Anurag Priyam
On Thu, Sep 27, 2012 at 11:39 AM, Junio C Hamano gits...@pobox.com wrote:
 Ramkumar Ramachandra artag...@gmail.com writes:

 I'd like to configure the location of ~/.gitconfig through an
 environment variable.  My usecase is a simple enough: I have a
 repository with all my dotfiles, and I don't want to symlink
 ~/dotfiles/.gitconfig from $HOME after cloning it.  Does anyone else
 think the feature will be useful?

 Not me. For that particular use case, my approach (long before I
 switched the vcs that controls my dotfiles to git) have always been
 to have ~/src that is version controlled, with a Makefile that
 builds/adjusts dotfiles appropriately for each box and installs them
 in the proper place.

Makefile is what I wanted to avoid when I suggested Ram that maybe Git
could _optionally_ read the location of global gitconfig from an
environment variable that can be exported in zshenv or bash_profile.

I don't think either way is the best way of managing dotfiles.  Just a
matter of preference.  The environment variable approach doesn't
require you to run `make` everytime you sync your dotfiles across
different machines, and that is what I like.

Also, Git allows configuring the location of template directory via an
environment variable (GIT_TEMPLATE_DIR).  Since Git has already fixed
the location of global gitconfig, it might as well read the location
of template directory from there (init.templatedir).  Why the added
flexibility?  Well, I have been exploiting the feature to manage git
templates through my zsh configuration.

-- 
Anurag Priyam
--
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 3/3] completion: improve shell expansion of items

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 02:28:55AM -0400, Jeff King wrote:

 Thanks for reminding me to time. I noticed your a31e626 while digging in
 the history, but forgot that I wanted to do a timing test. Sadly, the
 results are very discouraging. Doing a similar test to your 10,000-refs,
 I get:
 
   $ refs=$(seq 1 1)
 
   $ . git-completion.bash.old
   $ time __gitcomp_nl $refs
   real0m0.065s
   user0m0.064s
   sys 0m0.004s
 
   $ . git-completion.bash.new
   $ time __gitcomp_nl $refs
   real0m1.799s
   user0m1.828s
   sys 0m0.036s
 
 So, a 2700% slowdown. Yuck.
 
 I also tried running it through sed instead of iterating in bash. I know
 that some people will not like the fork, but curiously enough, it was
 not that much faster. Which makes me wonder if part of the slowdown is
 actually bash unquoting the result in compgen.

Ah. The problem is that most of the load comes from my patch 4/3, which
does a separate iteration. Here are the numbers after just patch 3:

  $ time __gitcomp_nl $refs
  real0m0.344s
  user0m0.392s
  sys 0m0.040s

Slower, but not nearly as painful. Here are the numbers using sed[1]
instead:

  $ time __gitcomp_nl $refs
  real0m0.100s
  user0m0.084s
  sys 0m0.016s

So a little slower, but probably acceptable. We could maybe do the same
trick on the output side (although it is a little trickier there,
because we need it in a bash array). Of course, this is Linux; the fork
for sed is way more expensive on some systems.

Still, I'd be curious to see it with the callers (e.g., __git_refs)
doing their own quoting. I'd worry that it would become a maintenance
headache, but perhaps we don't have that many lists we feed (there are a
lot of calls to gitcomp_nl, but they are mostly feeding __git_refs).

-Peff

[1] For reference, that patch is:

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 1fc43f7..5ff3742 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -225,16 +225,15 @@ __git_quote()
 fi
 fi
 
-# Quotes each element of an IFS-delimited list for shell reuse
+# Quotes each element of a newline-delimited list for shell reuse
 __git_quote()
 {
-   local i
-   local delim
-   for i in $1; do
-   local quoted=${i//\'/\'\\\'\'}
-   printf ${delim:+$IFS}'%s' $quoted
-   delim=t
-   done
+   echo $1 |
+   sed 
+ s/'/'''/g
+ s/^/'/
+ s/$/'/
+   
 }
 
 # Generates completion reply with compgen, appending a space to possible
--
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: bash completion with colour hints

2012-09-27 Thread Junio C Hamano
Simon Oosthoek soosth...@nieuwland.nl writes:

 I read the guide and now I have some questions:

 - It suggests to use the oldest commit that contains the bug and can
 support the fix. This would be the very first mention of __git_ps1
 function I think commit d3d717a4ad0c8d7329e79f7d0313baec57c6b585

You could claim that the lack of coloring is a bug, but nobody
complained about it so far (and I personally hate coloring in
prompts as they are distracting noise, and would reject a patch if
it weren't made conditional), so I would think this is more about
adding a feature the users can choose to use but they do not have
to.

We do not usually add new features to maintenance tracks, so the
result of applying the patch does not have to be merge-able to maint
or amything older.  I would base the patch on v1.7.12 (the latest
stable release) if I were you.

 - I read that git-prompt.sh is meant to support bash and zsh, I have
 only tested it on bash. Should I attempt to test it on zsh or is there a
 kind person with zsh as his/her shell to test it for me?

That is something you should ask on list, like you did here, but the
most effective way to do so is do so when you send a patch you
worked on and tested with bash.  Say I've tested it only with bash;
can you please take a look? and Cc the folks you find in the output
of git log contrib/completion/ who worked on making it workable
with zsh.
--
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: bash completion with colour hints

2012-09-27 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 27.09.2012 08:53:
 Simon Oosthoek soosth...@nieuwland.nl writes:
 
 I read the guide and now I have some questions:

 - It suggests to use the oldest commit that contains the bug and can
 support the fix. This would be the very first mention of __git_ps1
 function I think commit d3d717a4ad0c8d7329e79f7d0313baec57c6b585
 
 You could claim that the lack of coloring is a bug, but nobody
 complained about it so far (and I personally hate coloring in
 prompts as they are distracting noise, and would reject a patch if
 it weren't made conditional), so I would think this is more about
 adding a feature the users can choose to use but they do not have
 to.
 
 We do not usually add new features to maintenance tracks, so the
 result of applying the patch does not have to be merge-able to maint
 or amything older.  I would base the patch on v1.7.12 (the latest
 stable release) if I were you.
 
 - I read that git-prompt.sh is meant to support bash and zsh, I have
 only tested it on bash. Should I attempt to test it on zsh or is there a
 kind person with zsh as his/her shell to test it for me?

Actually, the instructions in the prompt script are not complete for zsh
(you need to activate expansion in the prompt), and I think there is
some breakage because bash uses \h etc. for PS1 format specifiers
whereas zsh uses '%h', and so the '%' in the prompt is a problem for zsh.

 That is something you should ask on list, like you did here, but the
 most effective way to do so is do so when you send a patch you
 worked on and tested with bash.  Say I've tested it only with bash;
 can you please take a look? and Cc the folks you find in the output
 of git log contrib/completion/ who worked on making it workable
 with zsh.
 

Additionally, there have been several attempts already for prompt. So
I'd suggest you check the list archives to see whether you addressed the
issues which were raised back then. I've actually been toying with that
myself lately. A few hints:

- You need to escape the escapes '\[', i.e. tell the shell that color
escapes produce zero length output, or else line wrapping is distorted.

- Bash interpretes '\' only when PS1 is assigned, not when an expansion
in PS1 produces it.

- Some don't like it colorful, so the coloring needs to depend on a
config setting or env variable.

- Coloring should be consistent with other coloring in Git, such as
'status -s -b'.

- Coloring provides the way to make the prompt characters analogous to
'status -s -b' because a green 'M' is different from red 'M', and is
much clearer then having to remember '+' vs. '*' (so there is a logic in
that).

From trying myself, I'm convinced that you need a clever combination of
PROMPT_COMMAND and PS1 to make this work. Setting PS1 in PROMPT_COMMAND
is probably a no-go because that makes it difficult to customize PS1. I
have something in the works which reproduces the current prompt but need
to clean it up further. The actual coloring would require setting a lot
of variables which communicate data from PROMPT_COMMAND to PS1, and I
actually don't like that.

An alternative approach would be:

- Tell users to activate git prompt by doing something like

PROMPT_COMMAND='__git_prompt [\u@\h \W (%s)]\$ '

rather than the current

PS1='[\u@\h \W$(__git_ps1  (%s))]\$ '

- set PS1 from __git_prompt

I'm not sure about the performance implications of re-setting PS1 on
(before) each prompt invocation, though. Would that be OK?

Michael
--
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: bash completion with colour hints

2012-09-27 Thread Simon Oosthoek

On 09/27/2012 10:53 AM, Michael J Gruber wrote:

We do not usually add new features to maintenance tracks, so the
result of applying the patch does not have to be merge-able to maint
or amything older.  I would base the patch on v1.7.12 (the latest
stable release) if I were you.



I now have a patch based on 1.7.12


- I read that git-prompt.sh is meant to support bash and zsh, I have
only tested it on bash. Should I attempt to test it on zsh or is there a
kind person with zsh as his/her shell to test it for me?


Actually, the instructions in the prompt script are not complete for zsh
(you need to activate expansion in the prompt), and I think there is
some breakage because bash uses \h etc. for PS1 format specifiers
whereas zsh uses '%h', and so the '%' in the prompt is a problem for zsh.


The only feature using it is untracked files, so that should probably be 
changed to work with zsh?




Additionally, there have been several attempts already for prompt. So
I'd suggest you check the list archives to see whether you addressed the
issues which were raised back then. I've actually been toying with that
myself lately. A few hints:


This would take me too much time



- You need to escape the escapes '\[', i.e. tell the shell that color
escapes produce zero length output, or else line wrapping is distorted.


A: this is something I've run into with my own contraption and I'm not 
sure I fixed it with my patch :-(


Can you give a more complete example of how that should work?



- Bash interpretes '\' only when PS1 is assigned, not when an expansion
in PS1 produces it.


is that true? how can \w produce the current working dir in the prompt?
Or do you mean it is translated to $PWD in some form internally?



- Some don't like it colorful, so the coloring needs to depend on a
config setting or env variable.


It is.



- Coloring should be consistent with other coloring in Git, such as
'status -s -b'.


That would be nice, I suppose the coloring is configurable for git, so 
the values would need to be dynamically imported from the git config. I 
don't know how to do that.




- Coloring provides the way to make the prompt characters analogous to
'status -s -b' because a green 'M' is different from red 'M', and is
much clearer then having to remember '+' vs. '*' (so there is a logic in
that).


Are you suggesting to use M instead of * or +? wouldn't that interfere 
with branchnames?




 From trying myself, I'm convinced that you need a clever combination of
PROMPT_COMMAND and PS1 to make this work. Setting PS1 in PROMPT_COMMAND
is probably a no-go because that makes it difficult to customize PS1. I
have something in the works which reproduces the current prompt but need
to clean it up further. The actual coloring would require setting a lot
of variables which communicate data from PROMPT_COMMAND to PS1, and I
actually don't like that.


I think it should be possible in PS1 only.



An alternative approach would be:

- Tell users to activate git prompt by doing something like

PROMPT_COMMAND='__git_prompt [\u@\h \W (%s)]\$ '

rather than the current

PS1='[\u@\h \W$(__git_ps1  (%s))]\$ '

- set PS1 from __git_prompt

I'm not sure about the performance implications of re-setting PS1 on
(before) each prompt invocation, though. Would that be OK?


I doubt you can set PS1 from a function (effectively a subshell?)

Thanks for your comments

I will send an updated patch later

/Simon
--
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: bash completion with colour hints

2012-09-27 Thread Andreas Schwab
Michael J Gruber g...@drmicha.warpmail.net writes:

 - Bash interpretes '\' only when PS1 is assigned, not when an expansion
 in PS1 produces it.

What you probably mean is that bash does not rescan expansions for
backslash escapes, thus only literal occurences in PS1 are processed
(which is consistent with ordinary expansion).

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


[PATCH] revision: add --reflog-message to grep reflog messages

2012-09-27 Thread Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 On Thu, Sep 27, 2012 at 2:28 AM, Junio C Hamano gits...@pobox.com wrote:
  The current commit_match() runs grep_buffer() on commit-buffer.  It
  probably makes sense to instead notice from opt that we are running
  log with -g, prepare a temporary strbuf and add in the reflog
  message to the string in commit-buffer, and run grep_buffer() on
  that temporary strbuf on it.

 Yeah. I was starting to think that way too, otherwise combining grep
 options would be a mess. I was hoping by injecting a fake reflog
 header, we could simplify reflog exceptions in pretty.c. But it
 was not that easy.

  I personally think it is sufficient ot just reuse --grep on
  concatenation of commit-buffer with Reflog message: checkout:
  moving from as/check-ignore to pu.

 --grep only reads the commit body. So either we append reflog
 message to commit body, or we put it in the header and add a new
 option for it. I don't like appending things to the commit body as
 --grep may hit reflog message while users do not mean so.

  If you really want to go fancier, you could add --grep-reflog option
  that behaves like the existing --author and --committer options to
  add header match elements to the grep expression, splice a fake
  reflog  header to the string copied from commit-buffer

 Inserting at the beginning of the commit like your demo patch works
 just fine and is simpler. I'm tempted to take the undocumented option
 name --reflog for this purpose. But it's probably not worth the risk.

 Documentation/rev-list-options.txt |  5 +
 grep.c |  1 +
 grep.h |  5 +++--
 revision.c | 19 +--
 t/t7810-grep.sh|  6 ++
 5 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 1fc2a18..aeaa58c 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -51,6 +51,11 @@ endif::git-rev-list[]
commits whose author matches any of the given patterns are
chosen (similarly for multiple `--committer=pattern`).
 
+--reflog-message=pattern::
+   Limit the commits output to ones with reflog entries that
+   match the specified pattern (regular expression). Ignored unless
+   --walk-reflogs is given.
+
 --grep=pattern::
 
Limit the commits output to ones with log message that
diff --git a/grep.c b/grep.c
index 898be6e..72ac1bf 100644
--- a/grep.c
+++ b/grep.c
@@ -697,6 +697,7 @@ static struct {
 } header_field[] = {
{ author , 7 },
{ committer , 10 },
+   { reflog , 7 },
 };
 
 static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
diff --git a/grep.h b/grep.h
index 8a28a67..1416ad7 100644
--- a/grep.h
+++ b/grep.h
@@ -29,9 +29,10 @@ enum grep_context {
 
 enum grep_header_field {
GREP_HEADER_AUTHOR = 0,
-   GREP_HEADER_COMMITTER
+   GREP_HEADER_COMMITTER,
+   GREP_HEADER_REFLOG,
+   GREP_HEADER_FIELD_MAX
 };
-#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)
 
 struct grep_pat {
struct grep_pat *next;
diff --git a/revision.c b/revision.c
index ae12e11..837051c 100644
--- a/revision.c
+++ b/revision.c
@@ -1595,6 +1595,9 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
} else if ((argcount = parse_long_opt(committer, argv, optarg))) {
add_header_grep(revs, GREP_HEADER_COMMITTER, optarg);
return argcount;
+   } else if ((argcount = parse_long_opt(reflog-message, argv, 
optarg))) {
+   add_header_grep(revs, GREP_HEADER_REFLOG, optarg);
+   return argcount;
} else if ((argcount = parse_long_opt(grep, argv, optarg))) {
add_message_grep(revs, optarg);
return argcount;
@@ -2212,8 +2215,20 @@ static int commit_match(struct commit *commit, struct 
rev_info *opt)
 {
if (!opt-grep_filter.pattern_list  !opt-grep_filter.header_list)
return 1;
-   return grep_buffer(opt-grep_filter,
-  commit-buffer, strlen(commit-buffer));
+   if (opt-reflog_info) {
+   int retval;
+   struct strbuf buf = STRBUF_INIT;
+   strbuf_addstr(buf, reflog );
+   get_reflog_message(buf, opt-reflog_info);
+   strbuf_addch(buf, '\n');
+   strbuf_addstr(buf, commit-buffer);
+   retval = grep_buffer(opt-grep_filter, buf.buf, buf.len);
+   strbuf_release(buf);
+   return retval;
+   } else {
+   return grep_buffer(opt-grep_filter,
+  commit-buffer, strlen(commit-buffer));
+   }
 }
 
 static inline int want_ancestry(struct rev_info *revs)
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 91db352..a0f519e 100755
--- 

Re: bash completion with colour hints

2012-09-27 Thread Simon Oosthoek

On 09/27/2012 10:53 AM, Michael J Gruber wrote:



 From trying myself, I'm convinced that you need a clever combination of
PROMPT_COMMAND and PS1 to make this work. Setting PS1 in PROMPT_COMMAND
is probably a no-go because that makes it difficult to customize PS1. I
have something in the works which reproduces the current prompt but need
to clean it up further. The actual coloring would require setting a lot
of variables which communicate data from PROMPT_COMMAND to PS1, and I
actually don't like that.

An alternative approach would be:

- Tell users to activate git prompt by doing something like

PROMPT_COMMAND='__git_prompt [\u@\h \W (%s)]\$ '

rather than the current

PS1='[\u@\h \W$(__git_ps1  (%s))]\$ '

- set PS1 from __git_prompt

I'm not sure about the performance implications of re-setting PS1 on
(before) each prompt invocation, though. Would that be OK?


After some research, I completely agree with your assessment!

I don't think the performance issue is that big and it's the only way to 
get the colors in there without messing up word wrapping.


I'm afraid that either this will need a new function just to get the 
coloring hints or that it will break the usage of __git_ps1 entirely (if 
you'd require the user to use it as PROMPT_COMMAND instead of via PS1)


And I suppose this will not be compatible with zsh?

/Simon

PS, I'd rather have colours with wrapping issues, than no colour hints. 
But I realise this has to be fixed before inclusion into the main tree.



--
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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Nguyen Thai Ngoc Duy
On Thu, Sep 27, 2012 at 7:47 AM, Shawn Pearce spea...@spearce.org wrote:
 Google has published a series of patches (see links below) to JGit to

Should discussions about this series happen in here, jgit mailing or
gerrit? I just want to make sure I'll discuss it at the right place.

 improve fetch and clone performance by adding compressed bitmaps to
 the pack-*.idx structure.

 Operation   Index V2   Index VE003
 Clone   37530ms (524.06 MiB) 82ms (524.06 MiB)
 Fetch (1 commit back)  75ms 107ms
 Fetch (10 commits back)   456ms (269.51 KiB)341ms (265.19 KiB)
 Fetch (100 commits back)  449ms (269.91 KiB)337ms (267.28 KiB)
 Fetch (1000 commits back)2229ms ( 14.75 MiB)189ms ( 14.42 MiB)
 Fetch (1 commits back)   2177ms ( 16.30 MiB)254ms ( 15.88 MiB)
 Fetch (10 commits back) 14340ms (185.83 MiB)   1655ms (189.39 MiB)

Beautiful. And curious, why do 100-1000 and 1-1 have such
big leaps in time (V2)?

 The basic gist of the implementation is a bitmap has a 1 bit set for
 each object that is reachable from the commit the bitmap is associated
 with. An index file may have a unique bitmap for hundreds of commits
 in the corresponding pack file. The set of objects to send is
 performed by doing a simple computation:

   OR (all want lines) AND NOT OR (all have lines)

 There are two key patches in the series that implement the file format
 change and logic involved:

 * https://git.eclipse.org/r/7939

   Defines the new E003 index format and the bit set
   implementation logic.

I suppose the index format is not set in stone yet? My java-foo is
rusty and I'm not familiar with jgit, so I more likely read things
wrong.

It seems the bitmap data follows directly after regular index content.
I'd like to see some sort of extension mechanism like in
$GIT_DIR/index, so that we don't have to increase pack index version
often. What I have in mind is optional commit cache to speed up
rev-list and merge, which could be stored in pack index too.

In PackIndexVE003 class

+   // Read the bitmaps for the Git types
+   SimpleDataInput dataInput = new SimpleDataInput(fd);
+   this.commits = readBitmap(dataInput);
+   this.trees = readBitmap(dataInput);
+   this.blobs = readBitmap(dataInput);
+   this.tags = readBitmap(dataInput);

Am I correct in saying that you have four different on-disk bitmaps,
one for each object type? If so, for compression efficient reasons?

 :-)

Definitely :-). I have shown my interest in this topic before. So I
should probably say that I'm going to work on this on C Git, but
slllwwwly. As this benefits the server side greatly, perhaps a
GitHubber ;-) might want to work on this on C Git, for GitHub itself
of course, and, as a side effect, make the rest of us happy?
-- 
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: [RFC PATCH] add t3420-rebase-topology

2012-09-27 Thread Chris Webb
Martin von Zweigbergk martinv...@gmail.com writes:

 On Tue, Sep 18, 2012 at 12:53 AM, Johannes Sixt j.s...@viscovery.net wrote:

  Why? Is it more like --root implies --force?
 
 It doesn't currently exactly imply --force, but the effect is the
 same. Also see my reply to Junio's email in this thread.
 
 Maybe Chris has some thoughts on this?

Hi Martin and Johannes. Sorry for the slow follow-up here.

You're right that rebase --root without --onto always creates a brand new
root as a result of the implementation using a sentinel commit. Clearly this
is what's wanted with --interactive, but rebase --root with neither --onto
nor --interactive is a slightly odd combination for which I struggle to
imagine a natural use. Perhaps you're right that for consistency it should
be a no-op unless --force-rebase is given?

If we did this, this combination would be a no-op unconditionally as by
definition we're always descended from the root of our current commit.
However, given the not-very-useful behaviour, I suspect that rebase --root
is much more likely to be a mistyped version of rebase -i --root than rebase
--root --force-rebase. (Unless I'm missing a reasonable use for this?
History linearisation perhaps?)

Best wishes,

Chris.
--
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: DWIM .git repository discovery

2012-09-27 Thread Drew Northup
On Wed, Sep 26, 2012 at 7:02 AM, Nguyen Thai Ngoc Duy pclo...@gmail.com wrote:
 On Wed, Sep 26, 2012 at 11:21 AM, Junio C Hamano gits...@pobox.com wrote:
 Nguyen Thai Ngoc Duy pclo...@gmail.com writes:

 I often find myself attempting to examine another repository,
 especially in projects that are closely related but put in different
 git repos. It's usually just a diff or log command

 git log --patch ../path/to/another/repo/path/to/file.c

 I personally do not think it is _too_ bad to internally do

 (cd ../path/to/another/repo/path/to 
  git log --patch file.c)


 As long as the .git discovery and path rewriting can be done
 automatically, that'd be nice. But..

I do not think that it should be the job of Git to guess how you would
like your paths recannonicalized. That is truly a pathway to insanity.

 but I doubt it is worth the numerous implications (I am not talking
 about implementation complexity at all, but the conceptual burden).

 For example, where in the working tree of the other project should
 the command run?  The output from log -p happens to be always
 relative to the top of the working tree, but that does not
 necessarily hold true for other subcommands.

And for us to presume that changing how all of those operate now by
default would be a good idea is most definitely folly.

 Returned paths should always be relative to cwd (well except diff/log
 which are prefixed by [ab]/). cd'ing internally like above makes it
 more confusing imo. Take grep for example, I find it natural for git
 grep foo -- ../other/repo/bar/ to return ../other/repo/bar/foo.c
 

In Junio's example it would be relative to the working directory—of
the subshell. Neither the shell nor Git is in a position to clean that
up much if at all.

 Prefix currently does not take ../blah form, but I see no reasons
 why it can't/shouldn't. $(cwd) is most likely outside the other
 project's working directory. An exception running from inside a
 submodule and examining the parent repository.

Is hacking the master project code from inside of a submodule what
this is actually about?

 For too long relative paths, we could even display in :/ pathspec
 notation. Users who don't recognize them either look up documentation,
 or gradually learn to drop the :/ part, even without know what it's
 for.

 Repo modification commands like git-add could cause greater confusion
 (I added and committed it (on the other repo), but when I pushed (on
 this repo), the changes aren't there). We could and probably should
 avoid dwim-ing these cases.

I think you just made a decent case for not doing too much DWIM
here. DWIM is a very fraught concept because you are assuming that
everybody is going to want to do things exactly (or nearly so) the way
you do.

 I think that this is a road to insanity; anybody who thinks along
 this line is already on the other side of the line, I would have to
 say ;-).

 We could go slowly and stop before being diagnosed insane. I mean the
 trick can be opted in for a command subset where it makes sense to do
 so.

I would recommend stopping now then.

-- 
-Drew Northup
--
As opposed to vegetable or mineral error?
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
--
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: DWIM .git repository discovery

2012-09-27 Thread Nguyen Thai Ngoc Duy
On Thu, Sep 27, 2012 at 7:22 PM, Drew Northup n1xim.em...@gmail.com wrote:
 I personally do not think it is _too_ bad to internally do

 (cd ../path/to/another/repo/path/to 
  git log --patch file.c)


 As long as the .git discovery and path rewriting can be done
 automatically, that'd be nice. But..

 I do not think that it should be the job of Git to guess how you would
 like your paths recannonicalized. That is truly a pathway to insanity.

I believe we are doing that. We move cwd internally to top worktree,
so we rewrite (well, prefix) all paths.

 Returned paths should always be relative to cwd (well except diff/log
 which are prefixed by [ab]/). cd'ing internally like above makes it
 more confusing imo. Take grep for example, I find it natural for git
 grep foo -- ../other/repo/bar/ to return ../other/repo/bar/foo.c
 

 In Junio's example it would be relative to the working directory—of
 the subshell. Neither the shell nor Git is in a position to clean that
 up much if at all.

That's implementation details.

 Prefix currently does not take ../blah form, but I see no reasons
 why it can't/shouldn't. $(cwd) is most likely outside the other
 project's working directory. An exception running from inside a
 submodule and examining the parent repository.

 Is hacking the master project code from inside of a submodule what
 this is actually about?

Hacking, no. Examining, yes. And in my case, no submodules. It's gnome
projects where a bunch of libraries (in their own repositories) may be
needed for an application. I stay in the application directory but
from time to time I'll need to look outside in other repositories.
-- 
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: CRLF, LF ... CR ?

2012-09-27 Thread Jens Bauer
Hi Junio and David.

Rule is in fact quite simple.
If it's a text-file and it contains a LF, a CRLF or a CR, then that's a 
line-break. :)
-So everywhere a LF is checked for, a CR should most likely be checked for.
Usually, when checking for CRLF, one is looking for the LF. If a CR precedes 
the LF, the CR is discarded.
It's done this way, in case we have a small buffer (say 100 bytes), and we read 
the file only 100 bytes each time.
If we searched for CR instead, we can't check the next character without a lot 
of clumsy or slow coding.
-But even today, I don't believe that a line would exceed 2000 characters; 
although, one could have a 16K buffer, then *if* the last character in that 
buffer is a CR, read ahead (just a single byte read would be acceptable in that 
case).

Unfortunately, it seems that it's not only old Mac OS users that have the 
problem:
http://stackoverflow.com/questions/10491564/git-and-cr-vs-lf-but-not-crlf

...That's a Windows user, who seem to be quite lazy - but if he already has a 
huge repository filled with CR, I do understand why he don't want to change 
things.
Here, adding scanning for CR later, can still solve his problem, because the 
files are not modified.

But on Mac OS X, there are also problems. If an application was written long 
ago (for Mac OS 9 or Carbon), it might still use CR instead of LF, if it's 
ported.

Anyway, Linus is right about not just popping code in, because someone, 
somewhere needs it for a single use - even if that is me. ;)
It's not leading to any kind of crashes if there's no scan for CR.
But on the other hand, it saves the community from FAQ about how to get it 
working, and it's always an advantage not modifying files more than necessary, 
when dealing with a VCS/SCM.

The safest way is to hunt for the LF, remembering the previous character. The 
following is just some untested code I wrote as an example on how it could 
approximately be done - it does not have to use this many lines. This is a 
buffer-based example:

#define LF 0x0a
#define CR 0x0d

const char  *buffer;/* buffer containing entire 
text file */
const char  *b; /* beginning of line */
const char  *s; /* source */
const char  *e; /* end pointer */
charc;
charlast;
const char  *l; /* pointer to last 
character */

s = buffer;
e = s[length];
b = s;
c = 0;
while(s  e)
{
last = c;
c = *s++;
if(LF == c) /* deal with Linux, 
UNIX and DOS */
{
b = l;
// we have a linefeed - new line.
l = s;
if(CR == last)
{
l = s - 1;
}
/* line contents are from b to l */
}
else if(CR == last) /* deal with old Mac OS and 
other weirdos. ;) */
{
b = l;
l = s - 1;
/* line contents are from b to l */
}
}

-As written above, it can be optimized. There's a small bug though; it doesn't 
scan if the very last character in the file is a CR.


Love
Jens

On Wed, 26 Sep 2012 23:16:38 -0700, Junio C Hamano wrote:
 David Aguilar dav...@gmail.com writes:
 
 That said, perhaps the autocrlf code is simple enough that it
 could be easily tweaked to also handle this special case,...
 
 I wouldn't be surprised if it is quite simple.
 
 We (actually Linus, IIRC) simply declared from the get-go that it is
 not worth spending any line of code only to worry about pre OSX
 Macintosh when we did the end-of-line stuff, and nobody so far
 showed any need.
 
--
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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Shawn Pearce
On Thu, Sep 27, 2012 at 5:17 AM, Nguyen Thai Ngoc Duy pclo...@gmail.com wrote:
 On Thu, Sep 27, 2012 at 7:47 AM, Shawn Pearce spea...@spearce.org wrote:
 Google has published a series of patches (see links below) to JGit to

 Should discussions about this series happen in here, jgit mailing or
 gerrit? I just want to make sure I'll discuss it at the right place.

I think we should have a concrete discussion about the implementation
in Java on the Gerrit changes (e.g.  you should fix this comment it
doesn't sufficiently describe the method), and a discussion about the
file format and algorithm here where everyone can contribute.

The format is named E003 because its still experimental. Its not set
in stone. Future iterations might be named E004, etc. If we get
something final we will look to rename it to just version 3.

 improve fetch and clone performance by adding compressed bitmaps to
 the pack-*.idx structure.

 Operation   Index V2   Index VE003
 Clone   37530ms (524.06 MiB) 82ms (524.06 MiB)
 Fetch (1 commit back)  75ms 107ms
 Fetch (10 commits back)   456ms (269.51 KiB)341ms (265.19 KiB)
 Fetch (100 commits back)  449ms (269.91 KiB)337ms (267.28 KiB)
 Fetch (1000 commits back)2229ms ( 14.75 MiB)189ms ( 14.42 MiB)
 Fetch (1 commits back)   2177ms ( 16.30 MiB)254ms ( 15.88 MiB)
 Fetch (10 commits back) 14340ms (185.83 MiB)   1655ms (189.39 MiB)

 Beautiful. And curious, why do 100-1000 and 1-1 have such
 big leaps in time (V2)?

We didn't investigate this. Colby just reported on what the current
JGit code does. I suspect there is something specific about the shape
of the history graph for this repository combined with the way JGit
wrote the pack file that caused these sorts of large increases.
Perhaps they could be smaller. Not sure I care anymore when the E003
approach gives us such low times. :-)

 The basic gist of the implementation is a bitmap has a 1 bit set for
 each object that is reachable from the commit the bitmap is associated
 with. An index file may have a unique bitmap for hundreds of commits
 in the corresponding pack file. The set of objects to send is
 performed by doing a simple computation:

   OR (all want lines) AND NOT OR (all have lines)

 There are two key patches in the series that implement the file format
 change and logic involved:

 * https://git.eclipse.org/r/7939

   Defines the new E003 index format and the bit set
   implementation logic.

 I suppose the index format is not set in stone yet?

For E003, yes, we already have some data encoded with it. But as a
file format change, no. We are willing to iterate on this if there is
tangible benefit displayed by an alternative. Future versions would
have to be E004 or some other new version number to disambiguate from
E003.

 My java-foo is
 rusty and I'm not familiar with jgit, so I more likely read things
 wrong.

Or maybe not. :-)

 It seems the bitmap data follows directly after regular index content.

Correct. It is after the regular content, but before the 2 SHA-1 trailers.

 I'd like to see some sort of extension mechanism like in
 $GIT_DIR/index, so that we don't have to increase pack index version
 often.

This might be worthwhile. I dislike the way $GIT_DIR/index encodes
extensions. Forcing an extension to fully materialize itself to
determine its length so the length can be placed before the data is
painful to work with when writing the file out to disk. I would prefer
writing an index catalog at the trailer of the file. We already
require random access to the index file, so its possible for a reader
to read a fixed size trailer record that has the 2 SHA-1s we normally
end an index with, and an extension catalog footer that has a length
and CRC-32 of the catalog. The catalog would immediately appear before
the footer, so a reader can find the start of the extension catalog by
subtracting from the end of the file the catalog length and the file
footer and catalog footer lengths. The catalog can then supply a
starting offset for each extension section, and writers don't need to
predict in advance how much data they need to store. Readers trying to
use extensions aren't really hurt, Git already randomly seeks to read
the tail of an index file to compare the pack SHA-1 before assuming
the index is valid.

 What I have in mind is optional commit cache to speed up
 rev-list and merge, which could be stored in pack index too.

We should also look into using the commit bitmap data to feed the
rev-list traversal. The bitmaps can tell us which objects are commits,
and their rough ordering given the packing rules. That may be
sufficient to feed the walker without having a priority queue.

 In PackIndexVE003 class

 +   // Read the bitmaps for the Git types
 +   SimpleDataInput dataInput = new SimpleDataInput(fd);
 +   this.commits = readBitmap(dataInput);
 + 

[PATCH] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig

2012-09-27 Thread Ramkumar Ramachandra

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Documentation/git-config.txt |3 +++
 path.c   |5 +
 t/t1306-xdg-files.sh |8 
 3 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index eaea079..c8db03f 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -205,6 +205,9 @@ $GIT_DIR/config::
User-specific configuration file. Also called global
configuration file.
 
+$GIT_GLOBAL_CONFIG::
+   Overrides the path of the global configuration file.
+
 $XDG_CONFIG_HOME/git/config::
Second user-specific configuration file. If $XDG_CONFIG_HOME is not set
or empty, $HOME/.config/git/config will be used. Any single-valued
diff --git a/path.c b/path.c
index cbbdf7d..9b09cee 100644
--- a/path.c
+++ b/path.c
@@ -131,10 +131,15 @@ char *git_path(const char *fmt, ...)
 
 void home_config_paths(char **global, char **xdg, char *file)
 {
+   char *global_config = getenv(GIT_GLOBAL_CONFIG);
char *xdg_home = getenv(XDG_CONFIG_HOME);
char *home = getenv(HOME);
char *to_free = NULL;
 
+   if (global_config) {
+   *global = mkpathdup(%s, global_config);
+   return;
+   }
if (!home) {
if (global)
*global = NULL;
diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
index 8b14ab1..5b0e08e 100755
--- a/t/t1306-xdg-files.sh
+++ b/t/t1306-xdg-files.sh
@@ -28,6 +28,14 @@ test_expect_success 'read config: xdg file exists and 
~/.gitconfig exists' '
test_cmp expected actual
 '
 
+test_expect_success 'read config: $GIT_GLOBAL_CONFIG is set and ~/.gitconfig 
exists' '
+   .gitconfig 
+   echo [alias] .gittestconfig 
+   echo   myalias = !echo in_gitconfig .gittestconfig 
+   echo in_gitconfig expected 
+   GIT_GLOBAL_CONFIG=~/.gittestconfig git myalias actual 
+   test_cmp expected actual
+'
 
 test_expect_success 'read with --get: xdg file exists and ~/.gitconfig 
doesn'\''t' '
rm .gitconfig 
-- 
1.7.8.1.362.g5d6df.dirty

--
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] diff: diff.context configuration gives default to -U

2012-09-27 Thread Jeff Muizelaar
Introduce a configuration variable diff.context that tells
Porcelain commands to use a non-default number of context
lines instead of 3 (the default).  With this variable, users
do not have to keep repeating git log -U8 from the command
line; instead, it becomes sufficient to say git config
diff.context 8 just once.

Signed-off-by: Jeff Muizelaar jmuizel...@mozilla.com
---
 Documentation/diff-config.txt |4 +
 diff.c|9 +++-
 t/t4060-diff-context.sh   |  127 +
 3 files changed, 139 insertions(+), 1 deletions(-)
 create mode 100755 t/t4060-diff-context.sh

diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 67a90a8..75ab8a5 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -56,6 +56,10 @@ diff.statGraphWidth::
Limit the width of the graph part in --stat output. If set, applies
to all commands generating --stat output except format-patch.
 
+diff.context::
+   Generate diffs with n lines of context instead of the default of
+   3. This value is overridden by the -U option.
+
 diff.external::
If this config variable is set, diff generation is not
performed using the internal diff machinery, but using the
diff --git a/diff.c b/diff.c
index 35d3f07..86e5f2a 100644
--- a/diff.c
+++ b/diff.c
@@ -26,6 +26,7 @@ static int diff_detect_rename_default;
 static int diff_rename_limit_default = 400;
 static int diff_suppress_blank_empty;
 static int diff_use_color_default = -1;
+static int diff_context_default = 3;
 static const char *diff_word_regex_cfg;
 static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
@@ -141,6 +142,12 @@ int git_diff_ui_config(const char *var, const char *value, 
void *cb)
diff_use_color_default = git_config_colorbool(var, value);
return 0;
}
+   if (!strcmp(var, diff.context)) {
+   diff_context_default = git_config_int(var, value);
+   if (diff_context_default  0)
+   return -1;
+   return 0;
+   }
if (!strcmp(var, diff.renames)) {
diff_detect_rename_default = git_config_rename(var, value);
return 0;
@@ -3170,7 +3177,7 @@ void diff_setup(struct diff_options *options)
options-break_opt = -1;
options-rename_limit = -1;
options-dirstat_permille = diff_dirstat_permille_default;
-   options-context = 3;
+   options-context = diff_context_default;
DIFF_OPT_SET(options, RENAME_EMPTY);
 
options-change = diff_change;
diff --git a/t/t4060-diff-context.sh b/t/t4060-diff-context.sh
new file mode 100755
index 000..76fa3c3
--- /dev/null
+++ b/t/t4060-diff-context.sh
@@ -0,0 +1,127 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Mozilla Foundation
+#
+
+test_description='diff.context configuration'
+
+. ./test-lib.sh
+
+cat  EOF  x
+firstline
+b
+c
+d
+e
+f
+preline
+postline
+i
+j
+k
+l
+m
+n
+EOF
+test_expect_success 'initial add' '
+   git update-index --add x 
+   git commit -m initial
+'
+
+cat  EOF  x
+firstline
+b
+c
+d
+e
+f
+preline
+1
+postline
+i
+j
+k
+l
+m
+n
+EOF
+
+test_expect_success 'next commit' '
+   git update-index --add x 
+   git commit -m next
+'
+cat  EOF  x
+firstline
+b
+c
+d
+e
+f
+preline
+2
+postline
+i
+j
+k
+l
+m
+n
+EOF
+
+
+
+
+test_expect_success 'diff.context affects log' '
+   git log -1 -p | grep -q -v firstline
+   git config diff.context 8 
+   git log -1 -p | grep -q firstline
+'
+test_expect_success 'different -U value' '
+   git config diff.context 8 
+   git log -U4 -1 | grep -q -v firstline
+'
+
+test_expect_success 'diff.context affects diff' '
+   git config diff.context 8 
+   git diff | grep -q firstline
+'
+
+test_expect_success 'plumbing not affected' '
+   git config diff.context 8 
+   git diff-files -p | grep -q -v firstline
+'
+
+cat  .git/config  EOF
+[diff]
+   context = no
+EOF
+test_expect_success 'config parsing' '
+   git diff 21 | grep -q bad config value
+'
+
+cat  .git/config  EOF
+[diff]
+   context = 0
+EOF
+test_expect_success 'config parsing' '
+   git diff | grep -v preline
+'
+
+cat  .git/config  EOF
+[diff]
+   context = -1
+EOF
+test_expect_success 'config parsing' '
+   git diff 21 | grep -q bad config file
+'
+
+cat  .git/config  EOF
+[diff]
+   context = 8
+EOF
+test_expect_success 'config parsing' '
+   git diff 21 | grep postline
+'
+
+
+test_done
-- 
1.7.4.4


--
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: Commit cache to speed up rev-list and merge

2012-09-27 Thread Shawn Pearce
On Thu, Sep 27, 2012 at 5:17 AM, Nguyen Thai Ngoc Duy pclo...@gmail.com wrote:
 I'd like to see some sort of extension mechanism like in
 $GIT_DIR/index, so that we don't have to increase pack index version
 often. What I have in mind is optional commit cache to speed up
 rev-list and merge, which could be stored in pack index too.

Can you share some of your ideas?

In Linus' Linux kernel tree there are currently about 323,178 commits.
If we store just the pre-parsed commit time as an int32 field this is
an additional 1.2 MiB of data in the pack-*.idx file, assuming we can
use additional data like pack offset position to correlate commit to
the parsed int. If we stored parent pointers in a similar way you
probably need at least 3.6 MiB of additional disk space on the index.
For example, use 12 bytes for each commit to store enough of the
parsed commit time to sort commits, and up to 2 parent pointers per
commit with a reserved magic value for octopus merges to mean the
commit itself has to be parsed to get the graph structure correct.
--
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 v4] Teach rm to remove submodules unless they contain a git directory

2012-09-27 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 Changes since v3:
 - Added get_ours_cache_pos() helper to only check stage 2 of a conflict
 - Added tests for modified submodules in the conflict case

Thanks.

 + /*
 +  * Skip unmerged entries except for populated submodules
 +  * that could loose history when removed.
 +  */

s/loose/lose/, I think.

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


Option for git bisect run to automatically clean up

2012-09-27 Thread Laszlo Papp
Hi everybody,

I have just run into a problem when I had to issue an explicit cleanup for
tracked files after a configure run in the Qt5 project. I have tried to
suggest to the people to bring up this idea on the mailing list in order to
get this further on. Unfortunately I did not have time to do so, especially
for the follow-up. I have also been told it is not a good way of asking on
IRC which surprised me a bit, but I am now bringing this up, and I try to
also make the follow-up. Hope it is ok.

Unfortunately my time is limited so I cannot contribute with that patch
myself, but I think it would be a cool convenience feature. Help is
appreciated. Thank you in advance!

Laszlo
--
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] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig

2012-09-27 Thread Matthieu Moy
Ramkumar Ramachandra artag...@gmail.com writes:

 diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
 index eaea079..c8db03f 100644
 --- a/Documentation/git-config.txt
 +++ b/Documentation/git-config.txt
 @@ -205,6 +205,9 @@ $GIT_DIR/config::
   User-specific configuration file. Also called global
   configuration file.
  
 +$GIT_GLOBAL_CONFIG::
 + Overrides the path of the global configuration file.
 +

I'm not particularly in favor of introducing another environment
variable, but if you are to introduce it, why just override the
configuration file, and not $HOME completely (e.g. to override
$HOME/.git-credentials too).

There was a patch proposing that here ($GIT_HOME to override $HOME):

  http://thread.gmane.org/gmane.comp.version-control.git/135447/focus=135494

I don't remember exactly what happened to the patch, I can't find an
explicit reason to reject it in the thread, but it seems it didn't make
its way to git.git.

 index cbbdf7d..9b09cee 100644
 --- a/path.c
 +++ b/path.c
 @@ -131,10 +131,15 @@ char *git_path(const char *fmt, ...)
  
  void home_config_paths(char **global, char **xdg, char *file)
  {
 + char *global_config = getenv(GIT_GLOBAL_CONFIG);
   char *xdg_home = getenv(XDG_CONFIG_HOME);
   char *home = getenv(HOME);
   char *to_free = NULL;
  
 + if (global_config) {
 + *global = mkpathdup(%s, global_config);
 + return;
 + }

If you return here, haven't you completely broken the XDG stuff, since
*xdg is set a few lines below in the function?

Also, I guess home_config_paths(..., ignore) will return the path to
the configuration file instead of the ignore file?

 --- a/t/t1306-xdg-files.sh
 +++ b/t/t1306-xdg-files.sh
 @@ -28,6 +28,14 @@ test_expect_success 'read config: xdg file exists and 
 ~/.gitconfig exists' '
   test_cmp expected actual
  '
  
 +test_expect_success 'read config: $GIT_GLOBAL_CONFIG is set and ~/.gitconfig 
 exists' '
 + .gitconfig 
 + echo [alias] .gittestconfig 
 + echo   myalias = !echo in_gitconfig .gittestconfig 
 + echo in_gitconfig expected 
 + GIT_GLOBAL_CONFIG=~/.gittestconfig git myalias actual 
 + test_cmp expected actual
 +'

You should check that git config --set works too, as the codepath for
writing to configuration is relatively different from the one to read.
For example, I *think* that git config --global will write to
$GIT_GLOBAL_CONFIG and git config without --global will ignore it, but
a test would be welcome.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] revision: add --reflog-message to grep reflog messages

2012-09-27 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---

Plase explain yourself in the space above.

 diff --git a/Documentation/rev-list-options.txt 
 b/Documentation/rev-list-options.txt
 index 1fc2a18..aeaa58c 100644
 --- a/Documentation/rev-list-options.txt
 +++ b/Documentation/rev-list-options.txt
 @@ -51,6 +51,11 @@ endif::git-rev-list[]
   commits whose author matches any of the given patterns are
   chosen (similarly for multiple `--committer=pattern`).
  
 +--reflog-message=pattern::
 + Limit the commits output to ones with reflog entries that
 + match the specified pattern (regular expression). Ignored unless
 + --walk-reflogs is given.
 +

I am debating myself if it is sane for this option to have no hint
that it is about limiting in its name.  --author/--committer
don't and it is clear from the context of the command that they are
not about setting author/committer, so --reflog-message may be
interpreted the same, perhaps.

The entry in the context above talks about multiple occurrence of
that option. Shouldn't this new one also say what happens when it is
given twice?

 diff --git a/grep.c b/grep.c
 index 898be6e..72ac1bf 100644
 --- a/grep.c
 +++ b/grep.c
 @@ -697,6 +697,7 @@ static struct {
  } header_field[] = {
   { author , 7 },
   { committer , 10 },
 + { reflog , 7 },
  };
  
  static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 diff --git a/grep.h b/grep.h
 index 8a28a67..1416ad7 100644
 --- a/grep.h
 +++ b/grep.h
 @@ -29,9 +29,10 @@ enum grep_context {
  
  enum grep_header_field {
   GREP_HEADER_AUTHOR = 0,
 - GREP_HEADER_COMMITTER
 + GREP_HEADER_COMMITTER,
 + GREP_HEADER_REFLOG,
 + GREP_HEADER_FIELD_MAX
  };
 -#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)

Please add comment to ensure that FIELD_MAX stays at the end; if you
ensure that, the result is much better than the original we know
committer is at the end so add one.

I think I wrote prep_header_patterns() and compile_grep_patterns()
carefully enough not to assume the headers are only the author and
committer names, so the various combinations i.e. all-match,
author(s), committer(s), grep(s), and reflog-message(s), should work
out of the box, but have you actually tested them?

I do not know offhand the matching side is prepared to take random
garbage fields.  IIRC, we strip the trailing timestamp from committer
and author header lines when we match, and a new code needs to be
added to control when that stripping should / should not kick in
depending on the header.

 diff --git a/revision.c b/revision.c
 index ae12e11..837051c 100644
 --- a/revision.c
 +++ b/revision.c
 @@ -1595,6 +1595,9 @@ static int handle_revision_opt(struct rev_info *revs, 
 int argc, const char **arg
   } else if ((argcount = parse_long_opt(committer, argv, optarg))) {
   add_header_grep(revs, GREP_HEADER_COMMITTER, optarg);
   return argcount;
 + } else if ((argcount = parse_long_opt(reflog-message, argv, 
 optarg))) {
 + add_header_grep(revs, GREP_HEADER_REFLOG, optarg);
 + return argcount;
   } else if ((argcount = parse_long_opt(grep, argv, optarg))) {
   add_message_grep(revs, optarg);
   return argcount;
 @@ -2212,8 +2215,20 @@ static int commit_match(struct commit *commit, struct 
 rev_info *opt)
  {
   if (!opt-grep_filter.pattern_list  !opt-grep_filter.header_list)
   return 1;
 - return grep_buffer(opt-grep_filter,
 -commit-buffer, strlen(commit-buffer));
 + if (opt-reflog_info) {
 + int retval;
 + struct strbuf buf = STRBUF_INIT;
 + strbuf_addstr(buf, reflog );
 + get_reflog_message(buf, opt-reflog_info);
 + strbuf_addch(buf, '\n');
 + strbuf_addstr(buf, commit-buffer);
 + retval = grep_buffer(opt-grep_filter, buf.buf, buf.len);
 + strbuf_release(buf);
 + return retval;
 + } else {
 + return grep_buffer(opt-grep_filter,
 +commit-buffer, strlen(commit-buffer));
 + }
  }

This part looks familiar and smells sane ;-)
--
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: Configuring the location of ~/.gitconfig

2012-09-27 Thread Junio C Hamano
Anurag Priyam anurag08pri...@gmail.com writes:

 Not me. For that particular use case, my approach (long before I
 switched the vcs that controls my dotfiles to git) have always been
 to have ~/src that is version controlled, with a Makefile that
 builds/adjusts dotfiles appropriately for each box and installs them
 in the proper place.

 Makefile is what I wanted to avoid when I suggested Ram that maybe Git
 could _optionally_ read the location of global gitconfig from an
 environment variable that can be exported in zshenv or bash_profile.

Where exactly do the zshenv and bash_profile you mention reside,
and how are their contents kept up to date?  I am guessing it is
/net/srv1/home/ram/.bash_profile on machines that mount user home
directories from srv1 NFS server and it is to have a string like
export GITCONFIG_FILE=/net/srv1/home/ram/dot/gitconfig or on those
machines.  The contents of ~ram/dot/gitconfig may be able to stay
the same across boxes, but how is ~ram/.bash_profile is set up?
Manually?

Again, I am not personally interested; I think aversion of Make is
the root cause of the disease.



--
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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 07:17:42PM +0700, Nguyen Thai Ngoc Duy wrote:

  Operation   Index V2   Index VE003
  Clone   37530ms (524.06 MiB) 82ms (524.06 MiB)
  Fetch (1 commit back)  75ms 107ms
  Fetch (10 commits back)   456ms (269.51 KiB)341ms (265.19 KiB)
  Fetch (100 commits back)  449ms (269.91 KiB)337ms (267.28 KiB)
  Fetch (1000 commits back)2229ms ( 14.75 MiB)189ms ( 14.42 MiB)
  Fetch (1 commits back)   2177ms ( 16.30 MiB)254ms ( 15.88 MiB)
  Fetch (10 commits back) 14340ms (185.83 MiB)   1655ms (189.39 MiB)
 
 Beautiful. And curious, why do 100-1000 and 1-1 have such
 big leaps in time (V2)?

Agreed. I'm very excited about these numbers.

Defines the new E003 index format and the bit set
implementation logic.
 [...]
 It seems the bitmap data follows directly after regular index content.
 I'd like to see some sort of extension mechanism like in
 $GIT_DIR/index, so that we don't have to increase pack index version
 often. What I have in mind is optional commit cache to speed up
 rev-list and merge, which could be stored in pack index too.

As I understand it, both the bitmaps and a commit cache are
theoretically optional. That is, git can do the job without them, but
they speed things up. If that is the case, do we need to bump the index
version at all? Why not store a plain v2 index, and then store an
additional file pack-XXX.reachable that contains the bitmaps and an
independent version number.

The sha1 in the filename makes sure that the reachability file is always
in sync with the actual pack data and index.  Old readers won't know
about the new file, and will ignore it. For new readers, if the file is
there they can use it; if it's missing (or its version is not
understood), they can fall back to the regular index.

I haven't looked at the details of the format change yet. If it is
purely an extra chunk of data at the end, this Just Works. If there are
changes to the earlier parts of the pack (e.g., I seem to recall the
commit cache idea wanted separate indices for each object type), we may
still need a v3. But it would be nice if we could make those changes
generic (e.g., just the separate indices, which might support many
different enhancements), and then let the actual feature work happen in
the separate files.

 Definitely :-). I have shown my interest in this topic before. So I
 should probably say that I'm going to work on this on C Git, but
 slllwwwly. As this benefits the server side greatly, perhaps a
 GitHubber ;-) might want to work on this on C Git, for GitHub itself
 of course, and, as a side effect, make the rest of us happy?

Yeah, GitHub is definitely interested in this. I may take a shot at it,
but I know David Barr (cc'd) is also interested in such things.

-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] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig

2012-09-27 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  Documentation/git-config.txt |3 +++
  path.c   |5 +
  t/t1306-xdg-files.sh |8 
  3 files changed, 16 insertions(+), 0 deletions(-)

 diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
 index eaea079..c8db03f 100644
 --- a/Documentation/git-config.txt
 +++ b/Documentation/git-config.txt
 @@ -205,6 +205,9 @@ $GIT_DIR/config::
   User-specific configuration file. Also called global
   configuration file.
  
 +$GIT_GLOBAL_CONFIG::
 + Overrides the path of the global configuration file.
 +

Why is this even a good idea?

  - Does it make sense not to read $HOME/.gitconfig or XDG stuff
when this is set?  Why skip them?  If it makes sense to skip
them, why doesn't it also skip $GIT_DIR/config and/or
/etc/gitconfig?

  - Why is it not the third user-specific configuration file instead?

  - Why is it not a list of paths to read configurations from?

  - Where does it end?

Not overly impressed, I'd have to say.

 diff --git a/path.c b/path.c
 index cbbdf7d..9b09cee 100644
 --- a/path.c
 +++ b/path.c
 @@ -131,10 +131,15 @@ char *git_path(const char *fmt, ...)
  
  void home_config_paths(char **global, char **xdg, char *file)
  {
 + char *global_config = getenv(GIT_GLOBAL_CONFIG);
   char *xdg_home = getenv(XDG_CONFIG_HOME);
   char *home = getenv(HOME);
   char *to_free = NULL;
  
 + if (global_config) {
 + *global = mkpathdup(%s, global_config);
 + return;
 + }
   if (!home) {
   if (global)
   *global = NULL;
 diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh
 index 8b14ab1..5b0e08e 100755
 --- a/t/t1306-xdg-files.sh
 +++ b/t/t1306-xdg-files.sh
 @@ -28,6 +28,14 @@ test_expect_success 'read config: xdg file exists and 
 ~/.gitconfig exists' '
   test_cmp expected actual
  '
  
 +test_expect_success 'read config: $GIT_GLOBAL_CONFIG is set and ~/.gitconfig 
 exists' '
 + .gitconfig 
 + echo [alias] .gittestconfig 
 + echo   myalias = !echo in_gitconfig .gittestconfig 
 + echo in_gitconfig expected 
 + GIT_GLOBAL_CONFIG=~/.gittestconfig git myalias actual 

How is this tilde expanded and by whom?
--
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] revision: add --reflog-message to grep reflog messages

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 10:09:28AM -0700, Junio C Hamano wrote:

  +--reflog-message=pattern::
  +   Limit the commits output to ones with reflog entries that
  +   match the specified pattern (regular expression). Ignored unless
  +   --walk-reflogs is given.
  +
 
 I am debating myself if it is sane for this option to have no hint
 that it is about limiting in its name.  --author/--committer
 don't and it is clear from the context of the command that they are
 not about setting author/committer, so --reflog-message may be
 interpreted the same, perhaps.

I also found the name confusing on first-read. While --author is an
example in one direction, the fact that --grep is not called --body
is a counter-example.

I'd much rather see it as --grep-reflog or something. You could also
do --grep-reflog-message, which would match a later
--grep-reflog-author, but I am not sure anybody would want the latter,
and it makes the current name a lot longer.

I actually think just checking the reflog when we call --grep would
the most common workflow, and requires no extra work from the user.  My
only hesitation is that if somebody _does_ want to distinguish, there's
no escape hatch. Of course, the reflog walker is already full of such
weird conflations (e.g., the rewriting of parent and date information).

-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] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 08:16:11PM +0530, Ramkumar Ramachandra wrote:

 diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
 index eaea079..c8db03f 100644
 --- a/Documentation/git-config.txt
 +++ b/Documentation/git-config.txt
 @@ -205,6 +205,9 @@ $GIT_DIR/config::
   User-specific configuration file. Also called global
   configuration file.
  
 +$GIT_GLOBAL_CONFIG::
 + Overrides the path of the global configuration file.
 +

Like the other reviews, I am not overly enthused. If we are going to add
a new variable, I think $GIT_HOME makes a lot more sense. But it really
sounds like using $XDG_CONFIG_HOME would be even simpler.

Also, have you considered using a config include? Like:

  $ echo '[include]path = ~/my-dotfiles/gitconfig' ~/.gitconfig

It's a one-time setup, and then you get updates inside my-dotfiles
forever. The one-time setup is annoying, but you have to bootstrap
somehow (e.g., you're going to have to copy a .profile or similar to get
the GIT_GLOBAL_CONFIG variable set).

-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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Shawn Pearce
On Thu, Sep 27, 2012 at 10:20 AM, Jeff King p...@peff.net wrote:
 On Thu, Sep 27, 2012 at 07:17:42PM +0700, Nguyen Thai Ngoc Duy wrote:

  Operation   Index V2   Index VE003
  Clone   37530ms (524.06 MiB) 82ms (524.06 MiB)
  Fetch (1 commit back)  75ms 107ms
  Fetch (10 commits back)   456ms (269.51 KiB)341ms (265.19 KiB)
  Fetch (100 commits back)  449ms (269.91 KiB)337ms (267.28 KiB)
  Fetch (1000 commits back)2229ms ( 14.75 MiB)189ms ( 14.42 MiB)
  Fetch (1 commits back)   2177ms ( 16.30 MiB)254ms ( 15.88 MiB)
  Fetch (10 commits back) 14340ms (185.83 MiB)   1655ms (189.39 MiB)

 Beautiful. And curious, why do 100-1000 and 1-1 have such
 big leaps in time (V2)?

 Agreed. I'm very excited about these numbers.

Defines the new E003 index format and the bit set
implementation logic.
 [...]
 It seems the bitmap data follows directly after regular index content.
 I'd like to see some sort of extension mechanism like in
 $GIT_DIR/index, so that we don't have to increase pack index version
 often. What I have in mind is optional commit cache to speed up
 rev-list and merge, which could be stored in pack index too.

 As I understand it, both the bitmaps and a commit cache are
 theoretically optional. That is, git can do the job without them, but
 they speed things up.

Yes, entirely true.

 If that is the case, do we need to bump the index
 version at all? Why not store a plain v2 index, and then store an
 additional file pack-XXX.reachable that contains the bitmaps and an
 independent version number.

This is the alternate version we considered internally. It was a bit
more work to define a 3rd file stream per pack in our backend storage
system, so we opted for a revision of an existing stream. We could
spend a bit more time and add a 3rd stream, keeping the index format
unmodified.

But we could have also done this with the CRC-32 table in index v2. We
didn't. If the data should almost always be there in order to provide
good service then we should really be embedding into the files.

I'm on the fence. I could go either way on this. E003 was just the
fastest way to prototype and start testing. We would probably be
equally happy with the 3rd stream.

 The sha1 in the filename makes sure that the reachability file is always
 in sync with the actual pack data and index.

Depending on the extension dependencies, you may need to also use the
trailer SHA-1 from the pack file itself, like the index does. E.g. the
bitmap data depends heavily on object order in the pack and is invalid
if you repack with a different ordering algorithm, or a different
delta set of results from delta compression.

  Old readers won't know
 about the new file, and will ignore it. For new readers, if the file is
 there they can use it; if it's missing (or its version is not
 understood), they can fall back to the regular index.

 I haven't looked at the details of the format change yet. If it is
 purely an extra chunk of data at the end, this Just Works.

Yes, its just extra chunk on the end.

 If there are
 changes to the earlier parts of the pack (e.g., I seem to recall the
 commit cache idea wanted separate indices for each object type), we may
 still need a v3. But it would be nice if we could make those changes
 generic (e.g., just the separate indices, which might support many
 different enhancements), and then let the actual feature work happen in
 the separate files.

Yes. One downside is these separate streams aren't removed when you
run git repack. But this could be fixed by  a modification to git
repack to clean up additional extensions with the same pack base name.

 Definitely :-). I have shown my interest in this topic before. So I
 should probably say that I'm going to work on this on C Git, but
 slllwwwly. As this benefits the server side greatly, perhaps a
 GitHubber ;-) might want to work on this on C Git, for GitHub itself
 of course, and, as a side effect, make the rest of us happy?

 Yeah, GitHub is definitely interested in this. I may take a shot at it,
 but I know David Barr (cc'd) is also interested in such things.

Building this in C is also dependent on having a good implementation
of EWAH compressed bitmaps available. So its a bit more work, but not
insurmountable by any means.
--
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: Commit cache to speed up rev-list and merge

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 08:51:51AM -0700, Shawn O. Pearce wrote:

 On Thu, Sep 27, 2012 at 5:17 AM, Nguyen Thai Ngoc Duy pclo...@gmail.com 
 wrote:
  I'd like to see some sort of extension mechanism like in
  $GIT_DIR/index, so that we don't have to increase pack index version
  often. What I have in mind is optional commit cache to speed up
  rev-list and merge, which could be stored in pack index too.
 
 Can you share some of your ideas?

Some of it is here:

  http://article.gmane.org/gmane.comp.version-control.git/203308

-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] diff: diff.context configuration gives default to -U

2012-09-27 Thread Junio C Hamano
Jeff Muizelaar jmuizel...@mozilla.com writes:

 Introduce a configuration variable diff.context that tells
 Porcelain commands to use a non-default number of context
 lines instead of 3 (the default).  With this variable, users
 do not have to keep repeating git log -U8 from the command
 line; instead, it becomes sufficient to say git config
 diff.context 8 just once.

 Signed-off-by: Jeff Muizelaar jmuizel...@mozilla.com
 ---
  Documentation/diff-config.txt |4 +
  diff.c|9 +++-
  t/t4060-diff-context.sh   |  127 
 +
  3 files changed, 139 insertions(+), 1 deletions(-)
  create mode 100755 t/t4060-diff-context.sh

Sigh, we don't have existing tests to check the number of context
lines given with -U option and we need to allocate a new test number
for it.  What is the gap between 4054 (the last one in the tests for
the diff family) and 4060 for?

 diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
 index 67a90a8..75ab8a5 100644
 --- a/Documentation/diff-config.txt
 +++ b/Documentation/diff-config.txt
 @@ -56,6 +56,10 @@ diff.statGraphWidth::
   Limit the width of the graph part in --stat output. If set, applies
   to all commands generating --stat output except format-patch.
  
 +diff.context::
 + Generate diffs with n lines of context instead of the default of
 + 3. This value is overridden by the -U option.
 +
  diff.external::
   If this config variable is set, diff generation is not
   performed using the internal diff machinery, but using the
 diff --git a/diff.c b/diff.c
 index 35d3f07..86e5f2a 100644
 --- a/diff.c
 +++ b/diff.c
 @@ -26,6 +26,7 @@ static int diff_detect_rename_default;
  static int diff_rename_limit_default = 400;
  static int diff_suppress_blank_empty;
  static int diff_use_color_default = -1;
 +static int diff_context_default = 3;
  static const char *diff_word_regex_cfg;
  static const char *external_diff_cmd_cfg;
  int diff_auto_refresh_index = 1;
 @@ -141,6 +142,12 @@ int git_diff_ui_config(const char *var, const char 
 *value, void *cb)
   diff_use_color_default = git_config_colorbool(var, value);
   return 0;
   }
 + if (!strcmp(var, diff.context)) {
 + diff_context_default = git_config_int(var, value);
 + if (diff_context_default  0)
 + return -1;
 + return 0;
 + }
   if (!strcmp(var, diff.renames)) {
   diff_detect_rename_default = git_config_rename(var, value);
   return 0;
 @@ -3170,7 +3177,7 @@ void diff_setup(struct diff_options *options)
   options-break_opt = -1;
   options-rename_limit = -1;
   options-dirstat_permille = diff_dirstat_permille_default;
 - options-context = 3;
 + options-context = diff_context_default;
   DIFF_OPT_SET(options, RENAME_EMPTY);
  
   options-change = diff_change;

Thanks; looks sensible.

 diff --git a/t/t4060-diff-context.sh b/t/t4060-diff-context.sh
 new file mode 100755
 index 000..76fa3c3
 --- /dev/null
 +++ b/t/t4060-diff-context.sh
 @@ -0,0 +1,127 @@
 +#!/bin/sh
 +#
 +# Copyright (c) 2012 Mozilla Foundation
 +#
 +
 +test_description='diff.context configuration'
 +
 +. ./test-lib.sh
 +
 +cat  EOF  x
 +firstline
 +b
 +c
 +d
 +e
 +f
 +preline
 +postline
 +i
 +j
 +k
 +l
 +m
 +n
 +EOF

I know ancient tests are written like this, but we are slowly trying
to migrate them to have these test-vector preparation inside
test_expect_success block, e.g.


test_expect_success setup '
cat x -\EOF 
firstline
b
...
n
EOF
git add x 
git commit -m initial
'

 +test_expect_success 'diff.context affects log' '
 + git log -1 -p | grep -q -v firstline
 + git config diff.context 8 
 + git log -1 -p | grep -q firstline
 +'

Three points:

 - Please avoid grep -q, which does not help people who ran tests
   (the output is hidden by default) and hurts people who want to
   debug tests.

 - Your test will ignore breakage from the first log 1 output and
   goes on running git config.  Make sure you got your  cascades
   right.

 - Because an error from the command on the upstream side of the
   pipe is ignored, we tend to prefer writing things like this:

git log -n 1 -p output 
grep -v firstline output 
...

 +cat  .git/config  EOF
 +[diff]
 + context = no
 +EOF
 +test_expect_success 'config parsing' '
 + git diff 21 | grep -q bad config value
 +'

How does the git diff command exit?  That is far more important
than the actual error message, but being on the left side of the
pipe, the exit status from the command is not being tested.
--
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  

Re: Commit cache to speed up rev-list and merge

2012-09-27 Thread Shawn Pearce
On Thu, Sep 27, 2012 at 10:39 AM, Jeff King p...@peff.net wrote:
 On Thu, Sep 27, 2012 at 08:51:51AM -0700, Shawn O. Pearce wrote:
 On Thu, Sep 27, 2012 at 5:17 AM, Nguyen Thai Ngoc Duy pclo...@gmail.com 
 wrote:
  I'd like to see some sort of extension mechanism like in
  $GIT_DIR/index, so that we don't have to increase pack index version
  often. What I have in mind is optional commit cache to speed up
  rev-list and merge, which could be stored in pack index too.

 Can you share some of your ideas?

 Some of it is here:

   http://article.gmane.org/gmane.comp.version-control.git/203308

Quoting from that patch:

On  2012-08-12 Nguyen Thai Ngoc Duy pclo...@gmail.com wrote:
 Long term we might gain slight lookup speedup if we know object type
 as search region is made smaller. But for that to happen, we need to
 propagate object type hint down to find_pack_entry_one() and friends.
 Possible thing to do, I think.

I'm not sure reclustering the index by object type is going to make a
worthwhile difference. Of 2.2m objects in the Linux tree, 320k are
commits. The difference between doing the binary search through all
objects vs. just commits is only 2 iterations more of binary search if
we assume the per-type ranges have their own fan-out tables.

 The main reason to group objects by type is to make it possible to
 create another sha1-something mapping for a particular object type,
 without wasting space for storing sha-1 keys again. For example, we
 can store commit caches, tree caches... at the end of the index as
 extensions.

Using ordinal position in the pack also works, and doesn't require
clustering objects by type.
--
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] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig

2012-09-27 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Ramkumar Ramachandra artag...@gmail.com writes:

 diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
 index eaea079..c8db03f 100644
 --- a/Documentation/git-config.txt
 +++ b/Documentation/git-config.txt
 @@ -205,6 +205,9 @@ $GIT_DIR/config::
  User-specific configuration file. Also called global
  configuration file.
  
 +$GIT_GLOBAL_CONFIG::
 +Overrides the path of the global configuration file.
 +

 I'm not particularly in favor of introducing another environment
 variable, but if you are to introduce it, why just override the
 configuration file, and not $HOME completely (e.g. to override
 $HOME/.git-credentials too).

 There was a patch proposing that here ($GIT_HOME to override $HOME):

I think both of these are at the entrance of slippery slope to
insanity I'd rather not to venture into.

If somebody hates ~/.dotmanyfiles so much, why not do HOME=~/dots/,
instead of having to set GIT_HOME to move ~/.gitconfig, FROTZ_HOME
to move ~/.frotzconfig, MAIL_HOME to move ~/.mailrc, etc.?

I wouldn't be surprised if some _other_ things break with your HOME
pointing at a directory inside your home directory, but then it may
be better to fix that other thing instead.
--
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 0/5] wildmatch series update

2012-09-27 Thread Junio C Hamano
Thanks; will re-queue.
--
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


[BUG] misplaced space in word diff

2012-09-27 Thread Yann Dirson
Here is a quick test case for a word-diff error, using v1.7.10.4
(debian wheezy package).

 Setting up:

tmp$ mkdir foo  cd foo
foo$ git init
Initialized empty Git repository in /tmp/foo/.git/
foo (master)$ echo '*.pydiff=python'  .gitattributes
foo (master)$ echo 'for name in bar.blurb:'  v1.py
foo (master)$ echo 'for name in foo.bar.blurb:'  v2.py

 This looks fine:

foo (master)$ git diff --no-index --word-diff=plain v1.py v2.py
diff --git a/v1.py b/v2.py
index a6a079d..2832331 100644
--- a/v1.py
+++ b/v2.py
@@ -1 +1 @@
for name in {+foo.+}bar.blurb:

 This however is not:

foo (master)$ git diff --no-index --word-diff=plain v2.py v1.py
diff --git a/v2.py b/v1.py
index 2832331..a6a079d 100644
--- a/v2.py
+++ b/v1.py
@@ -1 +1 @@
for name in[-foo.-] bar.blurb:
--
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] revision: add --reflog-message to grep reflog messages

2012-09-27 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 I actually think just checking the reflog when we call --grep would
 the most common workflow, and requires no extra work from the user.  My
 only hesitation is that if somebody _does_ want to distinguish, there's
 no escape hatch. Of course, the reflog walker is already full of such
 weird conflations (e.g., the rewriting of parent and date information).

Yes, that reasoning more-or-less matches the reason why I said we do
not necessarily want to go fancier.  But log -g output already
makes it fairly clear that the additional information is not part of
the regular log and is a some header-like thingy, so I actually am
OK with --reflog-grep and --grep being different.

The output from log --show-notes, on the other hand, is even more
conflated and a casual user would view it as part of the message, so
I would imagine that if we ever do the extention to cover notes
data, the normal --grep should apply to it.

--
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: DWIM .git repository discovery

2012-09-27 Thread Junio C Hamano
Drew Northup n1xim.em...@gmail.com writes:

 I think that this is a road to insanity; anybody who thinks along
 this line is already on the other side of the line, I would have to
 say ;-).

 We could go slowly and stop before being diagnosed insane. I mean the
 trick can be opted in for a command subset where it makes sense to do
 so.

 I would recommend stopping now then.

Likewise.  Didn't I already diagnose it as insane? ;-)
--
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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 10:35:47AM -0700, Shawn O. Pearce wrote:

  If that is the case, do we need to bump the index
  version at all? Why not store a plain v2 index, and then store an
  additional file pack-XXX.reachable that contains the bitmaps and an
  independent version number.
 
 This is the alternate version we considered internally. It was a bit
 more work to define a 3rd file stream per pack in our backend storage
 system, so we opted for a revision of an existing stream. We could
 spend a bit more time and add a 3rd stream, keeping the index format
 unmodified.

I'd rather make the choice that provides the best user experience, even
if it is a bit more code refactoring.

 But we could have also done this with the CRC-32 table in index v2. We
 didn't. If the data should almost always be there in order to provide
 good service then we should really be embedding into the files.

Yes, although there were other changes in v2, also (e.g., the fanout to
handle larger packfiles).  Bumping the version also made the transition
take a lot longer. We introduced the reading and writing code, but then
couldn't flip the default for quite a while. For big server providers
this is not as big a deal (we know which versions of git we will use,
and are OK with flipping a config bit). But it's one more tuning thing
to deal with for small or single-person servers.

I think clients will also want it. If we can make git rev-list
--objects --all faster (which this should be able to do), we can speed
up git prune, which in turn is by far the slowest part of git gc
--auto, since in the typical case we are only incrementally packing.

I also like that the general technique can be reused easily. We've
talked about a generation-number cache in the past. That would fit this
model as well. Removing a backwards-compatibility barrier makes it a lot
easier to experiment with these sorts of things.

  The sha1 in the filename makes sure that the reachability file is always
  in sync with the actual pack data and index.
 
 Depending on the extension dependencies, you may need to also use the
 trailer SHA-1 from the pack file itself, like the index does. E.g. the
 bitmap data depends heavily on object order in the pack and is invalid
 if you repack with a different ordering algorithm, or a different
 delta set of results from delta compression.

Interesting. I would have assumed it depended on order in the index. But
like I said, I haven't looked. I think you are still OK, though, because
the filename comes from the sha1 over the index file, which in turn
includes the sha1 over the packfile. Thus any change in the packfile
would give you a new pack and index name.

 Yes. One downside is these separate streams aren't removed when you
 run git repack. But this could be fixed by  a modification to git
 repack to clean up additional extensions with the same pack base name.

I don't think that's a big deal. We already do it with .keep files. If
you repack with an older version of git, you may have a stale
supplementary file wasting space. But that's OK. The next time you gc
with a newer version of git, we could detect and clean up such stale
files (we already do so for tmp_pack_* files).

-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: Commit cache to speed up rev-list and merge

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 10:45:32AM -0700, Shawn O. Pearce wrote:

 On  2012-08-12 Nguyen Thai Ngoc Duy pclo...@gmail.com wrote:
  Long term we might gain slight lookup speedup if we know object type
  as search region is made smaller. But for that to happen, we need to
  propagate object type hint down to find_pack_entry_one() and friends.
  Possible thing to do, I think.
 
 I'm not sure reclustering the index by object type is going to make a
 worthwhile difference. Of 2.2m objects in the Linux tree, 320k are
 commits. The difference between doing the binary search through all
 objects vs. just commits is only 2 iterations more of binary search if
 we assume the per-type ranges have their own fan-out tables.

To me the big win would be implicit indexing for items that are present
for every instance of a particular object type. So if we wanted to keep
the timestamp for every commit, you could have a pack-*.timestamps
that is literally just a packed list of uint32's, one per commit, where
the position of a commit's timestamp in the list is the same as its
position in the index of sha1s in the pack index.

That's simple to do if your index is just commits. But if it includes
all objects, then your list is sparse. So either you waste space by
making an empty slot for the non-commit objects, or you have an extra
level of indirection mapping the commit into the packed list, which is
going to double the storage in this case (though you could reuse that
extra mapping for the parent, generation number, etc, so it at least
gets amortized as you store more data). Or is there some clever solution
I'm missing?

For your extension, I don't think it matters. You're sparse even in the
commit-object space, so you have to store the mapping anyway. And your
data is big enough that the overhead isn't too painful.

-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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Shawn Pearce
On Thu, Sep 27, 2012 at 11:22 AM, Jeff King p...@peff.net wrote:

 I think clients will also want it. If we can make git rev-list
 --objects --all faster (which this should be able to do), we can speed
 up git prune, which in turn is by far the slowest part of git gc
 --auto, since in the typical case we are only incrementally packing.

Yes, the bitmap can also accelerate prune. We didn't implement this
but it is a trivial use of the existing bitmap.

  The sha1 in the filename makes sure that the reachability file is always
  in sync with the actual pack data and index.

 Depending on the extension dependencies, you may need to also use the
 trailer SHA-1 from the pack file itself, like the index does. E.g. the
 bitmap data depends heavily on object order in the pack and is invalid
 if you repack with a different ordering algorithm, or a different
 delta set of results from delta compression.

 Interesting. I would have assumed it depended on order in the index.

No. We tried that. Assigning bits by order in index (aka order of
SHA-1s sorted) results in horrible compression of the bitmap itself
because of the uniform distribution of SHA-1. Encoding instead by pack
order gets us really good bitmap compression, because object graph
traversal order tends to take reachability into account. So we see
long contiguous runs of 1s and get good compression. Sorting by SHA-1
just makes the space into swiss cheese.

 I think you are still OK, though, because
 the filename comes from the sha1 over the index file, which in turn
 includes the sha1 over the packfile. Thus any change in the packfile
 would give you a new pack and index name.

No. The pack file name is composed from the SHA-1 of the sorted SHA-1s
in the pack. Any change in compression settings or delta windows or
even just random scheduling variations when repacking can cause
offsets to slide, even if the set of objects being repacked has not
differed. The resulting pack and index will have the same file names
(as its the same set of objects), but the offset information and
ordering is now different.

Naming a pack after a SHA-1 is a fun feature. Naming it after the
SHA-1 of the object list was a mistake. It should have been named
after the SHA-1 in the trailer of the file, so that any single bit
modified within the pack stream itself would have caused a different
name to be used on the filesystem. But alas this is water under the
bridge and not likely to change anytime soon.

 Yes. One downside is these separate streams aren't removed when you
 run git repack. But this could be fixed by  a modification to git
 repack to clean up additional extensions with the same pack base name.

 I don't think that's a big deal. We already do it with .keep files. If
 you repack with an older version of git, you may have a stale
 supplementary file wasting space. But that's OK. The next time you gc
 with a newer version of git, we could detect and clean up such stale
 files (we already do so for tmp_pack_* files).

Yes, obviously.
--
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] diff: diff.context configuration gives default to -U

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 10:40:26AM -0700, Junio C Hamano wrote:

  +test_expect_success 'diff.context affects log' '
  +   git log -1 -p | grep -q -v firstline
  +   git config diff.context 8 
  +   git log -1 -p | grep -q firstline
  +'
 
 Three points:
 
  - Please avoid grep -q, which does not help people who ran tests
(the output is hidden by default) and hurts people who want to
debug tests.
 
  - Your test will ignore breakage from the first log 1 output and
goes on running git config.  Make sure you got your  cascades
right.
 
  - Because an error from the command on the upstream side of the
pipe is ignored, we tend to prefer writing things like this:
 
   git log -n 1 -p output 
 grep -v firstline output 

I agree with all of that. But also, is grep -v the right thing? I
think the intent of the test is firstline does not appear. But that is
not what  grep -v will tell you. It will tell you whether any line
that did not have firstline in it was shown (which it would be, since
there are a bunch of other lines shown).

I think ! grep firstline is what is needed here. Or even just
explicitly matching the diff that we expect via test_cmp. I like the
latter much better anyway, as a failure will show exactly what is wrong.
Whereas if the grep ends up not matching, there is no helpful output for
somebody reading the test.

We already produce nice messages from things like test_must_fail. Maybe
it would be nice to have:

  test_contains () {
  if ! grep $1 $2; then
  echo 2 File '$2' does not contain a line with '$1'
  return 1
  fi
  }

and likewise a test_not_contains or something to negate it. That makes
both the tests and their failure output readable.

-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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 11:36:19AM -0700, Shawn O. Pearce wrote:

  Interesting. I would have assumed it depended on order in the index.
 
 No. We tried that. Assigning bits by order in index (aka order of
 SHA-1s sorted) results in horrible compression of the bitmap itself
 because of the uniform distribution of SHA-1. Encoding instead by pack
 order gets us really good bitmap compression, because object graph
 traversal order tends to take reachability into account. So we see
 long contiguous runs of 1s and get good compression. Sorting by SHA-1
 just makes the space into swiss cheese.

Right, that makes a lot of sense.

  I think you are still OK, though, because
  the filename comes from the sha1 over the index file, which in turn
  includes the sha1 over the packfile. Thus any change in the packfile
  would give you a new pack and index name.
 
 No. The pack file name is composed from the SHA-1 of the sorted SHA-1s
 in the pack. Any change in compression settings or delta windows or
 even just random scheduling variations when repacking can cause
 offsets to slide, even if the set of objects being repacked has not
 differed. The resulting pack and index will have the same file names
 (as its the same set of objects), but the offset information and
 ordering is now different.

Are you sure? The trailer is computed over the sha1 of the actual pack
data (ordering, delta choices, and all), and is computed and written to
the packfile via sha1close (see pack-objects.c, ll. 753-763). That
trailer sha1 is fed into finish_tmp_packfile (l. 793).  That function
feeds it to write_idx_file, which starts a new sha1 computation that
includes the sorted sha1 list and other index info. But before we
sha1close that computation, we write the _original_ trailer sha1, adding
it to the new sha1 calculation. See pack-write.c, ll. 178-180.

And then that sha1 gets returned to finish_tmp_packfile, which uses it
to name the resulting files.

Am I reading the code wrong?

-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] git-web--browse: Fix open HTML help pages from iTerm

2012-09-27 Thread Junio C Hamano
Steffen Prohaska proha...@zib.de writes:

 iTerm is an alternative to the default terminal emulation program on Mac
 OS X.  git-web--browse wasn't aware of iTerm and failed to open HTML
 help pages when used in a shell session running in iTerm, reporting No
 known browser available.  Now it works as expected.

 Signed-off-by: Steffen Prohaska proha...@zib.de
 ---
  git-web--browse.sh | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/git-web--browse.sh b/git-web--browse.sh
 index 1e82726..95ecf65 100755
 --- a/git-web--browse.sh
 +++ b/git-web--browse.sh
 @@ -120,7 +120,8 @@ if test -z $browser ; then
   fi
   # SECURITYSESSIONID indicates an OS X GUI login session
   if test -n $SECURITYSESSIONID \
 - -o $TERM_PROGRAM = Apple_Terminal ; then
 + -o $TERM_PROGRAM = Apple_Terminal \
 + -o $TERM_PROGRAM = iTerm.app ; then
   browser_candidates=open $browser_candidates
   fi

I do not have anything against iTerm, but could we have a solution
that does not force us to keep adding 47 different terminal program
names to the list over the longer term (no pun intended)?  For
example, If on OS-X (which by the way does not seem to be checked
with the current logic) and environment TERM_PROGRAM is set to any
value, 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


[PATCH] diff: diff.context configuration gives default to -U

2012-09-27 Thread Jeff Muizelaar
Introduce a configuration variable diff.context that tells
Porcelain commands to use a non-default number of context
lines instead of 3 (the default).  With this variable, users
do not have to keep repeating git log -U8 from the command
line; instead, it becomes sufficient to say git config
diff.context 8 just once.

Signed-off-by: Jeff Muizelaar jmuizel...@mozilla.com
---

This fixes the tests to behave better and avoids 'grep -v'

 Documentation/diff-config.txt |4 +
 diff.c|9 +++-
 t/t4055-diff-context.sh   |  128 +
 3 files changed, 140 insertions(+), 1 deletions(-)
 create mode 100755 t/t4055-diff-context.sh

diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 67a90a8..75ab8a5 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -56,6 +56,10 @@ diff.statGraphWidth::
Limit the width of the graph part in --stat output. If set, applies
to all commands generating --stat output except format-patch.
 
+diff.context::
+   Generate diffs with n lines of context instead of the default of
+   3. This value is overridden by the -U option.
+
 diff.external::
If this config variable is set, diff generation is not
performed using the internal diff machinery, but using the
diff --git a/diff.c b/diff.c
index 35d3f07..86e5f2a 100644
--- a/diff.c
+++ b/diff.c
@@ -26,6 +26,7 @@ static int diff_detect_rename_default;
 static int diff_rename_limit_default = 400;
 static int diff_suppress_blank_empty;
 static int diff_use_color_default = -1;
+static int diff_context_default = 3;
 static const char *diff_word_regex_cfg;
 static const char *external_diff_cmd_cfg;
 int diff_auto_refresh_index = 1;
@@ -141,6 +142,12 @@ int git_diff_ui_config(const char *var, const char *value, 
void *cb)
diff_use_color_default = git_config_colorbool(var, value);
return 0;
}
+   if (!strcmp(var, diff.context)) {
+   diff_context_default = git_config_int(var, value);
+   if (diff_context_default  0)
+   return -1;
+   return 0;
+   }
if (!strcmp(var, diff.renames)) {
diff_detect_rename_default = git_config_rename(var, value);
return 0;
@@ -3170,7 +3177,7 @@ void diff_setup(struct diff_options *options)
options-break_opt = -1;
options-rename_limit = -1;
options-dirstat_permille = diff_dirstat_permille_default;
-   options-context = 3;
+   options-context = diff_context_default;
DIFF_OPT_SET(options, RENAME_EMPTY);
 
options-change = diff_change;
diff --git a/t/t4055-diff-context.sh b/t/t4055-diff-context.sh
new file mode 100755
index 000..8a31448
--- /dev/null
+++ b/t/t4055-diff-context.sh
@@ -0,0 +1,128 @@
+#!/bin/sh
+#
+# Copyright (c) 2012 Mozilla Foundation
+#
+
+test_description='diff.context configuration'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+   cat x -EOF 
+firstline
+b
+c
+d
+e
+f
+preline
+postline
+i
+j
+k
+l
+m
+n
+EOF
+   git update-index --add x 
+   git commit -m initial 
+   cat x -\EOF 
+firstline
+b
+c
+d
+e
+f
+preline
+1
+postline
+i
+j
+k
+l
+m
+n
+EOF
+   git update-index --add x 
+   git commit -m next 
+cat x -\EOF
+firstline
+b
+c
+d
+e
+f
+preline
+2
+postline
+i
+j
+k
+l
+m
+n
+EOF
+'
+
+test_expect_success 'diff.context affects log' '
+   git log -1 -p output 
+   ! grep firstline output 
+   git config diff.context 8 
+   git log -1 -p output 
+   grep firstline output
+'
+
+test_expect_success 'different -U value' '
+   git config diff.context 8 
+   git log -U4 -1 output 
+   ! grep firstline output
+'
+
+test_expect_success 'diff.context affects diff' '
+   git config diff.context 8 
+   git diff output 
+   grep firstline output
+'
+
+test_expect_success 'plumbing not affected' '
+   git config diff.context 8 
+   git diff-files -p  output 
+   ! grep firstline output
+'
+test_expect_success 'non-integer config parsing' '
+   cat  .git/config -\EOF 
+[diff]
+   context = no
+EOF
+   ! git diff 2output 
+   grep bad config value output
+'
+
+test_expect_success 'negative integer config parsing' '
+   cat .git/config -\EOF 
+[diff]
+   context = -1
+EOF
+   ! git diff 2output 
+   grep bad config file output
+'
+
+test_expect_success '0 config parsing' '
+   cat  .git/config -\EOF 
+[diff]
+   context = 0
+EOF
+   git diff output 
+   grep preline output
+'
+
+test_expect_success 'config parsing' '
+   cat .git/config -\EOF 
+[diff]
+   context = 8
+EOF
+   git diff output 
+   grep postline output
+'
+
+test_done
-- 
1.7.4.4


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

Re: [PATCH] git-web--browse: Fix open HTML help pages from iTerm

2012-09-27 Thread Steffen Prohaska

On Sep 27, 2012, at 9:11 PM, Junio C Hamano wrote:

 Steffen Prohaska proha...@zib.de writes:
 
 iTerm is an alternative to the default terminal emulation program on Mac
 OS X.  git-web--browse wasn't aware of iTerm and failed to open HTML
 help pages when used in a shell session running in iTerm, reporting No
 known browser available.  Now it works as expected.
 
 Signed-off-by: Steffen Prohaska proha...@zib.de
 ---
 git-web--browse.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
 
 diff --git a/git-web--browse.sh b/git-web--browse.sh
 index 1e82726..95ecf65 100755
 --- a/git-web--browse.sh
 +++ b/git-web--browse.sh
 @@ -120,7 +120,8 @@ if test -z $browser ; then
  fi
  # SECURITYSESSIONID indicates an OS X GUI login session
  if test -n $SECURITYSESSIONID \
 --o $TERM_PROGRAM = Apple_Terminal ; then
 +-o $TERM_PROGRAM = Apple_Terminal \
 +-o $TERM_PROGRAM = iTerm.app ; then
  browser_candidates=open $browser_candidates
  fi
 
 I do not have anything against iTerm, but could we have a solution
 that does not force us to keep adding 47 different terminal program
 names to the list over the longer term (no pun intended)?  For
 example, If on OS-X (which by the way does not seem to be checked
 with the current logic) and environment TERM_PROGRAM is set to any
 value, or something.

I googled a bit and it seems that TERM_PROGRAM is specific to OS X.
So simply testing whether TERM_PROGRAM is set to any value (without
additional check for OS X) might be good enough.

I am wondering whether anyone knows if TERM_PROGRAM is used on other
operating systems besides OS X.

Steffen
--
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 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks

2012-09-27 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 This series fixes longest_ancestor_length() so that it works even if
 prefix_list contains entries that involve symlinks.  The basic goal of
 the series is to call real_path() on each of the entries so that a
 textual comparison of the potential prefix to the front of path
 correctly decides whether the path is located inside of the entry.
 But along the way some other things had to be changed:

 * real_path() die()s if the path passed to it is invalid, whereas it
   is allowed for GIT_CEILING_DIRECTORIES to contain invalid paths.  So
   create a new function real_path_if_valid() that returns NULL for
   invalid paths.

 * Changing longest_ancestor_length() to call real_path_if_valid()
   would make the former very difficult to test (because the tests
   would depend on the contents of the whole filesystem).  Therefore,
   rewrite longest_ancestor_length() in terms of functions
   string_list_split(), string_list_longest_prefix(), and
   real_path_if_valid() which are tested individually.

 The net results of these changes are that:

 1. t1504 used to have to canonicalize TRASH_DIRECTORY to make itself
work even if the --root directory contains symlinks.  This
canonicalization is no longer necessary (and has been removed).

 2. t4035, which used to fail if the --root directory contained
symlinks, now works correctly in that situation.

 After this change, all tests pass if the --root directory does *not*
 contain symlinks, but t9903 still fails if the --root directory
 contains symlinks.  I haven't analyzed the cause of t9903's failure,
 but it does not appear to be related to the GIT_CEILING_DIRECTORIES
 feature.

I haven't read the actual patches yet, but the all of the above
sounds sensible.

 On the mailing list I suggested *purposely* inserting symlinks into
 the trash directory.* paths to test symlink handling more
 systematically.  This patch series does *NOT* make that change.

And that may be a sensible follow-up step once the dust settles.

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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread David Michael Barr
Hi all,

On Fri, Sep 28, 2012 at 3:20 AM, Jeff King p...@peff.net wrote:
 On Thu, Sep 27, 2012 at 07:17:42PM +0700, Nguyen Thai Ngoc Duy wrote:

  Operation   Index V2   Index VE003
  Clone   37530ms (524.06 MiB) 82ms (524.06 MiB)
  Fetch (1 commit back)  75ms 107ms
  Fetch (10 commits back)   456ms (269.51 KiB)341ms (265.19 KiB)
  Fetch (100 commits back)  449ms (269.91 KiB)337ms (267.28 KiB)
  Fetch (1000 commits back)2229ms ( 14.75 MiB)189ms ( 14.42 MiB)
  Fetch (1 commits back)   2177ms ( 16.30 MiB)254ms ( 15.88 MiB)
  Fetch (10 commits back) 14340ms (185.83 MiB)   1655ms (189.39 MiB)

 Beautiful. And curious, why do 100-1000 and 1-1 have such
 big leaps in time (V2)?

 Agreed. I'm very excited about these numbers.

+1

 Definitely :-). I have shown my interest in this topic before. So I
 should probably say that I'm going to work on this on C Git, but
 slllwwwly. As this benefits the server side greatly, perhaps a
 GitHubber ;-) might want to work on this on C Git, for GitHub itself
 of course, and, as a side effect, make the rest of us happy?

 Yeah, GitHub is definitely interested in this. I may take a shot at it,
 but I know David Barr (cc'd) is also interested in such things.

Yeah, I'm definitely interested, I love this stuff.

--
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: [PATCH 3/3] completion: improve shell expansion of items

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 02:43:38AM -0400, Jeff King wrote:

 Ah. The problem is that most of the load comes from my patch 4/3, which
 does a separate iteration. Here are the numbers after just patch 3:
 
   $ time __gitcomp_nl $refs
   real0m0.344s
   user0m0.392s
   sys 0m0.040s
 
 Slower, but not nearly as painful. Here are the numbers using sed[1]
 instead:
 
   $ time __gitcomp_nl $refs
   real0m0.100s
   user0m0.084s
   sys 0m0.016s
 
 So a little slower, but probably acceptable. We could maybe do the same
 trick on the output side (although it is a little trickier there,
 because we need it in a bash array). Of course, this is Linux; the fork
 for sed is way more expensive on some systems.

So something like the patch below does the quoting correctly (try
committing a file like name with spaces and doing git show
HEAD:TAB), and isn't too much slower:

  real0m0.114s
  user0m0.108s
  sys 0m0.004s

That's almost double the time without handling quoting, but keep in mind
this is on 10,000 entries (and the real sluggishness we are trying to
avoid is an order of magnitude). So it might be acceptable.

This is just a proof-of-concept patch. We'd probably want to replace the
perl below with a more complicated sed invocation  for portability (the
trickiness is that the output is shown to the user, so we very much
don't want to quote anything that does not have to be).

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index be800e0..20c09ef 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -225,6 +225,17 @@ fi
 fi
 fi
 
+# Quotes each element of a newline-delimited list for shell reuse
+__git_quote()
+{
+   echo $1 |
+   sed 
+ s/'/'''/g
+ s/^/'/
+ s/$/'/
+   
+}
+
 # Generates completion reply with compgen, appending a space to possible
 # completion words, if necessary.
 # It accepts 1 to 4 arguments:
@@ -261,7 +272,10 @@ __gitcomp_nl ()
 __gitcomp_nl ()
 {
local IFS=$'\n'
-   COMPREPLY=($(compgen -P ${2-} -S ${4- } -W $1 -- ${3-$cur}))
+   COMPREPLY=($(
+ compgen -P ${2-} -S ${4- } -W $(__git_quote $1) -- 
${3-$cur} |
+ perl -lne '/(.*?)( ?)$/; print quotemeta($1), $2'
+   ))
 }
 
 __git_heads ()

-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] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig

2012-09-27 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Also, have you considered using a config include? Like:

   $ echo '[include]path = ~/my-dotfiles/gitconfig' ~/.gitconfig

Very good suggestion.

 ... you have to bootstrap
 somehow (e.g., you're going to have to copy a .profile or similar to get
 the GIT_GLOBAL_CONFIG variable set).

Exactly my thought ;-)
--
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


Showing all stashed changes in one go

2012-09-27 Thread Yann Dirson
When I have a couple of stashed changes, it gets annoying to
repeatedly call git stash show -p stash@{N} until finding the
correct one.

Since git reflog show stash already does part of the job, I thought
that adding -p there to see the patch would help (at least it would
show the not-yet-staged parts, which would already be a good start).

But the output is then really strange: does it really print the delta
between every two reflog entries ?  I can't think of a situation where
it would be was we want - but then, my imagination is known to be
deficient when I hit a situation that does not do what I was expecting
at first :)

Is there another way I missed to get all those stash contents listed,
besides scriptically iterating ?
--
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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 02:52:29PM -0400, Jeff King wrote:

  No. The pack file name is composed from the SHA-1 of the sorted SHA-1s
  in the pack. Any change in compression settings or delta windows or
  even just random scheduling variations when repacking can cause
  offsets to slide, even if the set of objects being repacked has not
  differed. The resulting pack and index will have the same file names
  (as its the same set of objects), but the offset information and
  ordering is now different.
 
 Are you sure? The trailer is computed over the sha1 of the actual pack
 data (ordering, delta choices, and all), and is computed and written to
 the packfile via sha1close (see pack-objects.c, ll. 753-763). That
 trailer sha1 is fed into finish_tmp_packfile (l. 793).  That function
 feeds it to write_idx_file, which starts a new sha1 computation that
 includes the sorted sha1 list and other index info. But before we
 sha1close that computation, we write the _original_ trailer sha1, adding
 it to the new sha1 calculation. See pack-write.c, ll. 178-180.
 
 And then that sha1 gets returned to finish_tmp_packfile, which uses it
 to name the resulting files.
 
 Am I reading the code wrong?

And the answer is...yes. I'm blind.

The final bit of code in write_idx_file is:

sha1write(f, sha1, 20);
sha1close(f, NULL, ((opts-flags  WRITE_IDX_VERIFY)
? CSUM_CLOSE : CSUM_FSYNC));
git_SHA1_Final(sha1, ctx);

So we write the trailer, but the sha1 we pull out is _not_ the sha1 over
the index format. It is from ctx, not f; and hte former is from the
object list. Just like you said. :)

So yeah, we would want to put the pack trailer sha1 into the
supplementary index file, and check that it matches when we open it.
It's a slight annoyance, but it's O(1).

Anything which rewrote the pack and index would also want to rewrite
these supplementary files. So the worst case would be:

  1. Pack with a new version which builds the supplementary file.

  2. Repack with an old version which generates a pack with identical
 objects, but different ordering. It does not regenerate the
 supplementary file, because it does not know about it.

  3. Try to read with newer git.

Without the extra trailer check, we get a wrong answer. With the check,
we notice that the supplementary file is bogus, and fallback to the slow
path. Which I think is OK, considering that this is a reasonably
unlikely scenario to come up often (and it is no slower than it would be
if you generated a _new_ packfile in step 2).

-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: Showing all stashed changes in one go

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 10:00:06PM +0200, Yann Dirson wrote:

 When I have a couple of stashed changes, it gets annoying to
 repeatedly call git stash show -p stash@{N} until finding the
 correct one.
 
 Since git reflog show stash already does part of the job, I thought
 that adding -p there to see the patch would help (at least it would
 show the not-yet-staged parts, which would already be a good start).
 
 But the output is then really strange: does it really print the delta
 between every two reflog entries ?  I can't think of a situation where
 it would be was we want - but then, my imagination is known to be
 deficient when I hit a situation that does not do what I was expecting
 at first :)

This is a known issue. The reflog walker rewrites the parents of each
commit to make them look like a chain, but it means that your diffs are
between reflog entries, not to the true parents.

 Is there another way I missed to get all those stash contents listed,
 besides scriptically iterating ?

You can do:

  git rev-list -g ref | git log --stdin --no-walk other options

to show the individual commits with their true parents. But note that
stash commits are a little confusing (they are merges representing the
index and working tree state).

-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] config: introduce GIT_GLOBAL_CONFIG to override ~/.gitconfig

2012-09-27 Thread Matthieu Moy
Jeff King p...@peff.net writes:

 Also, have you considered using a config include? Like:

   $ echo '[include]path = ~/my-dotfiles/gitconfig' ~/.gitconfig

 It's a one-time setup, and then you get updates inside my-dotfiles
 forever. The one-time setup is annoying, but you have to bootstrap
 somehow (e.g., you're going to have to copy a .profile or similar to get
 the GIT_GLOBAL_CONFIG variable set).

I also strongly prefer symlinks or include over environment variables.
Relying on environment variables makes the setup a bit fragile, partly
because I often mess things up with my shell (e.g. I once had different
shell and environment variables in ~/.xsession and in actual shells,
hence different configuration depending on whether I launch stuff from
my window-manager or from a shell).

I could understand using environment variables for one-shot
configuration change (e.g. HOME=/tmp/ git whatever), but then $HOME is
sufficient in general.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 1/8] Introduce new static function real_path_internal()

2012-09-27 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 @@ -54,20 +73,36 @@ const char *real_path(const char *path)
   }
  
   if (*buf) {
 - if (!*cwd  !getcwd(cwd, sizeof(cwd)))
 - die_errno (Could not get current working 
 directory);
 + if (!*cwd  !getcwd(cwd, sizeof(cwd))) {
 + if (die_on_error)
 + die_errno(Could not get current 
 working directory);
 + else
 + goto error_out;
 + }
  
 - if (chdir(buf))
 - die_errno (Could not switch to '%s', buf);
 + if (chdir(buf)) {
 + if (die_on_error)
 + die_errno(Could not switch to '%s', 
 buf);
 + else
 + goto error_out;
 + }
 + }

The patch makes sense, but while you are touching this code, I have
to wonder if there is an easy way to tell, before entering the loop,
if we have to chdir() around in the loop.  That would allow us to
hoist the getcwd() that is done only so that we can come back to
where we started outside the loop, making it clear why the call is
there, instead of cryptic if (!*cwd  to ensure we do getcwd()
once and before doing any chdir().
--
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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 So yeah, we would want to put the pack trailer sha1 into the
 supplementary index file, and check that it matches when we open it.
 It's a slight annoyance, but it's O(1).

Yes.  If I am not mistaken, that is exactly how an .idx file makes
sure that it describes the matching .pack file (it has packfile
checksum in its trailer).  Otherwise you can repack the same set of
objects into a new .pack file and make existing .idx very confused.

--
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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Jeff King
On Thu, Sep 27, 2012 at 02:33:01PM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  So yeah, we would want to put the pack trailer sha1 into the
  supplementary index file, and check that it matches when we open it.
  It's a slight annoyance, but it's O(1).
 
 Yes.  If I am not mistaken, that is exactly how an .idx file makes
 sure that it describes the matching .pack file (it has packfile
 checksum in its trailer).  Otherwise you can repack the same set of
 objects into a new .pack file and make existing .idx very confused.

Yeah. In theory you wouldn't name the new packfile into place without
also generating a new index for it. But even if you do it right, there's
a race condition, and checking the trailer sha1 at least lets us know
that they don't match (I assume we just reject the index, then; I guess
this can result in an operation failing, but in practice it doesn't
really happen, as we don't bother packing unless there are actually new
objects).

-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 4/3] completion: quote completions we find

2012-09-27 Thread SZEDER Gábor
On Wed, Sep 26, 2012 at 05:57:00PM -0400, Jeff King wrote:
 + COMPREPLY[$i]=${COMPREPLY[$i]}$stripped

This reminded me to a mini-series collecting dust in my git repo,
which converts a few similar var=$var$something constructs to use the
+= append operator instead.

Now, Bash supports this += append operator since v3.1
(bash-3.1-alpha1, to be exact), which is around since July 2005, if I
can trust the mtime at ftp://ftp.cwru.edu/pub/bash/.  MSysgit ships
v3.1 so it already supports this, too.  So, what is the oldest Bash
version we care about for completion?

--
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] diff: diff.context configuration gives default to -U

2012-09-27 Thread Junio C Hamano
Jeff Muizelaar jmuizel...@mozilla.com writes:

 + if (!strcmp(var, diff.context)) {
 + diff_context_default = git_config_int(var, value);
 + if (diff_context_default  0)
 + return -1;
 + return 0;

I am somewhat torn on this part. This fails the entire command when
diff.context is set to non integer or negative integer, which means
trouble for a user of a future version of git that accepts such a
value to do something intelligent we do not anticipate today. The
useful configuration value cannot be given unless the user is
certain that .gitconfig file will never be read by older version of
git.

Perhaps it is OK, at least for now.  We'd have the same worry for
what is given to -Un anyway.

 + }
   if (!strcmp(var, diff.renames)) {
   diff_detect_rename_default = git_config_rename(var, value);
   return 0;
 @@ -3170,7 +3177,7 @@ void diff_setup(struct diff_options *options)
   options-break_opt = -1;
   options-rename_limit = -1;
   options-dirstat_permille = diff_dirstat_permille_default;
 - options-context = 3;
 + options-context = diff_context_default;
   DIFF_OPT_SET(options, RENAME_EMPTY);
  
   options-change = diff_change;
 diff --git a/t/t4055-diff-context.sh b/t/t4055-diff-context.sh
 new file mode 100755
 index 000..8a31448
 --- /dev/null
 +++ b/t/t4055-diff-context.sh
 @@ -0,0 +1,128 @@
 +#!/bin/sh
 +#
 +# Copyright (c) 2012 Mozilla Foundation
 +#
 +
 +test_description='diff.context configuration'
 +
 +. ./test-lib.sh
 +
 +test_expect_success 'setup' '
 + cat x -EOF 
 +firstline
 +b
 +...
 +n
 +EOF
 + git update-index --add x 
 + git commit -m initial 
 + cat x -\EOF 
 +firstline
 +b

The dash after here-document redirection (i.e. -) allows you to
indent the meat of the here-document and the end-of-here-document
marker.  Please take advantage of the facility to make the result
more readable.

Also unless there is something that needs interpolation by the
shell, we prefer to use quoted here-document (i.e. -\EOF) to
reduce the mental burden by the readers of the code.  So

test_expect_success setup '
cat x -\EOF 
firstline
b
...
n
EOF
use x in the test 
check the result
'

This comment applies to all other uses of - in this patch.

 +test_expect_success 'diff.context affects log' '
 + git log -1 -p output 
 + ! grep firstline output 
 + git config diff.context 8 
 + git log -1 -p output 
 + grep firstline output
 +'

Is there a reason to favor log -1 -p over something a lot simpler
like show?  Not requesting to change anything, but just being
curious.

 +test_expect_success 'different -U value' '
 + git config diff.context 8 
 + git log -U4 -1 output 
 + ! grep firstline output
 +'

OK, so -U4 overrides configured diff.context setting and you make
sure by asking -U4 that is too small to show firstline (but if 8
were used, the line would have been shown).

 +test_expect_success 'diff.context affects diff' '
 + git config diff.context 8 
 + git diff output 
 + grep firstline output
 +'

Don't we want to make sure that without diff.context the output will
give you the default 3 lines of context?  It is a common mistake
shared by people who want to demonstrate their shiny new toys to
test only the positive cases while forgetting to test non-regression.

 +test_expect_success 'plumbing not affected' '
 + git config diff.context 8 
 + git diff-files -p  output 

Style.  No SP between redirection and the filename, i.e.

git diff-files -p output 

 + ! grep firstline output
 +'

A blank line between tests.

 +test_expect_success 'non-integer config parsing' '
 + cat  .git/config -\EOF 

 +[diff]
 + context = no
 +EOF
 + ! git diff 2output 
 + grep bad config value output
 +'

We are not in the business of debugging grep, so writing ! grep ...
to make sure grep does not find something is perfectly fine, but
when expecting an error from git command, please write it like

test_must_fail git diff 2error 
test_i18ngrep bad config value error

instead.  The error messages meant for human consumption could be
localized, and use of test_i18ngrep allows use to ignore its error
condition when the test is run under funny locale.

 +test_expect_success 'negative integer config parsing' '
 + cat .git/config -\EOF 
 +[diff]
 + context = -1
 +EOF
 + ! git diff 2output 
 + grep bad config file output
 +'
 +
 +test_expect_success '0 config parsing' '
 + cat  .git/config -\EOF 
 +[diff]
 + context = 0
 +EOF
 + git diff output 
 + grep preline output
 +'

Hrm, is this correct?  Your test vectors change a single line that
is surrounded by preline and postline, so -U0 patch should show
only the change for that single line, 

Re: [PATCH 4/3] completion: quote completions we find

2012-09-27 Thread Junio C Hamano
SZEDER Gábor sze...@ira.uka.de writes:

 On Wed, Sep 26, 2012 at 05:57:00PM -0400, Jeff King wrote:
 +COMPREPLY[$i]=${COMPREPLY[$i]}$stripped

 This reminded me to a mini-series collecting dust in my git repo,
 which converts a few similar var=$var$something constructs to use the
 += append operator instead.

Is the benefit of rewriting it to var+=$something large enough to
worry about the below?

 Now, Bash supports this += append operator since v3.1
 (bash-3.1-alpha1, to be exact), which is around since July 2005, if I
 can trust the mtime at ftp://ftp.cwru.edu/pub/bash/.  MSysgit ships
 v3.1 so it already supports this, too.  So, what is the oldest Bash
 version we care about for completion?

--
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 3/8] longest_ancestor_length(): use string_list_split()

2012-09-27 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 - for (colon = ceil = prefix_list; *colon; ceil = colon+1) {
 - for (colon = ceil; *colon  *colon != PATH_SEP; colon++);
 - len = colon - ceil;
 + string_list_split(prefixes, prefix_list, PATH_SEP, -1);
 +
 + for (i = 0; i  prefixes.nr; i++) {
 + const char *ceil = prefixes.items[i].string;
 + int len = strlen(ceil);
 +

Much nicer than the yucky original ;-)

   if (len == 0 || len  PATH_MAX || !is_absolute_path(ceil))
   continue;
 - strlcpy(buf, ceil, len+1);
 + memcpy(buf, ceil, len+1);
   if (normalize_path_copy(buf, buf)  0)
   continue;

Why do you need this memcpy in the first place?  Isn't ceil already
a NUL terminated string unlike the original code that points into a
part of the prefix_list string?  IOW, why not

normalize_path_copy(buf, ceil);

or something?

Can normalize_path_copy() overflow buf[PATH_MAX+1] here (before or
after this patch)?
--
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 4/8] longest_ancestor_length(): explicitly filter list before loop

2012-09-27 Thread Junio C Hamano
Makes sense.
--
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 7/8] longest_ancestor_length(): resolve symlinks before comparing paths

2012-09-27 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 longest_ancestor_length() relies on a textual comparison of directory
 parts to find the part of path that overlaps with one of the paths in
 prefix_list.  But this doesn't work if any of the prefixes involves a
 symbolic link, because the directories will look different even though
 they might logically refer to the same directory.  So canonicalize the
 paths listed in prefix_list using real_path_if_valid() before trying
 to find matches.

 path is already in canonical form, so doesn't need to be canonicalized
 again.

 This fixes some problems with using GIT_CEILING_DIRECTORIES that
 contains paths involving symlinks, including t4035 if run with --root
 set to a path involving symlinks.

 Remove a number of tests of longest_ancestor_length().  It is awkward
 to test longest_ancestor_length() now, because its new path
 normalization behavior depends on the contents of the whole
 filesystem.  But we can live without the tests, because
 longest_ancestor_length() is now built of reusable components that are
 themselves tested separately: string_list_split(),
 string_list_longest_prefix(), and real_path_if_valid().

Errr, components may be correct but the way to combine and construct
could go faulty, so...

 Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
 ---
  path.c| 17 --
  t/t0060-path-utils.sh | 64 
 ---
  2 files changed, 10 insertions(+), 71 deletions(-)

 diff --git a/path.c b/path.c
 index 5cace83..981bb06 100644
 --- a/path.c
 +++ b/path.c
 @@ -570,22 +570,25 @@ int normalize_path_copy(char *dst, const char *src)
  
  static int normalize_path_callback(struct string_list_item *item, void 
 *cb_data)
  {
 - char buf[PATH_MAX+2];
 + char *buf;
   const char *ceil = item-string;
 - int len = strlen(ceil);
 + const char *realpath;
 + int len;
  
 - if (len == 0 || len  PATH_MAX || !is_absolute_path(ceil))
 + if (!*ceil || !is_absolute_path(ceil))
   return 0;
 - memcpy(buf, ceil, len+1);
 - if (normalize_path_copy(buf, buf)  0)
 + realpath = real_path_if_valid(ceil);
 + if (!realpath)
   return 0;
 - len = strlen(buf);
 + len = strlen(realpath);
 + buf = xmalloc(len + 2); /* Leave space for possible trailing slash */
 + strcpy(buf, realpath);
   if (len == 0 || buf[len-1] != '/') {
   buf[len++] = '/';
   buf[len++] = '\0';
   }

Nice.
--
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 4/3] completion: quote completions we find

2012-09-27 Thread SZEDER Gábor
On Thu, Sep 27, 2012 at 03:31:10PM -0700, Junio C Hamano wrote:
 SZEDER Gábor sze...@ira.uka.de writes:
 
  On Wed, Sep 26, 2012 at 05:57:00PM -0400, Jeff King wrote:
  +  COMPREPLY[$i]=${COMPREPLY[$i]}$stripped
 
  This reminded me to a mini-series collecting dust in my git repo,
  which converts a few similar var=$var$something constructs to use the
  += append operator instead.
 
 Is the benefit of rewriting it to var+=$something large enough to
 worry about the below?

That way we can get rid of a subshell in __gitcomp(), which means one
less fork() during every command or option completion for Windows
folks.  We can also get rid of two subshells during loading the
completion script.

And I would spare myself from a couple of merge conflicts, too ;)


  Now, Bash supports this += append operator since v3.1
  (bash-3.1-alpha1, to be exact), which is around since July 2005, if I
  can trust the mtime at ftp://ftp.cwru.edu/pub/bash/.  MSysgit ships
  v3.1 so it already supports this, too.  So, what is the oldest Bash
  version we care about for completion?

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


Fwd: Please pull git-l10n updates on maint branch

2012-09-27 Thread Jiang Xin
Hi, Junio

The following changes since commit e70d1632bdaf25a9ee528e78133cab319083eade:

  Further merging in preparation for 1.7.12.1 (2012-09-12 14:12:48 -0700)

are available in the git repository at:

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

for you to fetch changes up to 5b3ba7193f2f99fa1c23c1952f3e0f880e101ae2:

  Merge branch 'l10n-thynson' of git://github.com/thynson/git-po-zh_CN
into maint (2012-09-28 06:49:08 +0800)



Jiang Xin (2):
  Merge branch 'maint' of https://github.com/ralfth/git-po-de into maint
  Merge branch 'l10n-thynson' of
git://github.com/thynson/git-po-zh_CN into maint

Ralf Thielow (1):
  l10n: de.po: correct translation of a 'rebase' message

Thynson (2):
  l10n: Unify the translation for '(un)expected'
  l10n: Improve many translation for zh_CN

 po/de.po|   10 +-
 po/zh_CN.po |   46 +++---
 2 files changed, 28 insertions(+), 28 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


Please pull git-l10n updates on master branch

2012-09-27 Thread Jiang Xin
Hi, Junio

The following changes since commit 1084f3b844d80d84d2d318bc562b78514cd78028:

  The sixth batch for 1.8.0 (2012-09-14 12:34:11 -0700)

are available in the git repository at:

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

for you to fetch changes up to b2f4b6cec2c5c1e2f802ac4b4bd12d5b26461069:

  Merge git://github.com/gotgit/git-po-zh_CN (2012-09-28 07:03:43 +0800)



Jiang Xin (6):
  l10n: Update git.pot (825 new, 24 removed messages)
  l10n: zh.CN.po: msgmerge git.pot (1142t195f630u)
  Merge branch 'maint' of https://github.com/ralfth/git-po-de into maint
  Merge branch 'l10n-thynson' of
git://github.com/thynson/git-po-zh_CN into maint
  Merge branch 'maint'
  Merge git://github.com/gotgit/git-po-zh_CN

Peter Krefting (2):
  Update Swedish translation (1967t0f0u)
  l10n: Fixes to Swedish translation

Ralf Thielow (1):
  l10n: de.po: correct translation of a 'rebase' message

Thynson (2):
  l10n: Unify the translation for '(un)expected'
  l10n: Improve many translation for zh_CN

 po/de.po|   10 +-
 po/git.pot  | 6908 +++
 po/sv.po| 4433 +-
 po/zh_CN.po | 4687 +++-
 4 files changed, 13108 insertions(+), 2930 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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Nguyen Thai Ngoc Duy
On Thu, Sep 27, 2012 at 9:33 PM, Shawn Pearce spea...@spearce.org wrote:
 I'd like to see some sort of extension mechanism like in
 $GIT_DIR/index, so that we don't have to increase pack index version
 often.

 This might be worthwhile. I dislike the way $GIT_DIR/index encodes
 extensions. Forcing an extension to fully materialize itself to
 determine its length so the length can be placed before the data is
 painful to work with when writing the file out to disk. I would prefer
 writing an index catalog at the trailer of the file. We already
 require random access to the index file, so its possible for a reader
 to read a fixed size trailer record that has the 2 SHA-1s we normally
 end an index with, and an extension catalog footer that has a length
 and CRC-32 of the catalog. The catalog would immediately appear before
 the footer, so a reader can find the start of the extension catalog by
 subtracting from the end of the file the catalog length and the file
 footer and catalog footer lengths. The catalog can then supply a
 starting offset for each extension section, and writers don't need to
 predict in advance how much data they need to store. Readers trying to
 use extensions aren't really hurt, Git already randomly seeks to read
 the tail of an index file to compare the pack SHA-1 before assuming
 the index is valid.

Yeah, that's exactly what I had in mind. But perhaps a separate file
(or files?) may be better. On that point, should all extensions be in
one new extra file, or one extension per file? I prefer all extensions
in one file, so we only need a single additional stat() for extension
check instead of probing for the entire pack-XXX.* range. In that
case, the catalog trailer idea still applies.
-- 
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: Using bitmaps to accelerate fetch and clone

2012-09-27 Thread Nguyen Thai Ngoc Duy
On Fri, Sep 28, 2012 at 12:20 AM, Jeff King p...@peff.net wrote:
 Definitely :-). I have shown my interest in this topic before. So I
 should probably say that I'm going to work on this on C Git, but
 slllwwwly. As this benefits the server side greatly, perhaps a
 GitHubber ;-) might want to work on this on C Git, for GitHub itself
 of course, and, as a side effect, make the rest of us happy?

 Yeah, GitHub is definitely interested in this. I may take a shot at it,
 but I know David Barr (cc'd) is also interested in such things.

Great. Now I can just sit back and enjoy :)
-- 
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: Commit cache to speed up rev-list and merge

2012-09-27 Thread Nguyen Thai Ngoc Duy
On Fri, Sep 28, 2012 at 12:39 AM, Jeff King p...@peff.net wrote:
 On Thu, Sep 27, 2012 at 08:51:51AM -0700, Shawn O. Pearce wrote:

 On Thu, Sep 27, 2012 at 5:17 AM, Nguyen Thai Ngoc Duy pclo...@gmail.com 
 wrote:
  I'd like to see some sort of extension mechanism like in
  $GIT_DIR/index, so that we don't have to increase pack index version
  often. What I have in mind is optional commit cache to speed up
  rev-list and merge, which could be stored in pack index too.

 Can you share some of your ideas?

 Some of it is here:

   http://article.gmane.org/gmane.comp.version-control.git/203308

And an experiment was done earlier

http://thread.gmane.org/gmane.comp.version-control.git/194306/focus=194596
-- 
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


The ciabot hook code in contrib/ is obsolete - delete it

2012-09-27 Thread Eric S. Raymond
The CIA notification server died about two days ago, done in by
a screwup at the cloud service hosting is VM.  For various technical
reasons there cannot and will not be any effort to revive it.  If you want
the whole sordid tale, read CIA and the perils of overengineering at
http://esr.ibiblio.org/?p=4540.

Accordingly, the contrib/ciabot code is now obsolete and should be removed.

I have written a replacement service. irker, with a different (and
fundamentally simpler) design. Though I released irker just today, it
already has multiple deployments.  A lot of hackers like their IRC
commit notifications and have instantly seized on this option to get
such a service running again - this time as a distributed flock of
lightweight notifier proxy daemons that cannot be taken out by any
single-point failure. 

Interested persons may wish to monitor the freenode #cia channel,
which is morphing into a discussion about coordinating irker
deployment and building various proxy/symbiote/statistics-gathering 
services around it.

I'm shipping a generic repo-hook script that supports both git and
Subversion in the irker distribution, so there won't be any need
for git to carry a special hook.  I remain grateful for your previous 
cooperation in supporting and distributing the ciabot code. 
-- 
a href=http://www.catb.org/~esr/;Eric S. Raymond/a

You need only reflect that one of the best ways to get yourself a
reputation as a dangerous citizen these days is to go about repeating
the very phrases which our founding fathers used in the great struggle
for independence.   -- Attributed to Charles Austin Beard (1874-1948)
--
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