Re: git 1.9.1 tarball

2014-03-20 Thread Junio C Hamano
Jason St. John jstj...@purdue.edu writes:

 On Wed, Mar 19, 2014 at 8:09 PM, 乙酸鋰 ch3co...@gmail.com wrote:

 Hi,

 Where to find git 1.9.1 tarball?
 It is not uploaded to google code.
 --

 You can download a tarball for 1.9.1 from GitHub:
 https://github.com/git/git/archive/v1.9.1.tar.gz

 Jason

The announcement message starts like this:

The latest maintenance release Git v1.9.1 is now available at
the usual places.

The release tarballs are found at:

https://www.kernel.org/pub/software/scm/git/

It is somewhat strange that nobody seems to read the announcement
that has that exact information before asking.



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


[GSOC 2014]idea:Git Configuration API Improvement

2014-03-20 Thread Yao Zhao
Hello, Michael, Matthieu and peff,

My name is Yao and I am interested in Git Configuration API Improvements listed 
in idea page in Git. I came up some ideas and really want to discuss them with 
you.

First is about when to start reading configuration file to cache. My idea is 
the time user starts call command that need configuration information (need to 
read configuration file).

Second is about data structure. I read Peff's email listed on idea page. He 
indicated two methods and I prefer syntax tree. I think there should be three 
or more syntax tree in the cache. One for system, one for global and one for 
local. If user indicate a file to be configuration file, add one more tree. Or 
maybe we can build one tree and tag every node to indicate where it belongs to.

Third one is about when to write back to file, I am really confused about it. I 
think one way could be when user leave git repository using cd to go back. 
But I am not sure if git could detect user calls cd to leave repository.

Thank you,

Yao
--
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] rev-parse --parseopt: option argument name hints

2014-03-20 Thread Ilya Bobyr

On 3/19/2014 11:46 AM, Junio C Hamano wrote:

Ilya Bobyr ilya.bo...@gmail.com writes:


I can not find this particular patch in the latest What's cooking email.
Is there something I can do?

IIRC, I think I was waiting for the version with a new Usage text
section to the documentation you alluded to in this exchange
($gmane/243924):

 Ilya Bobyr ilya.bo...@gmail.com writes:

  On 3/11/2014 12:10 PM, Junio C Hamano wrote:
 
  Documentation on the whole argument parsing is quite short, so,...
  ...
  I though that an example just to describe `argh' while useful would
  look a bit disproportional, compared to the amount of text on
  --parseopt.
 
  But now that I've added a Usage text section to looks quite in place.
 
  I just realized that the second patch I sent did not contain the
  changes.  Sorry about - I will resend it.


Oh %)
I did sent it in the next minute.  And did receive a copy myself.
But it seems it never showed up in the list.
I am still a bit new to the tools, maybe I did something wrong.
Will try again :)


It does not seems like there is a lot of interest, so I am not sure
there will be a lot of discussion.
It is a minor fix and considering the number of the emails on the
list, I do not unexpected this kind of stuff to be very popular.
But it seems like a valid improvement to me.
Maybe I am missing something?

You did the right thing by sending a reminder message with a pointer
to help others locate the original (like the one I am responding
to), as nobody can keep up with a busy list traffic.


Thanks :)


Same questions about this one:

 [PATCH] gitk: replace SHA1 entry field on keyboard paste
 http://www.mail-archive.com/git@vger.kernel.org/msg45040.html

I think they are more or less similar, except that the second one is
just trivial.

I do not remember if I forwarded the patch to the area maintainer
Paul Mackerras pau...@samba.org, but if I didn't please do so
yourself.  The changes to gitk and git-gui come to me via their own
project repositories.


You did and I even replied with additional details, that I should have 
included as a cover letter.

I can see those messages in the web archive.
It seems that Paul Mackerras gitk repository is here: 
git://ozlabs.org/~paulus/gitk.git

At least that is what is online.  I do not see the change in there.
I will remind him about 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


[PATCH v3] rev-parse --parseopt: option argument name hints

2014-03-20 Thread Ilya Bobyr
Built-in commands can specify names for option arguments when usage text
is generated for a command.  sh based commands should be able to do the
same.

Option argument name hint is any text that comes after [*=?!] after the
argument name up to the first whitespace.  Underscores are replaced with
whitespace.  It is unlikely that an underscore would be useful in the
hint text.

Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
---
 Changed according to the last comments.  Added Usage text paragraph in the
 documentation and updated variable names.

 Documentation/git-rev-parse.txt |   34 --
 builtin/rev-parse.c |   17 -
 t/t1502-rev-parse-parseopt.sh   |   20 
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0d2cdcd..b8aabc9 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -284,13 +284,13 @@ Input Format
 
 'git rev-parse --parseopt' input format is fully text based. It has two parts,
 separated by a line that contains only `--`. The lines before the separator
-(should be more than one) are used for the usage.
+(should be one or more) are used for the usage.
 The lines after the separator describe the options.
 
 Each line of options has this format:
 
 
-opt_specflags* SP+ help LF
+opt_specflags*arg_hint? SP+ help LF
 
 
 `opt_spec`::
@@ -313,6 +313,12 @@ Each line of options has this format:
 
* Use `!` to not make the corresponding negated long option available.
 
+`arg_hint`::
+   `arg_hing`, if specified, is used as a name of the argument in the
+   help output, for options that take arguments. `arg_hint` is
+   terminated by the first whitespace. When output the name is shown in
+   angle braces.  Underscore symbols are replaced with spaces.
+
 The remainder of the line, after stripping the spaces, is used
 as the help associated to the option.
 
@@ -333,6 +339,8 @@ h,helpshow the help
 
 foo   some nifty option --foo
 bar=  some cool option --bar with an argument
+baz=arg   another cool option --baz with a named argument
+qux?path  qux may take a path argument but has meaning by itself
 
   An option group Header
 C?option C with an optional argument
@@ -340,6 +348,28 @@ C?option C with an optional argument
 eval $(echo $OPTS_SPEC | git rev-parse --parseopt -- $@ || echo exit $?)
 
 
+
+Usage text
+~~
+
+When $@ is -h or --help the above example would produce the following
+usage text:
+
+
+usage: some-command [options] args...
+
+some-command does foo and bar!
+
+-h, --helpshow the help
+--foo some nifty option --foo
+--bar ... some cool option --bar with an argument
+--bar arg   another cool option --baz with a named argument
+--qux[=path]qux may take a path argument but has meaning by 
itself
+
+An option group Header
+-C[...]   option C with an optional argument
+
+
 SQ-QUOTE
 
 
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 45901df..a4c9fdf 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -395,9 +395,10 @@ static int cmd_parseopt(int argc, const char **argv, const 
char *prefix)
usage[unb++] = strbuf_detach(sb, NULL);
}
 
-   /* parse: (short|short,long|long)[=?]? SP+ help */
+   /* parse: (short|short,long|long)[*=?!]*arghint? SP+ help */
while (strbuf_getline(sb, stdin, '\n') != EOF) {
const char *s;
+   const char *end;
struct option *o;
 
if (!sb.len)
@@ -419,6 +420,20 @@ static int cmd_parseopt(int argc, const char **argv, const 
char *prefix)
o-value = parsed;
o-flags = PARSE_OPT_NOARG;
o-callback = parseopt_dump;
+
+   /* Possible argument name hint */
+   end = s;
+   while (s  sb.buf  strchr(*=?!, s[-1]) == NULL)
+   --s;
+   if (s != sb.buf  s != end) {
+   char *a;
+   o-argh = a = xmemdupz(s, end - s);
+   while (a = strchr(a, '_'))
+   *a = ' ';
+   }
+   if (s == sb.buf)
+   s = end;
+
while (s  sb.buf  strchr(*=?!, s[-1])) {
switch (*--s) {
case '=':
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 83b1300..bf0db05 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -18,6 +18,17 @@ An option group Header
 -C[...]   option C with an optional argument
 -d, --data[=...]  short and long option with an optional argument
 
+Argument hints
+-b arg

Re: [GSOC 2014]idea:Git Configuration API Improvement

2014-03-20 Thread Michael Haggerty
On 03/20/2014 08:23 AM, Yao Zhao wrote:
 Third one is about when to write back to file, I am really confused
 about it. I think one way could be when user leave git repository
 using cd to go back. But I am not sure if git could detect user
 calls cd to leave repository.

I don't understand.  The cache would be in memory, and would only live
as long as a single git process.  Within that process, if somebody
wants to change the config, they might (for example) call one function
to lock the config file, a second function to change the value(s) in
memory, and then a third function to flush the new config out to disk
and unlock the config file again.  The cache would usually only live for
milliseconds, not minutes/hours, so I don't think your question really
makes sense.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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: [GSOC 2014]idea:Git Configuration API Improvement

2014-03-20 Thread Matthieu Moy
Hi,

Yao Zhao zhaox...@umn.edu writes:

 First is about when to start reading configuration file to cache. My
 idea is the time user starts call command that need configuration
 information (need to read configuration file).

I'd actually load the configuration lazily, when Git first requires a
configuration variable's value. Something like

int config_has_been_loaded = 0;

git_config() {
if (!config_has_been_loaded) {
load_config();
config_has_been_loaded = 1;
} else if (cache_is_outdated()) {
load_config();
} else { /* Nothing to do, we're good */ }
do_something_with_loaded_config();
}

 Second is about data structure. I read Peff's email listed on idea
 page. He indicated two methods and I prefer syntax tree.

Why?

(In general, explaining why you chose something is more important than
explaining what you chose)

 I think there should be three or more syntax tree in the cache. One
 for system, one for global and one for local. If user indicate a file
 to be configuration file, add one more tree. Or maybe we can build one
 tree and tag every node to indicate where it belongs to.

A tree (AST, Abstract syntax tree) can be interesting if you have some
source-to-source transformations to do on the configuration files (i.e.
edit the config files themselves).

For read-only accesses, I would find it more natural to have a
data-structure that reflects the configuration variables themselves, not
the way they appear in the config file. For example, a map (hashtable)
associating to each config variable the corresponding value (which may
be a scalar value or a list, depending on the variable).

But the really important part here is the API exposed to the user, not
the internal data-structure. A map would be more efficient (O(1) or
O(log(n)) access), but traversing the AST for each config request would
not really harm: this is currently what we're doing, except that we
currently re-parse the file each time. OTOH, the API should hide the AST
for most uses. If the user wants the value of configuration variable
foo, the code to do that should not be much more complex than
get_value_for_config_variable(foo). (well, I did oversimplify a bit
here).

 Third one is about when to write back to file, I am really confused
 about it. I think one way could be when user leave git repository
 using cd to go back. But I am not sure if git could detect user
 calls cd to leave repository.

There semes to be a misunderstanding here. The point of the project is
to have a per-process cache, but Git does not normally store a state in
memory between two calls. IOW, when you run

  git status
  cd ../
  git log

The call to git status creates a process, but the process dies before
you run cd. The call to git log is a different process. It can
re-use things that git status left on disk, but not in-memory data
structures.

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


[PATCH] builtin/apply.c: fuzzy_matchlines:trying to fix some inefficiencies

2014-03-20 Thread George Papanikolaou
Hi again guys,
I forgot to add the signed-of line to the tiny patch I sent earlier for GSOC.
Any ideas about the changes?
Thanks...

Signed-off-by: George Papanikolaou g3orge@gmail.com
---
 builtin/apply.c | 25 +
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index b0d0986..df2435f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -294,20 +294,16 @@ static int fuzzy_matchlines(const char *s1, size_t n1,
const char *last2 = s2 + n2 - 1;
int result = 0;
 
+   /* early return if both lines are empty */
+   if ((s1  last1)  (s2  last2))
+   return 1;
+
/* ignore line endings */
while ((*last1 == '\r') || (*last1 == '\n'))
last1--;
while ((*last2 == '\r') || (*last2 == '\n'))
last2--;
 
-   /* skip leading whitespace */
-   while (isspace(*s1)  (s1 = last1))
-   s1++;
-   while (isspace(*s2)  (s2 = last2))
-   s2++;
-   /* early return if both lines are empty */
-   if ((s1  last1)  (s2  last2))
-   return 1;
while (!result) {
result = *s1++ - *s2++;
/*
@@ -315,18 +311,15 @@ static int fuzzy_matchlines(const char *s1, size_t n1,
 * both buffers because we don't want a b to match
 * ab
 */
-   if (isspace(*s1)  isspace(*s2)) {
-   while (isspace(*s1)  s1 = last1)
-   s1++;
-   while (isspace(*s2)  s2 = last2)
-   s2++;
-   }
+   while (isspace(*s1)  s1 = last1)
+   s1++;
+   while (isspace(*s2)  s2 = last2)
+   s2++;
/*
 * If we reached the end on one side only,
 * lines don't match
 */
-   if (
-   ((s2  last2)  (s1 = last1)) ||
+   if (((s2  last2)  (s1 = last1)) ||
((s1  last1)  (s2 = last2)))
return 0;
if ((s1  last1)  (s2  last2))
-- 
1.9.0

--
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] builtin/apply.c: fuzzy_matchlines:trying to fix some inefficiencies

2014-03-20 Thread Michael Haggerty
Hello and welcome.  See the comments inline.

On 03/20/2014 02:32 AM, George Papanikolaou wrote:
 Hi fellows,
 I'm planning on applying on GSOC 2014...
 
 I tried my luck with that kinda weird microproject about inefficiencies,
 and I think I've discovered some.
 
 (also on a totally different mood, there are some warning about empty format
 strings during compilation that could easily be silenced with some #pragma
 calls on -Wformat-zero-length. Is there a way you're not adding this?)

The main reason is probably that #pragmas are compiler-specific.  It is
undesirable to clutter up the source code with ugly #pragmas that only
benefit people using gcc.

I think that most people who use gcc compile with
-Wno-format-zero-length.  FWIW, the options that I use are

O = 2
CFLAGS = -g -O$(O) -Wall -Werror -Wdeclaration-after-statement
-Wno-format-zero-length -Wno-format-security $(EXTRA_CFLAGS)

, which you can put in your config.mak.

 The empty buffers check could happen at the beggining.

s/beggining/beginning/

 Leading whitespace check was unnecessary.
 Some style changes
 
 Thanks.
 ---

Please pay attention to how patches have to be formatted:

The subject of the email and everything above the --- line is used as
the commit's log message.  This should only include information that
belongs in the Git project's permanent history, not incidental
information like the fact that you are applying for GSoC.

The commit message *should* include an explanation of *why* you are
making a change, any tradeoffs that might be involved, etc.

The commit message also *must* include a Signed-off-by line.

Other discussion, not intended for the commit message, should be placed
directly *under* the --- line.

  builtin/apply.c | 25 +
  1 file changed, 9 insertions(+), 16 deletions(-)
 
 diff --git a/builtin/apply.c b/builtin/apply.c
 index b0d0986..df2435f 100644
 --- a/builtin/apply.c
 +++ b/builtin/apply.c
 @@ -294,20 +294,16 @@ static int fuzzy_matchlines(const char *s1, size_t n1,
   const char *last2 = s2 + n2 - 1;
   int result = 0;
  
 + /* early return if both lines are empty */
 + if ((s1  last1)  (s2  last2))
 + return 1;
 +

Why is this an improvement?  Do you expect this function to be called
often for empty lines (as opposed, for example, to lines consisting
solely of whitespace characters)?

   /* ignore line endings */
   while ((*last1 == '\r') || (*last1 == '\n'))
   last1--;
   while ((*last2 == '\r') || (*last2 == '\n'))
   last2--;
  
 - /* skip leading whitespace */
 - while (isspace(*s1)  (s1 = last1))
 - s1++;
 - while (isspace(*s2)  (s2 = last2))
 - s2++;
 - /* early return if both lines are empty */
 - if ((s1  last1)  (s2  last2))
 - return 1;
   while (!result) {
   result = *s1++ - *s2++;
   /*
 @@ -315,18 +311,15 @@ static int fuzzy_matchlines(const char *s1, size_t n1,
* both buffers because we don't want a b to match
* ab
*/
 - if (isspace(*s1)  isspace(*s2)) {
 - while (isspace(*s1)  s1 = last1)
 - s1++;
 - while (isspace(*s2)  s2 = last2)
 - s2++;
 - }
 + while (isspace(*s1)  s1 = last1)
 + s1++;
 + while (isspace(*s2)  s2 = last2)
 + s2++;

The comment just above this change gives a justification for putting an
if statement surrounding the while statements.  Do you think the
comment's argument is incorrect?  If so, please explain why, and remove
or change the comment.

   /*
* If we reached the end on one side only,
* lines don't match
*/
 - if (
 - ((s2  last2)  (s1 = last1)) ||
 + if (((s2  last2)  (s1 = last1)) ||

This reformatting doesn't have anything to do with the rest of your
patch.  If it were important enough to make this change, then it should
be submitted as a separate patch.  But it is probably not important
enough, and is only code churn, so it should probably be omitted entirely.

   ((s1  last1)  (s2 = last2)))
   return 0;
   if ((s1  last1)  (s2  last2))
 


-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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] Rewritten fetch-pack.c:filter_refs() using starts_with() instead of memcmp()

2014-03-20 Thread MustafaOrkunAcar
Hi, I have completed one of the microprojects -14th one: Change 
fetch-pack.c:filter_refs() to use starts_with() instead of memcmp(). The only 
line in the function filter_refs() containing memcmp() is changed with 
starts_with(). I plan to apply for GSoC 2014. Any feedback is appreciated. 
Thanks. 
Signed-off-by: MustafaOrkunAcar mustafaorkuna...@gmail.com
---
 fetch-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index f061f1f..17823ab 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -506,7 +506,7 @@ static void filter_refs(struct fetch_pack_args *args,
int keep = 0;
next = ref-next;
 
-   if (!memcmp(ref-name, refs/, 5) 
+   if (starts_with(ref-name, refs/) 
check_refname_format(ref-name, 0))
; /* trash */
else {
-- 
1.9.1.286.g5172cb3

--
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/RFC 0/8] git-ls

2014-03-20 Thread Nguyễn Thái Ngọc Duy
Last time I tried this was more than two years ago [1]. It's time for
another try and see if the community has any interest in it.

The command is straight forward, it's a ls-like version for listing
things in git. It respects $LS_COLORS and does column output like GNU
ls. ls shows cached entries (but no recursion), ls -o
show untracked files. I want ls-tree, diff --name-only and diff
--name-only --cached too, but they are not implemented yet.

WIP quality, this is to gather comments on the idea.

[1] http://thread.gmane.org/gmane.comp.version-control.git/166405

Nguyễn Thái Ngọc Duy (8):
  Import $LS_COLORS parsing code from coreutils
  ls_colors.c: a bit of document on print_color_indicator input
  ls_colors.c: enable coloring on u+x files
  ls_colors.c: new color descriptors
  ls-files: add --color to highlight based on $LS_COLORS
  ls-files: add --column
  ls-files: support --max-depth
  Add git-ls, a user friendly version of ls-files and more

 Makefile   |   1 +
 builtin.h  |   1 +
 builtin/ls-files.c |  80 -
 git.c  |   1 +
 ls_colors.c (new)  | 487 +
 ls_colors.h (new)  |  20 +++
 6 files changed, 588 insertions(+), 2 deletions(-)
 create mode 100644 ls_colors.c
 create mode 100644 ls_colors.h

-- 
1.9.0.40.gaa8c3ea

--
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 1/8] Import $LS_COLORS parsing code from coreutils

2014-03-20 Thread Nguyễn Thái Ngọc Duy
This could could help highlight files in ls-files or status output, or
even diff --name-only (but that's questionable).

This code is from coreutils.git commit
7326d1f1a67edf21947ae98194f98c38b6e9e527 file src/ls.c. This is the
last GPL-2 commit before coreutils turns to GPL-3.

The code is reformatted to fit Git coding style, which is more than
just adding and removing spaces. For example, bool is replaced with
int, or true/false replaced with 1/0, or the use of git's error()
instead of error(3). There are also two #if 0 to make it build with
git-compat-util.h.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Makefile  |   1 +
 ls_colors.c (new) | 477 ++
 ls_colors.h (new) |  20 +++
 3 files changed, 498 insertions(+)
 create mode 100644 ls_colors.c
 create mode 100644 ls_colors.h

diff --git a/Makefile b/Makefile
index f818eec..f6a6e14 100644
--- a/Makefile
+++ b/Makefile
@@ -819,6 +819,7 @@ LIB_OBJS += list-objects.o
 LIB_OBJS += ll-merge.o
 LIB_OBJS += lockfile.o
 LIB_OBJS += log-tree.o
+LIB_OBJS += ls_colors.o
 LIB_OBJS += mailmap.o
 LIB_OBJS += match-trees.o
 LIB_OBJS += merge.o
diff --git a/ls_colors.c b/ls_colors.c
new file mode 100644
index 000..6385446
--- /dev/null
+++ b/ls_colors.c
@@ -0,0 +1,477 @@
+#include git-compat-util.h
+#include gettext.h
+#include ls_colors.h
+
+#define STREQ(a, b) (strcmp(a, b) == 0)
+
+enum indicator_no {
+   C_LEFT, C_RIGHT, C_END, C_NORM, C_FILE, C_DIR, C_LINK, C_FIFO, C_SOCK,
+   C_BLK, C_CHR, C_MISSING, C_ORPHAN, C_EXEC, C_DOOR, C_SETUID, C_SETGID,
+   C_STICKY, C_OTHER_WRITABLE, C_STICKY_OTHER_WRITABLE
+};
+
+#define FILETYPE_INDICATORS\
+  {\
+C_ORPHAN, C_FIFO, C_CHR, C_DIR, C_BLK, C_FILE, \
+C_LINK, C_SOCK, C_FILE, C_DIR  \
+  }
+
+struct bin_str {
+   size_t len; /* Number of bytes */
+   const char *string; /* Pointer to the same */
+};
+
+struct color_ext_type {
+   struct bin_str ext; /* The extension we're looking for */
+   struct bin_str seq; /* The sequence to output when we do */
+   struct color_ext_type *next;/* Next in list */
+};
+
+static const char *const indicator_name[]= {
+   lc, rc, ec, no, fi, di, ln, pi, so,
+   bd, cd, mi, or, ex, do, su, sg, st,
+   ow, tw, NULL
+};
+
+#define LEN_STR_PAIR(s) sizeof(s) - 1, s
+static struct bin_str color_indicator[] = {
+   { LEN_STR_PAIR(\033[) },  /* lc: Left of color sequence */
+   { LEN_STR_PAIR(m) },  /* rc: Right of color sequence */
+   { 0, NULL },/* ec: End color (replaces lc+no+rc) */
+   { LEN_STR_PAIR(0) },  /* no: Normal */
+   { LEN_STR_PAIR(0) },  /* fi: File: default */
+   { LEN_STR_PAIR(01;34) },  /* di: Directory: bright blue */
+   { LEN_STR_PAIR(01;36) },  /* ln: Symlink: bright cyan */
+   { LEN_STR_PAIR(33) }, /* pi: Pipe: yellow/brown */
+   { LEN_STR_PAIR(01;35) },  /* so: Socket: bright magenta */
+   { LEN_STR_PAIR(01;33) },  /* bd: Block device: bright yellow */
+   { LEN_STR_PAIR(01;33) },  /* cd: Char device: bright yellow */
+   { 0, NULL },/* mi: Missing file: undefined */
+   { 0, NULL },/* or: Orphaned symlink: undefined */
+   { LEN_STR_PAIR(01;32) },  /* ex: Executable: bright green */
+   { LEN_STR_PAIR(01;35) },  /* do: Door: bright magenta */
+   { LEN_STR_PAIR(37;41) },  /* su: setuid: white on red */
+   { LEN_STR_PAIR(30;43) },  /* sg: setgid: black on yellow */
+   { LEN_STR_PAIR(37;44) },  /* st: sticky: black on blue */
+   { LEN_STR_PAIR(34;42) },  /* ow: other-writable: blue on green */
+   { LEN_STR_PAIR(30;42) },  /* tw: ow w/ sticky: black on green */
+};
+
+static struct color_ext_type *color_ext_list = NULL;
+/* Buffer for color sequences */
+static char *color_buf;
+
+/*
+ * True means use colors to mark types.  Also define the different
+ * colors as well as the stuff for the LS_COLORS environment variable.
+ * The LS_COLORS variable is now in a termcap-like format.
+ */
+static int print_with_color;
+
+/*
+ * When true, in a color listing, color each symlink name according to the
+ * type of file it points to.  Otherwise, color them according to the `ln'
+ * directive in LS_COLORS.  Dangling (orphan) symlinks are treated specially,
+ * regardless.  This is set when `ln=target' appears in LS_COLORS.
+ */
+static int color_symlink_as_referent;
+
+/*
+ * Parse a string as part of the LS_COLORS variable; this may involve
+ * decoding all kinds of escape characters.  If equals_end is set an
+ * unescaped equal sign ends the string, otherwise only a : or \0
+ * does.  Set *OUTPUT_COUNT to the number of bytes output.  Return
+ * true if successful.
+ 

[PATCH 2/8] ls_colors.c: a bit of document on print_color_indicator input

2014-03-20 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 ls_colors.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/ls_colors.c b/ls_colors.c
index 6385446..d492ab3 100644
--- a/ls_colors.c
+++ b/ls_colors.c
@@ -401,6 +401,11 @@ static void put_indicator(const struct bin_str *ind)
putchar(*(p++));
 }
 
+/*
+ * mode is only used if stat_ok is true, else filetype is used. if
+ * linkok is -1, shows broken link. If linkok is zero and mode says
+ * it's a link, then show orphan link.
+ */
 void print_color_indicator(const char *name, mode_t mode, int linkok,
   int stat_ok, enum filetype filetype)
 {
-- 
1.9.0.40.gaa8c3ea

--
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 3/8] ls_colors.c: enable coloring on u+x files

2014-03-20 Thread Nguyễn Thái Ngọc Duy
git-compat-util.h does not seem to carry S_IXUGO. Anyway as far as Git
is concerned, we only care one executable bit. Hard code it.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 ls_colors.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/ls_colors.c b/ls_colors.c
index d492ab3..23f1e0b 100644
--- a/ls_colors.c
+++ b/ls_colors.c
@@ -427,10 +427,8 @@ void print_color_indicator(const char *name, mode_t mode, 
int linkok,
type = C_SETUID;
else if ((mode  S_ISGID) != 0)
type = C_SETGID;
-#if 0
-   else if ((mode  S_IXUGO) != 0)
+   else if ((mode  0100) != 0)
type = C_EXEC;
-#endif
} else if (S_ISDIR(mode)) {
if ((mode  S_ISVTX)  (mode  S_IWOTH))
type = C_STICKY_OTHER_WRITABLE;
-- 
1.9.0.40.gaa8c3ea

--
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 4/8] ls_colors.c: new color descriptors

2014-03-20 Thread Nguyễn Thái Ngọc Duy
After coreutils moved to GPL-3 a couple more color descriptors were
added. parse_ls_color() will abort if it finds any of these so just
add them recognized (but never actually use them).

Reference commits (in coreutils.git)

0df338f (ls --color: do not colorize files with multiple hard links by default 
- 2009-06-10)
adc62b5 (ls: clean up after wrapped+colored file names with clear-to-EOL - 
2008-12-31)
1e48b1f (ls: --color now highlights hard linked files, too - 2008-10-27)
84f6abf (ls: --color now highlights files with capabilities, too - 2008-08-01)
483297d (ls --color no longer outputs unnecessary escape sequences - 2008-02-12)

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 ls_colors.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/ls_colors.c b/ls_colors.c
index 23f1e0b..02fc632 100644
--- a/ls_colors.c
+++ b/ls_colors.c
@@ -5,9 +5,12 @@
 #define STREQ(a, b) (strcmp(a, b) == 0)
 
 enum indicator_no {
-   C_LEFT, C_RIGHT, C_END, C_NORM, C_FILE, C_DIR, C_LINK, C_FIFO, C_SOCK,
+   C_LEFT, C_RIGHT, C_END, C_RESET, C_NORM,
+   C_FILE, C_DIR, C_LINK, C_FIFO, C_SOCK,
C_BLK, C_CHR, C_MISSING, C_ORPHAN, C_EXEC, C_DOOR, C_SETUID, C_SETGID,
-   C_STICKY, C_OTHER_WRITABLE, C_STICKY_OTHER_WRITABLE
+   C_STICKY, C_OTHER_WRITABLE, C_STICKY_OTHER_WRITABLE,
+   C_CAP, C_MULTIHARDLINK, C_CLR_TO_EOL
+
 };
 
 #define FILETYPE_INDICATORS\
@@ -28,9 +31,9 @@ struct color_ext_type {
 };
 
 static const char *const indicator_name[]= {
-   lc, rc, ec, no, fi, di, ln, pi, so,
+   lc, rc, ec, rs, no, fi, di, ln, pi, so,
bd, cd, mi, or, ex, do, su, sg, st,
-   ow, tw, NULL
+   ow, tw, ca, mh, cl, NULL
 };
 
 #define LEN_STR_PAIR(s) sizeof(s) - 1, s
@@ -38,6 +41,7 @@ static struct bin_str color_indicator[] = {
{ LEN_STR_PAIR(\033[) },  /* lc: Left of color sequence */
{ LEN_STR_PAIR(m) },  /* rc: Right of color sequence */
{ 0, NULL },/* ec: End color (replaces lc+no+rc) */
+   { 0, NULL },/* rs: Reset to ordinary colors */
{ LEN_STR_PAIR(0) },  /* no: Normal */
{ LEN_STR_PAIR(0) },  /* fi: File: default */
{ LEN_STR_PAIR(01;34) },  /* di: Directory: bright blue */
@@ -55,6 +59,9 @@ static struct bin_str color_indicator[] = {
{ LEN_STR_PAIR(37;44) },  /* st: sticky: black on blue */
{ LEN_STR_PAIR(34;42) },  /* ow: other-writable: blue on green */
{ LEN_STR_PAIR(30;42) },  /* tw: ow w/ sticky: black on green */
+   { 0, NULL },/* ca: black on red */
+   { 0, NULL },/* mh: disabled by default */
+   { 0, NULL },/* cl: clear to end of line */
 };
 
 static struct color_ext_type *color_ext_list = NULL;
-- 
1.9.0.40.gaa8c3ea

--
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 5/8] ls-files: add --color to highlight based on $LS_COLORS

2014-03-20 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/ls-files.c | 38 --
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 47c3880..463280e 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -14,6 +14,8 @@
 #include resolve-undo.h
 #include string-list.h
 #include pathspec.h
+#include color.h
+#include ls_colors.h
 
 static int abbrev;
 static int show_deleted;
@@ -27,6 +29,7 @@ static int show_killed;
 static int show_valid_bit;
 static int line_terminator = '\n';
 static int debug_mode;
+static int use_color;
 
 static const char *prefix;
 static int max_prefix_len;
@@ -57,6 +60,33 @@ static void write_name(const char *name)
   stdout, line_terminator);
 }
 
+static void write_dir_entry(const struct dir_entry *ent)
+{
+   if (want_color(use_color)) {
+   static struct strbuf sb = STRBUF_INIT;
+   struct stat st;
+   int statok;
+   quote_path_relative(ent-name, prefix_len ? prefix : NULL, sb);
+   statok = stat(ent-name, st) == 0;
+   print_color_indicator(sb.buf, st.st_mode, 0, statok, 0);
+   fputs(sb.buf, stdout);
+   printf(%s%c, GIT_COLOR_RESET, line_terminator);
+   } else
+   write_name(ent-name);
+}
+
+static void write_ce_name(const struct cache_entry *ce)
+{
+   if (want_color(use_color)) {
+   static struct strbuf sb = STRBUF_INIT;
+   quote_path_relative(ce-name, prefix_len ? prefix : NULL, sb);
+   print_color_indicator(sb.buf, ce-ce_mode, 1, 1, 0);
+   fputs(sb.buf, stdout);
+   printf(%s%c, GIT_COLOR_RESET, line_terminator);
+   } else
+   write_name(ce-name);
+}
+
 static void show_dir_entry(const char *tag, struct dir_entry *ent)
 {
int len = max_prefix_len;
@@ -68,7 +98,7 @@ static void show_dir_entry(const char *tag, struct dir_entry 
*ent)
return;
 
fputs(tag, stdout);
-   write_name(ent-name);
+   write_dir_entry(ent);
 }
 
 static void show_other_files(struct dir_struct *dir)
@@ -170,7 +200,7 @@ static void show_ce_entry(const char *tag, const struct 
cache_entry *ce)
   find_unique_abbrev(ce-sha1,abbrev),
   ce_stage(ce));
}
-   write_name(ce-name);
+   write_ce_name(ce);
if (debug_mode) {
const struct stat_data *sd = ce-ce_stat_data;
 
@@ -501,6 +531,7 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
N_(if any file is not in the index, treat this as an 
error)),
OPT_STRING(0, with-tree, with_tree, N_(tree-ish),
N_(pretend that paths removed since tree-ish are 
still present)),
+   OPT__COLOR(use_color, N_(show color)),
OPT__ABBREV(abbrev),
OPT_BOOL(0, debug, debug_mode, N_(show debugging data)),
OPT_END()
@@ -548,6 +579,9 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
if (require_work_tree  !is_inside_work_tree())
setup_work_tree();
 
+   if (want_color(use_color))
+   parse_ls_color();
+
parse_pathspec(pathspec, 0,
   PATHSPEC_PREFER_CWD |
   PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
-- 
1.9.0.40.gaa8c3ea

--
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 6/8] ls-files: add --column

2014-03-20 Thread Nguyễn Thái Ngọc Duy
Default pathspec behavior is recursive which includes too many files
for effective column output. But if you can do

git ls-files --column ':(glob)*'

to limit to one level only. It's not exactly the same as GNU ls
(e.g. directories are never shown) but much closer.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/ls-files.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 463280e..a43abdb 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -16,6 +16,7 @@
 #include pathspec.h
 #include color.h
 #include ls_colors.h
+#include column.h
 
 static int abbrev;
 static int show_deleted;
@@ -476,6 +477,7 @@ static int option_parse_exclude_standard(const struct 
option *opt,
 int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 {
int require_work_tree = 0, show_tag = 0, i;
+   unsigned int colopts = 0;
const char *max_prefix;
struct dir_struct dir;
struct exclude_list *el;
@@ -532,6 +534,7 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
OPT_STRING(0, with-tree, with_tree, N_(tree-ish),
N_(pretend that paths removed since tree-ish are 
still present)),
OPT__COLOR(use_color, N_(show color)),
+   OPT_COLUMN(0, column, colopts, N_(show files in columns)),
OPT__ABBREV(abbrev),
OPT_BOOL(0, debug, debug_mode, N_(show debugging data)),
OPT_END()
@@ -576,6 +579,10 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
if (dir.exclude_per_dir)
exc_given = 1;
 
+   finalize_colopts(colopts, -1);
+   if (!line_terminator  explicitly_enable_column(colopts))
+   die(_(--column and -z are incompatible));
+
if (require_work_tree  !is_inside_work_tree())
setup_work_tree();
 
@@ -614,10 +621,19 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
die(ls-files --with-tree is incompatible with -s or 
-u);
overlay_tree_on_cache(with_tree, max_prefix);
}
+
+   if (column_active(colopts)) {
+   struct column_options copts;
+   memset(copts, 0, sizeof(copts));
+   run_column_filter(colopts, copts);
+   }
show_files(dir);
if (show_resolve_undo)
show_ru_info();
 
+   if (column_active(colopts))
+   stop_column_filter();
+
if (ps_matched) {
int bad;
bad = report_path_error(ps_matched, pathspec, prefix);
-- 
1.9.0.40.gaa8c3ea

--
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 7/8] ls-files: support --max-depth

2014-03-20 Thread Nguyễn Thái Ngọc Duy
The use case in mind is --max-depth=0 to stop recursion. With this we can do

git config --global alias.ls 'ls-files --column --color --max-depth=0'

and have git ls with an output very similar to GNU ls. Another
interesting one is

git config --global alias.lso 'ls-files --column --color --max-depth=0 -o 
--exclude-standard'

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/ls-files.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index a43abdb..2c51b0a 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -478,6 +478,7 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
 {
int require_work_tree = 0, show_tag = 0, i;
unsigned int colopts = 0;
+   int max_depth = -1;
const char *max_prefix;
struct dir_struct dir;
struct exclude_list *el;
@@ -535,6 +536,9 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
N_(pretend that paths removed since tree-ish are 
still present)),
OPT__COLOR(use_color, N_(show color)),
OPT_COLUMN(0, column, colopts, N_(show files in columns)),
+   { OPTION_INTEGER, 0, max-depth, max_depth, N_(depth),
+   N_(descend at most depth levels), PARSE_OPT_NONEG,
+   NULL, 1 },
OPT__ABBREV(abbrev),
OPT_BOOL(0, debug, debug_mode, N_(show debugging data)),
OPT_END()
@@ -591,8 +595,11 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
 
parse_pathspec(pathspec, 0,
   PATHSPEC_PREFER_CWD |
+  (max_depth != -1 ? PATHSPEC_MAXDEPTH_VALID : 0) |
   PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
   prefix, argv);
+   pathspec.max_depth = max_depth;
+   pathspec.recursive = 1;
 
/* Find common prefix for all pathspec's */
max_prefix = common_prefix(pathspec);
-- 
1.9.0.40.gaa8c3ea

--
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? git status --porcelain --branch is translated

2014-03-20 Thread Anarky
Hello,

The porcelain format of git status is described as not based on user
configuration.
But with --branch, behind/ahead are translated following the user's locale.
Is it normal that scripts need to take care of that?

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


Re: [PATCH] builtin/apply.c: fuzzy_matchlines:trying to fix some inefficiencies

2014-03-20 Thread George Papanikolaou
Hi,
Thanks for the feedback,

On Thu, Mar 20, 2014 at 11:58 AM, Michael Haggerty mhag...@alum.mit.edu wrote:

 Why is this an improvement?  Do you expect this function to be called
 often for empty lines (as opposed, for example, to lines consisting
 solely of whitespace characters)?


Yes, you are probably right, we are not gonna get much (if any)
completely empty lines


 The comment just above this change gives a justification for putting an
 if statement surrounding the while statements.  Do you think the
 comment's argument is incorrect?  If so, please explain why, and remove
 or change the comment.


I see what I did wrong. I thought since that the if-condition is double checked
(from the while clause) so I removed it.

Also this lead me to see that since the while clause is now unconditioned, there
is no point of it being replicated exactly the same above, so I
removed that too. =(

I'm trying to find other inefficiencies/irregularities on that
function. I'm currently
thinking on merging the first checks with a call to iswspace() or
something similar.

Also thanks for clarifying the way patches/mails work.

Cheers.

---
papanikge's surrogate email.
I may reply back.
http://www.5slingshots.com/
--
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] ls_colors.c: enable coloring on u+x files

2014-03-20 Thread Matthieu Moy
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:

 git-compat-util.h does not seem to carry S_IXUGO. Anyway as far as Git
 is concerned, we only care one executable bit. Hard code it.

Why not use S_IXUSR instead of a hardcoded value? (already used in
path.c, so shouldn't be a problem wrt portability)

-- 
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][GSOC] Selection of the verbose message is replaced with generated message in install_branch_config()

2014-03-20 Thread Aleksey Mokhovikov
On 03/19/2014 04:21 PM, Eric Sunshine wrote:
 Thanks for the resubmission. Comments below...
 
 On Tue, Mar 18, 2014 at 10:33 AM, Aleksey Mokhovikov
 moxobu...@gmail.com wrote:
 Subject: [PATCH][GSOC] Selection of the verbose message is replaced with 
 generated message in install_branch_config()
 
 Say [PATCH v2] to indicate your second attempt.
 
 This subject is quite long. Try to keep it short but informative.
 Mention the module or function first. Use imperative voice. You might
 say:
 
 Subject: install_branch_config: replace if-chain with table lookup

Thanks. This is my first experience with such newsgroups. I don't know 
explicitly how this mail-nntp newsgroups work
under the hood, so I was afraid, that if I'll change the subject, gmane will 
create a new thread instead of placing a
comment.

 Compare with original construction
 Pros:
 1) Remove if chain.
 2) Single table contains all messages with arguments in one construction.
 3) Less code duplication.
 Cons:
 1) Harder to associate the case with message.
 2) Harder for compiler to warn the programmer about not
enough arguments for format string.
 3) Harder to add non-string argument to messages.
 
 Nice summary. Do you draw any conclusions from it? Is the new code
 better? Easier to understand? Would it be better merely to refactor
 the 'if' statements a bit instead of using a table?

I think if that code is heavily developed at this time, then if chain is better,
because if construction is more simple and looks more flexible to major changes.
And if there is no plans to make huge but minor changes,
then table driven approach looks better for me. So I would wrote the if chain at
first, and later, If I had to change verbose message or something similar,
I could rewrite it.


 If !!(x) is out of the coding guide, then message_id construction
 can be rewritten in several other ways.
 The guideline tells that !!(x) is not welcome and should not be
 unless needed. But looks like this is normal place for it.
 
 Curious. !!x is indeed used regularly. Where did you read that it is
 not welcome?

In git/Documentation/CodingGuidelines:
   170   - Some clever tricks, like using the !! operator with arithmetic
   171 constructs, can be extremely confusing to others.  Avoid them,
   172 unless there is a compelling reason to use them.



 Unnecessary blank line insertion. This adds noise to the patch which
 distracts from the real changes.

  void install_branch_config(int flag, const char *local, const char *origin, 
 const char *remote)
  {
 const char *shortname = remote + 11;
 int remote_is_branch = starts_with(remote, refs/heads/);
 struct strbuf key = STRBUF_INIT;
 int rebasing = should_setup_rebase(origin);
 +   int message_id = (!!remote_is_branch  2) | (!!origin  1) | 
 (!!rebasing);
 
 It works, but it's also a fairly magical incantation, placing greater
 cognitive demands on the reader. You could achieve a similar result
 with a multi-dimensional table which is indexed directly by
 !!remote_is_branch, !!origin, and !!rebasing. (Whether you actually
 want to do so is a different question.)

I thought about a multidimensional table and about this approach before 
submitting a patch
and it looks easier for me to read without multidimensional table. But I 
mentioned that indexing can be rewritten
several ways. It will be even easier if the named function or macro is used:

#define BOOL_TO_BITFLAG(x, shift) (!!(x)  (shift))
...
int message_id = BOOL_TO_BITFLAG(remote_is_branch, 2) | BOOL_TO_BITFLAG(origin, 
1) | BOOL_TO_BITFLAG(rebasing, 0);


 +   const char *message_table[][4] =
 +   {{N_(Branch %s set up to track local ref %s.),
 + local, remote},
 +{N_(Branch %s set up to track local ref %s by rebasing.),
 + local, remote},
 +{N_(Branch %s set up to track remote ref %s.),
 + local, remote},
 +{N_(Branch %s set up to track remote ref %s by rebasing.),
 + local, remote},
 +{N_(Branch %s set up to track local branch %s.),
 + local, shortname},
 +{N_(Branch %s set up to track local branch %s by 
 rebasing.),
 + local, shortname},
 +{N_(Branch %s set up to track remote branch %s from %s.),
 + local, shortname, origin},
 +{N_(Branch %s set up to track remote branch %s from %s by 
 rebasing.),
 + local, shortname, origin}};
 
 Indeed, this is a reasonably decent way to keep the arguments and
 messages together and can simplify the code nicely. Unfortunately,
 this project is still restricted primarily to C89, so using
 non-constant C99 initializers is likely to prevent the patch from
 being accepted.

Strange. This is not a static variable. N_(x) is expanded to x - is just a 
marker for xgetext.
array dimensions are known on compile time. Thought 

Re: [PATCH 8/8] Add git-ls, a user friendly version of ls-files and more

2014-03-20 Thread Matthieu Moy
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:

 +int cmd_ls(int argc, const char **argv, const char *cmd_prefix)
 +{
 + struct argv_array av = ARGV_ARRAY_INIT;
 + argv_array_pushl(av, ls-files,
 +  --color, --column, --max-depth=0,
 +  --exclude-standard, --refresh-index,
 +  NULL);

I already have git ls as an alias for git ls-files
--exclude-standard, so this change would bring me even more goodies, I
like it.

I'm a bit hesitant on the --max-depth=0 part by default. I like the way
my git ls alias recurses by default, but that may be just because I'm
used to it. Also, disabling it is can only be done by adding
--max-depth=-1 to the command-line, which isn't very convenient. Perhaps
there should be a git ls -R shorthand for git ls --max-depth=-1.

-- 
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: [BUG] Segfault on git describe

2014-03-20 Thread Sylvestre Ledru
Sylvestre Ledru sylvestre at mozilla.com writes:


 As attachment, the backtrace. I removed about 87250 calls to the
 name_rev function. I guess that is a potential source of problem.
FYI, ulimit -s 10 (increase the stack size) fixes the issue.

Sylvestre


--
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] status: disable translation when --porcelain is used

2014-03-20 Thread Matthieu Moy
git status --branch --porcelain displays the status of the branch
(ahead, behind, gone), and used gettext to translate the string.

Use hardcoded strings when --porcelain is used, but keep the gettext
translation for git status --short which is essentially the same, but
meant to be read by a human.

Reported-by: Anarky ghostana...@gmail.com
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 The porcelain format of git status is described as not based on user
 configuration.
 But with --branch, behind/ahead are translated following the user's locale.
 Is it normal that scripts need to take care of that?

Indeed, I'd call that a bug. Here's a fix.

 wt-status.c | 15 ++-
 wt-status.h |  1 +
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index a452407..e55e5b9 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1509,19 +1509,23 @@ static void wt_shortstatus_print_tracking(struct 
wt_status *s)
return;
}
 
+   const char *gone   = s-no_gettext ? gone   : _(gone);
+   const char *behind = s-no_gettext ? behind  : _(behind );
+   const char *ahead  = s-no_gettext ? ahead   : _(ahead );
+
color_fprintf(s-fp, header_color,  [);
if (upstream_is_gone) {
-   color_fprintf(s-fp, header_color, _(gone));
+   color_fprintf(s-fp, header_color, gone);
} else if (!num_ours) {
-   color_fprintf(s-fp, header_color, _(behind ));
+   color_fprintf(s-fp, header_color, behind);
color_fprintf(s-fp, branch_color_remote, %d, num_theirs);
} else if (!num_theirs) {
-   color_fprintf(s-fp, header_color, _(ahead ));
+   color_fprintf(s-fp, header_color, ahead);
color_fprintf(s-fp, branch_color_local, %d, num_ours);
} else {
-   color_fprintf(s-fp, header_color, _(ahead ));
+   color_fprintf(s-fp, header_color, ahead);
color_fprintf(s-fp, branch_color_local, %d, num_ours);
-   color_fprintf(s-fp, header_color, _(, behind ));
+   color_fprintf(s-fp, header_color, , %s, behind);
color_fprintf(s-fp, branch_color_remote, %d, num_theirs);
}
 
@@ -1566,5 +1570,6 @@ void wt_porcelain_print(struct wt_status *s)
s-use_color = 0;
s-relative_paths = 0;
s-prefix = NULL;
+   s-no_gettext = 1;
wt_shortstatus_print(s);
 }
diff --git a/wt-status.h b/wt-status.h
index 30a4812..82f6ce6 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -50,6 +50,7 @@ struct wt_status {
enum commit_whence whence;
int nowarn;
int use_color;
+   int no_gettext;
int display_comment_prefix;
int relative_paths;
int submodule_summary;
-- 
1.9.1.295.g2661741

--
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] ls_colors.c: enable coloring on u+x files

2014-03-20 Thread Duy Nguyen
On Thu, Mar 20, 2014 at 6:46 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:

 git-compat-util.h does not seem to carry S_IXUGO. Anyway as far as Git
 is concerned, we only care one executable bit. Hard code it.

 Why not use S_IXUSR instead of a hardcoded value? (already used in
 path.c, so shouldn't be a problem wrt portability)

Hmm..maybe cache.h does something to that macro. Will drop this patch
and include cache.h.
-- 
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: Configuring a third-party git hook

2014-03-20 Thread Kevin
On Wed, Mar 19, 2014 at 12:16 PM, Chris Angelico ros...@gmail.com wrote:
 Two parts to the question, then. Firstly, is it acceptable to use 'git
 config' for a hook like this? And secondly, either: Is there a naming
 convention to follow? or, what alternative would you recommend?

1. I would say yes. git config is made to be extended and doesn't
require a config item to be known.
2. Namespacing the config items like you did is a good thing to do so
it won't interfere with other options.
--
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 v3] Documentation/gitk: Document new config file location

2014-03-20 Thread Astril Hayato
User config file location now complies with the XDG base directory specification

Signed-off-by: Astril Hayato astrilhay...@gmail.com
---
 Documentation/gitk.txt | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index 1e9e38a..7e03fcc 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -166,8 +166,14 @@ gitk --max-count=100 --all \-- Makefile::
 
 Files
 -
-Gitk creates the .gitk file in your $HOME directory to store preferences
-such as display options, font, and colors.
+User configuration and preferences are stored at:
+
+* '$XDG_CONFIG_HOME/git/gitk' if it exists, otherwise
+* '$HOME/.gitk' if it exists
+
+If neither of the above exist then '$XDG_CONFIG_HOME/git/gitk' is created and
+used by default. If '$XDG_CONFIG_HOME' is not set it defaults to
+'$HOME/.config' in all cases.
 
 History
 ---
-- 
1.9.0

--
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: GSoC proposal: port pack bitmap support to libgit2.

2014-03-20 Thread Yuxuan Shui
Hi,

Sorry for this late reply, I was busy for past few days.

On Fri, Mar 14, 2014 at 12:34 PM, Jeff King p...@peff.net wrote:
 On Wed, Mar 12, 2014 at 04:19:23PM +0800, Yuxuan Shui wrote:

 I'm Yuxuan Shui, a undergraduate student from China. I'm applying for
 GSoC 2014, and here is my proposal:

 I found this idea on the ideas page, and did some research about it.
 The pack bitmap patchset add a new .bitmap file for every pack file
 which contains the reachability information of selected commits. This
 information is used to speed up git fetching and cloning, and produce
 a very convincing results.

 The goal of my project is to port the pack bitmap implementation in
 core git to libgit2, so users of libgit2 could benefit from this
 optimization as well.

 Please let me know if my proposal makes sense, thanks.

 You'd want to flesh it out a bit more to show how you're thinking about
 tackling the problem:

   - What are the areas of libgit2 that you will need to touch? Be
 specific. What's the current state of the packing code? What
 files and functions will you need to touch?

Firstly I will need to implement bitmap creation in libgit2's
git_packbuilder_* functions (probably also git_odb_write_pack), so
libgit2 could support bitmap creation. Then I will need to change
git_revwalk_* functions to make them use bitmap. Since the operations
that can benefit from bitmap is, if my understanding is correct, all
using the git_revwalk_* functions, having bitmap support in revwalk
functions should be enough.

Files I need to touch probably are: revwalk.c pack-objects.c
If I need to change the API of packbuilder or revwalk functions I will
have to change the callers as well: push.c fetch.c and
transport/smart_protocol.c

I haven't read all the code to put together a list of functions I need
to change, but I think the list will be long.


   - What are the challenges you expect to encounter in porting the code?

The architecture differences between git and libgit2 will probably be
a challenge.


   - Can you give a detailed schedule of the summer's work? What will you
 work on in each week? What milestones do you expect to hit, and
 when?

I don't really have a plan, but I'll try to provide a rough schedule.

I'll read the code and try to understand the code, to the point where
I can start to add new code. This will probably take a week. For next
three or four weeks I should be implementing bitmap creation in
packbuilder. Then for the rest of time I will be optimizing revwalk
using bitmap.


 -Peff

--

Regards
Yuxuan Shui
--
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] Enable index-pack threading in msysgit.

2014-03-20 Thread Karsten Blees
Am 19.03.2014 01:46, schrieb sza...@chromium.org:
 This adds a Windows implementation of pread.  Note that it is NOT
 safe to intersperse calls to read() and pread() on a file
 descriptor.

This is a bad idea. You're basically fixing the multi-threaded issue twice, 
while at the same time breaking single-threaded read/pread interop on the mingw 
and msvc platform. Users of pread already have to take care that its not 
thread-safe on some platforms, now you're adding another breakage that has to 
be considered in future development.

The mingw_pread implementation in [1] is both thread-safe and allows mixing 
read/pread in single-threaded scenarios, why not use this instead?

[1] http://article.gmane.org/gmane.comp.version-control.git/242120

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

Duy's patch alone enables multi-threaded index-pack on all platforms (including 
cygwin), so IMO this should be a separate patch.

 + if (hand == INVALID_HANDLE_VALUE) {
 + errno = EBADF;
 + return -1;
 + }

This check is redundant, ReadFile already ckecks for invalid handles and 
err_win_to_posix converts to EBADF.

 +
 + LARGE_INTEGER offset_value;
 + offset_value.QuadPart = offset;
 +
 + DWORD bytes_read = 0;
 + OVERLAPPED overlapped = {0};
 + overlapped.Offset = offset_value.LowPart;
 + overlapped.OffsetHigh = offset_value.HighPart;
 + BOOL result = ReadFile(hand, buf, count, bytes_read, overlapped);
 +
 + ssize_t ret = bytes_read;
 +
 + if (!result  GetLastError() != ERROR_HANDLE_EOF)

According to MSDN docs, ReadFile never fails with ERROR_HANDLE_EOF, or is this 
another case where the documentation is wrong?

When a synchronous read operation reaches the end of a file, ReadFile returns 
TRUE and sets *lpNumberOfBytesRead to zero.

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


Loan Offer Company

2014-03-20 Thread finance06
We offer all purpose loan at 3% interest rate. Contact Us for more details by 
Email:santanderfinancegr...@gmail.com
--
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


[GSoC 2014] Replacing object loading/writing layer by libgit2

2014-03-20 Thread Guanglin Xu
Hello,

My name is Guanglin Xu. I am a second-year master student at Sun
Yat-sen University in China, majoring in Software Engineering. I am
also a perspective PhD student matriculated in the US. I'm planning on
doing summer projects which I can work remotely. The GSoC 2014 program
of Git project is a great choice.

I am kind of a skillful Git user with 4 years' experience in 3
projects. For example, I am a Top 5 contributor in LibVMI project
(https://github.com/bdpayne/libvmi); I host a team-made mobile app in
Github (https://itunes.apple.com/us/app/ying-yue/id689566688?ls=1mt=8).
For more of my projects see here
(http://www.andrew.cmu.edu/user/guanglin)

To get familiar with the flow of Git development, I worked on
microproject [2] and had corresponding conversations with Eric
Sunshine, Jacopo Notarstefano and Sun He in threads. Thank you for
their comments on my work.

Now I've submitted my proposal to google-melange. In brief, I propose
to replace object loading/writing layer by libgit2, which comes from
the GSoC 2014 ideas page of Git project. Particularly, since I didn't
realize where the hardest part is when I looked into the initial aim
of this idea, I added a performance requirement that the new
implementation should at least not run slower than previous one. Maybe
I underestimated specific challenge in working with Git, suggestions
or warnings for this topic are all welcomed.

Thanks for your consideration for GSoC 2014.

Guanglin
--
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] Enable index-pack threading in msysgit.

2014-03-20 Thread Stefan Zager
On Thu, Mar 20, 2014 at 6:54 AM, Karsten Blees karsten.bl...@gmail.com wrote:
 Am 19.03.2014 01:46, schrieb sza...@chromium.org:
 This adds a Windows implementation of pread.  Note that it is NOT
 safe to intersperse calls to read() and pread() on a file
 descriptor.

 This is a bad idea. You're basically fixing the multi-threaded issue twice, 
 while at the same time breaking single-threaded read/pread interop on the 
 mingw and msvc platform. Users of pread already have to take care that its 
 not thread-safe on some platforms, now you're adding another breakage that 
 has to be considered in future development.

 The mingw_pread implementation in [1] is both thread-safe and allows mixing 
 read/pread in single-threaded scenarios, why not use this instead?

 [1] http://article.gmane.org/gmane.comp.version-control.git/242120


That's not thread-safe.  There is, presumably, a point between the
first and second calls to lseek64 when the implicit position pointer
is incorrect.  If another thread executes its first call to lseek64 at
that time, then the file descriptor may end up with an incorrect
position pointer after all threads have finished.

 Duy's patch alone enables multi-threaded index-pack on all platforms 
 (including cygwin), so IMO this should be a separate patch.

Fair enough, and it's a good first step.  I would love to see it
landed soon.  On the Chrome project, we're currently distributing a
patched version of msysgit; I would very much like for us to stop
doing that, but the performance penalty is too significant right now.

Duy, would you like to re-post your patch without the new pread implementation?

Going forward, there is still a lot of performance that gets left on
the table when you rule out threaded file access.  There are not so
many calls to read, mmap, and pread in the code; it should be possible
to rationalize them and make them thread-safe -- at least, thread-safe
for posix-compliant systems and msysgit, which covers the great
majority of git users, I would hope.
--
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


[RFC][GSOC] Proposal Draft for GSoC, Suggest Changes

2014-03-20 Thread karthik nayak
Hello,
I have completed my microproject under the guidance of Eric, After
going through the code and
previous mailing lists. I have drafted my Proposal. Still going
through the code
as of now and figuring things out. Would be great to have your
suggestions on my proposal, so that i can improve it before submitting
it. Also have written the proposal in markdown for easier formatting.
Doesn't look pretty on plain text.
Thanks
Karthik


Git configuration API improvements

Abstract

Currently git_config() has a few issues which need to be addressed:

Reads and parses the configuration files each time.
Values cannot be unset,only can be set to false which has different
implications.
Repeated setting and un-setting of value in a particular new header,
leaves trails.

This project is to fix these problems while also retaining backward
compatibility
wherever git_config() is called, by implementing a cache for the config's in a
tree data structure, which provides for easier modification.

About Me

Name : Karthik Nayak
Email : karthik@gmail.com
College : BMS Institute of Technology
Studying : Engineering In Computer Science
IRC : nayak94
Phone : 91--XXX-XXX
Country : India
Interests : Guitar, Photography, Craft.
Github : KarthikNayak

Technical Experience

Have been Learning about the Linux Kernel and its implementation on the android
platform. Released also on XDA-Dev for the phones LG P500 and Xperia SP.
Working on a Library in C on various Sorting Techniques.
Contributed to the Open-Source Lab Manual for Colleges under VTU.
Active Member of Gnu/Linux Users Group in College and Free Software
Movement of Karnataka.

Why i Picked Git

This is my first attempt at GSOC and as I began going through the list
of organisations, what struck me is that
I haven't really used any of the software's of most of the listed
organisations. That's when I realized why
not contribute to something I use on a daily basis, this way I wont be
contributing only because I want to take
part in GSOC, rather I'd contribute because I would love to be a part
of something I use on a regular basis
and would be able to contribute to the project even after GSOC.

Proposal

Ideas Page : Git configuration API improvements

The Following improvements have to be made to how configs are handled in git :

Read all the config files once and store them in an appropriate data structure.

I suggest the use of an tree data structure to store the cache of the
config files.
I think tree data structure is a better choice over a hash - key data
structure as a tree data
structure although has a lower time efficiency than a hash - key data
structure while traversing
for a config request. A tree data structure can more optimal for
further improvements like
the problem with setting and unsetting of configs can be easily
handled as when a node under
a particular header is deleted the header can check if it has no
children nodes and on being true
can delete the header from the config file.

Change git_config() to iterate through the pre-read values in memory
rather than re-reading
the configuration files. This function should remain
backwards-compatible with the old implementation
so that callers don't have to all be rewritten at once.

Now whenever git_config() is called within a single invocation of git
it can traverse the
tree data structure already created and get the particular config.
This needs to maintain backward
compatibility. So the Basic functioning of functions like git_config()
and so on would change the
API should remain the same for the user invoking these calls.

Add new API functions that allow the cache to be inquired easily and
efficiently.
Rewrite callers to use the new API wherever possible.

Now that the base data structure and underlying changes have been made
for the data structure
to work have been made, we can now add various new API functions to
assist the usage of the data
structure. And also rewrite callers to use the new API's made available

Issues to be addressed

Headers and comments left are all configs under a header is deleted.

whenever we set and unset configs under a particular header it leaves
garbage value
behind, for example :

git config pull.rebase true
git config --unset pull.rebase
git config pull.rebase true
git config --unset pull.rebase

would result in :

[pull]
[pull]

And further changes made appear under the last header.
The issue also gives rise to comments being stranded within a header.

Possible Solution :

Make sure that the header is deleted whenever the last config under it
is deleted.
Also delete comments within a header and comments made above a particular config
when a config is removed and comments made above a header when the whole header
is being removed.

How to invalidate the cache correctly in the case that the
configuration is changed
while git is executing.

If config is being changed while git is currently running then the
changes need to be considered.

Possible Solution :

A simple 

Re: [PATCH] test-lib.sh: use printf instead of echo

2014-03-20 Thread Junio C Hamano
Jonathan Nieder jrnie...@gmail.com writes:

 Junio C Hamano wrote:
 ...
 I am a bit reluctant to name the helper sane_echo to declare echo
 that interprets backslashes in the string is insane, though.  For
 these print a single line uses, we are only interested in using a
 subset of the features offered by 'echo', but that does not mean the
 other features we do not want to trigger in our use is of no use to
 any sane person.

 In a portable script, uncareful use of 'echo' is always insane.

I agree that makes sense and I actually think that it is a bit
stronger than that.  If a script is meant to be portable, there is
no way to use echo on a string whose contents is unknown sanely.
There is no careful use is OK.

 In a script tailored to an environment where echo behaves consistently
 it is perfectly reasonable to use 'echo', but that's a different
 story.  In the context of git, saying Here is the thing you should
 always use instead of echo is a good thing, in my opinion.

That is true in my opinion, but that thing is also what you should
always use instead of printf '%s\n'.  A guideline more useful for
the users is Here is the thing you should always use when literally
emitting a single line., isn't 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: Configuring a third-party git hook

2014-03-20 Thread Chris Angelico
On Thu, Mar 20, 2014 at 11:53 PM, Kevin i...@ikke.info wrote:
 On Wed, Mar 19, 2014 at 12:16 PM, Chris Angelico ros...@gmail.com wrote:
 Two parts to the question, then. Firstly, is it acceptable to use 'git
 config' for a hook like this? And secondly, either: Is there a naming
 convention to follow? or, what alternative would you recommend?

 1. I would say yes. git config is made to be extended and doesn't
 require a config item to be known.
 2. Namespacing the config items like you did is a good thing to do so
 it won't interfere with other options.

Excellent! Thank you.

Is this documented anywhere? The git config man page says to look to
other git man pages:

https://www.kernel.org/pub/software/scm/git/docs/git-config.html#_variables

A comment there to the effect that Third party tools may also define
their own variables or something would make it clear that this is the
intention.

ChrisA
--
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 a third-party git hook

2014-03-20 Thread Junio C Hamano
Chris Angelico ros...@gmail.com writes:

 file. It doesn't really care about the full history, and wants to be
 reasonably fast (as the user is waiting for it). It's just a
 convenience, so correctness isn't a huge issue. The easiest way to
 keep it moving through quickly is to limit the search:

 $ git log ...other options... HEAD~100 some-file.pike

 The problem with this is that it doesn't work if HEAD doesn't have 100
 great-great-...-grandparents

Did you really mean that you are *not* interested in what happened
to the most recent 100 commits?  Or is it a typo of HEAD~100..?

git log -100 should traverse from the HEAD and stop after showing
at most 100 items, even if you only had 20 in the history.
--
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 03/26] t1400: Pass a legitimate newvalue to update command

2014-03-20 Thread Michael Haggerty
On 03/11/2014 10:41 PM, Brad King wrote:
 On Tue, Mar 11, 2014 at 4:06 PM, Junio C Hamano gits...@pobox.com wrote:
 I may be misremembering things, but your first sentence quoted above
 was exactly my reaction while reviewing the original change, and I
 might have even raised that as an issue myself, saying something
 like consistency across values is more important than type-saving
 in a machine format.
 
 For reference, the original design discussion of the format was here:
 
  http://thread.gmane.org/gmane.comp.version-control.git/233842
 
 I do not recall this issue being raised before, but now that it has
 been raised I fully agree:
 
  http://thread.gmane.org/gmane.comp.version-control.git/243754/focus=243862
 
 In -z mode an empty newvalue should be treated as missing just as
 it is for oldvalue.  This is obvious now in hindsight and I wish I
 had realized this at the time.  Back then I went through a lot of
 iterations on the format and missed this simplification in the final
 version :(

It's not your fault; anybody could have reviewed your code at the time
(I most of all, because I have been so active in this area of the code).

 Moving forward:
 
 The create command rejects a zero newvalue so the change in
 question for that command is merely the wording of the error message
 and there is no compatibility issue.
 
 The update command supports a zero newvalue so that it can
 be used for all operations (create, update, delete, verify) with
 the proper combination of old and new values.  The change in question
 makes an empty newvalue an error where it was previously treated
 as zero.  (BTW, Michael, I do not see a test case for the new error
 in your series.  Something like the patch below should work.)
 
 I am not against deprecating and removing
 the support for it in the longer term, though.
 
 As I reported in my above-linked response, I'm not depending on
 the old behavior myself.  Also if one were to start seeing this
 error then generated input needs only trivial changes to avoid it.
 If we do want to preserve compatibility for others then perhaps an
 empty newvalue with -z should produce:
 
  warning: update $ref: missing newvalue, treating as zero

This last suggestion is what I am implementing for the re-roll (coming
shortly).  Thanks for the discussion.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB

2014-03-20 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Thu, Mar 20, 2014 at 5:11 AM, Junio C Hamano gits...@pobox.com wrote:
 ...
 I know that the 512MiB default for the bitFileThreashold (aka
 forget about delta compression) came out of thin air.  It was just
 1GB is always too huge for anybody, so let's cut it in half and
 declare that value the initial version of a sane threashold,
 nothing more.

 So it might be that the problem is 512MiB is still too big, relative
 to the 16MiB of delta base cache, and the former may be what needs
 to be tweaked.  If a blob close to but below 512MiB is a problem for
 16MiB delta base cache, it would still be too big to cause the same
 problem for 128MiB delta base cache---it would evict all the other
 objects and then end up not being able to fit in the limit itself,
 busting the limit immediately, no?

 I would understand if the change were to update the definition of
 deltaBaseCacheLimit and link it to the value of bigFileThreashold,
 for example.  With the presented discussion, I am still not sure if
 we can say that bumping deltaBaseCacheLimit is the right solution to
 the description with the current setting is clearly wrong (which
 is a real issue).

 I vote make big_file_threshold smaller. 512MB is already unfriendly
 for many smaller machines. I'm thinking somewhere around 32MB-64MB
 (and maybe increase delta cache base limit to match).

These numbers match my gut feeling (e.g. 4k*4k*32-bit uncompressed
would be 64MB); delta cash base that is sized to the same as (or
perhaps twice as big as) that limit may be a good default.

 The only
 downside I see is large blobs will be packed  undeltified, which could
 increase pack size if you have lots of them.

I think that is something that can be tweaked, unless the user tells
us otherwise via command line override, when running the improved
gc --aggressive ;-)
--
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 a third-party git hook

2014-03-20 Thread Chris Angelico
On Fri, Mar 21, 2014 at 3:53 AM, Junio C Hamano gits...@pobox.com wrote:
 Chris Angelico ros...@gmail.com writes:

 file. It doesn't really care about the full history, and wants to be
 reasonably fast (as the user is waiting for it). It's just a
 convenience, so correctness isn't a huge issue. The easiest way to
 keep it moving through quickly is to limit the search:

 $ git log ...other options... HEAD~100 some-file.pike

 The problem with this is that it doesn't work if HEAD doesn't have 100
 great-great-...-grandparents

 Did you really mean that you are *not* interested in what happened
 to the most recent 100 commits?  Or is it a typo of HEAD~100..?

Oops, yes, HEAD~100.. is what I actually use in the source code. Same
difference; it doesn't work if there aren't that many commits.

 git log -100 should traverse from the HEAD and stop after showing
 at most 100 items, even if you only had 20 in the history.

Yes, and I use that to limit the results (to 10, actually); but
there's one degenerate case left, and that's a new or moved/renamed
file in a long-standing repository. Let's say the repo has 760 commits
(which is currently the case for Gypsum; I'd say this is fairly small
as repos go), and a file was moved a little while ago and then not
edited much.

$ git log plugins-more/threshtime.pike

Four results, the oldest being Move three plugins into -more which
moved the file without any edits at all. If I edit that file now, the
prepare-commit-msg hook will execute the following (or would, if I
hadn't set the config option):

$ git log --shortstat --full-diff -10 --oneline plugins-more/threshtime.pike
fca89fe Threshtime: Drop a comment from the old C++ plugin
 1 file changed, 1 insertion(+), 1 deletion(-)
df8bcf0 Threshtime: Make use of statusevent
 1 file changed, 2 insertions(+), 11 deletions(-)
1207213 Threshtime: Use the tooltip to hint at the converter
 1 file changed, 1 insertion(+)
c22dfbc Move three plugins into -more so they're loaded by default but
unloadable
 6 files changed, 426 insertions(+), 426 deletions(-)

Since it says -10 and hasn't found ten results yet, git log will
keep on searching back in history. I don't know of a way to say give
up searching once you find the commit that creates this file,
although that would also do what I want. The end result is the same,
but it's very slow if the git log isn't in the OS/disk cache. On my
main development box, it is cached, but I just tried it on my Windows
box and it took about fifteen seconds to finish; and 760 commits is
not huge as repositories go - the Pike repo has over 30,000 commits,
and git's own repo is of similar size.

Bounding the search is potentially a huge improvement here, since the
user's waiting. But the exact limit depends on the repo itself, and
it'd be nice to be able to disable it (huh, didn't find any
results... I'll de-limit the search and try again). Hence the config
option, which I'm very happy to hear *is* a viable technique.

ChrisA
--
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: optimise parse_dirstat_params() to only compare strings when necessary

2014-03-20 Thread Junio C Hamano
Dragos Foianu dragos.foi...@gmail.com writes:

 parse_dirstat_params() goes through a chain of if statements using
 strcmp to parse parameters. When the parameter is a digit, the
 value must go through all comparisons before the function realises
 it is a digit. Optimise this logic by only going through the chain
 of string compares when the parameter is not a digit.

This change could be an optimization only if parse_dirstat_params()
is called with a param that begins with a digit a lot more often
than with other forms of params, but that is a mere assumption.
Unless that assumption is substantiated, this change can be a
pessimization.

Even if the assumption were true (which I doubt), a simpler solution
to optimize such a call pattern would be to simply tweak of the
order if/else cascade to check if the param begins with a digit
first before checking other keywords, wouldn't it?  I am not sure
why you even need to change the structure into a nested if
statement.

 Signed-off-by: Dragos Foianu dragos.foi...@gmail.com
 ---
  diff.c | 37 +++--
  1 file changed, 19 insertions(+), 18 deletions(-)

 diff --git a/diff.c b/diff.c
 index e343191..733764e 100644
 --- a/diff.c
 +++ b/diff.c
 @@ -84,20 +84,25 @@ static int parse_dirstat_params(struct diff_options 
 *options, const char *params
   string_list_split_in_place(params, params_copy, ',', -1);
   for (i = 0; i  params.nr; i++) {
   const char *p = params.items[i].string;
 - if (!strcmp(p, changes)) {
 - DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
 - DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
 - } else if (!strcmp(p, lines)) {
 - DIFF_OPT_SET(options, DIRSTAT_BY_LINE);
 - DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
 - } else if (!strcmp(p, files)) {
 - DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
 - DIFF_OPT_SET(options, DIRSTAT_BY_FILE);
 - } else if (!strcmp(p, noncumulative)) {
 - DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE);
 - } else if (!strcmp(p, cumulative)) {
 - DIFF_OPT_SET(options, DIRSTAT_CUMULATIVE);
 - } else if (isdigit(*p)) {
 + if (!isdigit(*p)) {
 + if (!strcmp(p, changes)) {
 + DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
 + DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
 + } else if (!strcmp(p, lines)) {
 + DIFF_OPT_SET(options, DIRSTAT_BY_LINE);
 + DIFF_OPT_CLR(options, DIRSTAT_BY_FILE);
 + } else if (!strcmp(p, files)) {
 + DIFF_OPT_CLR(options, DIRSTAT_BY_LINE);
 + DIFF_OPT_SET(options, DIRSTAT_BY_FILE);
 + } else if (!strcmp(p, noncumulative)) {
 + DIFF_OPT_CLR(options, DIRSTAT_CUMULATIVE);
 + } else if (!strcmp(p, cumulative)) {
 + DIFF_OPT_SET(options, DIRSTAT_CUMULATIVE);
 + } else {
 + strbuf_addf(errmsg, _(  Unknown dirstat 
 parameter '%s'\n), p);
 + ret++;
 + }
 + } else  {
   char *end;
   int permille = strtoul(p, end, 10) * 10;
   if (*end == '.'  isdigit(*++end)) {
 @@ -114,11 +119,7 @@ static int parse_dirstat_params(struct diff_options 
 *options, const char *params
   p);
   ret++;
   }
 - } else {
 - strbuf_addf(errmsg, _(  Unknown dirstat parameter 
 '%s'\n), p);
 - ret++;
   }
 -
   }
   string_list_clear(params, 0);
   free(params_copy);
--
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


[GSoC] Inquiry about writing config file to disk

2014-03-20 Thread Tanay Abhra
Hi,

I have gone through commit.c, builtin/commit.c and api-config.txt but one
thing I cannot find is which functions handle writing config file to disk
after adding a new variable,value pair(for example git config my.option
true) . It is also marked TODO on the api-config.txt file. Can somebody help
me , I just want the starting function and will trace the function calls
after that.

Regards,
Tanay Abhra.


--
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: [GSOC 2014]idea:Git Configuration API Improvement

2014-03-20 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Why?

 (In general, explaining why you chose something is more important than
 explaining what you chose)

Good educational comment.  Thanks.

 A tree (AST, Abstract syntax tree) can be interesting if you have some
 source-to-source transformations to do on the configuration files (i.e.
 edit the config files themselves).

 For read-only accesses, I would find it more natural to have a
 data-structure that reflects the configuration variables themselves, not
 the way they appear in the config file.

... and one important thing that was left unsaid is that the
read-only accesses happen far more often than updates, so the data
structure must be optimized for the read-only look-up case.
--
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] ls_colors.c: enable coloring on u+x files

2014-03-20 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Thu, Mar 20, 2014 at 6:46 PM, Matthieu Moy
 matthieu@grenoble-inp.fr wrote:
 Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:

 git-compat-util.h does not seem to carry S_IXUGO. Anyway as far as Git
 is concerned, we only care one executable bit. Hard code it.

 Why not use S_IXUSR instead of a hardcoded value? (already used in
 path.c, so shouldn't be a problem wrt portability)

 Hmm..maybe cache.h does something to that macro. Will drop this patch
 and include cache.h.

Why even include cache.h for S_IXUSR?

In the context of the patch I see S_ISGID mentioned and other S_*
st_mode things are already in use in this function before this step,
and presumably you are using them without problems, no?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] status: disable translation when --porcelain is used

2014-03-20 Thread Junio C Hamano
Matthieu Moy matthieu@imag.fr writes:

 git status --branch --porcelain displays the status of the branch
 (ahead, behind, gone), and used gettext to translate the string.

 Use hardcoded strings when --porcelain is used, but keep the gettext
 translation for git status --short which is essentially the same, but
 meant to be read by a human.

 Reported-by: Anarky ghostana...@gmail.com
 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---
 The porcelain format of git status is described as not based on user
 configuration.
 But with --branch, behind/ahead are translated following the user's locale.
 Is it normal that scripts need to take care of that?

 Indeed, I'd call that a bug. Here's a fix.

Good thing to fix.  Thanks.

  wt-status.c | 15 ++-
  wt-status.h |  1 +
  2 files changed, 11 insertions(+), 5 deletions(-)

 diff --git a/wt-status.c b/wt-status.c
 index a452407..e55e5b9 100644
 --- a/wt-status.c
 +++ b/wt-status.c
 @@ -1509,19 +1509,23 @@ static void wt_shortstatus_print_tracking(struct 
 wt_status *s)
   return;
   }
  
 + const char *gone   = s-no_gettext ? gone   : _(gone);
 + const char *behind = s-no_gettext ? behind  : _(behind );
 + const char *ahead  = s-no_gettext ? ahead   : _(ahead );

Having to repeat the same string constant twice (and a half for the
variable name) each is an eyesore.  I wonder if we can do better,
perhaps with:

#define LABEL(string) (s-no_gettext ? (string) : _(string))

and then

color_fprintf(s-fp, header_color, LABEL(N_(gone)));

or something along those lines?

   color_fprintf(s-fp, header_color,  [);
   if (upstream_is_gone) {
 - color_fprintf(s-fp, header_color, _(gone));
 + color_fprintf(s-fp, header_color, gone);
   } else if (!num_ours) {
 - color_fprintf(s-fp, header_color, _(behind ));
 + color_fprintf(s-fp, header_color, behind);
   color_fprintf(s-fp, branch_color_remote, %d, num_theirs);
   } else if (!num_theirs) {
 - color_fprintf(s-fp, header_color, _(ahead ));
 + color_fprintf(s-fp, header_color, ahead);
   color_fprintf(s-fp, branch_color_local, %d, num_ours);
   } else {
 - color_fprintf(s-fp, header_color, _(ahead ));
 + color_fprintf(s-fp, header_color, ahead);
   color_fprintf(s-fp, branch_color_local, %d, num_ours);
 - color_fprintf(s-fp, header_color, _(, behind ));
 + color_fprintf(s-fp, header_color, , %s, behind);
   color_fprintf(s-fp, branch_color_remote, %d, num_theirs);
   }
  
 @@ -1566,5 +1570,6 @@ void wt_porcelain_print(struct wt_status *s)
   s-use_color = 0;
   s-relative_paths = 0;
   s-prefix = NULL;
 + s-no_gettext = 1;
   wt_shortstatus_print(s);
  }
 diff --git a/wt-status.h b/wt-status.h
 index 30a4812..82f6ce6 100644
 --- a/wt-status.h
 +++ b/wt-status.h
 @@ -50,6 +50,7 @@ struct wt_status {
   enum commit_whence whence;
   int nowarn;
   int use_color;
 + int no_gettext;
   int display_comment_prefix;
   int relative_paths;
   int submodule_summary;
--
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 v3] Documentation/gitk: Document new config file location

2014-03-20 Thread Junio C Hamano
Astril Hayato astrilhay...@gmail.com writes:

 User config file location now complies with the XDG base directory 
 specification

 Signed-off-by: Astril Hayato astrilhay...@gmail.com
 ---
  Documentation/gitk.txt | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

 diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
 index 1e9e38a..7e03fcc 100644
 --- a/Documentation/gitk.txt
 +++ b/Documentation/gitk.txt
 @@ -166,8 +166,14 @@ gitk --max-count=100 --all \-- Makefile::
  
  Files
  -
 -Gitk creates the .gitk file in your $HOME directory to store preferences
 -such as display options, font, and colors.
 +User configuration and preferences are stored at:
 +
 +* '$XDG_CONFIG_HOME/git/gitk' if it exists, otherwise
 +* '$HOME/.gitk' if it exists
 +
 +If neither of the above exist then '$XDG_CONFIG_HOME/git/gitk' is created and
 +used by default. If '$XDG_CONFIG_HOME' is not set it defaults to
 +'$HOME/.config' in all cases.

Thanks; looks good.
--
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 v3][GSOC] fsck: use bitwise-or assignment operator to set flag

2014-03-20 Thread Junio C Hamano
Hiroyuki Sano sh19910...@gmail.com writes:

 fsck_tree() has two different ways to set a flag,
 which are the followings:

   1. Using a if-statement that guards assignment.

   2. Using a bitwise-or assignment operator.

 Currently, many with the former way,
 and one with the latter way.

 In this patch, unify them to the latter way,
 because it makes the code shorter and easier to read,
 and it is brief and to the point.

Two issues:

 * In this patch, is redundant.

 * it is brief and to the point are equally applicable to both
   styles, so that is not a *reason* to choose one over the other.

If a condition were *not* brief and to the point, then a rewrite to
the latter style will make the resulting code worse:

if (a very complex condition
that potentially have to consume a
lot of brain-cycles to understand) {
has_that_condition = 1;
}

is a lot easier to extend than

has_that_condition = (a very complex condition
  that potentially have to consume a
  lot of brain-cycles to understand);

because it is a lot more likely that we would need to later extend
such a complex condition is more likely than a simple singleton
condition, and we could end up with

if (a very complex condition
that potentially have to consume a
lot of brain-cycles to understand) {
futher computation to check if
the condition really holds
will be added here later
if (does that condition really hold true?)
has_that_condition = 1;
}


which may be harder to express in the latter form.

In other words, it is brief and to the point merely _allows_ these
statements to be expressed in the latter form; it does not say
anything about which is better between the former and the latter.

 Signed-off-by: Hiroyuki Sano sh19910...@gmail.com
 ---
  fsck.c | 18 ++
  1 file changed, 6 insertions(+), 12 deletions(-)

 diff --git a/fsck.c b/fsck.c
 index b3022ad..abed62b 100644
 --- a/fsck.c
 +++ b/fsck.c
 @@ -165,18 +165,12 @@ static int fsck_tree(struct tree *item, int strict, 
 fsck_error error_func)
  
   sha1 = tree_entry_extract(desc, name, mode);
  
 - if (is_null_sha1(sha1))
 - has_null_sha1 = 1;
 - if (strchr(name, '/'))
 - has_full_path = 1;
 - if (!*name)
 - has_empty_name = 1;
 - if (!strcmp(name, .))
 - has_dot = 1;
 - if (!strcmp(name, ..))
 - has_dotdot = 1;
 - if (!strcmp(name, .git))
 - has_dotgit = 1;
 + has_null_sha1 |= is_null_sha1(sha1);
 + has_full_path |= !!strchr(name, '/');
 + has_empty_name |= !*name;
 + has_dot |= !strcmp(name, .);
 + has_dotdot |= !strcmp(name, ..);
 + has_dotgit |= !strcmp(name, .git);
   has_zero_pad |= *(char *)desc.buffer == '0';
   update_tree_entry(desc);
--
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


[RFC] [GSoC] Draft of Proposal for GSoC

2014-03-20 Thread Brian Bourn
Hi all,

This is a first draft of my Proposal for GSoC, I'd love feedback about
what I might be missing and any other files I should read regarding
this, so far I have read most of tag.c, branch.c,
builtin/for-each-ref.c, parse-options.c. once again I hope I can get
the same amount of helpful feedback as when I submitted my
Microproject.

My name is Brian Bourn, I'm currently a computer engineering student
at Columbia university in the city of New York.  I've used git since
my freshman year however this past week has been my first time
attempting to contribute to the project, and I loved it. I'd
particularly like to tackle Unifying git branch -l, git tag -l, and
git for-each-ref.  This functionality seems like an important update
to me as it will simplify usage of git throughout three different
commands, a noble pursuit which is not contained in any other project.

Going through the annals of the listserve thus far I've found a few
discussions which provide some insight towards this process as well as
some experimental patches that never seem to have made it
through[1][2][3][4]

I would start by beginning a deprecation plan for git branch -l very
similar to the one Junio presents in [5], moving -create-reflog to -g,

Following this I would begin the real work of the project which would
involve moving the following flag operations into a standard library
say 'list-options.h'

--contains [6]
--merged [7]
--no-merged[8]
--format
This Library would build these options for later interpretation by parse_options

Next I would implement these flags in the three files so that they are
uniform and the same formatting and list capabilities can be used on
all three. The formatting option will be especially useful for branch
and tag as it will allow users to better understand what is in each
ref that they grab.

For the most part I haven't finalized my weekly schedule but a basic
breakdown would be

Start-Midterm
Begin deprecation of -l
Spend some time reading *.c files even deeper
Build Library(dedicate Minimum one week per function moved)

Midterm-finish
Implement the list flags
Implement the format flags
(if time is left over, add some formatting)

Additionally I am thinking about adding some more formatting tools
such as numbering outputs. What do you all think of this?


[1]http://git.661346.n2.nabble.com/More-formatting-with-git-tag-l-tt6739049.html

[2]http://git.661346.n2.nabble.com/RFC-branch-list-branches-by-single-remote-tt6645679.html#a6725483

[3]http://git.661346.n2.nabble.com/RFC-PATCH-tag-make-list-exclude-lt-pattern-gt-tt7270451.html#a7338712

 
[4]http://git.661346.n2.nabble.com/RFC-branch-list-branches-by-single-remote-tt6645679.html#a6728878

[5]http://git.661346.n2.nabble.com/RFC-PATCH-0-2-RFC-POC-patterns-for-branch-list-tt6309233.html

[6]https://github.com/git/git/blob/master/builtin/branch.c#L817

[7] https://github.com/git/git/blob/master/builtin/branch.c#L849

[8] https://github.com/git/git/blob/master/builtin/branch.c#L843

Regards,
Brian Bourn
--
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 v3] rev-parse --parseopt: option argument name hints

2014-03-20 Thread Junio C Hamano
Ilya Bobyr ilya.bo...@gmail.com writes:

 Built-in commands can specify names for option arguments when usage text
 is generated for a command.  sh based commands should be able to do the
 same.

 Option argument name hint is any text that comes after [*=?!] after the
 argument name up to the first whitespace.  Underscores are replaced with
 whitespace.  It is unlikely that an underscore would be useful in the
 hint text.

 Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
 ---
  Changed according to the last comments.  Added Usage text paragraph in the
  documentation and updated variable names.

  Documentation/git-rev-parse.txt |   34 --
  builtin/rev-parse.c |   17 -
  t/t1502-rev-parse-parseopt.sh   |   20 
  3 files changed, 68 insertions(+), 3 deletions(-)

 diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
 index 0d2cdcd..b8aabc9 100644
 --- a/Documentation/git-rev-parse.txt
 +++ b/Documentation/git-rev-parse.txt
 @@ -284,13 +284,13 @@ Input Format
  
  'git rev-parse --parseopt' input format is fully text based. It has two 
 parts,
  separated by a line that contains only `--`. The lines before the separator
 -(should be more than one) are used for the usage.
 +(should be one or more) are used for the usage.
  The lines after the separator describe the options.
  
  Each line of options has this format:
  
  
 -opt_specflags* SP+ help LF
 +opt_specflags*arg_hint? SP+ help LF
  
  
  `opt_spec`::
 @@ -313,6 +313,12 @@ Each line of options has this format:
  
   * Use `!` to not make the corresponding negated long option available.
  
 +`arg_hint`::
 + `arg_hing`, if specified, is used as a name of the argument in the
 + help output, for options that take arguments. `arg_hint` is
 + terminated by the first whitespace. When output the name is shown in
 + angle braces.  Underscore symbols are replaced with spaces.

The last part is troubling (and sounds not very sane).  Do we do
such a munging anywhere else, or is it just here?  If the latter I'd
prefer not to see such a hack.

 @@ -333,6 +339,8 @@ h,helpshow the help
  
  foo   some nifty option --foo
  bar=  some cool option --bar with an argument
 +baz=arg   another cool option --baz with a named argument
 +qux?path  qux may take a path argument but has meaning by itself
  
An option group Header
  C?option C with an optional argument
 @@ -340,6 +348,28 @@ C?option C with an optional argument
  eval $(echo $OPTS_SPEC | git rev-parse --parseopt -- $@ || echo exit 
 $?)
  
  
 +
 +Usage text
 +~~
 +
 +When $@ is -h or --help the above example would produce the following
 +usage text:

Sounds like a good idea to add this; all the above arguments inside
double quotes should be typeset `as-typed`, though.

 @@ -419,6 +420,20 @@ static int cmd_parseopt(int argc, const char **argv, 
 const char *prefix)
   o-value = parsed;
   o-flags = PARSE_OPT_NOARG;
   o-callback = parseopt_dump;
 +
 + /* Possible argument name hint */
 + end = s;
 + while (s  sb.buf  strchr(*=?!, s[-1]) == NULL)
 + --s;
 + if (s != sb.buf  s != end) {
 + char *a;
 + o-argh = a = xmemdupz(s, end - s);
 + while (a = strchr(a, '_'))
 + *a = ' ';

... and without the underscore munging, we do not have to allocate
a new piece of memory, either.
--
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 v3][GSOC] fsck: use bitwise-or assignment operator to set flag

2014-03-20 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 In other words, it is brief and to the point merely _allows_ these
 statements to be expressed in the latter form; it does not say
 anything about which is better between the former and the latter.

In any case, that is a minor point.  I'll queue the patch on 'pu',
with a tweaked log message.

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


[PATCH v4] tests: use env to run commands with temporary env-var settings

2014-03-20 Thread David Tran
Ordinarily, we would say VAR=VAL command to execute a tested command with
environment variable(s) set only for that command. This however does not work
if 'command' is a shell function (most notably 'test_must_fail'); the result
of the assignment is retained and affects later commands.

To avoid this, we assigned and exported the environment variables and run such
test(s) in a subshell like this,

(
VAR=VAL 
export VAR 
test_must_fail git command to be tested
)

Using the env utility, we should be able to say

test_must_fail git command to be tested

which is much shorter and easier to read.

Signed-off-by: David Tran unsignedz...@gmail.com

---

Revision update: v1[1] and v2[2] fixed broken -chains and removed subshells
that are no longer needed. v3[3] fixed typos and mistakes in the commit
message itself.

Let's see if I replied correctly with send-email. Retrying this again.
How do I 'reply' to a thread using send-email?
Look for --in-reply-to option in man git-send-email.
Git prompts me for this but I don't know what to type. I tried typing just
the id, like 244379 and that didn't work. I guess I'll try
244379 at gmane.comp.version-control.git? Google didn't bring many results on
how to use it but many results of what it is and its goal.
I see 'Followup' on the upper right. Should do the right job.

Much better. I didn't spot any errors in the patch this time around.
Good.

One final note for future submissions: As a courtesy to reviewers,
explain (below the --- line) what changed in the current version,
and provide a reference to the previous attempt, like this [1].
Added above.

Looks familiar ;-) but it seems the changes from the original you
took it from all look worsening, not improvements, to me.
I learn more from rewriting than copying and pasting but I'll change most of it
back then.

Seems to be well done.  Thanks.
You're welcome.

I am David Tran a graduating CS/Math senior from Sonoma State University,
United States. I would like to work with git for GSoC'14, specifically the line
options for git rebase --interactive [4][5]. I have used git for a few years and
know how destructive but important rebase is to git. I have created a few shell
scripts here and there to make life using bash/zsh easier. I would like to
apply these skills and work with the best.

Github: unsignedzero

[1]: http://thread.gmane.org/gmane.comp.version-control.git/244256
[2]: http://thread.gmane.org/gmane.comp.version-control.git/244379
[3]: http://thread.gmane.org/gmane.comp.version-control.git/244406

[4]: http://thread.gmane.org/gmane.comp.version-control.git/243933/focus=243967
[5]: http://thread.gmane.org/gmane.comp.version-control.git/242701

Signed-off-by: David Tran unsignedz...@gmail.com
---
 t/t1300-repo-config.sh|   17 ++
 t/t1510-repo-setup.sh |4 +--
 t/t3200-branch.sh |   12 +--
 t/t3301-notes.sh  |   22 +++-
 t/t3404-rebase-interactive.sh |   69 -
 t/t3413-rebase-hook.sh|6 +---
 t/t4014-format-patch.sh   |   14 ++--
 t/t5305-include-tag.sh|4 +--
 t/t5602-clone-remote-exec.sh  |   13 ++--
 t/t5801-remote-helpers.sh |6 +--
 t/t6006-rev-list-format.sh|9 ++---
 t/t7006-pager.sh  |   18 ++-
 12 files changed, 42 insertions(+), 152 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index c9c426c..3e3f77b 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -974,24 +974,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '
 '

 test_expect_success 'nonexistent configuration' '
-   (
-   GIT_CONFIG=doesnotexist 
-   export GIT_CONFIG 
-   test_must_fail git config --list 
-   test_must_fail git config test.xyzzy
-   )
+   test_must_fail env GIT_CONFIG=doesnotexist git config --list 
+   test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy
 '

 test_expect_success SYMLINKS 'symlink to nonexistent configuration' '
ln -s doesnotexist linktonada 
ln -s linktonada linktolinktonada 
-   (
-   GIT_CONFIG=linktonada 
-   export GIT_CONFIG 
-   test_must_fail git config --list 
-   GIT_CONFIG=linktolinktonada 
-   test_must_fail git config --list
-   )
+   test_must_fail env GIT_CONFIG=linktonada git config --list 
+   test_must_fail env GIT_CONFIG=linktolinktonada git config --list
 '

 test_expect_success 'check split_cmdline return' 
diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index cf2ee78..e1b2a99 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -777,9 +777,7 @@ test_expect_success '#30: core.worktree and core.bare 
conflict (gitfile version)
setup_repo 30 $here/30 gitfile true 
(
cd 30 
-   

Re: [PATCH v3 2/2] log: add --show-linear-break to help see non-linear history

2014-03-20 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 Option explanation is in rev-list-options.txt. The interaction with -z
 is left undecided.

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

Thanks.

  * Revert back to the old option name --show-linear-break
  * Get rid of saved_linear, use another flag in struct object instead

I cannot offhand say if I like this change or not.  A flag bit is a
scarce and limited resource; commit slabs felt more suited for
implementation of corner case eye-candies.

  * Fix not showing the break bar after a root commit if the dag graph
has multiple roots

I definitely do not like the way a commit-list data structure is
abused to hold a phoney element that points at a NULL with its item
pointer.  Allocate a single bit in revs that says I haven't done
anything yet if you want to catch the first-ness without breaking
what commit_list_insert() and friends are expecting to see---they
never expect to see a NULL asked to be on the list, AFAIK.

  * Make it work with --graph (although I don't really see the point of
using both at the same time)

I do not see the point, either.  I vaguely recall that the previous
iteration refused the combination at the option parser level, which
I think would be the right thing to do.

  * Let the next contributor deal with -z

That is fine.
--
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] Import $LS_COLORS parsing code from coreutils

2014-03-20 Thread David Tran
Nguyễn Thái Ngọc Duy pclouds at gmail.com writes:

 This could could help highlight files in ls-files or status output, or
 even diff --name-only (but that's questionable).
 
 This code is from coreutils.git commit
 7326d1f1a67edf21947ae98194f98c38b6e9e527 file src/ls.c. This is the
 last GPL-2 commit before coreutils turns to GPL-3.
 

I don't know if this is something to consider but for my mac, I have another 
variable CLICOLOR which shows the colors if it is set. This is also true with 
FreeBSD[1] as well. I don't know if that should be checked if you're on those 
systems.

I think it would be nice to have --color flag as well if you want to enable 
color output for just that one output. 

[1]: https://unix.stackexchange.com/questions/2897/clicolor-and-ls-colors-in-
bash

--
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] builtin/apply.c: use iswspace() to detect line-ending-like chars

2014-03-20 Thread George Papanikolaou
Removing the bloat of checking for both '\r' and '\n' with the prettier
iswspace() function which checks for other characters as well. (read: \f \t \v)
---

This is one more try to clean up this fuzzy_matchlines() function as part of a
microproject for GSOC. The rest more clarrified microprojects were taken.
I'm obviously planning on applying.

Thanks

Signed-of-by: George 'papanikge' Papanikolaou g3orge@gmail.com

 builtin/apply.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index b0d0986..912a53a 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -295,9 +295,9 @@ static int fuzzy_matchlines(const char *s1, size_t n1,
int result = 0;
 
/* ignore line endings */
-   while ((*last1 == '\r') || (*last1 == '\n'))
+   while (iswspace(*last1))
last1--;
-   while ((*last2 == '\r') || (*last2 == '\n'))
+   while (iswspace(*last2))
last2--;
 
/* skip leading whitespace */
-- 
1.9.0

--
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: Lost commit during rebase!

2014-03-20 Thread Robert Dailey
Sorry it looks like I missed some basic documentation:
http://git-scm.com/docs/git-rebase

This issue (combining preserve merges with interactive option) is
described in the BUGS section.

Is there a way I can go about this without losing my merge commits?

On Thu, Mar 20, 2014 at 2:37 PM, Robert Dailey rcdailey.li...@gmail.com wrote:
 I have a local-only branch with just under 70 commits. I also have
 merge commits on this branch.

 The log of the top few commits on my branch looks like so:

 * de651ff   (20 minutes ago) (HEAD, survey) Robert Dailey - WIP:
 GOTO implementation
 * 2a68a23   (21 minutes ago) Robert Dailey - Move boost::phoenix
 include  namespace changes to own header file
 * e1cd568   (19 hours ago) Robert Dailey - WIP: GOTO flow changes
 * b039bb5   (19 hours ago) Robert Dailey - Remove superfluous
 include of own header
 * 4bdeb27   (20 hours ago) Robert Dailey - Rename NavigateBackwards()

 I had two commits that I wanted to squash and also reorder another
 one. The command I ran is:

 git rebase -pi `git merge-base origin/master survey`

 I ran this command with 'survey' branch checked out. Once my editor
 appears, the last few commits in the file look like so:

 pick 4bdeb27 Rename NavigateBackwards()
 pick b039bb5 Remove superfluous include of own header
 pick e1cd568 WIP: GOTO flow changes
 pick 2a68a23 Move boost::phoenix include  namespace changes to own header 
 file
 pick de651ff WIP: GOTO implementation

 I modify the last 3 commits in the file so now it looks like this:

 pick 4bdeb27 Rename NavigateBackwards()
 pick b039bb5 Remove superfluous include of own header
 pick 2a68a23 Move boost::phoenix include  namespace changes to own header 
 file
 pick e1cd568 WIP: GOTO flow changes
 squash de651ff WIP: GOTO implementation

 What I did was:

 1. Move 2a68a23 back one commit
 2. Mark de651ff for squash

 After this, I save the file, close, and rebase operations begin. After
 the rebase is done, my log looks like this:

 * 94d06df   (19 hours ago) (HEAD, survey) Robert Dailey - WIP:
 GOTO flow changes
 * b039bb5   (19 hours ago) Robert Dailey - Remove superfluous
 include of own header
 * 4bdeb27   (20 hours ago) Robert Dailey - Rename NavigateBackwards()

 Notice that the commit with description Move boost::phoenix include 
 namespace changes to own header file is missing! I looked at reflog:

 $ git reflog
 94d06df HEAD@{0}: rebase -i (finish): returning to refs/heads/survey
 94d06df HEAD@{1}: rebase -i (squash): WIP: GOTO flow changes
 e1cd568 HEAD@{2}: rebase -i (pick): updating HEAD
 2a7b27a HEAD@{3}: rebase -i (pick): Move boost::phoenix include 
 namespace changes to own header file
 b039bb5 HEAD@{4}: rebase -i (pick): updating HEAD
 4bdeb27 HEAD@{5}: rebase -i (pick): updating HEAD
 2e40bcd HEAD@{6}: rebase -i (pick): updating HEAD
 3ca6bb3 HEAD@{7}: rebase -i (pick): updating HEAD
 e63b1e5 HEAD@{8}: rebase -i (pick): updating HEAD
 4d40c00 HEAD@{9}: rebase -i (pick): updating HEAD
 ec078c1 HEAD@{10}: rebase -i (pick): updating HEAD
 de48c5d HEAD@{11}: rebase -i (pick): updating HEAD

 It shows that it picked the commit that's missing now. Is this a bug
 or am I not doing something right? I'm using Git for Windows version
 1.9.0
--
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


[GSoc PATCH 1/3] diff-no-index.c: rename read_directory()

2014-03-20 Thread Andrei Dinu

Avoid the conflict between read_directory() from diff-no-index.c and 
read_directory() from dir.h

Signed-off-by: Andrei Dinu mandrei.d...@gmail.com

---

 I plan on applying to GSOC 2014

 Submit again on the list for an older bug that I solved, to show you that 
 I received your feedback and I reviewed my code, numbering and partitioning 
 patches style. Thank you!


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

diff --git a/diff-no-index.c b/diff-no-index.c
index 8e10bff..5e4a76c 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -16,7 +16,7 @@
 #include builtin.h
 #include string-list.h
 
-static int read_directory(const char *path, struct string_list *list)
+static int read_directory_path(const char *path, struct string_list *list)
 {
DIR *dir;
struct dirent *e;
@@ -107,9 +107,9 @@ static int queue_diff(struct diff_options *o,
int i1, i2, ret = 0;
size_t len1 = 0, len2 = 0;
 
-   if (name1  read_directory(name1, p1))
+   if (name1  read_directory_path(name1, p1))
return -1;
-   if (name2  read_directory(name2, p2)) {
+   if (name2  read_directory_path(name2, p2)) {
string_list_clear(p1, 0);
return -1;
}
-- 
1.7.9.5


From d54129eacb45b307dacf2b7afebd1da40df17047 Mon Sep 17 00:00:00 2001
From: Andrei Dinu mandrei.d...@gmail.com
Date: Wed, 19 Mar 2014 17:42:08 +0200
Subject: [GSoc PATCH 2/3] diff-no-index.c: read_directory_path() use
 is_dot_or_dotdot().

Implement code so read_directory_path() use is_dot_or_dotdot() from dir.h
instead of strcmp().

Signed-off-by: Andrei Dinu mandrei.d...@gmail.com

---

 I plan on applying to GSOC 2014
 diff-no-index.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 5e4a76c..2d1165f 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -15,6 +15,7 @@
 #include log-tree.h
 #include builtin.h
 #include string-list.h
+#include dir.h
 
 static int read_directory_path(const char *path, struct string_list *list)
 {
@@ -25,7 +26,7 @@ static int read_directory_path(const char *path, struct 
string_list *list)
return error(Could not open directory %s, path);
 
while ((e = readdir(dir)))
-   if (strcmp(., e-d_name)  strcmp(.., e-d_name))
+   if (!is_dot_or_dotdot(e-d_name))
string_list_insert(list, e-d_name);
 
closedir(dir);
-- 
1.7.9.5


From 4843ad23675047163211c68434517c097435ebb7 Mon Sep 17 00:00:00 2001
From: Andrei Dinu mandrei.d...@gmail.com
Date: Wed, 19 Mar 2014 18:01:55 +0200
Subject: [GSoc PATCH 3/3] fsck.c: fsck_tree() now use is_dot_or_dotdot().

Rewrite fsck_tree() to use is_dot_or_dotdot() from dir.h instead 
of calling twice strcmp().

Signed-off-by: Andrei Dinu mandrei.d...@gmail.com

---

 I plan on applying to GSOC 2014.

 fsck.c |   11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/fsck.c b/fsck.c
index 64bf279..82a55ab 100644
--- a/fsck.c
+++ b/fsck.c
@@ -6,6 +6,7 @@
 #include commit.h
 #include tag.h
 #include fsck.h
+#include dir.h
 
 static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data)
 {
@@ -171,10 +172,12 @@ static int fsck_tree(struct tree *item, int strict, 
fsck_error error_func)
has_full_path = 1;
if (!*name)
has_empty_name = 1;
-   if (!strcmp(name, .))
-   has_dot = 1;
-   if (!strcmp(name, ..))
-   has_dotdot = 1;
+   if (is_dot_or_dotdot(name))
+   if (name[1] == '\0')
+   has_dot = 1;
+   else
+   has_dotdot = 1;
if (!strcmp(name, .git))
has_dotgit = 1;
has_zero_pad |= *(char *)desc.buffer == '0';
-- 
1.7.9.5


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


Re: [PATCH v3] rev-parse --parseopt: option argument name hints

2014-03-20 Thread Eric Sunshine
On Thu, Mar 20, 2014 at 4:44 AM, Ilya Bobyr ilya.bo...@gmail.com wrote:
 Built-in commands can specify names for option arguments when usage text
 is generated for a command.  sh based commands should be able to do the
 same.

 Option argument name hint is any text that comes after [*=?!] after the
 argument name up to the first whitespace.  Underscores are replaced with
 whitespace.  It is unlikely that an underscore would be useful in the
 hint text.

 Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
 ---
  Changed according to the last comments.  Added Usage text paragraph in the
  documentation and updated variable names.

As this is a high-traffic list, it can be difficult for reviewers to
remember all the comments regarding the previous version. It can help
a lot if you include a reference to the previous attempt, like this
[1].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/243216/focus=243945

One more comment below...

 diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
 index 0d2cdcd..b8aabc9 100644
 --- a/Documentation/git-rev-parse.txt
 +++ b/Documentation/git-rev-parse.txt
 @@ -313,6 +313,12 @@ Each line of options has this format:

 * Use `!` to not make the corresponding negated long option available.

 +`arg_hint`::
 +   `arg_hing`, if specified, is used as a name of the argument in the

arg_hing?

 +   help output, for options that take arguments. `arg_hint` is
 +   terminated by the first whitespace. When output the name is shown in
 +   angle braces.  Underscore symbols are replaced with spaces.
 +
  The remainder of the line, after stripping the spaces, is used
  as the help associated to the option.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Make XDF_NEED_MINIMAL default in blame.

2014-03-20 Thread Michael Andreen
Currently git blame has a big problem finding copies and moves when you
split up a big file into smaller ones. One example in the git repository
is 2cf565c, which split the documentation into smaller files.

In 582aa00 XDF_NEED_MINIMAL was removed as the default for performance
reasons, mainly for diff and rebase, but blame was also changed.

In 059a500 the problem with blame was noticed and the flag --minimal was
introduced. However this flag is not documented and it is not possible
to set when using git gui blame.

Setting XDF_NEED_MINIMAL as default has a small performance impact when
you run on a file with few modifications. However, if you run it on a
file with a bigger number of modifications, the performance impact is
small enough to not be noticable.

The previous behavior can still be activated with --no-minimal.

((2cf565c...))$ time PAGER=cat git blame -C -M
Documentation/git-ls-files.txt  /dev/null

real0m0.003s
user0m0.002s
sys 0m0.000s

((2cf565c...))$ time PAGER=cat git blame --minimal -C -M
Documentation/git-ls-files.txt  /dev/null

real0m0.010s
user0m0.009s
sys 0m0.000s

((2cf565c...))$ time PAGER=cat git blame -C -C -C -M
Documentation/git-ls-files.txt  /dev/null

real0m0.010s
user0m0.010s
sys 0m0.000s

((2cf565c...))$ time PAGER=cat git blame --minimal -C -C -C -M
Documentation/git-ls-files.txt  /dev/null

real0m0.028s
user0m0.027s
sys 0m0.000s

(master)$ time PAGER=cat git blame -C -C -C -M
Documentation/git-ls-files.txt  /dev/null

real0m2.338s
user0m2.283s
sys 0m0.056s

(master)$ time PAGER=cat git blame --minimal -C -C -C -M
Documentation/git-ls-files.txt  /dev/null

real0m2.355s
user0m2.285s
sys 0m0.069s

(master)$ time PAGER=cat git blame -C -M cache.h  /dev/null

real0m1.755s
user0m1.730s
sys 0m0.024s

(master)$ time PAGER=cat git blame --minimal -C -M cache.h  /dev/null

real0m1.785s
user0m1.770s
sys 0m0.014s

(master)$ time PAGER=cat git blame -C -C -C -M cache.h  /dev/null

real0m31.515s
user0m30.810s
sys 0m0.684s

(master)$ time PAGER=cat git blame --minimal -C -C -C -M cache.h 
/dev/null

real0m31.504s
user0m30.885s
sys 0m0.598s

Signed-off-by: Michael Andreen h...@ruin.nu
---
There hasn't been any arguments against this patch. Just updated the message 
with a note about --no-minimal.

Applies cleanly on both master and maint.

 builtin/blame.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index e5b5d71..0e7ebd0 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -42,7 +42,7 @@ static int show_root;
 static int reverse;
 static int blank_boundary;
 static int incremental;
-static int xdl_opts;
+static int xdl_opts = XDF_NEED_MINIMAL;
 static int abbrev = -1;
 static int no_whole_file_rename;
 
-- 
1.8.3.2


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


Re: [PATCH] Make XDF_NEED_MINIMAL default in blame.

2014-03-20 Thread Junio C Hamano
Michael Andreen h...@ruin.nu writes:

 There hasn't been any arguments against this patch. Just updated the message 
 with a note about --no-minimal.

There hasn't been any argument for this patch, either.

It is not like we are still in year 2007; timing result in a small
project like Git itself is not a good enough argument to change a
well established default at this late in the game, especially when
there are ways like command line options for users to specify their
preferred settings.
--
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] configure.ac: link with -liconv for locale_charset()

2014-03-20 Thread Junio C Hamano
Дилян Палаузов  dilyan.palau...@aegee.org writes:

 diff --git a/Makefile b/Makefile
 index dddaf4f..dce4694 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -59,9 +59,9 @@ all::
  # FreeBSD can use either, but MinGW and some others need to use
  # libcharset.h's locale_charset() instead.
  #
 -# Define CHARSET_LIB to you need to link with library other than -liconv to
 +# Define CHARSET_LIB to the library you need to link with in order to
  # use locale_charset() function.  On some platforms this needs to set to
 -# -lcharset
 +# -lcharset, on others to -liconv .
  #
  # Define LIBC_CONTAINS_LIBINTL if your gettext implementation doesn't
  # need -lintl when linking.

This was unfortunately lost in the noise and I missed it.  I think
the updated text is much more clear than the original.

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


Re: [PATCH] Make XDF_NEED_MINIMAL default in blame.

2014-03-20 Thread Michael Andreen
On Thursday, March 20, 2014 01:45:21 PM Junio C Hamano wrote:
 There hasn't been any argument for this patch, either.
 
 It is not like we are still in year 2007; timing result in a small
 project like Git itself is not a good enough argument to change a
 well established default at this late in the game, especially when
 there are ways like command line options for users to specify their
 preferred settings.

The main reason why I submitted it is because the current behavior is very 
unintuitive when you split big files into smaller ones. Like the one that was 
done in 2cf565c.

If you do:

 $ git checkout 2cf565c
 $ git blame -C -C -C -M Documentation/git-ls-files.txt

then blame will not be able to detect any moves, even though big chunks have 
been moved without whitespace changes. Worse is that if you do

 $ git gui blame Documentation/git-ls-files.txt

then that won't see any movies either, and there is no way to make git gui 
blame use --minimal.

I spent a lot of time at $work trying to code-review a big split like this, 
wondering why git gui blame and git blame -C -C -C -M couldn't detect 
moves when complete functions had been moved with no changes (not even 
whitespace). So by making it default it will hopefully help someone else from 
losing time investigating this.

It wasn't until I did a bisect on git, finding 582aa00 and later googling for 
XDF_NEED_MINIMAL to find 059a500, that I found out about --minimal. Which is 
not documented anywhere.

I guess it can be argued for just documenting --minimal and either patching 
git gui blame to use it, or make it configurable. However I think the 
current default is really confusing, and the reason for changing it back in 
2007 was for performance, so that was what I tried to focus on, hopefully 
showing that there aren't any noticeable differences.

/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: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB

2014-03-20 Thread David Kastrup
Junio C Hamano gits...@pobox.com writes:

 Duy Nguyen pclo...@gmail.com writes:

 The only
 downside I see is large blobs will be packed  undeltified, which could
 increase pack size if you have lots of them.

 I think that is something that can be tweaked, unless the user tells
 us otherwise via command line override, when running the improved
 gc --aggressive ;-)

deltaBaseCacheLimit is used for unpacking, not for packing.

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


Re: [PATCH] Enable index-pack threading in msysgit.

2014-03-20 Thread Karsten Blees
Am 20.03.2014 17:08, schrieb Stefan Zager:
 On Thu, Mar 20, 2014 at 6:54 AM, Karsten Blees karsten.bl...@gmail.com 
 wrote:
 Am 19.03.2014 01:46, schrieb sza...@chromium.org:
 This adds a Windows implementation of pread.  Note that it is NOT
 safe to intersperse calls to read() and pread() on a file
 descriptor.

 This is a bad idea. You're basically fixing the multi-threaded issue twice, 
 while at the same time breaking single-threaded read/pread interop on the 
 mingw and msvc platform. Users of pread already have to take care that its 
 not thread-safe on some platforms, now you're adding another breakage that 
 has to be considered in future development.

 The mingw_pread implementation in [1] is both thread-safe and allows mixing 
 read/pread in single-threaded scenarios, why not use this instead?

 [1] http://article.gmane.org/gmane.comp.version-control.git/242120
 
 
 That's not thread-safe.  There is, presumably, a point between the
 first and second calls to lseek64 when the implicit position pointer
 is incorrect.  If another thread executes its first call to lseek64 at
 that time, then the file descriptor may end up with an incorrect
 position pointer after all threads have finished.
 

Correct, a multi-threaded code section using pread has the effect of 
randomizing the file position (btw., this is also true for your pread 
implementation). This can be easily fixed by resetting the file position after 
pthread_join, if necessary. Currently there's just six callers of pthread_join.

 Going forward, there is still a lot of performance that gets left on
 the table when you rule out threaded file access.  There are not so
 many calls to read, mmap, and pread in the code; it should be possible
 to rationalize them and make them thread-safe -- at least, thread-safe
 for posix-compliant systems and msysgit, which covers the great
 majority of git users, I would hope.
 

IMO a mostly XSI compliant pread (or even the git_pread() emulation) is still 
better than forbidding the use of read() entirely. Switching from read to pread 
everywhere requires that all callers have to keep track of the file position, 
which means a _lot_ of code changes (read/xread/strbuf_read is used in ~70 
places throughout git). And how do you plan to deal with platforms that don't 
have a thread-safe pread (HP, Cygwin)?

Considering all that, Duy's solution of opening separate file descriptors per 
thread seems to be the best pattern for future multi-threaded work.

Karsten

--
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] Enable index-pack threading in msysgit.

2014-03-20 Thread Stefan Zager
On Thu, Mar 20, 2014 at 2:35 PM, Karsten Blees karsten.bl...@gmail.com wrote:
 Am 20.03.2014 17:08, schrieb Stefan Zager:

 Going forward, there is still a lot of performance that gets left on
 the table when you rule out threaded file access.  There are not so
 many calls to read, mmap, and pread in the code; it should be possible
 to rationalize them and make them thread-safe -- at least, thread-safe
 for posix-compliant systems and msysgit, which covers the great
 majority of git users, I would hope.


 IMO a mostly XSI compliant pread (or even the git_pread() emulation) is 
 still better than forbidding the use of read() entirely. Switching from read 
 to pread everywhere requires that all callers have to keep track of the file 
 position, which means a _lot_ of code changes (read/xread/strbuf_read is used 
 in ~70 places throughout git). And how do you plan to deal with platforms 
 that don't have a thread-safe pread (HP, Cygwin)?

 Considering all that, Duy's solution of opening separate file descriptors per 
 thread seems to be the best pattern for future multi-threaded work.

Does that mean you would endorse the (N threads) * (M pack files)
approach to threading checkout and status?  That seems kind of
crazy-town to me.  Not to mention that pack windows are not shared, so
this approach to multi-threading can have the side-effect of blowing
out memory consumption.  We have already had to dial back settings for
pack.threads and core.deltaBaseCacheLimit, because threaded index-pack
was causing OOM errors on 32-bit platforms.

Cygwin (and MSVC) should be able to share a mostly compliant pread
implementation.  I don't have any insight into NonstopKernel; does is
really not have a thread-safe pread implementation?  If so, then I
suppose we have to #ifdef NO_PREAD, just as we do now.

I realize that these are deep changes.  However, the performance of
msysgit on the chromium repositories is pretty awful, enough so to
motivate this work.

Stefan
--
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: What's cooking in git.git (Mar 2014, #04; Thu, 20)

2014-03-20 Thread Torsten Bögershausen

On 03/20/2014 10:09 PM, Junio C Hamano wrote:

* ap/remote-hg-skip-null-bookmarks (2014-03-19) 1 commit
  - remote-hg: do not fail on invalid bookmarks

  Will merge to 'next'.

Hmm, am I the only one who has 11 failures in test-hg-hg-git.sh,
like this:
(Tested under Debian 7, commit 04570b241ddb53ad2f355977bae541bd7ff32af5)



master cde5672] clear executable bit
 Author: A U Thor aut...@example.com
 1 file changed, 0 insertions(+), 0 deletions(-)
 mode change 100755 = 100644 alpha
WARNING: Ignoring invalid bookmark 'master'
To hg::../hgrepo-git
 ! [remote rejected] master - master
error: failed to push some refs to 'hg::../hgrepo-git'
not ok 1 - executable bit
[]
# failed 11 among 11 test(s)
1..11






--
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 1/4] Documentation: Fix misuses of nor

2014-03-20 Thread Justin Lebar
Signed-off-by: Justin Lebar jle...@google.com
---
 Documentation/CodingGuidelines  |  4 ++--
 Documentation/config.txt|  6 +++---
 Documentation/diff-generate-patch.txt   |  2 +-
 Documentation/diff-options.txt  |  2 +-
 Documentation/everyday.txt  |  2 +-
 Documentation/git-add.txt   |  4 ++--
 Documentation/git-count-objects.txt |  4 ++--
 Documentation/git-diff.txt  |  4 ++--
 Documentation/git-prune.txt |  2 +-
 Documentation/git-push.txt  |  2 +-
 Documentation/git-read-tree.txt |  2 +-
 Documentation/git-reset.txt |  6 +++---
 Documentation/git-show-branch.txt   |  2 +-
 Documentation/git-show-ref.txt  |  2 +-
 Documentation/howto/rebase-from-internal-branch.txt |  2 +-
 Documentation/howto/revert-a-faulty-merge.txt   |  4 ++--
 Documentation/howto/revert-branch-rebase.txt|  2 +-
 Documentation/merge-options.txt | 15 +++
 Documentation/pretty-formats.txt|  2 +-
 Documentation/pretty-options.txt|  2 +-
 Documentation/rev-list-options.txt  |  2 +-
 Documentation/technical/api-gitattributes.txt   |  2 +-
 Documentation/technical/pack-protocol.txt   |  8 
 Documentation/technical/protocol-common.txt |  2 +-
 Documentation/user-manual.txt   |  2 +-
 25 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index ef67b53..b99fa87 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -91,13 +91,13 @@ For shell scripts specifically (not exhaustive):
E.g.: my_function () {
 
  - As to use of grep, stick to a subset of BRE (namely, no \{m,n\},
-   [::], [==], nor [..]) for portability.
+   [::], [==], or [..]) for portability.
 
- We do not use \{m,n\};
 
- We do not use -E;
 
-   - We do not use ? nor + (which are \{0,1\} and \{1,\}
+   - We do not use ? or + (which are \{0,1\} and \{1,\}
  respectively in BRE) but that goes without saying as these
  are ERE elements not BRE (note that \? and \+ are not even part
  of BRE -- making them accessible from BRE is a GNU extension).
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f4d793..c26a7c8 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -78,8 +78,8 @@ be escaped: use `\` for `` and `\\` for `\`.
 
 The following escape sequences (beside `\` and `\\`) are recognized:
 `\n` for newline character (NL), `\t` for horizontal tabulation (HT, TAB)
-and `\b` for backspace (BS).  No other char escape sequence, nor octal
-char sequences are valid.
+and `\b` for backspace (BS).  Other char escape sequences (including octal
+escape sequences) are invalid.
 
 Variable values ending in a `\` are continued on the next line in the
 customary UNIX fashion.
@@ -827,7 +827,7 @@ color.diff::
commands will only use color when output is to the terminal.
Defaults to false.
 +
-This does not affect linkgit:git-format-patch[1] nor the
+This does not affect linkgit:git-format-patch[1] or the
 'git-diff-{asterisk}' plumbing commands.  Can be overridden on the
 command line with the `--color[=when]` option.
 
diff --git a/Documentation/diff-generate-patch.txt 
b/Documentation/diff-generate-patch.txt
index 55f499a..843a20b 100644
--- a/Documentation/diff-generate-patch.txt
+++ b/Documentation/diff-generate-patch.txt
@@ -174,7 +174,7 @@ added, from the point of view of that parent).
 In the above example output, the function signature was changed
 from both files (hence two `-` removals from both file1 and
 file2, plus `++` to mean one line that was added does not appear
-in either file1 nor file2).  Also eight other lines are the same
+in either file1 or file2).  Also eight other lines are the same
 from file1 but do not appear in file2 (hence prefixed with `+`).
 
 When shown by `git diff-tree -c`, it compares the parents of a
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9b37b2a..6cb083a 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -358,7 +358,7 @@ endif::git-log[]
 --irreversible-delete::
Omit the preimage for deletes, i.e. print only the header but not
the diff between the preimage and `/dev/null`. The resulting patch
-   is not meant to be applied with `patch` nor `git apply`; this is
+   is not meant to be applied with `patch` or `git apply`; this is
solely for people who want to just concentrate on reviewing the
text after the change. In addition, the output obviously lack
enough information to apply such a patch in reverse, even 

[PATCH 3/4] Fix misuses of nor in comments

2014-03-20 Thread Justin Lebar
Signed-off-by: Justin Lebar jle...@gmail.com
---
 Makefile| 2 +-
 builtin/apply.c | 2 +-
 builtin/checkout.c  | 2 +-
 builtin/log.c   | 2 +-
 builtin/pack-objects.c  | 2 +-
 builtin/reset.c | 4 ++--
 builtin/show-branch.c   | 2 +-
 column.c| 2 +-
 contrib/examples/git-checkout.sh| 2 +-
 contrib/examples/git-reset.sh   | 4 ++--
 contrib/fast-import/import-directories.perl | 4 ++--
 delta.h | 2 +-
 diff.c  | 2 +-
 git-am.sh   | 2 +-
 gitweb/gitweb.perl  | 2 +-
 http.h  | 4 ++--
 perl/Git/SVN.pm | 2 +-
 perl/Git/SVN/Migration.pm   | 2 +-
 pkt-line.h  | 2 +-
 remote.c| 2 +-
 sha1_file.c | 2 +-
 test-chmtime.c  | 2 +-
 22 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/Makefile b/Makefile
index dddaf4f..fc02788 100644
--- a/Makefile
+++ b/Makefile
@@ -159,7 +159,7 @@ all::
 #
 # Define NEEDS_LIBINTL_BEFORE_LIBICONV if you need libintl before libiconv.
 #
-# Define NO_INTPTR_T if you don't have intptr_t nor uintptr_t.
+# Define NO_INTPTR_T if you don't have intptr_t or uintptr_t.
 #
 # Define NO_UINTMAX_T if you don't have uintmax_t.
 #
diff --git a/builtin/apply.c b/builtin/apply.c
index b0d0986..6013e19 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4061,7 +4061,7 @@ static int write_out_one_reject(struct patch *patch)
return error(_(cannot open %s: %s), namebuf, strerror(errno));
 
/* Normal git tools never deal with .rej, so do not pretend
-* this is a git patch by saying --git nor give extended
+* this is a git patch by saying --git or giving extended
 * headers.  While at it, maybe please kompare that wants
 * the trailing TAB and some garbage at the end of line ;-).
 */
diff --git a/builtin/checkout.c b/builtin/checkout.c
index ada51fa..7f37d1a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -895,7 +895,7 @@ static int parse_branchname_arg(int argc, const char **argv,
 *   between A and B, A...B names that merge base.
 *
 *   (b) If something is _not_ a commit, either -- is present
-*   or something is not a path, no -t nor -b was given, and
+*   or something is not a path, no -t or -b was given, and
 *   and there is a tracking branch whose name is something
 *   in one and only one remote, then this is a short-hand to
 *   fork local something from that remote-tracking branch.
diff --git a/builtin/log.c b/builtin/log.c
index b97373d..39e8836 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -503,7 +503,7 @@ static void show_rev_tweak_rev(struct rev_info *rev, struct 
setup_revision_opt *
/* There was no -m on the command line */
rev-ignore_merges = 0;
if (!rev-first_parent_only  !rev-combine_merges) {
-   /* No --first-parent, -c, nor --cc */
+   /* No --first-parent, -c, or --cc */
rev-combine_merges = 1;
rev-dense_combined_merges = 1;
}
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 541667f..ef1f20e 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -999,7 +999,7 @@ static int pbase_tree_cache_ix_incr(int ix)
 static struct pbase_tree {
struct pbase_tree *next;
/* This is a phony cache entry; we are not
-* going to evict it nor find it through _get()
+* going to evict it or find it through _get()
 * mechanism -- this is for the toplevel node that
 * would almost always change with any commit.
 */
diff --git a/builtin/reset.c b/builtin/reset.c
index a991344..96dd4c9 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -309,7 +309,7 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
 
/* git reset tree [--] paths... can be used to
 * load chosen paths from the tree into the index without
-* affecting the working tree nor HEAD. */
+* affecting the working tree or HEAD. */
if (pathspec.nr) {
if (reset_type == MIXED)
warning(_(--mixed with paths is deprecated; use 'git 
reset -- paths' instead.));
@@ -327,7 +327,7 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
die(_(%s reset is not allowed in a bare repository),
_(reset_type_names[reset_type]));
 
-   /* Soft 

[PATCH 2/4] contrib: Fix misuses of nor

2014-03-20 Thread Justin Lebar
Signed-off-by: Justin Lebar jle...@google.com
---
 contrib/examples/git-commit.sh | 2 +-
 contrib/svn-fe/svn-fe.txt  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/examples/git-commit.sh b/contrib/examples/git-commit.sh
index 23ffb02..4aab1a6 100755
--- a/contrib/examples/git-commit.sh
+++ b/contrib/examples/git-commit.sh
@@ -280,7 +280,7 @@ case $#,$also,$only,$amend in
 0,,,*)
;;
 *,,,*)
-   only_include_assumed=# Explicit paths specified without -i nor -o; 
assuming --only paths...
+   only_include_assumed=# Explicit paths specified without -i or -o; 
assuming --only paths...
also=
;;
 esac
diff --git a/contrib/svn-fe/svn-fe.txt b/contrib/svn-fe/svn-fe.txt
index 1128ab2..a3425f4 100644
--- a/contrib/svn-fe/svn-fe.txt
+++ b/contrib/svn-fe/svn-fe.txt
@@ -40,8 +40,8 @@ manual page.
 NOTES
 -
 Subversion dumps do not record a separate author and committer for
-each revision, nor a separate display name and email address for
-each author.  Like git-svn(1), 'svn-fe' will use the name
+each revision, nor do they record a separate display name and email
+address for each author.  Like git-svn(1), 'svn-fe' will use the name
 
 -
 user user@UUID
-- 
1.9.0.279.gdc9e3eb

--
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: What's cooking in git.git (Mar 2014, #04; Thu, 20)

2014-03-20 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Quite a few topics are still outside 'pu' and I suspect some of the
 larger ones deserve deeper reviews to help moving them to 'next'.
 In principle, I'd prefer to keep any large topic that touch core
 part of the system cooking in 'next' for at least a full cycle, and
 the sooner they get merged to 'next', the better.  Help is greatly
 appreciated.
 ...
 * ks/tree-diff-nway (2014-03-04) 19 commits
  - combine-diff: speed it up, by using multiparent diff tree-walker directly
  - tree-diff: rework diff_tree() to generate diffs for multiparent cases as 
 well
  - Portable alloca for Git
  - tree-diff: reuse base str(buf) memory on sub-tree recursion
  - tree-diff: no need to call full diff_tree_sha1 from show_path()
  - tree-diff: rework diff_tree interface to be sha1 based
  - tree-diff: diff_tree() should now be static
  - tree-diff: remove special-case diff-emitting code for empty-tree cases
  - tree-diff: simplify tree_entry_pathcmp
  - tree-diff: show_path prototype is not needed anymore
  - tree-diff: rename compare_tree_entry - tree_entry_pathcmp
  - tree-diff: move all action-taking code out of compare_tree_entry()
  - tree-diff: don't assume compare_tree_entry() returns -1,0,1
  - tree-diff: consolidate code for emitting diffs and recursion in one place
  - tree-diff: show_tree() is not needed
  - tree-diff: no need to pass match to skip_uninteresting()
  - tree-diff: no need to manually verify that there is no mode change for a 
 path
  - combine-diff: move changed-paths scanning logic into its own function
  - combine-diff: move show_log_first logic/action out of paths scanning

  Instead of running N pair-wise diff-trees when inspecting a
  N-parent merge, find the set of paths that were touched by walking
  N+1 trees in parallel.  These set of paths can then be turned into
  N pair-wise diff-tree results to be processed through rename
  detections and such.  And N=2 case nicely degenerates to the usual
  2-way diff-tree, which is very nice.

So I started re-reading this series, and decided that it might be
easier to advance the topic piece-by-piece.  Will be merging up to
consolidate code for emitting diffs to 'next', after tweaking that
last commit a bit (see below).

Kirill Smelkov k...@mns.spb.ru writes:

 Currently both compare_tree_entry() and show_path() invoke opt diff

s/show_path/show_entry/;

 callbacks (opt-add_remove() and opt-change()), and also they both have
 code which decides whether to recurse into sub-tree, and whether to emit
 a tree as separate entry if DIFF_OPT_TREE_IN_RECURSIVE is set.

 I.e. we have code duplication and logic scattered on two places.

 Let's consolidate it...
 ...
 +/* convert path, t1/t2 - opt-diff_*() callbacks */
 +static void emit_diff(struct diff_options *opt, struct strbuf *path,
 +   struct tree_desc *t1, struct tree_desc *t2)
 +{
 + unsigned int mode1 = t1 ? t1-entry.mode : 0;
 + unsigned int mode2 = t2 ? t2-entry.mode : 0;
 +
 + if (mode1  mode2) {
 + opt-change(opt, mode1, mode2, t1-entry.sha1, t2-entry.sha1,
 + 1, 1, path-buf, 0, 0);

This is not too bad per-se, but it would have been even better if
this part was done as:

if (t1  t2) {
opt-change(opt, t1-entry.mode1, t1-entry.mode2,
t1-entry.sha1, t2-entry.sha1,
1, 1, path-buf, 0, 0);
}

Then we do not have to rely on an extra convention, 'mode == 0'
means the entry is missing, in addition to what we already have,
i.e. t == NULL means the entry is missing.

This is minor, so I will not squash such a change in while merging
to 'next', but we may want to revisit and fix it up as a follow up
patch once the series is settled.

 + }
 + else {

Style; I've merged these two lines into one, i.e.

} else {

There was another instance of it in show_path(), which I also
tweaked.

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


Re: [PATCH v2] Bump core.deltaBaseCacheLimit to 128MiB

2014-03-20 Thread Junio C Hamano
David Kastrup d...@gnu.org writes:

 Junio C Hamano gits...@pobox.com writes:

 Duy Nguyen pclo...@gmail.com writes:

 The only
 downside I see is large blobs will be packed  undeltified, which could
 increase pack size if you have lots of them.

 I think that is something that can be tweaked, unless the user tells
 us otherwise via command line override, when running the improved
 gc --aggressive ;-)

 deltaBaseCacheLimit is used for unpacking, not for packing.

Hmm, doesn't packing need to read existing data?
--
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: What's cooking in git.git (Mar 2014, #04; Thu, 20)

2014-03-20 Thread Junio C Hamano
Torsten Bögershausen tbo...@web.de writes:

 On 03/20/2014 10:09 PM, Junio C Hamano wrote:
 * ap/remote-hg-skip-null-bookmarks (2014-03-19) 1 commit
   - remote-hg: do not fail on invalid bookmarks

   Will merge to 'next'.
 Hmm, am I the only one who has 11 failures in test-hg-hg-git.sh,

Still?  I thought I pushed out an updated version.

 like this:
 (Tested under Debian 7, commit 04570b241ddb53ad2f355977bae541bd7ff32af5)



 master cde5672] clear executable bit
  Author: A U Thor aut...@example.com
  1 file changed, 0 insertions(+), 0 deletions(-)
  mode change 100755 = 100644 alpha
 WARNING: Ignoring invalid bookmark 'master'
 To hg::../hgrepo-git
  ! [remote rejected] master - master
 error: failed to push some refs to 'hg::../hgrepo-git'
 not ok 1 - executable bit
 []
 # failed 11 among 11 test(s)
 1..11
--
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/4] Fix misuses of nor in comments

2014-03-20 Thread Jason St. John
On Thu, Mar 20, 2014 at 6:16 PM, Justin Lebar jle...@google.com wrote:
 Signed-off-by: Justin Lebar jle...@gmail.com
 ---
  Makefile| 2 +-
  builtin/apply.c | 2 +-
  builtin/checkout.c  | 2 +-
  builtin/log.c   | 2 +-
  builtin/pack-objects.c  | 2 +-
  builtin/reset.c | 4 ++--
  builtin/show-branch.c   | 2 +-
  column.c| 2 +-
  contrib/examples/git-checkout.sh| 2 +-
  contrib/examples/git-reset.sh   | 4 ++--
  contrib/fast-import/import-directories.perl | 4 ++--
  delta.h | 2 +-
  diff.c  | 2 +-
  git-am.sh   | 2 +-
  gitweb/gitweb.perl  | 2 +-
  http.h  | 4 ++--
  perl/Git/SVN.pm | 2 +-
  perl/Git/SVN/Migration.pm   | 2 +-
  pkt-line.h  | 2 +-
  remote.c| 2 +-
  sha1_file.c | 2 +-
  test-chmtime.c  | 2 +-
  22 files changed, 26 insertions(+), 26 deletions(-)

 diff --git a/Makefile b/Makefile
 index dddaf4f..fc02788 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -159,7 +159,7 @@ all::
  #
  # Define NEEDS_LIBINTL_BEFORE_LIBICONV if you need libintl before libiconv.
  #
 -# Define NO_INTPTR_T if you don't have intptr_t nor uintptr_t.
 +# Define NO_INTPTR_T if you don't have intptr_t or uintptr_t.
  #
  # Define NO_UINTMAX_T if you don't have uintmax_t.
  #
 diff --git a/builtin/apply.c b/builtin/apply.c
 index b0d0986..6013e19 100644
 --- a/builtin/apply.c
 +++ b/builtin/apply.c
 @@ -4061,7 +4061,7 @@ static int write_out_one_reject(struct patch *patch)
 return error(_(cannot open %s: %s), namebuf, 
 strerror(errno));

 /* Normal git tools never deal with .rej, so do not pretend
 -* this is a git patch by saying --git nor give extended
 +* this is a git patch by saying --git or giving extended
  * headers.  While at it, maybe please kompare that wants
  * the trailing TAB and some garbage at the end of line ;-).
  */

I don't think the change from give to giving here is grammatically correct.

 diff --git a/builtin/checkout.c b/builtin/checkout.c
 index ada51fa..7f37d1a 100644
 --- a/builtin/checkout.c
 +++ b/builtin/checkout.c
 @@ -895,7 +895,7 @@ static int parse_branchname_arg(int argc, const char 
 **argv,
  *   between A and B, A...B names that merge base.
  *
  *   (b) If something is _not_ a commit, either -- is present
 -*   or something is not a path, no -t nor -b was given, and
 +*   or something is not a path, no -t or -b was given, and
  *   and there is a tracking branch whose name is something
  *   in one and only one remote, then this is a short-hand to
  *   fork local something from that remote-tracking branch.
 diff --git a/builtin/log.c b/builtin/log.c
 index b97373d..39e8836 100644
 --- a/builtin/log.c
 +++ b/builtin/log.c
 @@ -503,7 +503,7 @@ static void show_rev_tweak_rev(struct rev_info *rev, 
 struct setup_revision_opt *
 /* There was no -m on the command line */
 rev-ignore_merges = 0;
 if (!rev-first_parent_only  !rev-combine_merges) {
 -   /* No --first-parent, -c, nor --cc */
 +   /* No --first-parent, -c, or --cc */
 rev-combine_merges = 1;
 rev-dense_combined_merges = 1;
 }
 diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
 index 541667f..ef1f20e 100644
 --- a/builtin/pack-objects.c
 +++ b/builtin/pack-objects.c
 @@ -999,7 +999,7 @@ static int pbase_tree_cache_ix_incr(int ix)
  static struct pbase_tree {
 struct pbase_tree *next;
 /* This is a phony cache entry; we are not
 -* going to evict it nor find it through _get()
 +* going to evict it or find it through _get()
  * mechanism -- this is for the toplevel node that
  * would almost always change with any commit.
  */
 diff --git a/builtin/reset.c b/builtin/reset.c
 index a991344..96dd4c9 100644
 --- a/builtin/reset.c
 +++ b/builtin/reset.c
 @@ -309,7 +309,7 @@ int cmd_reset(int argc, const char **argv, const char 
 *prefix)

 /* git reset tree [--] paths... can be used to
  * load chosen paths from the tree into the index without
 -* affecting the working tree nor HEAD. */
 +* affecting the working tree or HEAD. */
 if (pathspec.nr) {
 if (reset_type == MIXED)
 warning(_(--mixed with paths is deprecated; use 'git 

[PATCH 0/4] Fix misuses of nor (v2)

2014-03-20 Thread Justin Lebar
I got annoyed by git's awkward use of nor in man pages and in git add -p, so
I went ahead and audited all uses of nor in the tree.  One might be able to
argue that some of the uses I've changed are technically acceptable, but that's
a pretty low bar to set for ourselves.  I aimed to make everything both correct
and idiomatic.

All I really care about is git-add--interactive.perl and (to a lesser extent)
the docs, so if any of these other changes are controversial or annoying to
take, I'm happy to drop them.

Changes from v1 are:

- Drop the l10n patch (changes will be automatically picked up later).
- Fold the inside-tests and outside-tests patches.
- CC different people, as Duy indicated he wasn't comfortable reviewing.

Justin Lebar (4):
  Documentation: Fix misuses of nor
  contrib: Fix misuses of nor
  Fix misuses of nor in comments
  Fix misuses of nor outside comments and in tests

 Documentation/CodingGuidelines  |  4 ++--
 Documentation/config.txt|  6 +++---
 Documentation/diff-generate-patch.txt   |  2 +-
 Documentation/diff-options.txt  |  2 +-
 Documentation/everyday.txt  |  2 +-
 Documentation/git-add.txt   |  4 ++--
 Documentation/git-count-objects.txt |  4 ++--
 Documentation/git-diff.txt  |  4 ++--
 Documentation/git-prune.txt |  2 +-
 Documentation/git-push.txt  |  2 +-
 Documentation/git-read-tree.txt |  2 +-
 Documentation/git-reset.txt |  6 +++---
 Documentation/git-show-branch.txt   |  2 +-
 Documentation/git-show-ref.txt  |  2 +-
 Documentation/howto/rebase-from-internal-branch.txt |  2 +-
 Documentation/howto/revert-a-faulty-merge.txt   |  4 ++--
 Documentation/howto/revert-branch-rebase.txt|  2 +-
 Documentation/merge-options.txt | 15 +++
 Documentation/pretty-formats.txt|  2 +-
 Documentation/pretty-options.txt|  2 +-
 Documentation/rev-list-options.txt  |  2 +-
 Documentation/technical/api-gitattributes.txt   |  2 +-
 Documentation/technical/pack-protocol.txt   |  8 
 Documentation/technical/protocol-common.txt |  2 +-
 Documentation/user-manual.txt   |  2 +-
 Makefile|  2 +-
 builtin/apply.c |  2 +-
 builtin/checkout.c  |  2 +-
 builtin/clean.c |  6 +++---
 builtin/commit.c|  2 +-
 builtin/log.c   |  2 +-
 builtin/pack-objects.c  |  2 +-
 builtin/reset.c |  4 ++--
 builtin/show-branch.c   |  2 +-
 column.c|  2 +-
 contrib/examples/git-checkout.sh|  2 +-
 contrib/examples/git-commit.sh  |  2 +-
 contrib/examples/git-reset.sh   |  4 ++--
 contrib/fast-import/import-directories.perl |  4 ++--
 contrib/svn-fe/svn-fe.txt   |  4 ++--
 delta.h |  2 +-
 diff.c  |  2 +-
 git-add--interactive.perl   |  4 ++--
 git-am.sh   |  2 +-
 gitweb/gitweb.perl  |  2 +-
 http.h  |  4 ++--
 perl/Git/SVN.pm |  6 +++---
 perl/Git/SVN/Migration.pm   |  2 +-
 pkt-line.h  |  2 +-
 remote.c|  2 +-
 sha1_file.c |  4 ++--
 t/t1001-read-tree-m-2way.sh |  2 +-
 t/t4005-diff-rename-2.sh|  2 +-
 t/t4009-diff-rename-4.sh|  2 +-
 t/t5304-prune.sh|  2 +-
 t/t6036-recursive-corner-cases.sh   |  2 +-
 t/t7104-reset.sh|  2 +-
 t/t9400-git-cvsserver-server.sh |  2 +-
 test-chmtime.c  |  2 +-
 59 files changed, 88 insertions(+), 89 deletions(-)

-- 
1.9.0.279.gdc9e3eb

--
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 4/4] Fix misuses of nor outside comments and in tests

2014-03-20 Thread Justin Lebar
Signed-off-by: Justin Lebar jle...@gmail.com
---
 builtin/clean.c   | 6 +++---
 builtin/commit.c  | 2 +-
 git-add--interactive.perl | 4 ++--
 perl/Git/SVN.pm   | 4 ++--
 sha1_file.c   | 2 +-
 t/t1001-read-tree-m-2way.sh   | 2 +-
 t/t4005-diff-rename-2.sh  | 2 +-
 t/t4009-diff-rename-4.sh  | 2 +-
 t/t5304-prune.sh  | 2 +-
 t/t6036-recursive-corner-cases.sh | 2 +-
 t/t7104-reset.sh  | 2 +-
 t/t9400-git-cvsserver-server.sh   | 2 +-
 12 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index 5502957..977a068 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -903,11 +903,11 @@ int cmd_clean(int argc, const char **argv, const char 
*prefix)
 
if (!interactive  !dry_run  !force) {
if (config_set)
-   die(_(clean.requireForce set to true and neither -i, 
-n nor -f given; 
+   die(_(clean.requireForce set to true and neither -i, 
-n, nor -f given; 
  refusing to clean));
else
-   die(_(clean.requireForce defaults to true and neither 
-i, -n nor -f given; 
- refusing to clean));
+   die(_(clean.requireForce defaults to true and neither 
-i, -n, nor -f given;
+  refusing to clean));
}
 
if (force  1)
diff --git a/builtin/commit.c b/builtin/commit.c
index 26b2986..5d594a4 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1123,7 +1123,7 @@ static int parse_and_validate_options(int argc, const 
char *argv[],
if (argc == 0  only  amend)
only_include_assumed = _(Clever... amending the last one with 
dirty index.);
if (argc  0  !also  !only)
-   only_include_assumed = _(Explicit paths specified without -i 
nor -o; assuming --only paths...);
+   only_include_assumed = _(Explicit paths specified without -i 
or -o; assuming --only paths...);
if (!cleanup_arg || !strcmp(cleanup_arg, default))
cleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;
else if (!strcmp(cleanup_arg, verbatim))
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 24bb1ab..32c2f9c 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1156,9 +1156,9 @@ sub help_patch_cmd {
print colored $help_color, EOF ;
 y - $verb this hunk$target
 n - do not $verb this hunk$target
-q - quit; do not $verb this hunk nor any of the remaining ones
+q - quit; do not $verb this hunk or any of the remaining ones
 a - $verb this hunk and all later hunks in the file
-d - do not $verb this hunk nor any of the later hunks in the file
+d - do not $verb this hunk or any of the later hunks in the file
 g - select a hunk to go to
 / - search for a hunk matching the given regex
 j - leave this hunk undecided, see next undecided hunk
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 62f3293..a59564f 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -480,8 +480,8 @@ sub refname {
# It cannot end with a slash /, we'll throw up on this because
# SVN can't have directories with a slash in their name, either:
if ($refname =~ m{/$}) {
-   die ref: '$refname' ends with a trailing slash, this is ,
-   not permitted by git nor Subversion\n;
+   die ref: '$refname' ends with a trailing slash; this is ,
+   not permitted by git or Subversion\n;
}
 
# It cannot have ASCII control character space, tilde ~, caret ^,
diff --git a/sha1_file.c b/sha1_file.c
index b79efe4..77dbb56 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1123,7 +1123,7 @@ static void report_helper(const struct string_list *list,
const char *msg;
switch (seen_bits) {
case 0:
-   msg = no corresponding .idx nor .pack;
+   msg = no corresponding .idx or .pack;
break;
case 1:
msg = no corresponding .idx;
diff --git a/t/t1001-read-tree-m-2way.sh b/t/t1001-read-tree-m-2way.sh
index acaab07..1731383 100755
--- a/t/t1001-read-tree-m-2way.sh
+++ b/t/t1001-read-tree-m-2way.sh
@@ -18,7 +18,7 @@ In the test, these paths are used:
 frotz   - not in H added in M
 nitfol  - in H, stays in M unmodified
 rezrov  - in H, deleted in M
-yomin   - not in H nor M
+yomin   - not in H or M
 '
 . ./test-lib.sh
 . $TEST_DIRECTORY/lib-read-tree.sh
diff --git a/t/t4005-diff-rename-2.sh b/t/t4005-diff-rename-2.sh
index 77d7f49..7d2c6e1 100755
--- a/t/t4005-diff-rename-2.sh
+++ b/t/t4005-diff-rename-2.sh
@@ -66,7 +66,7 @@ test_expect_success \
 
 # tree has COPYING and rezrov.  work tree has the same COPYING and
 # copy-edited COPYING.1, and unchanged rezrov.  We should not say
-# 

Re: git log omits deleting merges

2014-03-20 Thread Philip Oakley

From: Jeff King p...@peff.net

On Thu, Feb 20, 2014 at 08:35:33AM +0100, Ephrim Khong wrote:


Hi, git log seems to omit merge commits that delete a file
if --follow or
--diff-filter=D is given. Below is a testcase. I'm not sure if it is
desired
behaviour for --diff-filter=D, but it's probably not correct
that --follow
_removes_ the merge commit from the log output.


This is by design. Git-log does not calculate or show merge diffs
unless
-c or --cc is specified, and thus no diff-filter can match.


This is hard to discern from the log(1) man page as this conflates
commit inclusion (limiting?) with the diff formatting.

The -c and -cc options are listed in the diff formatting section, but
that's well down the man page. Even then, the note for the options 
doesn't say that it will cause the log output to now include the merge 
commits.


Perhaps the -c and -cc options should also be noted in the options or
the commit limiting section, but exactly where is probably contentious 
(maybe under the --merges).


(or I may have misunderstood)




echo log 1 - no output
# note that --diff-filter=A and M work as expected
# the merge does not show up for --diff-filter=ACDMRTUXB either
git log --pretty=oneline --diff-filter=D -- some_file


Try:

 git log -c --diff-filter=D -- some_file

which does show it.

-Peff
--

Philip
--

--
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 0/12] GIT_CONFIG in the test suite

2014-03-20 Thread Jeff King
On Wed, Mar 19, 2014 at 10:28:46AM -0700, Junio C Hamano wrote:

 [git config --file versus GIT_CONFIG=]

 Thanks.  Then I think it makes sense to do such a conversion but it
 probably should be done on top of this patch (we could do it before
 this patch), not as a part of this patch.

Here's a series that goes on top of what you queued in
dt/tests-with-env-not-subshell. Once I started cleaning, I noticed a lot
of room for improvement and modernization in t0001. I hope I didn't get
too carried away.

  [01/12]: t/Makefile: stop setting GIT_CONFIG
  [02/12]: t/test-lib: drop redundant unset of GIT_CONFIG
  [03/12]: t: drop useless sane_unset GIT_* calls
  [04/12]: t: stop using GIT_CONFIG to cross repo boundaries
  [05/12]: t: prefer git config --file to GIT_CONFIG with test_must_fail
  [06/12]: t: prefer git config --file to GIT_CONFIG
  [07/12]: t0001: make symlink reinit test more careful
  [08/12]: t0001: use test_path_is_*
  [09/12]: t0001: use test_config_global
  [10/12]: t0001: use test_must_fail
  [11/12]: t0001: drop useless subshells
  [12/12]: t0001: drop subshells just for cd

 t/Makefile  |   4 +-
 t/t0001-init.sh | 211 ++-
 t/t1300-repo-config.sh  |  28 ++---
 t/t1302-repo-version.sh |   2 +-
 t/t5701-clone-local.sh  |   6 +-
 t/t7400-submodule-basic.sh  |   5 +-
 t/t9130-git-svn-authors-file.sh |   3 +-
 t/t9154-git-svn-fancy-glob.sh   |   6 +-
 t/t9400-git-cvsserver-server.sh |   1 -
 t/test-lib.sh   |   1 -
 10 files changed, 87 insertions(+), 180 deletions(-)

-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


[PATCH 01/12] t/Makefile: stop setting GIT_CONFIG

2014-03-20 Thread Jeff King
Once upon a time, the setting of GIT_CONFIG in the
environment could affect how tests ran. Commit 9c3796f (Fix
setting config variables with an alternative GIT_CONFIG,
2006-06-20) unconditionally set GIT_CONFIG in the Makefile
when running tests to give us a known starting point.

This is insufficient for running the tests outside of the
Makefile, however, and 8565d2d (Make tests independent of
global config files, 2007-02-15) later set GIT_CONFIG
directly in test-lib.sh. At that point the Makefile setting
was redundant, but we never removed it. Let's do so now.

Signed-off-by: Jeff King p...@peff.net
---
 t/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/Makefile b/t/Makefile
index 2373a04..8fd1a72 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -36,11 +36,11 @@ test: pre-clean $(TEST_LINT)
$(MAKE) aggregate-results-and-cleanup
 
 prove: pre-clean $(TEST_LINT)
-   @echo *** prove ***; GIT_CONFIG=.git/config $(PROVE) --exec 
'$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
+   @echo *** prove ***; $(PROVE) --exec '$(SHELL_PATH_SQ)' 
$(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS)
$(MAKE) clean-except-prove-cache
 
 $(T):
-   @echo *** $@ ***; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ 
$(GIT_TEST_OPTS)
+   @echo *** $@ ***; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
 
 pre-clean:
$(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'
-- 
1.9.0.560.g01ceb46

--
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/4] Fix misuses of nor in comments

2014-03-20 Thread Justin Lebar
Thanks for the quick reply.

When I send a new patch, should I fold these changes into the original
commit, or should I send them as a separate commit?

 diff --git a/builtin/apply.c b/builtin/apply.c
 index b0d0986..6013e19 100644
 --- a/builtin/apply.c
 +++ b/builtin/apply.c
 @@ -4061,7 +4061,7 @@ static int write_out_one_reject(struct patch *patch)
 return error(_(cannot open %s: %s), namebuf, 
 strerror(errno));

 /* Normal git tools never deal with .rej, so do not pretend
 -* this is a git patch by saying --git nor give extended
 +* this is a git patch by saying --git or giving extended
  * headers.  While at it, maybe please kompare that wants
  * the trailing TAB and some garbage at the end of line ;-).
  */

 I don't think the change from give to giving here is grammatically 
 correct.

Is it?  I might be misunderstanding the sentence, then.  I parse the
new sentence as

  Do not pretend this is a git patch by
  - saying --git, or
  - giving extended headers.

Giving is definitely awkward, but I'm not sure of a better word.

I'm happy to rephrase this, but I'm not sure how.  I don't think the
original makes much sense, but I'm also happy to leave it.

 How about ``If none of always, never, or auto is specified, then 
 setting layout
 implies always.``?

Sure.

 To leave nor here, I think you need to replace not with neither.

I think it actually works after the change, but unfortunately Garner's
doesn't give me a lot of ammunition to back up that feeling.  :)

How about We don't expect this to be set by the Makefile or by the
user (via CFLAGS).

 This would be better worded as If src_buffer and *src_buffer are not NULL, 
 it should ...

Done.

-Justin
--
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 02/12] t/test-lib: drop redundant unset of GIT_CONFIG

2014-03-20 Thread Jeff King
This is already handled by the mass GIT_* unsetting added by
95a1d12 (tests: scrub environment of GIT_* variables,
2011-03-15).

Signed-off-by: Jeff King p...@peff.net
---
 t/test-lib.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1531c24..625f06e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -649,7 +649,6 @@ else # normal case, use ../bin-wrappers only unless 
$with_dashes:
fi
 fi
 GIT_TEMPLATE_DIR=$GIT_BUILD_DIR/templates/blt
-unset GIT_CONFIG
 GIT_CONFIG_NOSYSTEM=1
 GIT_ATTR_NOSYSTEM=1
 export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM 
GIT_ATTR_NOSYSTEM
-- 
1.9.0.560.g01ceb46

--
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 03/12] t: drop useless sane_unset GIT_* calls

2014-03-20 Thread Jeff King
Several test scripts manually unset GIT_CONFIG and other
GIT_* variables. These are generally taken care of for us by
test-lib.sh already.

Unsetting these is not only useless, but can be confusing to
a reader, who may wonder why some tests in a script unset
them and others do not (t0001 is particularly guilty of this
inconsistency, probably because many of its tests predate
the test-lib.sh environment-cleansing).

Note that we cannot always get rid of such unsetting. For
example, t9130 can drop the GIT_CONFIG unset, but not the
GIT_DIR one, because lib-git-svn.sh sets the latter. And in
t1000, we unset GIT_TEMPLATE_DIR, which is explicitly
initialized by test-lib.sh.

Signed-off-by: Jeff King p...@peff.net
---
I suppose one could make an argument that test-lib.sh may later change
the set of variables it clears, and these unsets are documenting an
explicit need of each test. I'd find that more compelling if it were
actually applied consistently.

 t/t0001-init.sh | 15 ---
 t/t9130-git-svn-authors-file.sh |  1 -
 t/t9400-git-cvsserver-server.sh |  1 -
 3 files changed, 17 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 9fb582b..ddc8160 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -25,7 +25,6 @@ check_config () {
 
 test_expect_success 'plain' '
(
-   sane_unset GIT_DIR GIT_WORK_TREE 
mkdir plain 
cd plain 
git init
@@ -35,7 +34,6 @@ test_expect_success 'plain' '
 
 test_expect_success 'plain nested in bare' '
(
-   sane_unset GIT_DIR GIT_WORK_TREE 
git init --bare bare-ancestor.git 
cd bare-ancestor.git 
mkdir plain-nested 
@@ -47,7 +45,6 @@ test_expect_success 'plain nested in bare' '
 
 test_expect_success 'plain through aliased command, outside any git repo' '
(
-   sane_unset GIT_DIR GIT_WORK_TREE 
HOME=$(pwd)/alias-config 
export HOME 
mkdir alias-config 
@@ -65,7 +62,6 @@ test_expect_success 'plain through aliased command, outside 
any git repo' '
 
 test_expect_failure 'plain nested through aliased command' '
(
-   sane_unset GIT_DIR GIT_WORK_TREE 
git init plain-ancestor-aliased 
cd plain-ancestor-aliased 
echo [alias] aliasedinit = init .git/config 
@@ -78,7 +74,6 @@ test_expect_failure 'plain nested through aliased command' '
 
 test_expect_failure 'plain nested in bare through aliased command' '
(
-   sane_unset GIT_DIR GIT_WORK_TREE 
git init --bare bare-ancestor-aliased.git 
cd bare-ancestor-aliased.git 
echo [alias] aliasedinit = init config 
@@ -91,7 +86,6 @@ test_expect_failure 'plain nested in bare through aliased 
command' '
 
 test_expect_success 'plain with GIT_WORK_TREE' '
if (
-   sane_unset GIT_DIR 
mkdir plain-wt 
cd plain-wt 
GIT_WORK_TREE=$(pwd) git init
@@ -104,7 +98,6 @@ test_expect_success 'plain with GIT_WORK_TREE' '
 
 test_expect_success 'plain bare' '
(
-   sane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG 
mkdir plain-bare-1 
cd plain-bare-1 
git --bare init
@@ -114,7 +107,6 @@ test_expect_success 'plain bare' '
 
 test_expect_success 'plain bare with GIT_WORK_TREE' '
if (
-   sane_unset GIT_DIR GIT_CONFIG 
mkdir plain-bare-2 
cd plain-bare-2 
GIT_WORK_TREE=$(pwd) git --bare init
@@ -128,7 +120,6 @@ test_expect_success 'plain bare with GIT_WORK_TREE' '
 test_expect_success 'GIT_DIR bare' '
 
(
-   sane_unset GIT_CONFIG 
mkdir git-dir-bare.git 
GIT_DIR=git-dir-bare.git git init
) 
@@ -138,7 +129,6 @@ test_expect_success 'GIT_DIR bare' '
 test_expect_success 'init --bare' '
 
(
-   sane_unset GIT_DIR GIT_WORK_TREE GIT_CONFIG 
mkdir init-bare.git 
cd init-bare.git 
git init --bare
@@ -149,7 +139,6 @@ test_expect_success 'init --bare' '
 test_expect_success 'GIT_DIR non-bare' '
 
(
-   sane_unset GIT_CONFIG 
mkdir non-bare 
cd non-bare 
GIT_DIR=.git git init
@@ -160,7 +149,6 @@ test_expect_success 'GIT_DIR non-bare' '
 test_expect_success 'GIT_DIR  GIT_WORK_TREE (1)' '
 
(
-   sane_unset GIT_CONFIG 
mkdir git-dir-wt-1.git 
GIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-1.git git init
) 
@@ -170,7 +158,6 @@ test_expect_success 'GIT_DIR  GIT_WORK_TREE (1)' '
 test_expect_success 'GIT_DIR  GIT_WORK_TREE (2)' '
 
if (
-   sane_unset GIT_CONFIG 
mkdir git-dir-wt-2.git 
GIT_WORK_TREE=$(pwd) 

[PATCH 05/12] t: prefer git config --file to GIT_CONFIG with test_must_fail

2014-03-20 Thread Jeff King
This lets us get rid of an extra env invocation in the
middle, and is slightly more readable.

Signed-off-by: Jeff King p...@peff.net
---
The case that started this all...

This is also the only reason this series needs to go on top of David's
patch.

 t/t1300-repo-config.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index cd23d07..e355aa1 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -961,15 +961,15 @@ test_expect_success SYMLINKS 'symlinked configuration' '
 '
 
 test_expect_success 'nonexistent configuration' '
-   test_must_fail env GIT_CONFIG=doesnotexist git config --list 
-   test_must_fail env GIT_CONFIG=doesnotexist git config test.xyzzy
+   test_must_fail git config --file=doesnotexist --list 
+   test_must_fail git config --file=doesnotexist test.xyzzy
 '
 
 test_expect_success SYMLINKS 'symlink to nonexistent configuration' '
ln -s doesnotexist linktonada 
ln -s linktonada linktolinktonada 
-   test_must_fail env GIT_CONFIG=linktonada git config --list 
-   test_must_fail env GIT_CONFIG=linktolinktonada git config --list
+   test_must_fail git config --file=linktonada --list 
+   test_must_fail git config --file=linktolinktonada --list
 '
 
 test_expect_success 'check split_cmdline return' 
-- 
1.9.0.560.g01ceb46

--
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 06/12] t: prefer git config --file to GIT_CONFIG

2014-03-20 Thread Jeff King
Doing:

  GIT_CONFIG=foo git config ...

is equivalent to:

  git config --file=foo ...

The latter is easier to read and slightly less error-prone,
because of issues with one-shot variables and shell
functions (e.g., you cannot use the former with
test_must_fail).

Note that we explicitly leave one case in t1300 which checks
the same operation on both GIT_CONFIG and git config
--file. They are equivalent in the code these days, but
this will make sure it remains so.

Signed-off-by: Jeff King p...@peff.net
---
Unlike the last patch, this one has no tangible benefits besides Peff
thinks it looks better. I also tend to think that GIT_CONFIG is
something that it would be nice to get rid of in the long run, but I
don't have any immediate plans to do so.

 t/t1300-repo-config.sh  | 20 ++--
 t/t1302-repo-version.sh |  2 +-
 t/t7400-submodule-basic.sh  |  5 ++---
 t/t9130-git-svn-authors-file.sh |  2 +-
 t/t9154-git-svn-fancy-glob.sh   |  6 +++---
 5 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index e355aa1..85c6637 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -461,7 +461,7 @@ test_expect_success 'new variable inserts into proper 
section' '
test_cmp expect .git/config
 '
 
-test_expect_success 'alternative GIT_CONFIG (non-existing file should fail)' '
+test_expect_success 'alternative --file (non-existing file should fail)' '
test_must_fail git config --file non-existing-config -l
 '
 
@@ -495,10 +495,10 @@ test_expect_success 'refer config from subdirectory' '
 
 '
 
-test_expect_success 'refer config from subdirectory via GIT_CONFIG' '
+test_expect_success 'refer config from subdirectory via --file' '
(
cd x 
-   GIT_CONFIG=../other-config git config --get ein.bahn actual 
+   git config --file=../other-config --get ein.bahn actual 
test_cmp expect actual
)
 '
@@ -510,8 +510,8 @@ cat  expect  EOF
park = ausweis
 EOF
 
-test_expect_success '--set in alternative GIT_CONFIG' '
-   GIT_CONFIG=other-config git config anwohner.park ausweis 
+test_expect_success '--set in alternative file' '
+   git config --file=other-config anwohner.park ausweis 
test_cmp expect other-config
 '
 
@@ -942,11 +942,11 @@ test_expect_success 'inner whitespace kept verbatim' '
 
 test_expect_success SYMLINKS 'symlinked configuration' '
ln -s notyet myconfig 
-   GIT_CONFIG=myconfig git config test.frotz nitfol 
+   git config --file=myconfig test.frotz nitfol 
test -h myconfig 
test -f notyet 
-   test z$(GIT_CONFIG=notyet git config test.frotz) = znitfol 
-   GIT_CONFIG=myconfig git config test.xyzzy rezrov 
+   test z$(git config --file=notyet test.frotz) = znitfol 
+   git config --file=myconfig test.xyzzy rezrov 
test -h myconfig 
test -f notyet 
cat expect -\EOF 
@@ -954,8 +954,8 @@ test_expect_success SYMLINKS 'symlinked configuration' '
rezrov
EOF
{
-   GIT_CONFIG=notyet git config test.frotz 
-   GIT_CONFIG=notyet git config test.xyzzy
+   git config --file=notyet test.frotz 
+   git config --file=notyet test.xyzzy
} actual 
test_cmp expect actual
 '
diff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh
index 0e47662..0d9388a 100755
--- a/t/t1302-repo-version.sh
+++ b/t/t1302-repo-version.sh
@@ -19,7 +19,7 @@ test_expect_success 'setup' '
 
test_create_repo test 
test_create_repo test2 
-   GIT_CONFIG=test2/.git/config git config core.repositoryformatversion 99
+   git config --file=test2/.git/config core.repositoryformatversion 99
 '
 
 test_expect_success 'gitdir selection on normal repos' '
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index c28e8d8..7c88245 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -249,8 +249,7 @@ test_expect_success 'submodule add in subdirectory with 
relative path should fai
 '
 
 test_expect_success 'setup - add an example entry to .gitmodules' '
-   GIT_CONFIG=.gitmodules \
-   git config submodule.example.url git://example.com/init.git
+   git config --file=.gitmodules submodule.example.url 
git://example.com/init.git
 '
 
 test_expect_success 'status should fail for unmapped paths' '
@@ -264,7 +263,7 @@ test_expect_success 'setup - map path in .gitmodules' '
path = init
 EOF
 
-   GIT_CONFIG=.gitmodules git config submodule.example.path init 
+   git config --file=.gitmodules submodule.example.path init 
 
test_cmp expect .gitmodules
 '
diff --git a/t/t9130-git-svn-authors-file.sh b/t/t9130-git-svn-authors-file.sh
index a812783..c44de26 100755
--- a/t/t9130-git-svn-authors-file.sh
+++ b/t/t9130-git-svn-authors-file.sh
@@ -67,7 +67,7 @@ test_expect_success 'fetch fails on ee' '

[PATCH 08/12] t0001: use test_path_is_*

2014-03-20 Thread Jeff King
t0001 predates the test_path_is_* helpers, and uses test
-f and test -d directly. Using the helpers provides
better debugging output, and are a little more robust.
As opposed to ! test -d, test_path_is_missing will
actually makes sure the path does not exist at all.

Signed-off-by: Jeff King p...@peff.net
---
 t/t0001-init.sh | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 5245711..fdcf4b3 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -199,13 +199,13 @@ test_expect_success 'init with --template (blank)' '
cd template-plain 
git init
) 
-   test -f template-plain/.git/info/exclude 
+   test_path_is_file template-plain/.git/info/exclude 
(
mkdir template-blank 
cd template-blank 
git init --template=
) 
-   ! test -f template-blank/.git/info/exclude
+   test_path_is_missing template-blank/.git/info/exclude
 '
 
 test_expect_success 'init with init.templatedir set' '
@@ -263,7 +263,7 @@ test_expect_success 'init creates a new directory' '
rm -fr newdir 
(
git init newdir 
-   test -d newdir/.git/refs
+   test_path_is_dir newdir/.git/refs
)
 '
 
@@ -271,7 +271,7 @@ test_expect_success 'init creates a new bare directory' '
rm -fr newdir 
(
git init --bare newdir 
-   test -d newdir/refs
+   test_path_is_dir newdir/refs
)
 '
 
@@ -280,7 +280,7 @@ test_expect_success 'init recreates a directory' '
(
mkdir newdir 
git init newdir 
-   test -d newdir/.git/refs
+   test_path_is_dir newdir/.git/refs
)
 '
 
@@ -289,14 +289,14 @@ test_expect_success 'init recreates a new bare directory' 
'
(
mkdir newdir 
git init --bare newdir 
-   test -d newdir/refs
+   test_path_is_dir newdir/refs
)
 '
 
 test_expect_success 'init creates a new deep directory' '
rm -fr newdir 
git init newdir/a/b/c 
-   test -d newdir/a/b/c/.git/refs
+   test_path_is_dir newdir/a/b/c/.git/refs
 '
 
 test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. 
shared)' '
@@ -306,7 +306,7 @@ test_expect_success POSIXPERM 'init creates a new deep 
directory (umask vs. shar
# the repository itself should follow shared
umask 002 
git init --bare --shared=0660 newdir/a/b/c 
-   test -d newdir/a/b/c/refs 
+   test_path_is_dir newdir/a/b/c/refs 
ls -ld newdir/a newdir/a/b  lsab.out 
! grep -v ^drwxrw[sx]r-x lsab.out 
ls -ld newdir/a/b/c  lsc.out 
@@ -319,7 +319,7 @@ test_expect_success 'init notices EEXIST (1)' '
(
newdir 
test_must_fail git init newdir 
-   test -f newdir
+   test_path_is_file newdir
)
 '
 
@@ -329,7 +329,7 @@ test_expect_success 'init notices EEXIST (2)' '
mkdir newdir 
newdir/a
test_must_fail git init newdir/a/b 
-   test -f newdir/a
+   test_path_is_file newdir/a
)
 '
 
@@ -345,15 +345,15 @@ test_expect_success POSIXPERM,SANITY 'init notices EPERM' 
'
 test_expect_success 'init creates a new bare directory with global --bare' '
rm -rf newdir 
git --bare init newdir 
-   test -d newdir/refs
+   test_path_is_dir newdir/refs
 '
 
 test_expect_success 'init prefers command line to GIT_DIR' '
rm -rf newdir 
mkdir otherdir 
GIT_DIR=otherdir git --bare init newdir 
-   test -d newdir/refs 
-   ! test -d otherdir/refs
+   test_path_is_dir newdir/refs 
+   test_path_is_missing otherdir/refs
 '
 
 test_expect_success 'init with separate gitdir' '
@@ -361,7 +361,7 @@ test_expect_success 'init with separate gitdir' '
git init --separate-git-dir realgitdir newdir 
echo gitdir: `pwd`/realgitdir expected 
test_cmp expected newdir/.git 
-   test -d realgitdir/refs
+   test_path_is_dir realgitdir/refs
 '
 
 test_expect_success 're-init on .git file' '
@@ -375,8 +375,8 @@ test_expect_success 're-init to update git link' '
) 
echo gitdir: `pwd`/surrealgitdir expected 
test_cmp expected newdir/.git 
-   test -d surrealgitdir/refs 
-   ! test -d realgitdir/refs
+   test_path_is_dir surrealgitdir/refs 
+   test_path_is_missing realgitdir/refs
 '
 
 test_expect_success 're-init to move gitdir' '
@@ -388,7 +388,7 @@ test_expect_success 're-init to move gitdir' '
) 
echo gitdir: `pwd`/realgitdir expected 
test_cmp expected newdir/.git 
-   test -d realgitdir/refs
+   test_path_is_dir realgitdir/refs

[PATCH 07/12] t0001: make symlink reinit test more careful

2014-03-20 Thread Jeff King
In the final test of t0001, we have a repo whose .git is a
symlink to a directory here, and we use
--separate-git-dir to migrate that to a .git file pointing
to a different directory. We check that the data is migrated
to the new directory and that .git looks like a git-file.

We also check that here is not a directory, which is
slightly misleading. It should not be a directory, but
neither should it be gone. It is the actual resting place of
the git-file, and .git remains a symlink to it.

Let's check that more explicitly, both to make our test more
robust, and to make further cleanups in this area more
obvious.

Signed-off-by: Jeff King p...@peff.net
---
 t/t0001-init.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 9b05fdf..5245711 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -402,8 +402,8 @@ test_expect_success SYMLINKS 're-init to move gitdir 
symlink' '
) 
echo gitdir: `pwd`/realgitdir expected 
test_cmp expected newdir/.git 
-   test -d realgitdir/refs 
-   ! test -d newdir/here
+   test_cmp expected newdir/here 
+   test -d realgitdir/refs
 '
 
 test_done
-- 
1.9.0.560.g01ceb46

--
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 09/12] t0001: use test_config_global

2014-03-20 Thread Jeff King
We hand-set several config options using :

  git config -f $HOME/.gitconfig ...

Instead, we can use test_config_global. Not only is this
more readable, but it cleans up for us so that subsequent
tests aren't polluted by our settings.

Signed-off-by: Jeff King p...@peff.net
---
 t/t0001-init.sh | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index fdcf4b3..9515da3 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -211,9 +211,8 @@ test_expect_success 'init with --template (blank)' '
 test_expect_success 'init with init.templatedir set' '
mkdir templatedir-source 
echo Content templatedir-source/file 
+   test_config_global init.templatedir ${HOME}/templatedir-source 
(
-   test_config=${HOME}/.gitconfig 
-   git config -f $test_config  init.templatedir 
${HOME}/templatedir-source 
mkdir templatedir-set 
cd templatedir-set 
sane_unset GIT_TEMPLATE_DIR 
@@ -225,10 +224,9 @@ test_expect_success 'init with init.templatedir set' '
 '
 
 test_expect_success 'init --bare/--shared overrides system/global config' '
+   test_config_global core.bare false 
+   test_config_global core.sharedRepository 0640 
(
-   test_config=$HOME/.gitconfig 
-   git config -f $test_config core.bare false 
-   git config -f $test_config core.sharedRepository 0640 
mkdir init-bare-shared-override 
cd init-bare-shared-override 
git init --bare --shared=0666
@@ -239,9 +237,8 @@ test_expect_success 'init --bare/--shared overrides 
system/global config' '
 '
 
 test_expect_success 'init honors global core.sharedRepository' '
+   test_config_global core.sharedRepository 0666 
(
-   test_config=$HOME/.gitconfig 
-   git config -f $test_config core.sharedRepository 0666 
mkdir shared-honor-global 
cd shared-honor-global 
git init
-- 
1.9.0.560.g01ceb46

--
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 v3] rev-parse --parseopt: option argument name hints

2014-03-20 Thread Ilya Bobyr

On 3/20/2014 11:38 AM, Junio C Hamano wrote:

Ilya Bobyr ilya.bo...@gmail.com writes:


Built-in commands can specify names for option arguments when usage text
is generated for a command.  sh based commands should be able to do the
same.

Option argument name hint is any text that comes after [*=?!] after the
argument name up to the first whitespace.  Underscores are replaced with
whitespace.  It is unlikely that an underscore would be useful in the
hint text.

Signed-off-by: Ilya Bobyr ilya.bo...@gmail.com
---
  Changed according to the last comments.  Added Usage text paragraph in the
  documentation and updated variable names.

  Documentation/git-rev-parse.txt |   34 --
  builtin/rev-parse.c |   17 -
  t/t1502-rev-parse-parseopt.sh   |   20 
  3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0d2cdcd..b8aabc9 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -284,13 +284,13 @@ Input Format
  
  'git rev-parse --parseopt' input format is fully text based. It has two parts,

  separated by a line that contains only `--`. The lines before the separator
-(should be more than one) are used for the usage.
+(should be one or more) are used for the usage.
  The lines after the separator describe the options.
  
  Each line of options has this format:
  
  

-opt_specflags* SP+ help LF
+opt_specflags*arg_hint? SP+ help LF
  
  
  `opt_spec`::

@@ -313,6 +313,12 @@ Each line of options has this format:
  
  	* Use `!` to not make the corresponding negated long option available.
  
+`arg_hint`::

+   `arg_hing`, if specified, is used as a name of the argument in the
+   help output, for options that take arguments. `arg_hint` is
+   terminated by the first whitespace. When output the name is shown in
+   angle braces.  Underscore symbols are replaced with spaces.

The last part is troubling (and sounds not very sane).  Do we do
such a munging anywhere else, or is it just here?  If the latter I'd
prefer not to see such a hack.


The following commands have spaces in argument names in the -h output 
for one or two arguments:

  * clone
  * commit
  * merge

A number of commands use dashes to separate words in arguments names.

git notes is the only command that uses an underscore in one argument 
name.


At the moment space is used to separate option specification from the 
help line.  As argument name hint is part of the option specification it 
ends at the first space.


It seems a bit unfair if sh based commands would not be able to use 
spaces while the build-in ones can.
As underscores are not used in the UI (at least that was my impression 
so far), I thought that to be a good option.


Do you think a different kind of escaping should be used? Backslashes?
Or no spaces?


@@ -333,6 +339,8 @@ h,helpshow the help
  
  foo   some nifty option --foo

  bar=  some cool option --bar with an argument
+baz=arg   another cool option --baz with a named argument
+qux?path  qux may take a path argument but has meaning by itself
  
An option group Header

  C?option C with an optional argument
@@ -340,6 +348,28 @@ C?option C with an optional argument
  eval $(echo $OPTS_SPEC | git rev-parse --parseopt -- $@ || echo exit $?)
  
  
+

+Usage text
+~~
+
+When $@ is -h or --help the above example would produce the following
+usage text:

Sounds like a good idea to add this; all the above arguments inside
double quotes should be typeset `as-typed`, though.


Thanks, I will fix that.


@@ -419,6 +420,20 @@ static int cmd_parseopt(int argc, const char **argv, const 
char *prefix)
o-value = parsed;
o-flags = PARSE_OPT_NOARG;
o-callback = parseopt_dump;
+
+   /* Possible argument name hint */
+   end = s;
+   while (s  sb.buf  strchr(*=?!, s[-1]) == NULL)
+   --s;
+   if (s != sb.buf  s != end) {
+   char *a;
+   o-argh = a = xmemdupz(s, end - s);
+   while (a = strchr(a, '_'))
+   *a = ' ';

... and without the underscore munging, we do not have to allocate
a new piece of memory, either.


We would have to do it any way to have the string zero terminated.
The list of arguments that holds the lines been parsed is const char *.

But I do not think this is an argument to be considered when designing 
the user interface :)


Never the less if there is a way not to allocate extra memory that I am 
missing - let me know, I would remove the allocation.

--
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 10/12] t0001: use test_must_fail

2014-03-20 Thread Jeff King
We've hand-rolled several if statements looking for
failures. We can use test_must_fail here, which is shorter
and more robust.

Note that we modify the commands slightly (to use git init
foo rather than cd foo  git init) to avoid dealing with
a subshell, but this should not affect the outcome.

Signed-off-by: Jeff King p...@peff.net
---
I'm pretty sure we can actually drop the mkdir in each of
these cases, too, but I was trying to leave things as close
to the original as possible.

 t/t0001-init.sh | 38 +++---
 1 file changed, 11 insertions(+), 27 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 9515da3..4560bba 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -85,15 +85,8 @@ test_expect_failure 'plain nested in bare through aliased 
command' '
 '
 
 test_expect_success 'plain with GIT_WORK_TREE' '
-   if (
-   mkdir plain-wt 
-   cd plain-wt 
-   GIT_WORK_TREE=$(pwd) git init
-   )
-   then
-   echo Should have failed -- GIT_WORK_TREE should not be used
-   false
-   fi
+   mkdir plain-wt 
+   test_must_fail env GIT_WORK_TREE=$(pwd)/plain-wt git init plain-wt
 '
 
 test_expect_success 'plain bare' '
@@ -106,15 +99,10 @@ test_expect_success 'plain bare' '
 '
 
 test_expect_success 'plain bare with GIT_WORK_TREE' '
-   if (
-   mkdir plain-bare-2 
-   cd plain-bare-2 
-   GIT_WORK_TREE=$(pwd) git --bare init
-   )
-   then
-   echo Should have failed -- GIT_WORK_TREE should not be used
-   false
-   fi
+   mkdir plain-bare-2 
+   test_must_fail \
+   env GIT_WORK_TREE=$(pwd)/plain-bare-2 \
+   git --bare init plain-bare-2
 '
 
 test_expect_success 'GIT_DIR bare' '
@@ -156,15 +144,11 @@ test_expect_success 'GIT_DIR  GIT_WORK_TREE (1)' '
 '
 
 test_expect_success 'GIT_DIR  GIT_WORK_TREE (2)' '
-
-   if (
-   mkdir git-dir-wt-2.git 
-   GIT_WORK_TREE=$(pwd) GIT_DIR=git-dir-wt-2.git git --bare init
-   )
-   then
-   echo Should have failed -- --bare should not be used
-   false
-   fi
+   mkdir git-dir-wt-2.git 
+   test_must_fail env \
+   GIT_WORK_TREE=$(pwd) \
+   GIT_DIR=git-dir-wt-2.git \
+   git --bare init
 '
 
 test_expect_success 'reinit' '
-- 
1.9.0.560.g01ceb46

--
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 11/12] t0001: drop useless subshells

2014-03-20 Thread Jeff King
Many tests use subshells, but don't actually change the
shell environment. They were probably cargo-culted from
earlier tests which did need subshells. Drop the useless
ones.

Signed-off-by: Jeff King p...@peff.net
---
These ones should produce no behavior change at all; they're purely
mechanical (foo  bar) to foo  bar (though of course I did them
by hand, because you need to know that foo and bar do not affect the
environment).

 t/t0001-init.sh | 61 +
 1 file changed, 22 insertions(+), 39 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 4560bba..55a68bc 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -106,11 +106,8 @@ test_expect_success 'plain bare with GIT_WORK_TREE' '
 '
 
 test_expect_success 'GIT_DIR bare' '
-
-   (
-   mkdir git-dir-bare.git 
-   GIT_DIR=git-dir-bare.git git init
-   ) 
+   mkdir git-dir-bare.git 
+   GIT_DIR=git-dir-bare.git git init 
check_config git-dir-bare.git true unset
 '
 
@@ -242,36 +239,28 @@ test_expect_success 'init rejects insanely long 
--template' '
 
 test_expect_success 'init creates a new directory' '
rm -fr newdir 
-   (
-   git init newdir 
-   test_path_is_dir newdir/.git/refs
-   )
+   git init newdir 
+   test_path_is_dir newdir/.git/refs
 '
 
 test_expect_success 'init creates a new bare directory' '
rm -fr newdir 
-   (
-   git init --bare newdir 
-   test_path_is_dir newdir/refs
-   )
+   git init --bare newdir 
+   test_path_is_dir newdir/refs
 '
 
 test_expect_success 'init recreates a directory' '
rm -fr newdir 
-   (
-   mkdir newdir 
-   git init newdir 
-   test_path_is_dir newdir/.git/refs
-   )
+   mkdir newdir 
+   git init newdir 
+   test_path_is_dir newdir/.git/refs
 '
 
 test_expect_success 'init recreates a new bare directory' '
rm -fr newdir 
-   (
-   mkdir newdir 
-   git init --bare newdir 
-   test_path_is_dir newdir/refs
-   )
+   mkdir newdir 
+   git init --bare newdir 
+   test_path_is_dir newdir/refs
 '
 
 test_expect_success 'init creates a new deep directory' '
@@ -297,30 +286,24 @@ test_expect_success POSIXPERM 'init creates a new deep 
directory (umask vs. shar
 
 test_expect_success 'init notices EEXIST (1)' '
rm -fr newdir 
-   (
-   newdir 
-   test_must_fail git init newdir 
-   test_path_is_file newdir
-   )
+   newdir 
+   test_must_fail git init newdir 
+   test_path_is_file newdir
 '
 
 test_expect_success 'init notices EEXIST (2)' '
rm -fr newdir 
-   (
-   mkdir newdir 
-   newdir/a
-   test_must_fail git init newdir/a/b 
-   test_path_is_file newdir/a
-   )
+   mkdir newdir 
+   newdir/a
+   test_must_fail git init newdir/a/b 
+   test_path_is_file newdir/a
 '
 
 test_expect_success POSIXPERM,SANITY 'init notices EPERM' '
rm -fr newdir 
-   (
-   mkdir newdir 
-   chmod -w newdir 
-   test_must_fail git init newdir/a/b
-   )
+   mkdir newdir 
+   chmod -w newdir 
+   test_must_fail git init newdir/a/b
 '
 
 test_expect_success 'init creates a new bare directory with global --bare' '
-- 
1.9.0.560.g01ceb46

--
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 12/12] t0001: drop subshells just for cd

2014-03-20 Thread Jeff King
Many tests do something like:

  (
mkdir foo 
cd foo 
git init
  )

You can do the same these days with git init foo, which
makes the tests shorter and simpler to read.

Signed-off-by: Jeff King p...@peff.net
---
Unlike the last patch, this one _could_ have an affect. I made the
assumption that git init foo would behave sanely, but that other
complex things should be left alone. E.g., ones that set GIT_DIR in the
environment to a relative path might be affected based on when git does
the chdir.

 t/t0001-init.sh | 56 +---
 1 file changed, 9 insertions(+), 47 deletions(-)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index 55a68bc..68549d1 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -24,11 +24,7 @@ check_config () {
 }
 
 test_expect_success 'plain' '
-   (
-   mkdir plain 
-   cd plain 
-   git init
-   ) 
+   git init plain 
check_config plain/.git false unset
 '
 
@@ -90,11 +86,7 @@ test_expect_success 'plain with GIT_WORK_TREE' '
 '
 
 test_expect_success 'plain bare' '
-   (
-   mkdir plain-bare-1 
-   cd plain-bare-1 
-   git --bare init
-   ) 
+   git --bare init plain-bare-1 
check_config plain-bare-1 true unset
 '
 
@@ -112,12 +104,7 @@ test_expect_success 'GIT_DIR bare' '
 '
 
 test_expect_success 'init --bare' '
-
-   (
-   mkdir init-bare.git 
-   cd init-bare.git 
-   git init --bare
-   ) 
+   git init --bare init-bare.git 
check_config init-bare.git true unset
 '
 
@@ -166,26 +153,14 @@ test_expect_success 'reinit' '
 test_expect_success 'init with --template' '
mkdir template-source 
echo content template-source/file 
-   (
-   mkdir template-custom 
-   cd template-custom 
-   git init --template=../template-source
-   ) 
+   git init --template=../template-source template-custom 
test_cmp template-source/file template-custom/.git/file
 '
 
 test_expect_success 'init with --template (blank)' '
-   (
-   mkdir template-plain 
-   cd template-plain 
-   git init
-   ) 
+   git init template-plain 
test_path_is_file template-plain/.git/info/exclude 
-   (
-   mkdir template-blank 
-   cd template-blank 
-   git init --template=
-   ) 
+   git init --template= template-blank 
test_path_is_missing template-blank/.git/info/exclude
 '
 
@@ -207,11 +182,7 @@ test_expect_success 'init with init.templatedir set' '
 test_expect_success 'init --bare/--shared overrides system/global config' '
test_config_global core.bare false 
test_config_global core.sharedRepository 0640 
-   (
-   mkdir init-bare-shared-override 
-   cd init-bare-shared-override 
-   git init --bare --shared=0666
-   ) 
+   git init --bare --shared=0666 init-bare-shared-override 
check_config init-bare-shared-override true unset 
test x0666 = \
x`git config -f init-bare-shared-override/config core.sharedRepository`
@@ -219,22 +190,13 @@ test_expect_success 'init --bare/--shared overrides 
system/global config' '
 
 test_expect_success 'init honors global core.sharedRepository' '
test_config_global core.sharedRepository 0666 
-   (
-   mkdir shared-honor-global 
-   cd shared-honor-global 
-   git init
-   ) 
+   git init shared-honor-global 
test x0666 = \
x`git config -f shared-honor-global/.git/config core.sharedRepository`
 '
 
 test_expect_success 'init rejects insanely long --template' '
-   (
-   insane=$(printf x%0dx 1) 
-   mkdir test 
-   cd test 
-   test_must_fail git init --template=$insane
-   )
+   test_must_fail git init --template=$(printf x%0dx 1) test
 '
 
 test_expect_success 'init creates a new directory' '
-- 
1.9.0.560.g01ceb46
--
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 v3][GSOC] fsck: use bitwise-or assignment operator to set flag

2014-03-20 Thread Hiroyuki
Junio and Eric,

Thank you for the reviews and helpful advice.
I should have read more past commit messages before send patch.


Regards,


2014-03-21 3:20 GMT+09:00 Junio C Hamano gits...@pobox.com:
 Hiroyuki Sano sh19910...@gmail.com writes:

 fsck_tree() has two different ways to set a flag,
 which are the followings:

   1. Using a if-statement that guards assignment.

   2. Using a bitwise-or assignment operator.

 Currently, many with the former way,
 and one with the latter way.

 In this patch, unify them to the latter way,
 because it makes the code shorter and easier to read,
 and it is brief and to the point.

 Two issues:

  * In this patch, is redundant.

  * it is brief and to the point are equally applicable to both
styles, so that is not a *reason* to choose one over the other.

 If a condition were *not* brief and to the point, then a rewrite to
 the latter style will make the resulting code worse:

 if (a very complex condition
 that potentially have to consume a
 lot of brain-cycles to understand) {
 has_that_condition = 1;
 }

 is a lot easier to extend than

 has_that_condition = (a very complex condition
   that potentially have to consume a
   lot of brain-cycles to understand);

 because it is a lot more likely that we would need to later extend
 such a complex condition is more likely than a simple singleton
 condition, and we could end up with

 if (a very complex condition
 that potentially have to consume a
 lot of brain-cycles to understand) {
 futher computation to check if
 the condition really holds
 will be added here later
 if (does that condition really hold true?)
 has_that_condition = 1;
 }


 which may be harder to express in the latter form.

 In other words, it is brief and to the point merely _allows_ these
 statements to be expressed in the latter form; it does not say
 anything about which is better between the former and the latter.

 Signed-off-by: Hiroyuki Sano sh19910...@gmail.com
 ---
  fsck.c | 18 ++
  1 file changed, 6 insertions(+), 12 deletions(-)

 diff --git a/fsck.c b/fsck.c
 index b3022ad..abed62b 100644
 --- a/fsck.c
 +++ b/fsck.c
 @@ -165,18 +165,12 @@ static int fsck_tree(struct tree *item, int strict, 
 fsck_error error_func)

   sha1 = tree_entry_extract(desc, name, mode);

 - if (is_null_sha1(sha1))
 - has_null_sha1 = 1;
 - if (strchr(name, '/'))
 - has_full_path = 1;
 - if (!*name)
 - has_empty_name = 1;
 - if (!strcmp(name, .))
 - has_dot = 1;
 - if (!strcmp(name, ..))
 - has_dotdot = 1;
 - if (!strcmp(name, .git))
 - has_dotgit = 1;
 + has_null_sha1 |= is_null_sha1(sha1);
 + has_full_path |= !!strchr(name, '/');
 + has_empty_name |= !*name;
 + has_dot |= !strcmp(name, .);
 + has_dotdot |= !strcmp(name, ..);
 + has_dotgit |= !strcmp(name, .git);
   has_zero_pad |= *(char *)desc.buffer == '0';
   update_tree_entry(desc);



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


Re: [BUG] Segfault on git describe

2014-03-20 Thread Jeff King
On Wed, Mar 19, 2014 at 10:34:29PM +, Dragos Foianu wrote:

 The name_rev function recursively calls itself which is why the backtrace is
 so big. Unfortunately, for repos with long histories it can lead to Stack
 Overflows. This is pretty much what happened in your case.
 
 I tested it on my computer and I get the same results. I managed to get it
 working by doubling my default stacksize:
 
 ulimit -S -s 16192
 
 Considering your project is a very large one and merely allocating a few
 more resources fixes the problem, I'm not sure it warrants rewriting the
 function to use less stack. You will have to wait for one of the maintainers
 to give you a definitive answer.

Thanks for looking into the problem.

As a general rule, I think we are interested in reducing recursion in
functions which can go O(depth of history) or deeper. I'd say that
O(lg(depth of history)) is probably OK.

There is some precedent in 941ba8d (Eliminate recursion in
setting/clearing marks in commit list, 2012-01-14), for example.

Also, in:

  abe601b (sha1_file: remove recursion in unpack_entry, 2013-03-27)

  790d96c (sha1_file: remove recursion in packed_object_info,
  2013-03-25)

  2baad22 (index-pack: eliminate recursion in find_unresolved_deltas,
  2012-01-14)

though I think those recurse in the size of delta chain, which is not
nearly so big.

So I think we'd be happy to see it converted to an iterative process
(probably with a stack on the heap). In addition to name-rev, I believe
that tag --contains will recurse down the longest history path, too (I
think there may have been experimental patches for the latter, but you'd
have to search the list archive).

-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: Configuring a third-party git hook

2014-03-20 Thread Jeff King
On Fri, Mar 21, 2014 at 03:51:16AM +1100, Chris Angelico wrote:

  1. I would say yes. git config is made to be extended and doesn't
  require a config item to be known.
  2. Namespacing the config items like you did is a good thing to do so
  it won't interfere with other options.
 
 Excellent! Thank you.
 
 Is this documented anywhere? The git config man page says to look to
 other git man pages:
 
 https://www.kernel.org/pub/software/scm/git/docs/git-config.html#_variables
 
 A comment there to the effect that Third party tools may also define
 their own variables or something would make it clear that this is the
 intention.

I think this sentence from the section you linked is meant to express
that:

  You will find a description of non-core porcelain configuration
  variables in the respective porcelain documentation.

but it is rather opaque, isn't it? You did not know it, but your hook is
a non-core porcelain. :)

I think it could probably be re-worded, and possibly even indicate to
authors of other programs that they are free to make up their own
variables (but should take care with namespacing them appropriately).

Would you like to try your hand at writing a patch?

-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 v2] Bump core.deltaBaseCacheLimit to 128MiB

2014-03-20 Thread Jeff King
On Wed, Mar 19, 2014 at 01:38:32PM +0100, David Kastrup wrote:

 The default of 16MiB causes serious thrashing for large delta chains
 combined with large files.

Does it make much sense to bump this without also bumping
MAX_DELTA_CACHE in sha1_file.c? In my measurements of linux.git, bumping
the memory limit did not help much without also bumping the number of
slots.

I guess that just bumping the memory limit would help with repos which
have deltas on large-ish files (whereas the kernel just has a lot of
deltas on a lot of little directories), but I'd be curious how much.

If you have before-and-after numbers for just this patch on some
repository, that would be an interesting thing to put in the commit
message.

-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: [GSoc PATCH 1/3] diff-no-index.c: rename read_directory()

2014-03-20 Thread Eric Sunshine
On Thu, Mar 20, 2014 at 3:52 PM, Andrei Dinu mandrei.d...@gmail.com wrote:
 Avoid the conflict between read_directory() from diff-no-index.c and
 read_directory() from dir.h

This message is lacking a bit of information. Since diff-no-index.c
does not presently include dir.h, the reader is left to wonder why you
need to resolve a non-existent conflict. You should explain that
diff-no-index.c will soon be including dir.h, which is why the
function must be renamed. You might say something like:

A subsequent patch will include dir.h in diff-no-index.c to
access is_dot_or_dotdot(), however, dir.h declares a
read_directory() which conflicts with a (different)
read_directory() defined in diff-no-index.c. Rename
diff-no-index.c's read_directory() to avoid the conflict.

 Signed-off-by: Andrei Dinu mandrei.d...@gmail.com

 ---

  I plan on applying to GSOC 2014

  Submit again on the list for an older bug that I solved, to show you that
  I received your feedback and I reviewed my code, numbering and partitioning
  patches style. Thank you!

Unfortunately, all three patches arrived in a single email message,
which is undesirable. It's a good idea to try sending patches to
yourself first (using git send-email) to make sure that everything
comes through okay; then send to the list.

 diff --git a/diff-no-index.c b/diff-no-index.c
 index 8e10bff..5e4a76c 100644
 --- a/diff-no-index.c
 +++ b/diff-no-index.c
 @@ -16,7 +16,7 @@
  #include builtin.h
  #include string-list.h

 -static int read_directory(const char *path, struct string_list *list)
 +static int read_directory_path(const char *path, struct string_list *list)
  {
 DIR *dir;
 struct dirent *e;
 @@ -107,9 +107,9 @@ static int queue_diff(struct diff_options *o,
 int i1, i2, ret = 0;
 size_t len1 = 0, len2 = 0;

 -   if (name1  read_directory(name1, p1))
 +   if (name1  read_directory_path(name1, p1))
 return -1;
 -   if (name2  read_directory(name2, p2)) {
 +   if (name2  read_directory_path(name2, p2)) {

The patch looks reasonable. The name is a bit odd, but not terribly important.

 string_list_clear(p1, 0);
 return -1;
 }
 --
 1.7.9.5


 From d54129eacb45b307dacf2b7afebd1da40df17047 Mon Sep 17 00:00:00 2001
 From: Andrei Dinu mandrei.d...@gmail.com
 Date: Wed, 19 Mar 2014 17:42:08 +0200
 Subject: [GSoc PATCH 2/3] diff-no-index.c: read_directory_path() use
  is_dot_or_dotdot().

You can drop the . from the end of this line.

 Implement code so read_directory_path() use is_dot_or_dotdot() from dir.h
 instead of strcmp().

You're not really implementing any code, so this is a bit misleading,
and you don't need to explain that it's from dir.h since that's
obvious from the fact that you are including dir.h You might say
instead:

Use is_dot_or_dotdot() instead of manually checking against .
or ...

 Signed-off-by: Andrei Dinu mandrei.d...@gmail.com

 ---

  I plan on applying to GSOC 2014
  diff-no-index.c |3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/diff-no-index.c b/diff-no-index.c
 index 5e4a76c..2d1165f 100644
 --- a/diff-no-index.c
 +++ b/diff-no-index.c
 @@ -15,6 +15,7 @@
  #include log-tree.h
  #include builtin.h
  #include string-list.h
 +#include dir.h

  static int read_directory_path(const char *path, struct string_list *list)
  {
 @@ -25,7 +26,7 @@ static int read_directory_path(const char *path, struct 
 string_list *list)
 return error(Could not open directory %s, path);

 while ((e = readdir(dir)))
 -   if (strcmp(., e-d_name)  strcmp(.., e-d_name))
 +   if (!is_dot_or_dotdot(e-d_name))

Looks fine.

 string_list_insert(list, e-d_name);

 closedir(dir);
 --
 1.7.9.5


 From 4843ad23675047163211c68434517c097435ebb7 Mon Sep 17 00:00:00 2001
 From: Andrei Dinu mandrei.d...@gmail.com
 Date: Wed, 19 Mar 2014 18:01:55 +0200
 Subject: [GSoc PATCH 3/3] fsck.c: fsck_tree() now use is_dot_or_dotdot().

Drop now and the ..

 Rewrite fsck_tree() to use is_dot_or_dotdot() from dir.h instead
 of calling twice strcmp().

No need to explain that it is from dir.h.

 Signed-off-by: Andrei Dinu mandrei.d...@gmail.com

 ---

  I plan on applying to GSOC 2014.

  fsck.c |   11 +++
  1 file changed, 7 insertions(+), 4 deletions(-)

 diff --git a/fsck.c b/fsck.c
 index 64bf279..82a55ab 100644
 --- a/fsck.c
 +++ b/fsck.c
 @@ -6,6 +6,7 @@
  #include commit.h
  #include tag.h
  #include fsck.h
 +#include dir.h

  static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data)
  {
 @@ -171,10 +172,12 @@ static int fsck_tree(struct tree *item, int strict, 
 fsck_error error_func)
 has_full_path = 1;
 if (!*name)
 has_empty_name = 1;
 -   if (!strcmp(name, .))
 - 

Re: [PATCHv2] fsck.c:fsck_commit() use skip_prefix() and starts_with()

2014-03-20 Thread Eric Sunshine
On Wed, Mar 19, 2014 at 9:44 AM, Othman Darraz darraz...@gmail.com wrote:
 Subject: fsck.c:fsck_commit() use skip_prefix() and starts_with()

Add a space after the colon. Add a colon after fsck_commit().

 make buffer const char* because the content of the memory referenced by this 
 variable doesn't change in this function.

Wrap lines to 65-70 characters. Capitalize start of sentence. You
don't need to spell out 'const char*'. It's okay to say that you're
just making it 'const'.

Conceptually, this is a distinct change, so it deserves its own patch.
Thus, your submission should become a two-patch series. See
documentation for git format-patch and git send-email to learn how
to create a multi-part patch series.

 Add cast to buffer to match fsck_ident signature which is (char**,...)

But, is _this_ cast really needed? In my earlier review, I asked if
you could drop the cast you had in that attempt by making 'buffer'
const. I also mentioned that you might have to fix some small fallout
from making it const. That was a hint that you should dig a bit
deeper. It is possible to eliminate the casts. See if you can figure
out how.

 use skip_prefix() instead of memcmp() to avoid using magic number.

Capitalize start of sentence. You might want to explain a bit that
skip_prefix() conveys more clearly than memcmp() that the code is
checking the string for a prefix. An added benefit is that it allows
you to eliminate some magic numbers.

 Signed-off-by: Othman Darraz darraz...@gmail.com
 ---
 I am planning to apply for GSOC2014
  fsck.c | 27 ++-
  1 file changed, 14 insertions(+), 13 deletions(-)

 diff --git a/fsck.c b/fsck.c
 index 5eae856..128d426 100644
 --- a/fsck.c
 +++ b/fsck.c
 @@ -284,21 +284,22 @@ static int fsck_ident(char **ident, struct object *obj, 
 fsck_error error_func)

  static int fsck_commit(struct commit *commit, fsck_error error_func)
  {
 -   char *buffer = commit-buffer;
 +   const char *buffer = commit-buffer;
 unsigned char tree_sha1[20], sha1[20];
 struct commit_graft *graft;
 int parents = 0;
 int err;
 -
 -   if (starts_with(buffer, tree ))
 +   buffer = skip_prefix(buffer, tree );

Unfortunately, this patch is quite broken since it is based upon your
previous patch rather than upon a clean copy of fsck.c. I'll abort the
review at this point since this and your previous attempt are
intermingled, and the patch does not provide a clear picture of how
the original fsck.c gets changed.

 +   if (!buffer)
 return error_func(commit-object, FSCK_ERROR, invalid 
 format - expected 'tree' line);
 -   if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')()
 +   if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n')
 return error_func(commit-object, FSCK_ERROR, invalid 
 'tree' line format - bad sha1);
 -   buffer += 46;
 -   while (!memcmp(buffer, parent , 7)) {
 -   if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\n')
 +   buffer += 41;
 +   while (starts_with(buffer, parent )) {
 +   buffer = skip_prefix(buffer, parent );
 +   if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n')
 return error_func(commit-object, FSCK_ERROR, 
 invalid 'parent' line format - bad sha1);
 -   buffer += 48;
 +   buffer += 41;
 parents++;
 }
 graft = lookup_commit_graft(commit-object.sha1);
 @@ -322,16 +323,16 @@ static int fsck_commit(struct commit *commit, 
 fsck_error error_func)
 if (p || parents)
 return error_func(commit-object, FSCK_ERROR, 
 parent objects missing);
 }
 -   if (starts_with(buffer, author ))
 +   buffer = skip_prefix(buffer, author );
 +   if (!buffer)
 return error_func(commit-object, FSCK_ERROR, invalid 
 format - expected 'author' line);
 -   buffer += 7;
 -   err = fsck_ident(buffer, commit-object, error_func);
 +   err = fsck_ident((char **)buffer, commit-object, error_func);
 if (err)
 return err;
 -   buffer = (char* )skip_prefix(buffer,committer );
 +   buffer = skip_prefix(buffer,committer );
 if (!buffer)
 return error_func(commit-object, FSCK_ERROR, invalid 
 format - expected 'committer' line);
 -   err = fsck_ident(buffer, commit-object, error_func);
 +   err = fsck_ident((char **)buffer, commit-object, error_func);
 if (err)
 return err;
 if (!commit-tree)
 --
 1.9.0.258.g00eda23.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


Re: [PATCHv2] branch.c: simplify chain of if statements

2014-03-20 Thread Eric Sunshine
On Wed, Mar 19, 2014 at 7:12 PM, Dragos Foianu dragos.foi...@gmail.com wrote:
 Eric Sunshine sunshine at sunshineco.com writes:
 On Tue, Mar 18, 2014 at 6:31 PM, Eric Sunshine sunshine at
 sunshineco.com wrote:

 One other observation: You have a one-off error in your out-of-bounds
 check. It should be 'index = sizeof...'

 Well this is embarrassing.

It's a good illustration of the value of the review process. It's easy
to overlook omissions and problems in our one's work because one reads
it with the bias of knowing what it's _supposed_ to say. Reviewers
(hopefully) don't have such bias: they read the code afresh.

 Thank you again for the feedback. It's incredibily helpful and I learned a
 lot from submitting these patches. Making the code simple is harder than it
 appears at first sight.

 I'm not sure it's worth pursuing the table approach further, especially
 since a solution has already been accepted and merged into the codebase.

Agreed.

 In this case, is it okay to try another microproject? I was thinking about
 trying #17 (finding bugs/inefficiencies in builtin/apply.c), but I've
 already had my one micro project.

According to the description for #17, there are plenty of opportunities, so...

 All the best,
 Dragos
--
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: [PATCHv2] branch.c: simplify chain of if statements

2014-03-20 Thread Eric Sunshine
On Thu, Mar 20, 2014 at 8:40 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Wed, Mar 19, 2014 at 7:12 PM, Dragos Foianu dragos.foi...@gmail.com 
 wrote:
 Eric Sunshine sunshine at sunshineco.com writes:
 On Tue, Mar 18, 2014 at 6:31 PM, Eric Sunshine sunshine at
 sunshineco.com wrote:

 One other observation: You have a one-off error in your out-of-bounds
 check. It should be 'index = sizeof...'

 Well this is embarrassing.

 It's a good illustration of the value of the review process. It's easy
 to overlook omissions and problems in our one's work because one reads
 it with the bias of knowing what it's _supposed_ to say. Reviewers
 (hopefully) don't have such bias: they read the code afresh.

And, this is a perfect example. I knew that I wanted to say problems
in one's own work, and even though I proof-read, I still missed that
I wrote problems in our one's work.
--
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] disable grafts during fetch/push/bundle

2014-03-20 Thread Jeff King
On Wed, Mar 19, 2014 at 03:39:42PM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  On Fri, Mar 07, 2014 at 08:08:37AM +0100, Christian Couder wrote:
 
   Be it graft or replace, I do not think we want to invite people to
   use these mechansims too lightly to locally rewrite their history
   willy-nilly without fixing their mistakes at the object layer with
   commit --amend, rebase, bfg, etc. in the longer term.  So in
   that sense, adding a command to make it easy is not something I am
   enthusiastic about.
  
   On the other hand, if the user does need to use graft or replace
   (perhaps to prepare for casting the fixed history in stone with
   filter-branch), it would be good to help them avoid making mistakes
   while doing so and tool support may be a way to do so.
  
   So, ... I am of two minds.
  ...
  I do not think the features we are talking about are significantly more
  dangerous than git replace is in the first place. If we want to make
  people aware of the dangers, perhaps git-replace.1 is the right place to
  do it.
 
 Sure.
 
 So should we take the four-patch series for git replace --edit?

I think that is certainly going in the right direction, but it is
missing documentation and tests still. Aside from a one-liner bug (which
Christian pointed out on the list), I do not think it will _hurt_
anybody. But it probably should be finished before seeing the light of
day. I'd be happy if you wanted to pick it up for pu or even next
waiting and do that finishing in-tree.

Otherwise, I may eventually get to it and re-roll the whole completed
series.

-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 v3 2/2] log: add --show-linear-break to help see non-linear history

2014-03-20 Thread Duy Nguyen
On Fri, Mar 21, 2014 at 2:15 AM, Junio C Hamano gits...@pobox.com wrote:
  * Get rid of saved_linear, use another flag in struct object instead

 I cannot offhand say if I like this change or not.  A flag bit is a
 scarce and limited resource; commit slabs felt more suited for
 implementation of corner case eye-candies.

My thinking was like this:

OK an int for a flag is wasteful and Junio suggested that unsigned
char is used. But that still wastes 7 bits. So what if I rename it to
commit_flags and make it usable as a flag storage for other parts as
well? Wait don't we have some flags in struct object#flags. It turns
out we have _7_ flags left that nobody touches. Let's take one. If we
run out of flags in future, we can bring back commit_flags slab,
rearrange the flags and move rarely used ones to commit_flags.
-- 
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: [PATCH] Enable index-pack threading in msysgit.

2014-03-20 Thread Duy Nguyen
On Fri, Mar 21, 2014 at 4:56 AM, Stefan Zager sza...@chromium.org wrote:
 Considering all that, Duy's solution of opening separate file descriptors 
 per thread seems to be the best pattern for future multi-threaded work.

 Does that mean you would endorse the (N threads) * (M pack files)
 approach to threading checkout and status?  That seems kind of
 crazy-town to me.  Not to mention that pack windows are not shared, so
 this approach to multi-threading can have the side-effect of blowing
 out memory consumption.

Maybe we could protect and share the delta cache. Pack windows are
mmap'd so we should not need to worry about their memory consumption.

 We have already had to dial back settings for
 pack.threads and core.deltaBaseCacheLimit, because threaded index-pack
 was causing OOM errors on 32-bit platforms.

Hm.. I don't think index-pack uses sha1_file.c heavily. Local pack
access is only needed for verifying identical objects (and that should
never happen often). Something is fishy with these OOM errors.

 Cygwin (and MSVC) should be able to share a mostly compliant pread
 implementation.  I don't have any insight into NonstopKernel; does is
 really not have a thread-safe pread implementation?  If so, then I
 suppose we have to #ifdef NO_PREAD, just as we do now.

 I realize that these are deep changes.  However, the performance of
 msysgit on the chromium repositories is pretty awful, enough so to
 motivate this work.
-- 
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: [PATCH] Enable index-pack threading in msysgit.

2014-03-20 Thread Duy Nguyen
On Thu, Mar 20, 2014 at 11:08 PM, Stefan Zager sza...@chromium.org wrote:
 Duy, would you like to re-post your patch without the new pread 
 implementation?

I will but let me try out the sliding window idea first. My quick
tests on git.git show me we may only need 21k mmap instead of 177k
pread. That hints some potential performance improvement.
-- 
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


  1   2   >