Re: git 1.9.1 tarball
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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.
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.
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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!
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()
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
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.
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.
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()
Дилян Палаузов 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.
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
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.
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.
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)
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
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
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
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)
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
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)
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
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)
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
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
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
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
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
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
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
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
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
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_*
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
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
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
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
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
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
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
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
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
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
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()
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()
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
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
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
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
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.
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.
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