Re: [PATCH] attempt connects in parallel for IPv6-capable builds
Am 29.01.2016 um 02:41 schrieb Eric Wong: Junio C Hamano wrote: Eric Wong writes: getaddrinfo() may return multiple addresses, not all of which are equally performant. In some cases, a user behind a non-IPv6 capable network may get an IPv6 address which stalls connect(). Instead of waiting synchronously for a connect() to timeout, use non-blocking connect() in parallel and take the first successful connection. This may increase network traffic and server load slightly, but makes the worst-case user experience more bearable when one lacks permissions to edit /etc/gai.conf to favor IPv4 addresses. Umm. I am not sure what to think about this change--I generally do not like a selfish "I'll try to use whatever resource given to me to make my process go faster, screw the rest of the world" approach and I cannot decide if this falls into that category. I'll wait for opinions from others. No problem, I can also make it cheaper for servers to handle aborted connections in git-daemon: standalone: 1) use recv with MSG_PEEK or FIONREAD to determine if there's readable data in the socket before forking (and avoid forking for zero-bytes-written connections) 2) use TCP_DEFER_ACCEPT in Linux and dataready filter in FreeBSD for standalone git-daemon to delay accept() inetd: 3) suppress die("The remote end hung up unexpectedly") if no bytes are read at all At some point in the future, I would love to have git-daemon implement something like IDLE in IMAP (to avoid having clients poll for updates). Perhaps the standalone changes above would make sense there, too. Before you submit a patch in that direction (or resubmit the patch under discussion here), could you please find someone to test your patch on Windows first? A lot of the infrastructure mentioned may not be available there or may not work as expected. (I admit that I'm just hand-waving, I haven't tested your patch.) Thanks, -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
GIT 2.7.0 is listed on Software Informer
Good day! Software.informer.com would like to inform you that your product GIT 2.7.0 has been reviewed by our editors and your program got "100% Clean Award" http://git.software.informer.com/. We would be grateful if you place our award with a link to our review on your website. On our part, we can offer featuring your program in our Today's Highlight block. This block is shown in the rotator at the top of the main page and also on every page of our website in the upper right corner. We also offer you to take advantage of our free storage by hosting your installation package on our servers and listing us as one of the mirror downloads for your application. There is a selection of predesigned buttons available to fit the look of your website. Please let me know if you're interested in any of these offers. We are on the list of the world's 500 most visited websites with over 700,000 unique visitors every day, so this could get your application some extra exposure. Kind regards, Kasey Bloome -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 04/10] grep/icase: avoid kwsset on literal non-ascii strings
On Fri, Jan 29, 2016 at 1:18 AM, Eric Sunshine wrote: > On Thu, Jan 28, 2016 at 6:56 AM, Nguyễn Thái Ngọc Duy > wrote: >> When we detect the pattern is just a literal string, we avoid heavy >> regex engine and use fast substring search implemented in kwsset.c. >> But kws uses git-ctype which is locale-independent so it does not know >> how to fold case properly outside ascii range. Let regcomp or pcre >> take care of this case instead. Slower, but accurate. >> >> Signed-off-by: Nguyễn Thái Ngọc Duy >> --- >> diff --git a/grep.c b/grep.c >> @@ -398,12 +399,16 @@ static int is_fixed(const char *s, size_t len) >> static void compile_regexp(struct grep_pat *p, struct grep_opt *opt) >> { >> + int icase_non_ascii; >> int err; >> >> p->word_regexp = opt->word_regexp; >> p->ignore_case = opt->ignore_case; >> + icase_non_ascii = >> + (opt->regflags & REG_ICASE || p->ignore_case) && >> + has_non_ascii(p->pattern); >> >> - if (is_fixed(p->pattern, p->patternlen)) >> + if (!icase_non_ascii && is_fixed(p->pattern, p->patternlen)) > > These double negatives (!non_ascii) here and in patch 5/10 are > difficult to grok. I wonder if a different name, such as > icase_ascii_only would help. By the way, this is such a minor issue, and since there are only two cases (in patches 4/10 and 5/10), it's not worth worrying about, and certainly not worth a reroll. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 10/10] diffcore-pickaxe: support case insensitive match on non-ascii
On Thu, Jan 28, 2016 at 6:56 AM, Nguyễn Thái Ngọc Duy wrote: > Similar to the "grep -F -i" case, we can't use kws on icase search > outside ascii range, so we quote the string and pass it to regcomp as > a basic regexp and let regex engine deal with case sensitivity. > > The new test is put in t7812 instead of t4209-log-pickaxe because > lib-gettext.sh might cause problems elsewhere, probably.. s/\.\.$/./ > Noticed-by: Plamen Totev > Signed-off-by: Nguyễn Thái Ngọc 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 v5 05/10] grep/icase: avoid kwsset when -F is specified
On Thu, Jan 28, 2016 at 6:56 AM, Nguyễn Thái Ngọc Duy wrote: > Similar to the previous commit, we can't use kws on icase search > outside ascii range. But we can't simply pass the pattern to > regcomp/pcre like the previous commit because it may contain regex > special characters, so we need to quote the regex first. > > To avoid misquote traps that could lead to undefined behavior, we > always stick to basic regex engine in this case. We don't need fancy > features for grepping a literal string anyway. > > basic_regex_quote_buf() assumes that if the pattern is in a multibyte > encoding, ascii chars must be unambiguously encoded as single > bytes. This is true at least for UTF-8. For others, let's wait until > people yell up. Chances are nobody uses multibyte, non utf-8 charsets > any more.. s/any more../anymore./ > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > diff --git a/grep.c b/grep.c > @@ -397,6 +398,24 @@ static int is_fixed(const char *s, size_t len) > +static void compile_fixed_regexp(struct grep_pat *p, struct grep_opt *opt) > +{ > + struct strbuf sb = STRBUF_INIT; > + int err; > + > + basic_regex_quote_buf(&sb, p->pattern); > + err = regcomp(&p->regexp, sb.buf, opt->regflags & ~REG_EXTENDED); > + if (opt->debug) > + fprintf(stderr, "fixed%s\n", sb.buf); Did you want a space or colon or something after "fixed" for human consumption? (I realize that the test case doesn't care.) > + strbuf_release(&sb); > + if (err) { > + char errbuf[1024]; > + regerror(err, &p->regexp, errbuf, 1024); I guess this was copy/pasted from compile_regexp(), but for this new code, perhaps: s/1024/sizeof(errbuf)/ > + regfree(&p->regexp); > + compile_regexp_failed(p, errbuf); > + } > +} -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 04/10] grep/icase: avoid kwsset on literal non-ascii strings
On Thu, Jan 28, 2016 at 6:56 AM, Nguyễn Thái Ngọc Duy wrote: > When we detect the pattern is just a literal string, we avoid heavy > regex engine and use fast substring search implemented in kwsset.c. > But kws uses git-ctype which is locale-independent so it does not know > how to fold case properly outside ascii range. Let regcomp or pcre > take care of this case instead. Slower, but accurate. > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > diff --git a/grep.c b/grep.c > @@ -398,12 +399,16 @@ static int is_fixed(const char *s, size_t len) > static void compile_regexp(struct grep_pat *p, struct grep_opt *opt) > { > + int icase_non_ascii; > int err; > > p->word_regexp = opt->word_regexp; > p->ignore_case = opt->ignore_case; > + icase_non_ascii = > + (opt->regflags & REG_ICASE || p->ignore_case) && > + has_non_ascii(p->pattern); > > - if (is_fixed(p->pattern, p->patternlen)) > + if (!icase_non_ascii && is_fixed(p->pattern, p->patternlen)) These double negatives (!non_ascii) here and in patch 5/10 are difficult to grok. I wonder if a different name, such as icase_ascii_only would help. > p->fixed = 1; > else if (opt->fixed) { > p->fixed = 1; -- To unsubscribe from this list: send the line "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: Bugs in git filter-branch (git replace related)
On Thu, Jan 28, 2016 at 02:46:40PM +, Anatoly Borodin wrote: > The `git replace` makes the second commit empty (use the file content from > the first commit). It should disappear after `git filter-branch`, but it > doesn't happen. > > Bug 1: the empty commit stays. I'm not sure if this is a bug or not. The "empty commit" check works by checking the tree sha1s, without doing a full diff respecting replace refs. You're expecting git to notice a tree change, even though it never even examined the tree in the first place (because you didn't give it a tree or index filter). Try: git filter-branch --prune-empty --tree-filter true master which will force git to go through the motions of checking out the replaced content and re-examining it. This will run much more slowly, as it actually touches the filesystem. In the general case, it would be interesting if filter-branch (or a similar tool) could "cement" replacement objects into place as efficiently as possible. But I'm not sure whether that should be the default mode for filter-branch. > Bug 2: the replace refs are not ignored (they can epresent blobs, trees etc, > but even if they represent commits - should they be rewritten?). You told it "--all", which is passed to rev-list, where it means "all refs". I agree that running filter-branch on refs/replace is probably not going to yield useful results, but I'm not sure if it is filter-branch's responsibility to second-guess the rev-list options. Probably the documentation for filter-branch should recommend "--branches --tags" instead of "--all", though. -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] attempt connects in parallel for IPv6-capable builds
On 2016-01-29 04.04, Junio C Hamano wrote: > Eric Wong writes: > >> getaddrinfo() may return multiple addresses, not all of which >> are equally performant. In some cases, a user behind a non-IPv6 >> capable network may get an IPv6 address which stalls connect(). > > I'd assume that you are not solving a hypothetical problem, but you > may (at least sometimes) have to reach outside world from such a > network environment. I further assume that git_tcp_connect() is not > the only activity you do from such a network, and other network > activities are similarly affected. > > How do you work around the same issue for connections that do not go > through git_tcp_connect()? The same issue would affect Git traffic > going over git-remote-curl, and also your usual Web browser traffic, > no? > > What I am getting at is if it is saner to solve the issue like how > curl(1) solves it with its -4/-6 command line options, e.g. by > adding a pair of configuration variables "net.ipv[46] = true/false". (Please don't do parallel connects as a default) I like the -4 / -6 Out of my head these kind of setups are not desired. (getaddrinfo returns an address and the the following connect fails) But it may be hard to fully avoid them in practice, and some RFC have been written, this may be a starting point: https://www.rfc-editor.org/rfc/rfc5014.txt -- To unsubscribe from this list: send the line "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: fast-import fails in read-only tree
On Thu, Jan 28, 2016 at 05:17:36PM -0500, Stefan Monnier wrote: > I recently discovered that "git fast-import" signals an error if used in > a tree to which we do not have write-access, because it tries to create > a "objects/pack/tmp_pack_XXX" file even before starting to process > the commands. > > Usually this is not a problem (we'll create new commits and such, so > write-access is indeed necessary), but in my case I was using > fast-import only for its "reading" operations (in order to combine > several inter-dependent "cat-file" operations into a single git > session). The primary goal of fast-import is to write that packfile. It kind of sounds like you are using the wrong tool for the job. Can you elaborate on what you are sending to fast-import (preferably with a concrete example)? There may be a way to accomplish the same thing with read-only tools like cat-file. -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 v5 03/10] test-regex: expose full regcomp() to the command line
On Thu, Jan 28, 2016 at 06:56:16PM +0700, Nguyễn Thái Ngọc Duy wrote: > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > diff --git a/test-regex.c b/test-regex.c > @@ -1,19 +1,63 @@ > int main(int argc, char **argv) > { > - char *pat = "[^={} \t]+"; > - char *str = "={}\nfred"; > + const char *pat; > + const char *str; > + int flags = 0; > regex_t r; > regmatch_t m[1]; > > - if (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE)) > + if (argc == 1) { > + /* special case, bug check */ > + pat = "[^={} \t]+"; > + str = "={}\nfred"; > + flags = REG_EXTENDED | REG_NEWLINE; > + } else { > + argv++; > + pat = *argv++; > + str = *argv++; I realize that this is just a test program, but it might be a good idea to insert: if (argc < 3) die("usage: ..."); prior to the *argv++ dereferences to give a controlled failure rather than an outright crash when an incorrect number of arguments is given. More below... > + while (*argv) { > + struct reg_flag *rf; > + for (rf = reg_flags; rf->name; rf++) > + if (!strcmp(*argv, rf->name)) { > + flags |= rf->flag; > + break; > + } > + if (!rf->name) > + die("do not recognize %s", *argv); > + argv++; > + } > + git_setup_gettext(); > + } > + > + if (regcomp(&r, pat, flags)) > die("failed regcomp() for pattern '%s'", pat); > - if (regexec(&r, str, 1, m, 0)) > - die("no match of pattern '%s' to string '%s'", pat, str); > + if (regexec(&r, str, 1, m, 0)) { > + if (argc == 1) > + die("no match of pattern '%s' to string '%s'", pat, > str); > + return 1; > + } > > /* http://sourceware.org/bugzilla/show_bug.cgi?id=3957 */ > - if (m[0].rm_so == 3) /* matches '\n' when it should not */ > + if (argc == 1 && m[0].rm_so == 3) /* matches '\n' when it should not */ > die("regex bug confirmed: re-build git with NO_REGEX=1"); Again, I realize that this is just a test program, but sprinkling this 'argc == 1' special case throughout the code makes it unnecessarily difficult to follow. Some alternatives: 1. Rename the existing test-regex to test-regex-bug (or test-regex-bugs), and then name the new general purpose program test-regex. 2. Drop the special case altogether and have the program emit the matched text on stdout (in addition to the exit code indicating success/failure). Most callers will care only about the exit status, but the one special case in t0070 which wants to check for the glibc bug can do so itself: test_expect_success 'check for a bug in the regex routines' ' # if this test fails, re-build git with NO_REGEX=1 printf "fred" >expect && test-regex "[^={} \t]+" "={}\nfred" EXTENDED NEWLINE >actual && test_cmp expect actual ' Of course, that doesn't actually work because "\n" in the 'str' argument isn't really a newline, so test-regex would have to do a bit of preprocessing of 'str' first (which might be as simple as calling unquote_c_style() or something). 3. [less desirable] Move the 'argc == 1' special case to its own function, which will result in a bit of duplicated code, but the result should at least be easier to follow. > exit(0); > -- > 2.7.0.288.g1d8ad15 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
NEW ARRIVALS, CISCO,CPU's,Memory, LAPTOP AND AVAYA
Dear Sir/Madam, Clean tested working pulls CPUs and QTYs in stock. 115 X X5650 65 X E5410 75 X X5660 145 X E5530 100 X E5645 40 X X5680 75 X X5690 Brand new sealed IP phones and QTYs in stock. 55 x CP-7937G 77 x CP-7942G 54 x CP-7945G 75 x CP-7962G .. 45 x Avaya 9630 65 x Avaya 9641 55 x Avaya 9640 All NIB. Here is our current stock list. SSD drives and 750 gig 2.5" sata drives We also have 250 x i5 dell 133 x HP 360 x Lenevo all grade A Here is the qty in stock for laptops 150 x E6500 Dell Latitude E6500 Core 2 Duo 2.80GHz T9600 4GB 160GB DVD+/-RW Win 7 60 x E6510 Dell Ltitude E6510. 15.6" Intel Core i5- M520 @ 2.40GHz. 4GB. 320GB DVD+/-RW Win 7 30 X 8530P HP EliteBook 8530p 15.4" Laptop 2.53GHz Core 2 Duo T9400, 4GB RAM, 160GB HDD, Win 100 x 8740W HP EliteBook 8740w 17" 2.4Ghz Core i5 6GB 250GB Win 7 Pr 45 x 8760W HP EliteBook 8760W 17.3" 2nd Gen Intel Core i7 2.70GHz 8GB 320GB Windows 7 Pro 50 x DELL 6520Dell latitude e6520 15.6" i5-2520M 2.5Ghz, 8GB Ram, 500GB HD Win 7 @ $115 120 x DELL 6420 Laptop Dell Latitude E6420 14" Intel Core i5 2.4Ghz 4GB RAM 250GB HDD DVD Win 7 150 x T410 Lenovo ThinkPad Laptop T410s 14.1" 2.40GHz Core i5 4GB RAM 160GB Win 7 180 x 8440P HP EliteBook 8440p 14" M520 Core i5 2.4GHz 4GB 250GB Win 7 Let me know if you're interested. We are very open to offers and willing to work with you to make sure that we have a deal. Thank You Barbara Johnson Laison Computech Inc 210 N Scoring Ave, Rialto California, 92376 Tel: +1-657-232-7047 Fax: +1-347-214-047 sa...@laisoncomputertech.us -- To unsubscribe from this list: send the line "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] pass transport verbosity down to git_connect
On Thu, Jan 28, 2016 at 07:19:30PM -0800, Junio C Hamano wrote: > I just reviewed the output that are protected by CONNECT_VERBOSE; > they look somewhere between pure debugging aid (like the protocol > dump that are shown by "fetch -vv") and progress display, and at > least to me they are much closer to the latter than the former, in > the sense that they are not _so_ annoying as the protocol dump that > are clearly not meant for the end users, and that they say "I am > looking up this host's address", "Now connecting to this host:port", > etc. > > So, I personally find this addtional output not _too_ bad if we give > it with "fetch -v" (not limiting to "fetch -vv"). Yeah, I do not feel that strongly, and am OK if it is attached to a single "-v". I don't think we make any promises about scraping stderr, so it is really just about end-user experience. It is mostly just my gut feeling on what I'd expect based on other parts of git (especially "fetch -vv" in other circumstances). -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] pass transport verbosity down to git_connect
Eric Wong writes: > On the other hand, I'm not sure if there's anything parsing the stderr > out of "git fetch -v" right now. It would also affect end-user experience, depending on what new pieces of lines are emitted to the terminal. From "git fetch -v", I expect to see the transfer progress and the final listing of fetched and updated refs, and as long as the line "remote: Compressing objects" and the lines that follow do not get garbled, I would imagine it would be fine. ... remote: Compressing objects: 100% (1195/1195), done. remote: Total 1869 (delta 1551), reused 841 (delta 672) Receiving objects: 100% (1869/1869), 462.80 KiB | 0 bytes/s, done. Resolving deltas: 100% (1551/1551), completed with 335 local objects. From $over_there * branchpu -> FETCH_HEAD > In that case, perhaps only changing > "-vv" (and documenting it) is a better idea. I just reviewed the output that are protected by CONNECT_VERBOSE; they look somewhere between pure debugging aid (like the protocol dump that are shown by "fetch -vv") and progress display, and at least to me they are much closer to the latter than the former, in the sense that they are not _so_ annoying as the protocol dump that are clearly not meant for the end users, and that they say "I am looking up this host's address", "Now connecting to this host:port", etc. So, I personally find this addtional output not _too_ bad if we give it with "fetch -v" (not limiting to "fetch -vv"). I dunno. -- To unsubscribe from this list: send the line "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] stripspace: Call U+0020 a "space" instead of a "blank"
I couldn't find any other examples of people referring to this character as a "blank". Signed-off-by: Alex Henrie --- builtin/stripspace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/stripspace.c b/builtin/stripspace.c index 7ff8434..15e716e 100644 --- a/builtin/stripspace.c +++ b/builtin/stripspace.c @@ -35,7 +35,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix) N_("skip and remove all lines starting with comment character"), STRIP_COMMENTS), OPT_CMDMODE('c', "comment-lines", &mode, - N_("prepend comment character and blank to each line"), + N_("prepend comment character and space to each line"), COMMENT_LINES), OPT_END() }; -- 2.7.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] blame: display a more helpful error message if the file was deleted
Sorry, wrong patch...this issue has already been fixed -Alex -- To unsubscribe from this list: send the line "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] blame: display a more helpful error message if the file was deleted
`git blame 22414770 generate-cmdlist.perl` currently results in: fatal: cannot stat path '22414770': No such file or directory This patch changes the error message to: fatal: ambiguous argument 'generate-cmdlist.perl': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git [...] -- [...]'" That way, the user knows to rewrite the command as `git blame 22414770 -- generate-cmdlist.perl`. Signed-off-by: Alex Henrie --- builtin/blame.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/builtin/blame.c b/builtin/blame.c index 55bf5fa..9461a73 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -2704,8 +2704,6 @@ parse_done: argv[argc - 1] = "--"; setup_work_tree(); - if (!file_exists(path)) - die_errno("cannot stat path '%s'", path); } revs.disable_stdin = 1; -- 2.7.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] attempt connects in parallel for IPv6-capable builds
Eric Wong writes: > getaddrinfo() may return multiple addresses, not all of which > are equally performant. In some cases, a user behind a non-IPv6 > capable network may get an IPv6 address which stalls connect(). I'd assume that you are not solving a hypothetical problem, but you may (at least sometimes) have to reach outside world from such a network environment. I further assume that git_tcp_connect() is not the only activity you do from such a network, and other network activities are similarly affected. How do you work around the same issue for connections that do not go through git_tcp_connect()? The same issue would affect Git traffic going over git-remote-curl, and also your usual Web browser traffic, no? What I am getting at is if it is saner to solve the issue like how curl(1) solves it with its -4/-6 command line options, e.g. by adding a pair of configuration variables "net.ipv[46] = true/false". -- To unsubscribe from this list: send the line "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] mergetool: reorder vim/gvim buffers in three-way diffs
When invoking default (g)vimdiff three-way merge, the merged file is loaded as the first buffer but moved to the bottom as the fourth window. This causes a disconnect between vim commands that operate on window positions (e.g. CTRL-W_w) and those that operate on buffer index (e.g. do/dp). This change reorders the buffers to have the same index as windows while keeping the cursor default to the merged result as the bottom window. Signed-off-by: Dickson Wong --- mergetools/vimdiff | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mergetools/vimdiff b/mergetools/vimdiff index 1ddfbfc..74ea6d5 100644 --- a/mergetools/vimdiff +++ b/mergetools/vimdiff @@ -2,22 +2,22 @@ diff_cmd () { "$merge_tool_path" -R -f -d \ -c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE" } merge_cmd () { touch "$BACKUP" case "$1" in gvimdiff|vimdiff) if $base_present then - "$merge_tool_path" -f -d -c 'wincmd J' \ - "$MERGED" "$LOCAL" "$BASE" "$REMOTE" + "$merge_tool_path" -f -d -c '4wincmd w | wincmd J' \ + "$LOCAL" "$BASE" "$REMOTE" "$MERGED" else "$merge_tool_path" -f -d -c 'wincmd l' \ "$LOCAL" "$MERGED" "$REMOTE" fi ;; gvimdiff2|vimdiff2) "$merge_tool_path" -f -d -c 'wincmd l' \ "$LOCAL" "$MERGED" "$REMOTE" ;; gvimdiff3|vimdiff3) -- 2.6.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] attempt connects in parallel for IPv6-capable builds
Junio C Hamano wrote: > Eric Wong writes: > > > getaddrinfo() may return multiple addresses, not all of which > > are equally performant. In some cases, a user behind a non-IPv6 > > capable network may get an IPv6 address which stalls connect(). > > Instead of waiting synchronously for a connect() to timeout, use > > non-blocking connect() in parallel and take the first successful > > connection. > > > > This may increase network traffic and server load slightly, but > > makes the worst-case user experience more bearable when one > > lacks permissions to edit /etc/gai.conf to favor IPv4 addresses. > > Umm. I am not sure what to think about this change--I generally do > not like a selfish "I'll try to use whatever resource given to me > to make my process go faster, screw the rest of the world" approach > and I cannot decide if this falls into that category. > > I'll wait for opinions from others. No problem, I can also make it cheaper for servers to handle aborted connections in git-daemon: standalone: 1) use recv with MSG_PEEK or FIONREAD to determine if there's readable data in the socket before forking (and avoid forking for zero-bytes-written connections) 2) use TCP_DEFER_ACCEPT in Linux and dataready filter in FreeBSD for standalone git-daemon to delay accept() inetd: 3) suppress die("The remote end hung up unexpectedly") if no bytes are read at all At some point in the future, I would love to have git-daemon implement something like IDLE in IMAP (to avoid having clients poll for updates). Perhaps the standalone changes above would make sense there, too. -- To unsubscribe from this list: send the line "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] pass transport verbosity down to git_connect
Jeff King wrote: > On Thu, Jan 28, 2016 at 10:51:23PM +, Eric Wong wrote: > > -static int connect_setup(struct transport *transport, int for_push, int > > verbose) > > +static int connect_setup(struct transport *transport, int for_push) > > { > > struct git_transport_data *data = transport->data; > > + int flags = transport->verbose > 0 ? CONNECT_VERBOSE : 0; > > Do we want to trigger this only for "transport->verbose > 1"? > > Right now, "git fetch -v" gives us a verbose status table (i.e., > includes up-to-date refs), but no more debugging than that. Should we > reserve more debug-ish information like for "git fetch -vv"? I'm not sure, I've never used "-v" at all in the past with fetch. On one hand, I suspect someone who looks up "-v" and uses it is likely wondering: "why is it so slow?" At least, that's what I did before resorting to strace :) On the other hand, I'm not sure if there's anything parsing the stderr out of "git fetch -v" right now. In that case, perhaps only changing "-vv" (and documenting it) is a better idea. I've always been of the opinion stderr is for humans and test suites, only; and not considered an interface somebody should be parsing. For reference, "curl -v" includes connection info which I rely on all the time. Junio: I'm leaning towards putting the test into t/t5570-git-daemon.sh to avoid potential port conflicts if that's OK with you. It doesn't seem like we have a lot of fetch tests on the git:// at all. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/10] Fix icase grep on non-ascii
Nguyễn Thái Ngọc Duy writes: > The series fixes grep choosing fast path that only works correctly for > ascii. This is a resend of v4 [1] after rebase. I think v4 just went > off radar for some reason, nothing was wrong with it (or nobody told > me about it). Or nobody found it interesting, perhaps, in which case we cannot say "nothing was wrong" or "nothing was good" with it ;-) Will find time to take a look if nothing more urgent and interesting (read: regression fixes) comes up in the meantime. Thanks. > > [1] http://thread.gmane.org/gmane.comp.version-control.git/273381/focus=276288 > > Nguyễn Thái Ngọc Duy (10): > grep: allow -F -i combination > grep: break down an "if" stmt in preparation for next changes > test-regex: expose full regcomp() to the command line > grep/icase: avoid kwsset on literal non-ascii strings > grep/icase: avoid kwsset when -F is specified > grep/pcre: prepare locale-dependent tables for icase matching > gettext: add is_utf8_locale() > grep/pcre: support utf-8 > diffcore-pickaxe: "share" regex error handling code > diffcore-pickaxe: support case insensitive match on non-ascii > > builtin/grep.c | 2 +- > diffcore-pickaxe.c | 27 > gettext.c| 24 ++- > gettext.h| 1 + > grep.c | 44 ++-- > grep.h | 1 + > quote.c | 37 + > quote.h | 1 + > t/t7812-grep-icase-non-ascii.sh (new +x) | 71 > > t/t7813-grep-icase-iso.sh (new +x) | 19 + > test-regex.c | 56 ++--- > 11 files changed, 262 insertions(+), 21 deletions(-) > create mode 100755 t/t7812-grep-icase-non-ascii.sh > create mode 100755 t/t7813-grep-icase-iso.sh -- To unsubscribe from this list: send the line "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] pass transport verbosity down to git_connect
On Thu, Jan 28, 2016 at 10:51:23PM +, Eric Wong wrote: > While working in connect.c to perform non-blocking connections, > I noticed calling "git fetch -v" was not causing the progress > messages inside git_tcp_connect_sock to be emitted as I > expected. > > Looking at history, it seems connect_setup has never been called > with the verbose parameter. Since transport already has a > "verbose" field, use that field instead of another parameter > in connect_setup. That makes sense, but... > -static int connect_setup(struct transport *transport, int for_push, int > verbose) > +static int connect_setup(struct transport *transport, int for_push) > { > struct git_transport_data *data = transport->data; > + int flags = transport->verbose > 0 ? CONNECT_VERBOSE : 0; Do we want to trigger this only for "transport->verbose > 1"? Right now, "git fetch -v" gives us a verbose status table (i.e., includes up-to-date refs), but no more debugging than that. Should we reserve more debug-ish information like for "git fetch -vv"? -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] convert: legitimately disable clean/smudge filter with an empty override
larsxschnei...@gmail.com writes: > - if (ca.drv) { > + if (ca.drv && ca.drv->smudge && strlen(ca.drv->smudge)) { You are not interested in its length, but if it is an empty string or not, so I'd tweak this like so: > + if (ca.drv && ca.drv->smudge && *ca.drv->smudge) { -- To unsubscribe from this list: send the line "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] pass transport verbosity down to git_connect
Eric Wong writes: > While working in connect.c to perform non-blocking connections, > I noticed calling "git fetch -v" was not causing the progress > messages inside git_tcp_connect_sock to be emitted as I > expected. Nice. Can we demonstrate and protect this fix with simple tests? Thanks. Will queue. > transport.c | 11 ++- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/transport.c b/transport.c > index 67f3666..9ae7184 100644 > --- a/transport.c > +++ b/transport.c > @@ -481,9 +481,10 @@ static int set_git_option(struct git_transport_options > *opts, > return 1; > } > > -static int connect_setup(struct transport *transport, int for_push, int > verbose) > +static int connect_setup(struct transport *transport, int for_push) > { > struct git_transport_data *data = transport->data; > + int flags = transport->verbose > 0 ? CONNECT_VERBOSE : 0; > > if (data->conn) > return 0; > @@ -491,7 +492,7 @@ static int connect_setup(struct transport *transport, int > for_push, int verbose) > data->conn = git_connect(data->fd, transport->url, >for_push ? data->options.receivepack : >data->options.uploadpack, > - verbose ? CONNECT_VERBOSE : 0); > + flags); > > return 0; > } > @@ -501,7 +502,7 @@ static struct ref *get_refs_via_connect(struct transport > *transport, int for_pus > struct git_transport_data *data = transport->data; > struct ref *refs; > > - connect_setup(transport, for_push, 0); > + connect_setup(transport, for_push); > get_remote_heads(data->fd[0], NULL, 0, &refs, >for_push ? REF_NORMAL : 0, >&data->extra_have, > @@ -536,7 +537,7 @@ static int fetch_refs_via_pack(struct transport > *transport, > args.update_shallow = data->options.update_shallow; > > if (!data->got_remote_heads) { > - connect_setup(transport, 0, 0); > + connect_setup(transport, 0); > get_remote_heads(data->fd[0], NULL, 0, &refs_tmp, 0, >NULL, &data->shallow); > data->got_remote_heads = 1; > @@ -812,7 +813,7 @@ static int git_transport_push(struct transport > *transport, struct ref *remote_re > > if (!data->got_remote_heads) { > struct ref *tmp_refs; > - connect_setup(transport, 1, 0); > + connect_setup(transport, 1); > > get_remote_heads(data->fd[0], NULL, 0, &tmp_refs, REF_NORMAL, >NULL, &data->shallow); -- To unsubscribe from this list: send the line "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] attempt connects in parallel for IPv6-capable builds
Eric Wong writes: > getaddrinfo() may return multiple addresses, not all of which > are equally performant. In some cases, a user behind a non-IPv6 > capable network may get an IPv6 address which stalls connect(). > Instead of waiting synchronously for a connect() to timeout, use > non-blocking connect() in parallel and take the first successful > connection. > > This may increase network traffic and server load slightly, but > makes the worst-case user experience more bearable when one > lacks permissions to edit /etc/gai.conf to favor IPv4 addresses. Umm. I am not sure what to think about this change--I generally do not like a selfish "I'll try to use whatever resource given to me to make my process go faster, screw the rest of the world" approach and I cannot decide if this falls into that category. I'll wait for opinions from others. Thanks. > Signed-off-by: Eric Wong > --- > connect.c | 118 > ++ > 1 file changed, 104 insertions(+), 14 deletions(-) > > diff --git a/connect.c b/connect.c > index fd7ffe1..74d2bb5 100644 > --- a/connect.c > +++ b/connect.c > @@ -14,6 +14,42 @@ > static char *server_capabilities; > static const char *parse_feature_value(const char *, const char *, int *); > > +#ifdef SOCK_NONBLOCK /* Linux-only flag */ > +# define GIT_SOCK_NONBLOCK SOCK_NONBLOCK > +#else > +# define GIT_SOCK_NONBLOCK 0 > +#endif > + > +static int socket_nb(int domain, int type, int protocol) > +{ > + static int flags = GIT_SOCK_NONBLOCK; > + int fd = socket(domain, type | flags, protocol); > + > + /* new headers, old kernel? */ > + if (fd < 0 && errno == EINVAL && flags != 0) { > + flags = 0; > + fd = socket(domain, type, protocol); > + } > + > + /* couldn't use SOCK_NONBLOCK, set non-blocking the old way */ > + if (flags == 0 && fd >= 0) { > + int fl = fcntl(fd, F_GETFL); > + > + if (fcntl(fd, F_SETFL, fl | O_NONBLOCK) < 0) > + die_errno("failed to set nonblocking flag\n"); > + } > + > + return fd; > +} > + > +static void set_blocking(int fd) > +{ > + int fl = fcntl(fd, F_GETFL); > + > + if (fcntl(fd, F_SETFL, fl & ~O_NONBLOCK) < 0) > + die_errno("failed to clear nonblocking flag\n"); > +} > + > static int check_ref(const char *name, unsigned int flags) > { > if (!flags) > @@ -351,6 +387,9 @@ static int git_tcp_connect_sock(char *host, int flags) > struct addrinfo hints, *ai0, *ai; > int gai; > int cnt = 0; > + nfds_t n = 0, nfds = 0; > + struct pollfd *fds = NULL; > + struct addrinfo **inprogress = NULL; > > get_host_and_port(&host, &port); > if (!*port) > @@ -371,20 +410,76 @@ static int git_tcp_connect_sock(char *host, int flags) > fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, > port); > > for (ai0 = ai; ai; ai = ai->ai_next, cnt++) { > - sockfd = socket(ai->ai_family, > - ai->ai_socktype, ai->ai_protocol); > - if ((sockfd < 0) || > - (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0)) { > + size_t cur; > + int fd = socket_nb(ai->ai_family, ai->ai_socktype, > + ai->ai_protocol); > + if (fd < 0) { > strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n", > host, cnt, ai_name(ai), strerror(errno)); > - if (0 <= sockfd) > - close(sockfd); > - sockfd = -1; > continue; > } > + > + if (connect(fd, ai->ai_addr, ai->ai_addrlen) < 0 && > + errno != EINPROGRESS) { > + strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n", > + host, cnt, ai_name(ai), strerror(errno)); > + close(fd); > + continue; > + } > + > if (flags & CONNECT_VERBOSE) > - fprintf(stderr, "%s ", ai_name(ai)); > - break; > + fprintf(stderr, "%s (started)\n", ai_name(ai)); > + > + nfds = n + 1; > + cur = n; > + ALLOC_GROW(fds, nfds, cur); > + cur = n; > + ALLOC_GROW(inprogress, nfds, cur); > + inprogress[n] = ai; > + fds[n].fd = fd; > + fds[n].events = POLLIN|POLLOUT; > + fds[n].revents = 0; > + n = nfds; > + } > + > + /* > + * nfds is tiny, no need to limit loop based on poll() retval, > + * just do not let poll sleep forever if nfds is zero > + */ > + if (nfds > 0) > + poll(fds, nfds, -1); > + > + for (n = 0; n < nfds && sockfd < 0; n++) { > + if (fds[n].revents & (POLLERR|POLLHUP))
Re: [PATCH 2/2] stash: use "stash--helper"
On 28 January 2016 at 21:41, Stefan Beller wrote: > On Thu, Jan 28, 2016 at 1:25 PM, Matthias Aßhauer wrote: https://github.com/git/git/pull/191 >>> >>> Oh I see you're using the pull-request to email translator, cool! Yay! >> Yes, I did. It definitly makes things easier if you are not used to mailing >> lists, but it was also a bit of a kerfuffle. I tried to start working on >> coverletter support, but I couldn't get it to accept the amazon SES >> credentials I provided. I ended up manually submiting the coverletter. It >> also didn't like my name. Apologies for that - https://github.com/rtyley/submitgit/pull/26 has just been deployed, which should resolve the encoding for non-US ASCII characters - if you feel like submitting another patch, and want to put the eszett back into your GitHub account display name, I'd be interested to know how that goes. > Not sure if Roberto, the creator of that tool, follows the mailing > list. I cc'd him. I don't closely follow the mailing list, so thanks for the cc! Roberto -- To unsubscribe from this list: send the line "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] push --force-with-lease: Fix ref status reporting
Andrew Wheeler writes: > diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh > index c402d8d..c65033f 100755 > --- a/t/t5533-push-cas.sh > +++ b/t/t5533-push-cas.sh > @@ -101,7 +101,8 @@ test_expect_success 'push to update (allowed, tracking)' ' > ( > cd dst && > test_commit D && > - git push --force-with-lease=master origin master > + git push --force-with-lease=master origin master 2>err && > + ! grep "forced update" err > ) && > git ls-remote dst refs/heads/master >expect && > git ls-remote src refs/heads/master >actual && > @@ -114,7 +115,8 @@ test_expect_success 'push to update (allowed even though > no-ff)' ' > cd dst && > git reset --hard HEAD^ && > test_commit D && > - git push --force-with-lease=master origin master > + git push --force-with-lease=master origin master 2>err && > + grep "forced update" err > ) && > git ls-remote dst refs/heads/master >expect && > git ls-remote src refs/heads/master >actual && > @@ -147,7 +149,8 @@ test_expect_success 'push to delete (allowed)' ' > setup_srcdst_basic && > ( > cd dst && > - git push --force-with-lease=master origin :master > + git push --force-with-lease=master origin :master 2>err && > + grep deleted err > ) && > >expect && > git ls-remote src refs/heads/master >actual && These all look OK (I am not sure about message i18n, though). Do we not test a case where --force-with-lease push is rejected due to REF_STATUS_REJECT_STALE? 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 1/2] stash--helper: implement "git stash--helper"
Matthias Asshauer writes: > From: Matthias Aßhauer > > This patch implements a new "git stash--helper" builtin plumbing > command that will be used to migrate "git-stash.sh" to C. > > We start by implementing only the "--non-patch" option that will > handle the core part of the non-patch stashing. > > Signed-off-by: Matthias Aßhauer > --- > Makefile| 2 ++ > builtin.h | 1 + > builtin/stash--helper.c | 13 ++ > git.c | 1 + > stash.c | 65 > + > stash.h | 11 + Hmph, why not have everything inside builtin/stash--helper.c? I do not quite see a point of having the other two "library-ish" looking files. Also I personally feel that it would be easier to review when these two patches are squashed into one. I had to go back and forth while reading the "non-patch" C function to see what logic from the scripted version it is trying to replace. > diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c > new file mode 100644 > index 000..542e782 > --- /dev/null > +++ b/builtin/stash--helper.c > @@ -0,0 +1,13 @@ > +#include "../stash.h" > +#include > + > +static const char builtin_stash__helper_usage[] = { > + "Usage: git stash--helper --non-patch " > +}; > + > +int cmd_stash__helper(int argc, const char **argv, const char *prefix) > +{ > + if (argc == 4 && !strcmp("--non-patch", argv[1])) > + return stash_non_patch(argv[2], argv[3], prefix); > + usage(builtin_stash__helper_usage); > +} This is meant to replace this sequence: git read-tree --index-output="$TMPindex" -m $i_tree && GIT_INDEX_FILE="$TMPindex" && export GIT_INDEX_FILE && git diff --name-only -z HEAD -- >"$TMP-stagenames" && git update-index -z --add --remove --stdin <"$TMP-stagenames" && git write-tree && rm -f "$TMPindex" And outside of this section of the script, $TMPindex is never looked at after this part finishes (which is obvious as the last thing the section does is to remove it). As you are rewriting this whole section in C, you should wonder if you can do it without using a temporary file in the filesystem at all. > diff --git a/stash.c b/stash.c > new file mode 100644 > index 000..c3d6e67 > --- /dev/null > +++ b/stash.c > @@ -0,0 +1,65 @@ > +#include "stash.h" > +#include "strbuf.h" > + > +static int prepare_update_index_argv(struct argv_array *args, > + struct strbuf *buf) > +{ > + struct strbuf **bufs, **b; > + > + bufs = strbuf_split(buf, '\0'); > + for (b = bufs; *b; b++) > + argv_array_pushf(args, "%s", (*b)->buf); > + argv_array_push(args, "--"); > + strbuf_list_free(bufs); > + > + return 0; > +} > + > +int stash_non_patch(const char *tmp_indexfile, const char *i_tree, > + const char *prefix) > +{ > + int result; > + struct child_process read_tree = CHILD_PROCESS_INIT; > + struct child_process diff = CHILD_PROCESS_INIT; > + struct child_process update_index = CHILD_PROCESS_INIT; > + struct child_process write_tree = CHILD_PROCESS_INIT; > + struct strbuf buf = STRBUF_INIT; > + > + argv_array_push(&read_tree.args, "read-tree"); > + argv_array_pushf(&read_tree.args, "--index-output=%s", tmp_indexfile); > + argv_array_pushl(&read_tree.args, "-m", i_tree, NULL); > + > + argv_array_pushl(&diff.args, "diff", "--name-only", "-z", "HEAD", "--", > + NULL); > + > + argv_array_pushl(&update_index.args, "update-index", "--add", > + "--remove", NULL); > + > + argv_array_push(&write_tree.args, "write-tree"); Honestly, I had high hopes after seeing the "we are rewriting it in C" but I am not enthused after seeing this. I was hoping that the rewritten version would do this all in-core, by calling these functions that we already have: * read_index() to read the current index into the_index; * unpack_trees() to overlay the contents of i_tree to the contents of the index; * run_diff_index() to make the comparison between the result of the above and HEAD to collect the paths that are different (you'd use DIFF_FORMAT_CALLBACK mechanism to collect paths---see wt-status.c for existing code that already does this for hints); * add_to_index() to add or remove paths you found in the previous step to the in-core index; * write_cache_as_tree() to write out the resulting index of the above sequence of calls to a new tree object; * sha1_to_hex() to convert that resulting tree object name to hex format; * puts() to output the result. Actually, because i_tree is coming from $(git write-tree) that was done earlier on the current index, the unpack_trees() step should not even be necessary. The first three lines of the scripted version: git read-tree --index-output="$TMPindex" -m $i_tree && GIT_INDEX_FILE="$TMPindex" && export GIT_INDEX_FILE
[PATCH] pass transport verbosity down to git_connect
While working in connect.c to perform non-blocking connections, I noticed calling "git fetch -v" was not causing the progress messages inside git_tcp_connect_sock to be emitted as I expected. Looking at history, it seems connect_setup has never been called with the verbose parameter. Since transport already has a "verbose" field, use that field instead of another parameter in connect_setup. Signed-off-by: Eric Wong --- Related, but independent of my other change to connect.c: http://mid.gmane.org/20160128115720.ga1...@dcvr.yhbt.net ("attempt connects in parallel for IPv6-capable builds") transport.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/transport.c b/transport.c index 67f3666..9ae7184 100644 --- a/transport.c +++ b/transport.c @@ -481,9 +481,10 @@ static int set_git_option(struct git_transport_options *opts, return 1; } -static int connect_setup(struct transport *transport, int for_push, int verbose) +static int connect_setup(struct transport *transport, int for_push) { struct git_transport_data *data = transport->data; + int flags = transport->verbose > 0 ? CONNECT_VERBOSE : 0; if (data->conn) return 0; @@ -491,7 +492,7 @@ static int connect_setup(struct transport *transport, int for_push, int verbose) data->conn = git_connect(data->fd, transport->url, for_push ? data->options.receivepack : data->options.uploadpack, -verbose ? CONNECT_VERBOSE : 0); +flags); return 0; } @@ -501,7 +502,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus struct git_transport_data *data = transport->data; struct ref *refs; - connect_setup(transport, for_push, 0); + connect_setup(transport, for_push); get_remote_heads(data->fd[0], NULL, 0, &refs, for_push ? REF_NORMAL : 0, &data->extra_have, @@ -536,7 +537,7 @@ static int fetch_refs_via_pack(struct transport *transport, args.update_shallow = data->options.update_shallow; if (!data->got_remote_heads) { - connect_setup(transport, 0, 0); + connect_setup(transport, 0); get_remote_heads(data->fd[0], NULL, 0, &refs_tmp, 0, NULL, &data->shallow); data->got_remote_heads = 1; @@ -812,7 +813,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re if (!data->got_remote_heads) { struct ref *tmp_refs; - connect_setup(transport, 1, 0); + connect_setup(transport, 1); get_remote_heads(data->fd[0], NULL, 0, &tmp_refs, REF_NORMAL, NULL, &data->shallow); -- EW -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
fast-import fails in read-only tree
I recently discovered that "git fast-import" signals an error if used in a tree to which we do not have write-access, because it tries to create a "objects/pack/tmp_pack_XXX" file even before starting to process the commands. Usually this is not a problem (we'll create new commits and such, so write-access is indeed necessary), but in my case I was using fast-import only for its "reading" operations (in order to combine several inter-dependent "cat-file" operations into a single git session). 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: [PATCHv4 1/2] submodule: port resolve_relative_url from shell to C
On Thu, Jan 28, 2016 at 2:03 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> +static char *relative_url(const char *remote_url, >> + const char *url, >> + const char *up_path) >> +{ >> + int is_relative = 0; >> + int colonsep = 0; >> + char *out; >> + char *remoteurl = xstrdup(remote_url); >> + struct strbuf sb = STRBUF_INIT; >> + size_t len = strlen(remoteurl); >> + >> + if (is_dir_sep(remoteurl[len])) >> + remoteurl[len] = '\0'; >> + >> + if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl)) >> + is_relative = 0; >> + else { >> + is_relative = 1; >> + /* >> + * Prepend a './' to ensure all relative >> + * remoteurls start with './' or '../' >> + */ >> + if (!starts_with_dot_slash(remoteurl) && >> + !starts_with_dot_dot_slash(remoteurl)) { >> + strbuf_reset(&sb); >> + strbuf_addf(&sb, "./%s", remoteurl); >> + free(remoteurl); >> + remoteurl = strbuf_detach(&sb, NULL); >> + } >> + } >> + /* >> + * When the url starts with '../', remove that and the >> + * last directory in remoteurl. >> + */ >> + while (url) { >> + if (starts_with_dot_dot_slash(url)) { >> + url += 3; >> + colonsep |= chop_last_dir(&remoteurl, is_relative); >> + } else if (starts_with_dot_slash(url)) >> + url += 2; >> + else >> + break; >> + } >> + strbuf_reset(&sb); >> + strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url); >> + >> + if (starts_with_dot_slash(sb.buf)) >> + out = xstrdup(sb.buf + 2); >> + else >> + out = xstrdup(sb.buf); >> + strbuf_reset(&sb); >> + >> + free(remoteurl); > > This is a rather strange place to put this free(), as you are done > with it a bit earlier, but it's OK. I briefly wondered if the code > becomes easier to follow with fewer free(remoteurl) if this local > variable is made into a strbuf, but I didn't seriously think it > through. Right. As I did not touch that particular free with the resend, I wondered how it came there, too. And I think I had it at the end of the function and then realized the return just after the current position would leak it, so I moved it minimally up. If I'll resend again, I'll move it up to where it was last used. > > Otherwise looking good. > > Thanks. Thanks, 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: [PATCH] convert: legitimately disable clean/smudge filter with an empty override
Lars Schneider writes: > If the clean/smudge command of a Git filter driver (filter..smudge and > filter..clean) is set to an empty string ("") and the filter driver is > not required (filter..required=false) then Git will run successfully. > However, Git will print an error for every file that is affected by the > filter. > > Teach Git to consider an empty clean/smudge filter as legitimately disabled > and do not print an error message if the filter is not required. That makes more sense to me. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv4 1/2] submodule: port resolve_relative_url from shell to C
Stefan Beller writes: > +static char *relative_url(const char *remote_url, > + const char *url, > + const char *up_path) > +{ > + int is_relative = 0; > + int colonsep = 0; > + char *out; > + char *remoteurl = xstrdup(remote_url); > + struct strbuf sb = STRBUF_INIT; > + size_t len = strlen(remoteurl); > + > + if (is_dir_sep(remoteurl[len])) > + remoteurl[len] = '\0'; > + > + if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl)) > + is_relative = 0; > + else { > + is_relative = 1; > + /* > + * Prepend a './' to ensure all relative > + * remoteurls start with './' or '../' > + */ > + if (!starts_with_dot_slash(remoteurl) && > + !starts_with_dot_dot_slash(remoteurl)) { > + strbuf_reset(&sb); > + strbuf_addf(&sb, "./%s", remoteurl); > + free(remoteurl); > + remoteurl = strbuf_detach(&sb, NULL); > + } > + } > + /* > + * When the url starts with '../', remove that and the > + * last directory in remoteurl. > + */ > + while (url) { > + if (starts_with_dot_dot_slash(url)) { > + url += 3; > + colonsep |= chop_last_dir(&remoteurl, is_relative); > + } else if (starts_with_dot_slash(url)) > + url += 2; > + else > + break; > + } > + strbuf_reset(&sb); > + strbuf_addf(&sb, "%s%s%s", remoteurl, colonsep ? ":" : "/", url); > + > + if (starts_with_dot_slash(sb.buf)) > + out = xstrdup(sb.buf + 2); > + else > + out = xstrdup(sb.buf); > + strbuf_reset(&sb); > + > + free(remoteurl); This is a rather strange place to put this free(), as you are done with it a bit earlier, but it's OK. I briefly wondered if the code becomes easier to follow with fewer free(remoteurl) if this local variable is made into a strbuf, but I didn't seriously think it through. Otherwise looking good. 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 2/2] stash: use "stash--helper"
On Thu, Jan 28, 2016 at 1:25 PM, Matthias Aßhauer wrote: >> You had some good measurements in the coverletter, which is not going to be >> recorded in the projects history. This part however would be part of the >> commit. >> So you could move the speed improvements here (as well as the other >> reasoning) on why this is a good idea. :) > > I considered that, but I thought it would inflate the size of the commit > message quite a bit and represents a pretty temporary information as I'm > planning to port more code. No worries about too large commit messages. ;) See dcd1742e56ebb944c4ff62346da4548e1e3be675 as an example for commit message per code raio what Jeff usually produces. :) > Any further progression on this would make the old meassurements kind of > obsolete IMHO. Well it records that this specific step was beneficial, too, on the platforms you measured on. If it turns out to there is a regression after you rewrote lots of code, it is still traceable that this commit was done in good faith. > I decided to move it to the coverletter, because it is only valid information > if you consider both commits. If the general opinion on here is that I should > add it to the commit message though, I'll gladly update it. Heh, true. However you enable the speedup in the second patch. If you were to apply only the first (add the helper), you'd not see the difference, so maybe it's worth adding it to the second commit message. > >>> https://github.com/git/git/pull/191 >> >> Oh I see you're using the pull-request to email translator, cool! > > Yes, I did. It definitly makes things easier if you are not used to mailing > lists, but it was also a bit of a kerfuffle. I tried to start working on > coverletter support, but I couldn't get it to accept the amazon SES > credentials I provided. I ended up manually submiting the coverletter. It > also didn't like my name. Not sure if Roberto, the creator of that tool, follows the mailing list. I cc'd him. > > Thank you for your quick feedback. > -- To unsubscribe from this list: send the line "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 00/20] Let Git's tests pass on Windows
Johannes Schindelin writes: >> For what it's worth, I ran the test suite on Mac OS X and FreeBSD, as >> well, with this series applied and didn't run across any problems. I >> also read through v3 and, other than the micro nit in patch 11/20, >> didn't find anything upon which to comment. > > Thank you so much! I really appreciate your feedback, and I have a lot of > respect for reviewers that go through a 19 or 20 strong patch series. Yeah, especially with multiple rerolls. Thanks, both. Will requeue. -- To unsubscribe from this list: send the line "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 11/20] tests: turn off git-daemon tests if FIFOs are not available
Johannes Schindelin writes: >> > diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh >> > @@ -23,6 +23,11 @@ then >> > +if ! test_have_prereq PIPE >> >> Maybe: >> >> if test_have_prereq !PIPE >> >> ? > > Darn. Of course I only looked for '! .*MINGW', but I should have looked > for '! test_have_prereq' in the patches. Wow. Talk about fine-toothed comb ;-) Will squash in. Thanks for a set of sharp eyes. -- To unsubscribe from this list: send the line "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] travis-ci: run previously failed tests first, then slowest to fastest
Clemens Buchacher writes: > On Wed, Jan 27, 2016 at 12:49:31PM -0800, Junio C Hamano wrote: >> Junio C Hamano writes: >> >> > I wonder what would break if we ask this question instead: >> > >> > We do not know if the working tree file and the indexed data >> > match. Let's see if "git checkout" of that path would leave the >> > same data as what currently is in the working tree file. > > If we do this, then git diff should show the diff between > convert_to_worktree(index state) and the worktree state. I agree with you that, when ce_compare_data(), i.e. "does the index match the working tree?", says "they match", "git diff" (show me the change to go from the index to the worknig tree) should show empty to be consistent, and for that to happen under the above definition of ce_compare_data(), "git diff" needs to be comparing the data in the index after converting it to the working tree representation with the data in the working tree. And that unfortunately is a very good reason why this approach should not be taken. "git diff" (show me the change to go from the index to the working tree) is a preview of what we would see in "git diff --cached" (show me the change to go from HEAD to the index) if we did "git add", and it is a preview of what we would see in "git show" (show me the change of what the last commit did) if we did "git commit -a". It is crazy for these latter comparisons to happen in the working tree (aka "smudged") representation of the data, IOW, these two must compare the "clean" representation. It also is crazy for "git diff" to be using different representation from these two. This alone makes the above idea a non-starter X-<. Besides, I do not think the above approach really solves the issue, either. After "git reset --hard" to have the contents in the index dumped to the working tree, if your core.autocrlf is flipped, "git checkout" of the same path would result in a working tree representation of the data that is different from what you have in the working tree, so we would declare that the working tree is not clean, even though nobody actually touched them in the meantime. This is less of an issue than having data in the index that is inconsistent with the convert_to_git() setting (i.e. eol and clean filter conversion that happens when you "git add"), but it still is fundamentally the same issue. Oh, bummer, I thought it was a nice approach. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
AW: [PATCH 2/2] stash: use "stash--helper"
> You had some good measurements in the coverletter, which is not going to be > recorded in the projects history. This part however would be part of the > commit. > So you could move the speed improvements here (as well as the other > reasoning) on why this is a good idea. :) I considered that, but I thought it would inflate the size of the commit message quite a bit and represents a pretty temporary information as I'm planning to port more code. Any further progression on this would make the old meassurements kind of obsolete IMHO. I decided to move it to the coverletter, because it is only valid information if you consider both commits. If the general opinion on here is that I should add it to the commit message though, I'll gladly update it. >> https://github.com/git/git/pull/191 > > Oh I see you're using the pull-request to email translator, cool! Yes, I did. It definitly makes things easier if you are not used to mailing lists, but it was also a bit of a kerfuffle. I tried to start working on coverletter support, but I couldn't get it to accept the amazon SES credentials I provided. I ended up manually submiting the coverletter. It also didn't like my name. Thank you for your quick feedback. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] tag-ref and tag object binding
On Tue, Jan 26, 2016 at 01:13:16PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > On Tue, Jan 26, 2016 at 10:29:42AM -0500, Santiago Torres wrote: > > > >> > If you cannot trust those with write access to a repo that you are > >> > pulling and installing from you might want to re-check where you are > >> > pulling or installing from ;) > >> > >> Yeah, I see your point, but mechanisms to ensure the server's origin can > >> be bypassed (e.g., a MITM). I don't think it would hurt to ensure the > >> source pointed to is the source itself. The tag signature can help us do > >> this. > > > > Right. I think the more interesting use case here is "I trust the > > upstream repository owner, but I do not trust their hosting site of > > choice." > > Yup, and push-certificate is there to help with that issue. Yes, I agree, but wouldn't this provide an in-band solution to this very particular scenario. In order to provide the spureous tag, you have to provide the tagname it should be pointing to (or tamper with the tag object). Push certificates can address many other sorts of attacks, but are not in-band in this sense are they? Thanks! -Santiago. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] tag-ref and tag object binding
Hello, sorry for being MIA yesterday among all this movement... On Wed, Jan 27, 2016 at 08:53:08AM +0100, Michael J Gruber wrote: > Jeff King venit, vidit, dixit 27.01.2016 08:33: > > On Wed, Jan 27, 2016 at 08:23:02AM +0100, Michael J Gruber wrote: > > > >>> Tag objects already have a "tag" header, which is part of the signed > >>> content. If you use "git verify-tag -v", you can check both that the > >>> signature is valid and that the tag is the one you are expecting. > >> > >> Yes, that's what I described in my last paragraph, using the term > >> (embedded) tag "name" which is technically wrong (it's not the tag > >> object's name, which would be a sha1) but the natural term for users. > > > > Indeed. I should have read further back in the quoting. :) > > > >>> Git pretty much punts on all of these issues and assumes either a human > >>> or a smarter tool is looking at the verification output. But I don't > >>> think it would hurt to build in some features to let git automatically > >>> check some things, if only to avoid callers duplicating work to > >>> implement the checks themselves. > >> > >> That is really a can of worms for several reasons: > >> [...] > >> So, for those who shy away from for-each-ref and such, we may add the > >> header check to verify-tag, with a big warning about the marginal gain > >> in security (or the requirements for an actual gain). > > > > Yeah, definitely. My thinking was that `verify-tag` could learn a series > > of optional consistency checks, enabled by command line options, and > > verifying programs (or humans) could turn them on to avoid having to > > replicate them manually. So something like: > > > > git verify-tag \ > > --verify-tagger-matches-key \ > > --verify-tag-matches-ref \ # or --verify-tag-matches=v2.0.0 > > v2.0.0 > > > > or to implement more specific policy, maybe an option to check for a > > _specific_ tagger, either by email (as provided by gpg) or even key-id. > > > > Those are all things that are not _too_ hard to do if you're willing to > > parse gpg or git output, but we could make life easier for our callers. > > And hopefully by asking for specific, concrete checks, it doesn't > > introduce a false sense of security. I.e., we're not making a foolproof > > tool; we're making building blocks that one could use for a more > > foolproof tool. > > OK, let's make a tool that helps fooling as well as proofing :) > > I'll look into the tag header check. Maybe "--check-tagname"? "check" > seems to imply less than "verify". This seems like exactly what I was thinking of. What I believe would be helpful is to provide upstream tools the means to automatically verify tags (--check-tagname could return non-0 if the tagname doesn't match), could this possibly be the default behavior (--no-check-tagname instead)? What worries me is something like this experiment with pip: (git-tag2) santiago@LykOS:~$ pip install -e git+https://github.com/santiagotorres/django/@1.8.3#egg=django Obtaining django from git+https://github.com/santiagotorres/django/@1.8.3#egg=django Cloning https://github.com/santiagotorres/django/ (to 1.8.3) to ./.virtualenvs/git-tag2/src/django Installing collected packages: django Running setup.py develop for django Successfully installed django (git-tag2) santiago@LykOS:~$ django-admin.py --version 1.4.10 I only had to swap the tag refs and push for this simulation. Needless to say, this deprectated django version (1.4.10) is vulnerable to a wide range known exploits that include session hijacking, DoS, "MySQL typecasting" and XSS. And the person who served this tampered tag could exploit the webserver right away (knows who got this vulnerable package). Of course, this could also trick a CI system and have people package the wrong version of django. I agree that nothing beats manual inspection of a paranoid developer, but these tools are widely used today and could avoid these edge cases. > > As for the gpg related stuff: We provide the full diagnostic output from > gpg on request. But I think a mismatch between the signing key's uid and > the taggers' name/email is more common than not, and on the other hand a > signature by a key identified by its uid is meaningless unless you keep > your keyring tidy... We could punt on that and let users identify the > key by any means that gpg allows, of course, and check that the > signature comes from whatever "gpg --list-key " gives as > long as it's unique. > > But maybe I'm biased by my zoo of uids/emails addresses/full name > spellings... and general paranoia regarding the term "verify" ;) Yeah, I believe the gpg related stuff might be a little out of scope. I think, for the sake of this RFC, that we could assume that tools/people have acess to the right key and could perform verification. As it was already pointed out, key distribution and a project-specific gpg keychain could be integrated. The problem I see is that, even with the right key and a pro
Re: [PATCH 2/2] stash: use "stash--helper"
On Thu, Jan 28, 2016 at 12:36 PM, Matthias Asshauer wrote: > From: Matthias Aßhauer > > Use the new "git stash--helper" builtin. It should be faster than the old > shell code and is a first step to move > more shell code to C. You had some good measurements in the coverletter, which is not going to be recorded in the projects history. This part however would be part of the commit. So you could move the speed improvements here (as well as the other reasoning) on why this is a good idea. :) > > Signed-off-by: Matthias Aßhauer > --- > git-stash.sh | 10 +- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/git-stash.sh b/git-stash.sh > index c7c65e2..973c77b 100755 > --- a/git-stash.sh > +++ b/git-stash.sh > @@ -112,15 +112,7 @@ create_stash () { > then > > # state of the working tree > - w_tree=$( ( > - git read-tree --index-output="$TMPindex" -m $i_tree && > - GIT_INDEX_FILE="$TMPindex" && > - export GIT_INDEX_FILE && > - git diff --name-only -z HEAD -- >"$TMP-stagenames" && > - git update-index -z --add --remove --stdin > <"$TMP-stagenames" && > - git write-tree && > - rm -f "$TMPindex" > - ) ) || > + w_tree=$(git stash--helper --non-patch "$TMPindex" $i_tree) || > die "$(gettext "Cannot save the current worktree > state")" > > else > > -- > https://github.com/git/git/pull/191 Oh I see you're using the pull-request to email translator, cool! > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "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/2] stash--helper: implement "git stash--helper"
From: Matthias Aßhauer This patch implements a new "git stash--helper" builtin plumbing command that will be used to migrate "git-stash.sh" to C. We start by implementing only the "--non-patch" option that will handle the core part of the non-patch stashing. Signed-off-by: Matthias Aßhauer --- Makefile| 2 ++ builtin.h | 1 + builtin/stash--helper.c | 13 ++ git.c | 1 + stash.c | 65 + stash.h | 11 + 6 files changed, 93 insertions(+) create mode 100644 builtin/stash--helper.c create mode 100644 stash.c create mode 100644 stash.h diff --git a/Makefile b/Makefile index fc2f1ab..88c07ea 100644 --- a/Makefile +++ b/Makefile @@ -792,6 +792,7 @@ LIB_OBJS += shallow.o LIB_OBJS += sideband.o LIB_OBJS += sigchain.o LIB_OBJS += split-index.o +LIB_OBJS += stash.o LIB_OBJS += strbuf.o LIB_OBJS += streaming.o LIB_OBJS += string-list.o @@ -913,6 +914,7 @@ BUILTIN_OBJS += builtin/send-pack.o BUILTIN_OBJS += builtin/shortlog.o BUILTIN_OBJS += builtin/show-branch.o BUILTIN_OBJS += builtin/show-ref.o +BUILTIN_OBJS += builtin/stash--helper.o BUILTIN_OBJS += builtin/stripspace.o BUILTIN_OBJS += builtin/submodule--helper.o BUILTIN_OBJS += builtin/symbolic-ref.o diff --git a/builtin.h b/builtin.h index 6b95006..f1c8b39 100644 --- a/builtin.h +++ b/builtin.h @@ -118,6 +118,7 @@ extern int cmd_send_pack(int argc, const char **argv, const char *prefix); extern int cmd_shortlog(int argc, const char **argv, const char *prefix); extern int cmd_show(int argc, const char **argv, const char *prefix); extern int cmd_show_branch(int argc, const char **argv, const char *prefix); +extern int cmd_stash__helper(int argc, const char **argv, const char *prefix); extern int cmd_status(int argc, const char **argv, const char *prefix); extern int cmd_stripspace(int argc, const char **argv, const char *prefix); extern int cmd_submodule__helper(int argc, const char **argv, const char *prefix); diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c new file mode 100644 index 000..542e782 --- /dev/null +++ b/builtin/stash--helper.c @@ -0,0 +1,13 @@ +#include "../stash.h" +#include + +static const char builtin_stash__helper_usage[] = { + "Usage: git stash--helper --non-patch " +}; + +int cmd_stash__helper(int argc, const char **argv, const char *prefix) +{ + if (argc == 4 && !strcmp("--non-patch", argv[1])) + return stash_non_patch(argv[2], argv[3], prefix); + usage(builtin_stash__helper_usage); +} diff --git a/git.c b/git.c index da278c3..9829ee8 100644 --- a/git.c +++ b/git.c @@ -470,6 +470,7 @@ static struct cmd_struct commands[] = { { "show-branch", cmd_show_branch, RUN_SETUP }, { "show-ref", cmd_show_ref, RUN_SETUP }, { "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE }, + { "stash--helper", cmd_stash__helper, RUN_SETUP | NEED_WORK_TREE }, { "status", cmd_status, RUN_SETUP | NEED_WORK_TREE }, { "stripspace", cmd_stripspace }, { "submodule--helper", cmd_submodule__helper, RUN_SETUP }, diff --git a/stash.c b/stash.c new file mode 100644 index 000..c3d6e67 --- /dev/null +++ b/stash.c @@ -0,0 +1,65 @@ +#include "stash.h" +#include "strbuf.h" + +static int prepare_update_index_argv(struct argv_array *args, + struct strbuf *buf) +{ + struct strbuf **bufs, **b; + + bufs = strbuf_split(buf, '\0'); + for (b = bufs; *b; b++) + argv_array_pushf(args, "%s", (*b)->buf); + argv_array_push(args, "--"); + strbuf_list_free(bufs); + + return 0; +} + +int stash_non_patch(const char *tmp_indexfile, const char *i_tree, + const char *prefix) +{ + int result; + struct child_process read_tree = CHILD_PROCESS_INIT; + struct child_process diff = CHILD_PROCESS_INIT; + struct child_process update_index = CHILD_PROCESS_INIT; + struct child_process write_tree = CHILD_PROCESS_INIT; + struct strbuf buf = STRBUF_INIT; + + argv_array_push(&read_tree.args, "read-tree"); + argv_array_pushf(&read_tree.args, "--index-output=%s", tmp_indexfile); + argv_array_pushl(&read_tree.args, "-m", i_tree, NULL); + + argv_array_pushl(&diff.args, "diff", "--name-only", "-z", "HEAD", "--", + NULL); + + argv_array_pushl(&update_index.args, "update-index", "--add", + "--remove", NULL); + + argv_array_push(&write_tree.args, "write-tree"); + + read_tree.env = + diff.env = + update_index.env = + write_tree.env = prefix; + + read_tree.use_shell = + diff.use_shell = + update_index.use_shell = + write_tree.use_shell = 1; + + read_tree.git_cmd = + diff.git_cmd = + update_index.git_cmd = + write_tree.git_cmd = 1; + + result = run
[PATCH 2/2] stash: use "stash--helper"
From: Matthias Aßhauer Use the new "git stash--helper" builtin. It should be faster than the old shell code and is a first step to move more shell code to C. Signed-off-by: Matthias Aßhauer --- git-stash.sh | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/git-stash.sh b/git-stash.sh index c7c65e2..973c77b 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -112,15 +112,7 @@ create_stash () { then # state of the working tree - w_tree=$( ( - git read-tree --index-output="$TMPindex" -m $i_tree && - GIT_INDEX_FILE="$TMPindex" && - export GIT_INDEX_FILE && - git diff --name-only -z HEAD -- >"$TMP-stagenames" && - git update-index -z --add --remove --stdin <"$TMP-stagenames" && - git write-tree && - rm -f "$TMPindex" - ) ) || + w_tree=$(git stash--helper --non-patch "$TMPindex" $i_tree) || die "$(gettext "Cannot save the current worktree state")" else -- https://github.com/git/git/pull/191 -- To unsubscribe from this list: send the line "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] push --force-with-lease: Fix ref status reporting
Ignore -- I left an extra blank line. v3 is sent. On Thu, Jan 28, 2016 at 2:20 PM, Andrew Wheeler wrote: > From: Andrew Wheeler > > The --force--with-lease push option leads to less > detailed status information than --force. In particular, > the output indicates that a reference was fast-forwarded, > even when it was force-updated. > > Modify the --force-with-lease ref status logic to leverage > the --force ref status logic when the "lease" conditions > are met. > > Also, enhance tests to validate output status reporting. > > Signed-off-by: Andrew Wheeler > --- > remote.c| 16 +--- > t/t5533-push-cas.sh | 9 ++--- > 2 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/remote.c b/remote.c > index 9d34b5a..bad6213 100644 > --- a/remote.c > +++ b/remote.c > @@ -1545,11 +1545,8 @@ void set_ref_status_for_push(struct ref *remote_refs, > int send_mirror, > } > > /* > -* Bypass the usual "must fast-forward" check but > -* replace it with a weaker "the old value must be > -* this value we observed". If the remote ref has > -* moved and is now different from what we expect, > -* reject any push. > +* If the remote ref has moved and is now different > +* from what we expect, reject any push. > * > * It also is an error if the user told us to check > * with the remote-tracking branch to find the value > @@ -1560,10 +1557,14 @@ void set_ref_status_for_push(struct ref *remote_refs, > int send_mirror, > if (ref->expect_old_no_trackback || > oidcmp(&ref->old_oid, &ref->old_oid_expect)) > reject_reason = REF_STATUS_REJECT_STALE; > + else > + /* If the ref isn't stale then force the > update. */ > + force_ref_update = 1; > } > > /* > -* The usual "must fast-forward" rules. > +* If the update isn't already rejected then check > +* the usual "must fast-forward" rules. > * > * Decide whether an individual refspec A:B can be > * pushed. The push will succeed if any of the > @@ -1580,9 +1581,10 @@ void set_ref_status_for_push(struct ref *remote_refs, > int send_mirror, > * > * (4) it is forced using the +A:B notation, or by > * passing the --force argument > +* > */ > > - else if (!ref->deletion && !is_null_oid(&ref->old_oid)) { > + if (!reject_reason && !ref->deletion && > !is_null_oid(&ref->old_oid)) { > if (starts_with(ref->name, "refs/tags/")) > reject_reason = > REF_STATUS_REJECT_ALREADY_EXISTS; > else if (!has_object_file(&ref->old_oid)) > diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh > index c402d8d..c65033f 100755 > --- a/t/t5533-push-cas.sh > +++ b/t/t5533-push-cas.sh > @@ -101,7 +101,8 @@ test_expect_success 'push to update (allowed, tracking)' ' > ( > cd dst && > test_commit D && > - git push --force-with-lease=master origin master > + git push --force-with-lease=master origin master 2>err && > + ! grep "forced update" err > ) && > git ls-remote dst refs/heads/master >expect && > git ls-remote src refs/heads/master >actual && > @@ -114,7 +115,8 @@ test_expect_success 'push to update (allowed even though > no-ff)' ' > cd dst && > git reset --hard HEAD^ && > test_commit D && > - git push --force-with-lease=master origin master > + git push --force-with-lease=master origin master 2>err && > + grep "forced update" err > ) && > git ls-remote dst refs/heads/master >expect && > git ls-remote src refs/heads/master >actual && > @@ -147,7 +149,8 @@ test_expect_success 'push to delete (allowed)' ' > setup_srcdst_basic && > ( > cd dst && > - git push --force-with-lease=master origin :master > + git push --force-with-lease=master origin :master 2>err && > + grep deleted err > ) && > >expect && > git ls-remote src refs/heads/master >actual && > -- > 1.7.11.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
[PATCH v3] push --force-with-lease: Fix ref status reporting
From: Andrew Wheeler The --force--with-lease push option leads to less detailed status information than --force. In particular, the output indicates that a reference was fast-forwarded, even when it was force-updated. Modify the --force-with-lease ref status logic to leverage the --force ref status logic when the "lease" conditions are met. Also, enhance tests to validate output status reporting. Signed-off-by: Andrew Wheeler --- remote.c| 15 --- t/t5533-push-cas.sh | 9 ++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/remote.c b/remote.c index 9d34b5a..3ceac07 100644 --- a/remote.c +++ b/remote.c @@ -1545,11 +1545,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, } /* -* Bypass the usual "must fast-forward" check but -* replace it with a weaker "the old value must be -* this value we observed". If the remote ref has -* moved and is now different from what we expect, -* reject any push. +* If the remote ref has moved and is now different +* from what we expect, reject any push. * * It also is an error if the user told us to check * with the remote-tracking branch to find the value @@ -1560,10 +1557,14 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, if (ref->expect_old_no_trackback || oidcmp(&ref->old_oid, &ref->old_oid_expect)) reject_reason = REF_STATUS_REJECT_STALE; + else + /* If the ref isn't stale then force the update. */ + force_ref_update = 1; } /* -* The usual "must fast-forward" rules. +* If the update isn't already rejected then check +* the usual "must fast-forward" rules. * * Decide whether an individual refspec A:B can be * pushed. The push will succeed if any of the @@ -1582,7 +1583,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, * passing the --force argument */ - else if (!ref->deletion && !is_null_oid(&ref->old_oid)) { + if (!reject_reason && !ref->deletion && !is_null_oid(&ref->old_oid)) { if (starts_with(ref->name, "refs/tags/")) reject_reason = REF_STATUS_REJECT_ALREADY_EXISTS; else if (!has_object_file(&ref->old_oid)) diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh index c402d8d..c65033f 100755 --- a/t/t5533-push-cas.sh +++ b/t/t5533-push-cas.sh @@ -101,7 +101,8 @@ test_expect_success 'push to update (allowed, tracking)' ' ( cd dst && test_commit D && - git push --force-with-lease=master origin master + git push --force-with-lease=master origin master 2>err && + ! grep "forced update" err ) && git ls-remote dst refs/heads/master >expect && git ls-remote src refs/heads/master >actual && @@ -114,7 +115,8 @@ test_expect_success 'push to update (allowed even though no-ff)' ' cd dst && git reset --hard HEAD^ && test_commit D && - git push --force-with-lease=master origin master + git push --force-with-lease=master origin master 2>err && + grep "forced update" err ) && git ls-remote dst refs/heads/master >expect && git ls-remote src refs/heads/master >actual && @@ -147,7 +149,8 @@ test_expect_success 'push to delete (allowed)' ' setup_srcdst_basic && ( cd dst && - git push --force-with-lease=master origin :master + git push --force-with-lease=master origin :master 2>err && + grep deleted err ) && >expect && git ls-remote src refs/heads/master >actual && -- 1.7.11.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
[PATCH v2] push --force-with-lease: Fix ref status reporting
From: Andrew Wheeler The --force--with-lease push option leads to less detailed status information than --force. In particular, the output indicates that a reference was fast-forwarded, even when it was force-updated. Modify the --force-with-lease ref status logic to leverage the --force ref status logic when the "lease" conditions are met. Also, enhance tests to validate output status reporting. Signed-off-by: Andrew Wheeler --- remote.c| 16 +--- t/t5533-push-cas.sh | 9 ++--- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/remote.c b/remote.c index 9d34b5a..bad6213 100644 --- a/remote.c +++ b/remote.c @@ -1545,11 +1545,8 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, } /* -* Bypass the usual "must fast-forward" check but -* replace it with a weaker "the old value must be -* this value we observed". If the remote ref has -* moved and is now different from what we expect, -* reject any push. +* If the remote ref has moved and is now different +* from what we expect, reject any push. * * It also is an error if the user told us to check * with the remote-tracking branch to find the value @@ -1560,10 +1557,14 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, if (ref->expect_old_no_trackback || oidcmp(&ref->old_oid, &ref->old_oid_expect)) reject_reason = REF_STATUS_REJECT_STALE; + else + /* If the ref isn't stale then force the update. */ + force_ref_update = 1; } /* -* The usual "must fast-forward" rules. +* If the update isn't already rejected then check +* the usual "must fast-forward" rules. * * Decide whether an individual refspec A:B can be * pushed. The push will succeed if any of the @@ -1580,9 +1581,10 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror, * * (4) it is forced using the +A:B notation, or by * passing the --force argument +* */ - else if (!ref->deletion && !is_null_oid(&ref->old_oid)) { + if (!reject_reason && !ref->deletion && !is_null_oid(&ref->old_oid)) { if (starts_with(ref->name, "refs/tags/")) reject_reason = REF_STATUS_REJECT_ALREADY_EXISTS; else if (!has_object_file(&ref->old_oid)) diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh index c402d8d..c65033f 100755 --- a/t/t5533-push-cas.sh +++ b/t/t5533-push-cas.sh @@ -101,7 +101,8 @@ test_expect_success 'push to update (allowed, tracking)' ' ( cd dst && test_commit D && - git push --force-with-lease=master origin master + git push --force-with-lease=master origin master 2>err && + ! grep "forced update" err ) && git ls-remote dst refs/heads/master >expect && git ls-remote src refs/heads/master >actual && @@ -114,7 +115,8 @@ test_expect_success 'push to update (allowed even though no-ff)' ' cd dst && git reset --hard HEAD^ && test_commit D && - git push --force-with-lease=master origin master + git push --force-with-lease=master origin master 2>err && + grep "forced update" err ) && git ls-remote dst refs/heads/master >expect && git ls-remote src refs/heads/master >actual && @@ -147,7 +149,8 @@ test_expect_success 'push to delete (allowed)' ' setup_srcdst_basic && ( cd dst && - git push --force-with-lease=master origin :master + git push --force-with-lease=master origin :master 2>err && + grep deleted err ) && >expect && git ls-remote src refs/heads/master >actual && -- 1.7.11.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: Starting on a microproject for GSoC
Moritz Neeb writes: > the next Google Summer of Code is not too far away. I expect git to > apply for it and hopefully have some student spots which in turn I plan > to apply. It was recommended elsewhere and on this list as well, that it > is beneficial to engage with the community early, that's why I am > writing to you already now, before all this formal stuff has begun. It is unknown if we are going to participate in GSoC this year. One question you must ask yourself first: will you still be interested in hacking Git if we decide not to take GSoC students this year? The GSoC microprojects are not about seeing who writes great code. What we want to see primarily with them is how well candidates interact with the community, responding to reviewers, showing agreement or disagreement with received comments, making arguments in a constructive way when opinions differ, updating their submissions with suggested improvements in a timely matter to keep the ball rolling, etc. GSoC micros are primarily designed to be small and can be finished within a short timeframe, but expected to still have enough iterations with reviewers that candidates can use as an opportunity to demonstrate how well they can work with the community. Suppose a candidate already tackled a micro, went through multiple iterations with reviewers and came up with a polished solution. When another candidate comes late and sends in a very similar "answer" to the same micro, without meaningful interactions with the reviewers and the community, the latter candidate would not be demonstrating how good a "fit" s/he is in the community at all. On the other hand, if the latter candidate approaches the same micro somebody else attacked in a different way, s/he would have his or her own interactions with the reviewers and would be demonstrating his or her ability to work with us. So in that sense, they are not "quiz" that has a single right answer, and we do not necessarily have problems if multiple candidates attack the same micro. Now, if your answer to the "first" question is "No", then you might want to avoid wasting your effort investing too early in preparing for an event that may not happen. You may want to stop reading here and wait until GSoC micros are posted (if we decide to participate this year, that is). If the answer is "Yes", then welcome to the Git development community ;-) The purpose of GSoC micro I explained above also means that people like you, who are interested in hacking Git, can start early and do their own things to demonstrate that they can work well with our community, which may give them a head start. When they apply to be a GSoC student (if we participate this year), we would already have enough datapoint to convince ourselves that it would be a good idea to pick them (even without them doing GSoC micro). > The second task, to allow "-" as a short-hand for "@{-1}" in more > places seems to be still open for reset, although someone almost > finished it (cf. $gmane/265417). I suppose just fixing/revising > this would be kind of a too low hanging fruit? More interesting > (also because I am a bit earlier) might be to unify everything, as > suggested by Junio in $gmane/265260. Or implementing it for > another branch command, e.g. rebase. This actually is a very tricky one to do well. > If all of that is considered as not relevant, I might just go for > a newer idea, like converting strbuf_getline_lf() callers to > strbuf_getline(), as suggested in $gmane/284104. It is a good sign that you are familiar with a recent list discussion. There are other "once this topic settles, we would probably want to do this and that kind of cleanups on top" left behind that haven't made my "leftover bits" page (which I update only after the discussion dies on the list and there is no sign that others will be picking them up soonish). Have fun. -- To unsubscribe from this list: send the line "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/2] Make stash a builtin
These are the first two Patches in an upcomming series of Patches to convert git stash into a builtin command. This is mainly based on the general performance of scripts running inside an interpreter (bash in this case) running inside a POSIX emulation layer on Windows. Shell, perl and python scripts are generaly a lot faster on Unix-like systems compared to Windows systems. That does not mean that Unix-like systems won't benefit from more native Git commands, but the effect is bigger on Git for Windows. These two patches port over the core non-patch part of git-stash into native code using a separate helper command. This helper command is intended as a temporary meassure and as such it's subject to change. For this reason, I did not implement new regression tests, documentation or localizations for this command. I meassured the changes in excecution time for the stash related regression tests on the same hardware running Windows 8.1 and Kubuntu 15.10. Each result is the difference between the average of five meassurements (six on Linux, because I lost count on the first run of meassurements) each before and after these changes. I meassured the following changes: Windows: t3903 real -5,10% user -0,94% sys +0,16% (10ms) t3904 real -0,30% user -2,98% (20ms) sys +5,03% t3905 real -4,03% user -8,13% sys +17,42% t3906 real -2,57% user +1,94% sys +1,59% Linux: t3903 real +0,63% user +10,87% (3ms) sys +4,29% (4ms) t3904 real -7,29% user -30,61% sys +5,77% (4ms) t3905 real -7,29% user -33,33% (2ms) sys +20% (2ms) t3906 real -0,88% user -1,08% (1ms) sys -2,22% I added the asolute times where I think the difference is below the meassurement precission (4ms on Linux) and on the two lowest absolute differences on windows. A full log of all meassurement runs is available at https://gist.github.com/rimrul/82adf3b368ed633263d2. Please note that according to Johannes Schindelin, maintainer of Git for Windows, the meassuring of sys time on Windows is unreliable. With that in mind, in summary this is a slight increase in performance on Linux, and a more noticeable increase on Windows. -- To unsubscribe from this list: send the line "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 4/6] worktree: new config file hierarchy
Duy Nguyen writes: >> I cannot see why it cannot live in $C/common/config, which would be >> read as the fourth alternative in your earlier enumeration. What I >> am missing? > > I wasn't clear. The last paragraph is about already released git > binaries, which does not care about $C/common/config. Suppose you add > a new worktree with new git binary, which will move common stuff out > of .git/config to common/config, on a shared repo. If the old binary > reads that repo, it does not see common config, but it does not know > where common config is either. Ah, OK. Would it make it simpler to invent a specific value for 'xxx' that denotes the main worktree (hence $C/worktrees/xxx/config will always be read by worktrees including the primary one), not to add $C/common/ anything, and use $C/config as the common one instead? Then the repository format version can live in $C/config that would be noticed by existing versions of Git. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Bugs in git filter-branch (git replace related)
Hi All! There are two bugs in `git filter-branch`, present in the most recent versions (d10e2cb9d0299a26f43d57dd5bdcf2b3f86a30b3), as well as in the old ones (I couldn't find a version where it works properly). The script: #!/bin/sh set -e GIT_EXEC_PATH=/tmp/git export GIT_EXEC_PATH GIT=$GIT_EXEC_PATH/git rm -rf a mkdir a cd a $GIT init echo aaa > a.txt && $GIT add a.txt && $GIT commit -a -m a echo bbb > a.txt && $GIT add a.txt && $GIT commit -a -m b echo ccc > a.txt && $GIT add a.txt && $GIT commit -a -m c $GIT replace f761ec192d9f0dca3329044b96ebdb12839dbff6 72943a16fb2c8f38f9dde202b7a70ccc19c52f34 echo && echo One && $GIT filter-branch --prune-empty -- master echo && echo Two && $GIT filter-branch --prune-empty -- --all The output is: ... One Rewrite 98af0305b778bf56e25a0d4f85acdf82f435f9b3 (3/3) (0 seconds passed, remaining 0 predicted) WARNING: Ref 'refs/heads/master' is unchanged Two Rewrite 98af0305b778bf56e25a0d4f85acdf82f435f9b3 (3/3) (0 seconds passed, remaining 0 predicted) WARNING: Ref 'refs/heads/master' is unchanged error: object 72943a16fb2c8f38f9dde202b7a70ccc19c52f34 is a blob, not a commit error: object 72943a16fb2c8f38f9dde202b7a70ccc19c52f34 is a blob, not a commit fatal: ambiguous argument 'refs/replace/f761ec192d9f0dca3329044b96ebdb12839dbff6^0': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git [...] -- [...]' WARNING: Ref 'refs/replace/f761ec192d9f0dca3329044b96ebdb12839dbff6' is unchanged The `git replace` makes the second commit empty (use the file content from the first commit). It should disappear after `git filter-branch`, but it doesn't happen. Bug 1: the empty commit stays. Bug 2: the replace refs are not ignored (they can epresent blobs, trees etc, but even if they represent commits - should they be rewritten?). Any ideas? PS I've found http://article.gmane.org/gmane.comp.version-control.git/220931 , seems like the bug 1. But it's old! -- To unsubscribe from this list: send the line "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 1/6] worktree: new repo extension to manage worktree behaviors
On Thu, Jan 28, 2016 at 5:12 AM, Junio C Hamano wrote: >> +WORKTREE VERSIONS AND MIGRATION >> +--- >> +Multiple worktree is still an experimental feature and evolving. Every >> +time the behavior is changed, the "worktree version" is stepped >> +up. Worktree version is stored as a configuration variable >> +extensions.worktree. > > s/stepped up/incremented/ > > More seriously, are we confident that the overall worktree support > is mature enough by now that once we add an experimental feature X > at version 1, we can promise to keep maintaining it forever at > version N for any positive integer N? I hate to sound overly > negative, but I am getting an impression that we are not quite > there, and it is still not ready for production use. I completely overlooked this config file issue in the first round, so there's a good chance I will fail to realize it's still incomplete again. > It would be beneficial both for us and our users if we can keep our > hand untied for at least several more releases to allow us try > various random experimental features, fully intending to drop any of > them if the ideas do not pan out. Yes it's best if we can somehow communicate with the users about that. If a line or two in release announcement is good enough, great. Otherwise maybe print a line every time the user executes "git worktree"? -- 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 v3 4/6] worktree: new config file hierarchy
On Thu, Jan 28, 2016 at 5:22 AM, Junio C Hamano wrote: >> With this patch, since worktree v1, the repo config file (or worktree >> config file in multi worktree context) is no longer shared. Main >> worktree reads $C/config. Linked worktrees read $C/worktrees/xxx/config >> and a new file, $C/worktrees/config. Sharing is done via this new >> file. The read hierarchy for a worktree becomes >> >> 1) system config >> 2) XDG config >> 3) user config >> 4) $C/common/config >> 5) $C/worktrees/xxx/config (or $C/config for main worktree) >> >> Compare to an alternative scheme where $C/config contains both shared >> variables and main-worktree-only ones, this is a cleaner design. >> >> * We do not have to check every single variable name to see if it's >> shared or per-worktree when reading config files. >> >> * We do not enforce any particular variable split. If a variable >> is in $C/worktrees/config, it is shared. Putting core.worktree in >> $C/worktrees/config is punished the same way the variable is put in >> $HOME/.gitconfig, for example. >> >> * We will provide a new "git config --repo" to access this new config >> file. In single-worktree context, or worktree v0, --repo is an alias >> of --local. >> >> There is one problem though. We store worktree version in config file >> and expect that all worktrees must share the same version (i.e. read >> the same config file). But the share-ness of per-repo config files is >> changed based on worktree version. Where do we put extensions.worktree >> then? > > I cannot see why it cannot live in $C/common/config, which would be > read as the fourth alternative in your earlier enumeration. What I > am missing? I wasn't clear. The last paragraph is about already released git binaries, which does not care about $C/common/config. Suppose you add a new worktree with new git binary, which will move common stuff out of .git/config to common/config, on a shared repo. If the old binary reads that repo, it does not see common config, but it does not know where common config is either. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 07/10] gettext: add is_utf8_locale()
This function returns true if git is running under an UTF-8 locale. pcre in the next patch will need this. is_encoding_utf8() is used instead of strcmp() to catch both "utf-8" and "utf8" suffixes. When built with no gettext support, we peek in several env variables to detect UTF-8. pcre library might support utf-8 even if libc is built without locale support.. The peeking code is a copy from compat/regex/regcomp.c Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: Nguyễn Thái Ngọc Duy --- gettext.c | 24 ++-- gettext.h | 1 + 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/gettext.c b/gettext.c index a268a2c..db727ea 100644 --- a/gettext.c +++ b/gettext.c @@ -18,6 +18,8 @@ # endif #endif +static const char *charset; + /* * Guess the user's preferred languages from the value in LANGUAGE environment * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined. @@ -65,7 +67,6 @@ static int test_vsnprintf(const char *fmt, ...) return ret; } -static const char *charset; static void init_gettext_charset(const char *domain) { /* @@ -172,8 +173,27 @@ int gettext_width(const char *s) { static int is_utf8 = -1; if (is_utf8 == -1) - is_utf8 = !strcmp(charset, "UTF-8"); + is_utf8 = is_utf8_locale(); return is_utf8 ? utf8_strwidth(s) : strlen(s); } #endif + +int is_utf8_locale(void) +{ +#ifdef NO_GETTEXT + if (!charset) { + const char *env = getenv("LC_ALL"); + if (!env || !*env) + env = getenv("LC_CTYPE"); + if (!env || !*env) + env = getenv("LANG"); + if (!env) + env = ""; + if (strchr(env, '.')) + env = strchr(env, '.') + 1; + charset = xstrdup(env); + } +#endif + return is_encoding_utf8(charset); +} diff --git a/gettext.h b/gettext.h index 33696a4..7eee64a 100644 --- a/gettext.h +++ b/gettext.h @@ -90,5 +90,6 @@ const char *Q_(const char *msgid, const char *plu, unsigned long n) #endif const char *get_preferred_languages(void); +extern int is_utf8_locale(void); #endif -- 2.7.0.288.g1d8ad15 -- To unsubscribe from this list: send the line "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] attempt connects in parallel for IPv6-capable builds
getaddrinfo() may return multiple addresses, not all of which are equally performant. In some cases, a user behind a non-IPv6 capable network may get an IPv6 address which stalls connect(). Instead of waiting synchronously for a connect() to timeout, use non-blocking connect() in parallel and take the first successful connection. This may increase network traffic and server load slightly, but makes the worst-case user experience more bearable when one lacks permissions to edit /etc/gai.conf to favor IPv4 addresses. Signed-off-by: Eric Wong --- connect.c | 118 ++ 1 file changed, 104 insertions(+), 14 deletions(-) diff --git a/connect.c b/connect.c index fd7ffe1..74d2bb5 100644 --- a/connect.c +++ b/connect.c @@ -14,6 +14,42 @@ static char *server_capabilities; static const char *parse_feature_value(const char *, const char *, int *); +#ifdef SOCK_NONBLOCK /* Linux-only flag */ +# define GIT_SOCK_NONBLOCK SOCK_NONBLOCK +#else +# define GIT_SOCK_NONBLOCK 0 +#endif + +static int socket_nb(int domain, int type, int protocol) +{ + static int flags = GIT_SOCK_NONBLOCK; + int fd = socket(domain, type | flags, protocol); + + /* new headers, old kernel? */ + if (fd < 0 && errno == EINVAL && flags != 0) { + flags = 0; + fd = socket(domain, type, protocol); + } + + /* couldn't use SOCK_NONBLOCK, set non-blocking the old way */ + if (flags == 0 && fd >= 0) { + int fl = fcntl(fd, F_GETFL); + + if (fcntl(fd, F_SETFL, fl | O_NONBLOCK) < 0) + die_errno("failed to set nonblocking flag\n"); + } + + return fd; +} + +static void set_blocking(int fd) +{ + int fl = fcntl(fd, F_GETFL); + + if (fcntl(fd, F_SETFL, fl & ~O_NONBLOCK) < 0) + die_errno("failed to clear nonblocking flag\n"); +} + static int check_ref(const char *name, unsigned int flags) { if (!flags) @@ -351,6 +387,9 @@ static int git_tcp_connect_sock(char *host, int flags) struct addrinfo hints, *ai0, *ai; int gai; int cnt = 0; + nfds_t n = 0, nfds = 0; + struct pollfd *fds = NULL; + struct addrinfo **inprogress = NULL; get_host_and_port(&host, &port); if (!*port) @@ -371,20 +410,76 @@ static int git_tcp_connect_sock(char *host, int flags) fprintf(stderr, "done.\nConnecting to %s (port %s) ... ", host, port); for (ai0 = ai; ai; ai = ai->ai_next, cnt++) { - sockfd = socket(ai->ai_family, - ai->ai_socktype, ai->ai_protocol); - if ((sockfd < 0) || - (connect(sockfd, ai->ai_addr, ai->ai_addrlen) < 0)) { + size_t cur; + int fd = socket_nb(ai->ai_family, ai->ai_socktype, + ai->ai_protocol); + if (fd < 0) { strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n", host, cnt, ai_name(ai), strerror(errno)); - if (0 <= sockfd) - close(sockfd); - sockfd = -1; continue; } + + if (connect(fd, ai->ai_addr, ai->ai_addrlen) < 0 && + errno != EINPROGRESS) { + strbuf_addf(&error_message, "%s[%d: %s]: errno=%s\n", + host, cnt, ai_name(ai), strerror(errno)); + close(fd); + continue; + } + if (flags & CONNECT_VERBOSE) - fprintf(stderr, "%s ", ai_name(ai)); - break; + fprintf(stderr, "%s (started)\n", ai_name(ai)); + + nfds = n + 1; + cur = n; + ALLOC_GROW(fds, nfds, cur); + cur = n; + ALLOC_GROW(inprogress, nfds, cur); + inprogress[n] = ai; + fds[n].fd = fd; + fds[n].events = POLLIN|POLLOUT; + fds[n].revents = 0; + n = nfds; + } + + /* +* nfds is tiny, no need to limit loop based on poll() retval, +* just do not let poll sleep forever if nfds is zero +*/ + if (nfds > 0) + poll(fds, nfds, -1); + + for (n = 0; n < nfds && sockfd < 0; n++) { + if (fds[n].revents & (POLLERR|POLLHUP)) + continue; + if (fds[n].revents & POLLOUT) { + int err; + socklen_t len = (socklen_t)sizeof(err); + int rc = getsockopt(fds[n].fd, SOL_SOCKET, SO_ERROR, + &err, &len); + if (rc != 0) + die_errno("getsockopt errno=%s\n", +
[PATCH v5 05/10] grep/icase: avoid kwsset when -F is specified
Similar to the previous commit, we can't use kws on icase search outside ascii range. But we can't simply pass the pattern to regcomp/pcre like the previous commit because it may contain regex special characters, so we need to quote the regex first. To avoid misquote traps that could lead to undefined behavior, we always stick to basic regex engine in this case. We don't need fancy features for grepping a literal string anyway. basic_regex_quote_buf() assumes that if the pattern is in a multibyte encoding, ascii chars must be unambiguously encoded as single bytes. This is true at least for UTF-8. For others, let's wait until people yell up. Chances are nobody uses multibyte, non utf-8 charsets any more.. Helped-by: René Scharfe Noticed-by: Plamen Totev Signed-off-by: Nguyễn Thái Ngọc Duy --- grep.c | 25 - quote.c | 37 + quote.h | 1 + t/t7812-grep-icase-non-ascii.sh | 26 ++ 4 files changed, 88 insertions(+), 1 deletion(-) diff --git a/grep.c b/grep.c index f2180a2..fa96a29 100644 --- a/grep.c +++ b/grep.c @@ -5,6 +5,7 @@ #include "diff.h" #include "diffcore.h" #include "commit.h" +#include "quote.h" static int grep_source_load(struct grep_source *gs); static int grep_source_is_binary(struct grep_source *gs); @@ -397,6 +398,24 @@ static int is_fixed(const char *s, size_t len) return 1; } +static void compile_fixed_regexp(struct grep_pat *p, struct grep_opt *opt) +{ + struct strbuf sb = STRBUF_INIT; + int err; + + basic_regex_quote_buf(&sb, p->pattern); + err = regcomp(&p->regexp, sb.buf, opt->regflags & ~REG_EXTENDED); + if (opt->debug) + fprintf(stderr, "fixed%s\n", sb.buf); + strbuf_release(&sb); + if (err) { + char errbuf[1024]; + regerror(err, &p->regexp, errbuf, 1024); + regfree(&p->regexp); + compile_regexp_failed(p, errbuf); + } +} + static void compile_regexp(struct grep_pat *p, struct grep_opt *opt) { int icase_non_ascii; @@ -411,7 +430,11 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt) if (!icase_non_ascii && is_fixed(p->pattern, p->patternlen)) p->fixed = 1; else if (opt->fixed) { - p->fixed = 1; + p->fixed = !icase_non_ascii; + if (!p->fixed) { + compile_fixed_regexp(p, opt); + return; + } } else p->fixed = 0; diff --git a/quote.c b/quote.c index fe884d2..c67adb7 100644 --- a/quote.c +++ b/quote.c @@ -440,3 +440,40 @@ void tcl_quote_buf(struct strbuf *sb, const char *src) } strbuf_addch(sb, '"'); } + +void basic_regex_quote_buf(struct strbuf *sb, const char *src) +{ + char c; + + if (*src == '^') { + /* only beginning '^' is special and needs quoting */ + strbuf_addch(sb, '\\'); + strbuf_addch(sb, *src++); + } + if (*src == '*') + /* beginning '*' is not special, no quoting */ + strbuf_addch(sb, *src++); + + while ((c = *src++)) { + switch (c) { + case '[': + case '.': + case '\\': + case '*': + strbuf_addch(sb, '\\'); + strbuf_addch(sb, c); + break; + + case '$': + /* only the end '$' is special and needs quoting */ + if (*src == '\0') + strbuf_addch(sb, '\\'); + strbuf_addch(sb, c); + break; + + default: + strbuf_addch(sb, c); + break; + } + } +} diff --git a/quote.h b/quote.h index 99e04d3..362d315 100644 --- a/quote.h +++ b/quote.h @@ -67,5 +67,6 @@ extern char *quote_path_relative(const char *in, const char *prefix, extern void perl_quote_buf(struct strbuf *sb, const char *src); extern void python_quote_buf(struct strbuf *sb, const char *src); extern void tcl_quote_buf(struct strbuf *sb, const char *src); +extern void basic_regex_quote_buf(struct strbuf *sb, const char *src); #endif diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh index 6eff490..aba6b15 100755 --- a/t/t7812-grep-icase-non-ascii.sh +++ b/t/t7812-grep-icase-non-ascii.sh @@ -20,4 +20,30 @@ test_expect_success REGEX_LOCALE 'grep literal string, no -F' ' git grep -i "TILRAUN: HALLÓ HEIMUR!" ' +test_expect_success REGEX_LOCALE 'grep literal string, with -F' ' + git grep --debug -i -F "TILRAUN: Halló Heimur!" 2>&1 >/dev/null | +grep fixed >debug1 && + echo "fixedTILRAUN: Halló Heimur!" >expect1 && + test_c
[PATCH v5 10/10] diffcore-pickaxe: support case insensitive match on non-ascii
Similar to the "grep -F -i" case, we can't use kws on icase search outside ascii range, so we quote the string and pass it to regcomp as a basic regexp and let regex engine deal with case sensitivity. The new test is put in t7812 instead of t4209-log-pickaxe because lib-gettext.sh might cause problems elsewhere, probably.. Noticed-by: Plamen Totev Signed-off-by: Nguyễn Thái Ngọc Duy --- diffcore-pickaxe.c | 11 +++ t/t7812-grep-icase-non-ascii.sh | 7 +++ 2 files changed, 18 insertions(+) diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c index 69c6567..0a5f877 100644 --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -7,6 +7,8 @@ #include "diffcore.h" #include "xdiff-interface.h" #include "kwset.h" +#include "commit.h" +#include "quote.h" typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two, struct diff_options *o, @@ -212,6 +214,15 @@ void diffcore_pickaxe(struct diff_options *o) cflags |= REG_ICASE; err = regcomp(®ex, needle, cflags); regexp = ®ex; + } else if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE) && + has_non_ascii(needle)) { + struct strbuf sb = STRBUF_INIT; + int cflags = REG_NEWLINE | REG_ICASE; + + basic_regex_quote_buf(&sb, needle); + err = regcomp(®ex, sb.buf, cflags); + strbuf_release(&sb); + regexp = ®ex; } else { kws = kwsalloc(DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE) ? tolower_trans_tbl : NULL); diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh index 8896410..a5475bb 100755 --- a/t/t7812-grep-icase-non-ascii.sh +++ b/t/t7812-grep-icase-non-ascii.sh @@ -61,4 +61,11 @@ test_expect_success REGEX_LOCALE 'grep string with regex, with -F' ' test_cmp expect2 debug2 ' +test_expect_success REGEX_LOCALE 'pickaxe -i on non-ascii' ' + git commit -m first && + git log --format=%f -i -S"TILRAUN: HALLÓ HEIMUR!" >actual && + echo first >expected && + test_cmp expected actual +' + test_done -- 2.7.0.288.g1d8ad15 -- To unsubscribe from this list: send the line "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 v5 06/10] grep/pcre: prepare locale-dependent tables for icase matching
The default tables are usually built with C locale and only suitable for LANG=C or similar. This should make case insensitive search work correctly for all single-byte charsets. Signed-off-by: Nguyễn Thái Ngọc Duy --- grep.c | 8 ++-- grep.h | 1 + t/t7813-grep-icase-iso.sh (new +x) | 19 +++ 3 files changed, 26 insertions(+), 2 deletions(-) create mode 100755 t/t7813-grep-icase-iso.sh diff --git a/grep.c b/grep.c index fa96a29..921339a 100644 --- a/grep.c +++ b/grep.c @@ -324,11 +324,14 @@ static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt) int erroffset; int options = PCRE_MULTILINE; - if (opt->ignore_case) + if (opt->ignore_case) { + if (has_non_ascii(p->pattern)) + p->pcre_tables = pcre_maketables(); options |= PCRE_CASELESS; + } p->pcre_regexp = pcre_compile(p->pattern, options, &error, &erroffset, - NULL); + p->pcre_tables); if (!p->pcre_regexp) compile_regexp_failed(p, error); @@ -362,6 +365,7 @@ static void free_pcre_regexp(struct grep_pat *p) { pcre_free(p->pcre_regexp); pcre_free(p->pcre_extra_info); + pcre_free((void *)p->pcre_tables); } #else /* !USE_LIBPCRE */ static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt) diff --git a/grep.h b/grep.h index 95f197a..cee4357 100644 --- a/grep.h +++ b/grep.h @@ -48,6 +48,7 @@ struct grep_pat { regex_t regexp; pcre *pcre_regexp; pcre_extra *pcre_extra_info; + const unsigned char *pcre_tables; kwset_t kws; unsigned fixed:1; unsigned ignore_case:1; diff --git a/t/t7813-grep-icase-iso.sh b/t/t7813-grep-icase-iso.sh new file mode 100755 index 000..efef7fb --- /dev/null +++ b/t/t7813-grep-icase-iso.sh @@ -0,0 +1,19 @@ +#!/bin/sh + +test_description='grep icase on non-English locales' + +. ./lib-gettext.sh + +test_expect_success GETTEXT_ISO_LOCALE 'setup' ' + printf "TILRAUN: Hall� Heimur!" >file && + git add file && + LC_ALL="$is_IS_iso_locale" && + export LC_ALL +' + +test_expect_success GETTEXT_ISO_LOCALE,LIBPCRE 'grep pcre string' ' + git grep --perl-regexp -i "TILRAUN: H.ll� Heimur!" && + git grep --perl-regexp -i "TILRAUN: H.LL� HEIMUR!" +' + +test_done -- 2.7.0.288.g1d8ad15 -- To unsubscribe from this list: send the line "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 v5 09/10] diffcore-pickaxe: "share" regex error handling code
There's another regcomp code block coming in this function. By moving the error handling code out of this block, we don't have to add the same error handling code in the new block. Signed-off-by: Nguyễn Thái Ngọc Duy --- diffcore-pickaxe.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c index 7715c13..69c6567 100644 --- a/diffcore-pickaxe.c +++ b/diffcore-pickaxe.c @@ -204,20 +204,13 @@ void diffcore_pickaxe(struct diff_options *o) int opts = o->pickaxe_opts; regex_t regex, *regexp = NULL; kwset_t kws = NULL; + int err = 0; if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) { - int err; int cflags = REG_EXTENDED | REG_NEWLINE; if (DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE)) cflags |= REG_ICASE; err = regcomp(®ex, needle, cflags); - if (err) { - /* The POSIX.2 people are surely sick */ - char errbuf[1024]; - regerror(err, ®ex, errbuf, 1024); - regfree(®ex); - die("invalid regex: %s", errbuf); - } regexp = ®ex; } else { kws = kwsalloc(DIFF_OPT_TST(o, PICKAXE_IGNORE_CASE) @@ -225,6 +218,13 @@ void diffcore_pickaxe(struct diff_options *o) kwsincr(kws, needle, strlen(needle)); kwsprep(kws); } + if (err) { + /* The POSIX.2 people are surely sick */ + char errbuf[1024]; + regerror(err, ®ex, errbuf, 1024); + regfree(®ex); + die("invalid regex: %s", errbuf); + } /* Might want to warn when both S and G are on; I don't care... */ pickaxe(&diff_queued_diff, o, regexp, kws, -- 2.7.0.288.g1d8ad15 -- To unsubscribe from this list: send the line "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 v5 08/10] grep/pcre: support utf-8
In the previous change in this function, we add locale support for single-byte encodings only. It looks like pcre only supports utf-* as multibyte encodings, the others are left in the cold (which is fine). We need to enable PCRE_UTF8 so pcre can find character boundary correctly. It's needed for case folding (when --ignore-case is used) or '*', '+' or similar syntax is used. The "has_non_ascii()" check is to be on the conservative side. If there's non-ascii in the pattern, the searched content could still be in utf-8, but we can treat it just like a byte stream and everything should work. If we force utf-8 based on locale only and pcre validates utf-8 and the file content is in non-utf8 encoding, things break. Noticed-by: Plamen Totev Helped-by: Plamen Totev Signed-off-by: Nguyễn Thái Ngọc Duy --- grep.c | 2 ++ t/t7812-grep-icase-non-ascii.sh | 15 +++ 2 files changed, 17 insertions(+) diff --git a/grep.c b/grep.c index 921339a..2e4f71d 100644 --- a/grep.c +++ b/grep.c @@ -329,6 +329,8 @@ static void compile_pcre_regexp(struct grep_pat *p, const struct grep_opt *opt) p->pcre_tables = pcre_maketables(); options |= PCRE_CASELESS; } + if (is_utf8_locale() && has_non_ascii(p->pattern)) + options |= PCRE_UTF8; p->pcre_regexp = pcre_compile(p->pattern, options, &error, &erroffset, p->pcre_tables); diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh index aba6b15..8896410 100755 --- a/t/t7812-grep-icase-non-ascii.sh +++ b/t/t7812-grep-icase-non-ascii.sh @@ -20,6 +20,21 @@ test_expect_success REGEX_LOCALE 'grep literal string, no -F' ' git grep -i "TILRAUN: HALLÓ HEIMUR!" ' +test_expect_success GETTEXT_LOCALE,LIBPCRE 'grep pcre utf-8 icase' ' + git grep --perl-regexp"TILRAUN: H.lló Heimur!" && + git grep --perl-regexp -i "TILRAUN: H.lló Heimur!" && + git grep --perl-regexp -i "TILRAUN: H.LLÓ HEIMUR!" +' + +test_expect_success GETTEXT_LOCALE,LIBPCRE 'grep pcre utf-8 string with "+"' ' + printf "TILRAUN: Hallóó Heimur!" >file2 && + git add file2 && + git grep -l --perl-regexp "TILRAUN: H.lló+ Heimur!" >actual && + echo file >expected && + echo file2 >>expected && + test_cmp expected actual +' + test_expect_success REGEX_LOCALE 'grep literal string, with -F' ' git grep --debug -i -F "TILRAUN: Halló Heimur!" 2>&1 >/dev/null | grep fixed >debug1 && -- 2.7.0.288.g1d8ad15 -- To unsubscribe from this list: send the line "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 v5 04/10] grep/icase: avoid kwsset on literal non-ascii strings
When we detect the pattern is just a literal string, we avoid heavy regex engine and use fast substring search implemented in kwsset.c. But kws uses git-ctype which is locale-independent so it does not know how to fold case properly outside ascii range. Let regcomp or pcre take care of this case instead. Slower, but accurate. Helped-by: René Scharfe Noticed-by: Plamen Totev Signed-off-by: Nguyễn Thái Ngọc Duy --- grep.c | 7 ++- t/t7812-grep-icase-non-ascii.sh (new +x) | 23 +++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100755 t/t7812-grep-icase-non-ascii.sh diff --git a/grep.c b/grep.c index e739d20..f2180a2 100644 --- a/grep.c +++ b/grep.c @@ -4,6 +4,7 @@ #include "xdiff-interface.h" #include "diff.h" #include "diffcore.h" +#include "commit.h" static int grep_source_load(struct grep_source *gs); static int grep_source_is_binary(struct grep_source *gs); @@ -398,12 +399,16 @@ static int is_fixed(const char *s, size_t len) static void compile_regexp(struct grep_pat *p, struct grep_opt *opt) { + int icase_non_ascii; int err; p->word_regexp = opt->word_regexp; p->ignore_case = opt->ignore_case; + icase_non_ascii = + (opt->regflags & REG_ICASE || p->ignore_case) && + has_non_ascii(p->pattern); - if (is_fixed(p->pattern, p->patternlen)) + if (!icase_non_ascii && is_fixed(p->pattern, p->patternlen)) p->fixed = 1; else if (opt->fixed) { p->fixed = 1; diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh new file mode 100755 index 000..6eff490 --- /dev/null +++ b/t/t7812-grep-icase-non-ascii.sh @@ -0,0 +1,23 @@ +#!/bin/sh + +test_description='grep icase on non-English locales' + +. ./lib-gettext.sh + +test_expect_success GETTEXT_LOCALE 'setup' ' + printf "TILRAUN: Halló Heimur!" >file && + git add file && + LC_ALL="$is_IS_locale" && + export LC_ALL +' + +test_have_prereq GETTEXT_LOCALE && +test-regex "HALLÓ" "Halló" ICASE && +test_set_prereq REGEX_LOCALE + +test_expect_success REGEX_LOCALE 'grep literal string, no -F' ' + git grep -i "TILRAUN: Halló Heimur!" && + git grep -i "TILRAUN: HALLÓ HEIMUR!" +' + +test_done -- 2.7.0.288.g1d8ad15 -- To unsubscribe from this list: send the line "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 v5 02/10] grep: break down an "if" stmt in preparation for next changes
Signed-off-by: Nguyễn Thái Ngọc Duy --- grep.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/grep.c b/grep.c index 7b2b96a..e739d20 100644 --- a/grep.c +++ b/grep.c @@ -403,9 +403,11 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt) p->word_regexp = opt->word_regexp; p->ignore_case = opt->ignore_case; - if (opt->fixed || is_fixed(p->pattern, p->patternlen)) + if (is_fixed(p->pattern, p->patternlen)) p->fixed = 1; - else + else if (opt->fixed) { + p->fixed = 1; + } else p->fixed = 0; if (p->fixed) { -- 2.7.0.288.g1d8ad15 -- To unsubscribe from this list: send the line "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 v5 03/10] test-regex: expose full regcomp() to the command line
Signed-off-by: Nguyễn Thái Ngọc Duy --- test-regex.c | 56 ++-- 1 file changed, 50 insertions(+), 6 deletions(-) diff --git a/test-regex.c b/test-regex.c index 0dc598e..3b5641c 100644 --- a/test-regex.c +++ b/test-regex.c @@ -1,19 +1,63 @@ #include "git-compat-util.h" +#include "gettext.h" + +struct reg_flag { + const char *name; + int flag; +}; + +static struct reg_flag reg_flags[] = { + { "EXTENDED",REG_EXTENDED }, + { "NEWLINE", REG_NEWLINE}, + { "ICASE", REG_ICASE }, + { "NOTBOL", REG_NOTBOL }, +#ifdef REG_STARTEND + { "STARTEND",REG_STARTEND }, +#endif + { NULL, 0 } +}; int main(int argc, char **argv) { - char *pat = "[^={} \t]+"; - char *str = "={}\nfred"; + const char *pat; + const char *str; + int flags = 0; regex_t r; regmatch_t m[1]; - if (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE)) + if (argc == 1) { + /* special case, bug check */ + pat = "[^={} \t]+"; + str = "={}\nfred"; + flags = REG_EXTENDED | REG_NEWLINE; + } else { + argv++; + pat = *argv++; + str = *argv++; + while (*argv) { + struct reg_flag *rf; + for (rf = reg_flags; rf->name; rf++) + if (!strcmp(*argv, rf->name)) { + flags |= rf->flag; + break; + } + if (!rf->name) + die("do not recognize %s", *argv); + argv++; + } + git_setup_gettext(); + } + + if (regcomp(&r, pat, flags)) die("failed regcomp() for pattern '%s'", pat); - if (regexec(&r, str, 1, m, 0)) - die("no match of pattern '%s' to string '%s'", pat, str); + if (regexec(&r, str, 1, m, 0)) { + if (argc == 1) + die("no match of pattern '%s' to string '%s'", pat, str); + return 1; + } /* http://sourceware.org/bugzilla/show_bug.cgi?id=3957 */ - if (m[0].rm_so == 3) /* matches '\n' when it should not */ + if (argc == 1 && m[0].rm_so == 3) /* matches '\n' when it should not */ die("regex bug confirmed: re-build git with NO_REGEX=1"); exit(0); -- 2.7.0.288.g1d8ad15 -- To unsubscribe from this list: send the line "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 v5 00/10] Fix icase grep on non-ascii
The series fixes grep choosing fast path that only works correctly for ascii. This is a resend of v4 [1] after rebase. I think v4 just went off radar for some reason, nothing was wrong with it (or nobody told me about it). [1] http://thread.gmane.org/gmane.comp.version-control.git/273381/focus=276288 Nguyễn Thái Ngọc Duy (10): grep: allow -F -i combination grep: break down an "if" stmt in preparation for next changes test-regex: expose full regcomp() to the command line grep/icase: avoid kwsset on literal non-ascii strings grep/icase: avoid kwsset when -F is specified grep/pcre: prepare locale-dependent tables for icase matching gettext: add is_utf8_locale() grep/pcre: support utf-8 diffcore-pickaxe: "share" regex error handling code diffcore-pickaxe: support case insensitive match on non-ascii builtin/grep.c | 2 +- diffcore-pickaxe.c | 27 gettext.c| 24 ++- gettext.h| 1 + grep.c | 44 ++-- grep.h | 1 + quote.c | 37 + quote.h | 1 + t/t7812-grep-icase-non-ascii.sh (new +x) | 71 t/t7813-grep-icase-iso.sh (new +x) | 19 + test-regex.c | 56 ++--- 11 files changed, 262 insertions(+), 21 deletions(-) create mode 100755 t/t7812-grep-icase-non-ascii.sh create mode 100755 t/t7813-grep-icase-iso.sh -- 2.7.0.288.g1d8ad15 -- To unsubscribe from this list: send the line "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 v5 01/10] grep: allow -F -i combination
-F means "no regex", not "case sensitive" so it should not override -i Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/grep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/grep.c b/builtin/grep.c index 5526fd7..4be0df5 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -809,7 +809,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (!opt.pattern_list) die(_("no pattern given.")); - if (!opt.fixed && opt.ignore_case) + if (opt.ignore_case) opt.regflags |= REG_ICASE; compile_grep_patterns(&opt); -- 2.7.0.288.g1d8ad15 -- To unsubscribe from this list: send the line "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: Starting on a microproject for GSoC
On 01/28/2016 02:18 AM, Stefan Beller wrote: > On Wed, Jan 27, 2016 at 4:40 PM, Moritz Neeb wrote: >> Before I may introduce myself: I'm a Computer Science student in >> Germany, coming towards the end of my Masters. I am an enthusiastic git >> user that's why I'd like to give something back. > > Giving back is a noble thing. To have most fun at it, you need to ask > yourself: > What is the most obnoxious part of Git that you personally use? What was > the latest problem you had, which you'd want to fix? If you identified > that, is that the right size to fix it? (Usually it is way bigger than > thought, > but you can ask around if there are approaches for solving that problem ;) You're right, creating something that is in the end relevant and useful for myself is even more fun. I have some itches, I will work on specifying them. I have the feeling though, that for solving the daily issues and itches it's not necessary to dig into the core of git. For example, I sometimes create a commit with the wrong email address (I try to separate between work/private stuff) and that annoys me. I think this could be solved by some hook rather than modifying git itself. > I just realized there are some micro projects on the 2014 page > as well which haven't been solved yet. (maybe, they are not > striked through) Yeah, I realized too that there are some points not striked through in [0]. Might be just not up to date, for example number 15. seems to be solved ($gmane//244020). Looking into the code, 14. is solved as well. For 17. there could be something left. Thanks, Moritz [0] http://git.github.io/SoC-2014-Microprojects.html -- To unsubscribe from this list: send the line "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] convert: legitimately disable clean/smudge filter with an empty override
On 25 Jan 2016, at 02:25, Junio C Hamano wrote: > larsxschnei...@gmail.com writes: > >> From: Lars Schneider >> >> A clean/smudge filter can be disabled if set to an empty string. However, >> Git will try to run the empty string as command which results in a error >> message per processed file. > > The above two sentences do not make any sense to me. You observe > that the command refuses to work when the variable is set to an > empty string--why then can you claim "filter can be disabled if set > to an empty string"? I'd consider that the system is not working > with such a configuration, i.e. "filter cannot be disabled by > setting it to empty; such a request will result in an error". If I am not mistaken then Git exits with 0 (success) and an error message if the filter command is empty and the filter is not required. If the filter command is empty and the filter is required then Git will exit with 1 (error). How about this? If the clean/smudge command of a Git filter driver (filter..smudge and filter..clean) is set to an empty string ("") and the filter driver is not required (filter..required=false) then Git will run successfully. However, Git will print an error for every file that is affected by the filter. Teach Git to consider an empty clean/smudge filter as legitimately disabled and do not print an error message if the filter is not required. Thanks, Lars > >> Teach Git to consider an empty clean/smudge filter as legitimately disabled >> and do not print an error message. > > On the other hand, this does make sense to me, as I do not think of > a good way to say "earlier configuration entry said we should use > this filter, but we actually do not want to use that one (or any)" > because a configuration, unlike attributes entry, cannot be reset. > The closest you can do is to set it to empty, so it may be a good > new feature to do this. > > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git archive should use vendor extension in pax header
Hello, > > > Users can always go back to the original format. At least I don't > > > expect this new format becoming the default too quickly. > > This is the most crucial issue here, as far as I am concerned: there are > already tons of .zip files out there that were created by git archive, and > there will inevitably be loads of tons more *having the current pax header > format*. > > So tools wanting to deal with Git archives will have to handle those as > well, i.e. do *precisely* as René suggested and use get-tar-commit-id. As > such, the value of changing the format *now* is a bit like closing the > barn's door after pretty much all of the horses left (except the old one > that has a few troubles getting up in the morning but that is too nice to > the kids to shoot). That's not really an argument. The situation you describes applies to all file formats and it always ends in the same way: A new file format is designed and then slowly adopted by the rest of the users, in case of git I imagine this to be a quick process taking maybe a year or two. Newly created files use the new file format and old files still hang around but their importance is dwindling until you can safely support only the new format. But to get there, a new file format has to be adopted in the first place. > > Sure thing. If this is going to be implemented, I would add options > > to choose what / what style of metadata to include. > > Why not put your money where your mouth is? I.e. get your head down into > the code and come up with a patch (because otherwise it is unlikely that > your idea will go anywhere)? I'd love to but I prefer to ask if there is interest in such a change in the first place. I'm not going to waste my time implementing this and then being told that the git project is not interested in this kind of functionality. So can someone give me a clear signal? > Ciao, > Johannes Yours sincerely, Robert Clausecker -- () ascii ribbon campaign - for an 8-bit clean world /\ - against html email - against proprietary attachments -- To unsubscribe from this list: send the line "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 v2 6/9] init-db: handle config errors
When creating default files we do not check for error codes returned by `git_config_set` functions. This may cause the user to end up with an inconsistent repository without any indication for the user. Fix this problem by dying with an error message when we are unable to write the configuration files to disk. Signed-off-by: Patrick Steinhardt --- builtin/init-db.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/builtin/init-db.c b/builtin/init-db.c index 07229d6..ef19048 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -227,7 +227,7 @@ static int create_default_files(const char *template_path) /* This forces creation of new config file */ xsnprintf(repo_version_string, sizeof(repo_version_string), "%d", GIT_REPO_VERSION); - git_config_set("core.repositoryformatversion", repo_version_string); + git_config_set_or_die("core.repositoryformatversion", repo_version_string); /* Check filemode trustability */ path = git_path_buf(&buf, "config"); @@ -241,18 +241,18 @@ static int create_default_files(const char *template_path) if (filemode && !reinit && (st1.st_mode & S_IXUSR)) filemode = 0; } - git_config_set("core.filemode", filemode ? "true" : "false"); + git_config_set_or_die("core.filemode", filemode ? "true" : "false"); if (is_bare_repository()) - git_config_set("core.bare", "true"); + git_config_set_or_die("core.bare", "true"); else { const char *work_tree = get_git_work_tree(); - git_config_set("core.bare", "false"); + git_config_set_or_die("core.bare", "false"); /* allow template config file to override the default */ if (log_all_ref_updates == -1) - git_config_set("core.logallrefupdates", "true"); + git_config_set_or_die("core.logallrefupdates", "true"); if (needs_work_tree_config(get_git_dir(), work_tree)) - git_config_set("core.worktree", work_tree); + git_config_set_or_die("core.worktree", work_tree); } if (!reinit) { @@ -265,12 +265,12 @@ static int create_default_files(const char *template_path) S_ISLNK(st1.st_mode)) unlink(path); /* good */ else - git_config_set("core.symlinks", "false"); + git_config_set_or_die("core.symlinks", "false"); /* Check if the filesystem is case-insensitive */ path = git_path_buf(&buf, "CoNfIg"); if (!access(path, F_OK)) - git_config_set("core.ignorecase", "true"); + git_config_set_or_die("core.ignorecase", "true"); probe_utf8_pathname_composition(); } @@ -386,8 +386,8 @@ int init_db(const char *template_dir, unsigned int flags) xsnprintf(buf, sizeof(buf), "%d", OLD_PERM_EVERYBODY); else die("BUG: invalid value for shared_repository"); - git_config_set("core.sharedrepository", buf); - git_config_set("receive.denyNonFastforwards", "true"); + git_config_set_or_die("core.sharedrepository", buf); + git_config_set_or_die("receive.denyNonFastforwards", "true"); } if (!(flags & INIT_DB_QUIET)) { -- 2.7.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
[PATCH v2 5/9] branch: handle config errors when unsetting upstream
When we try to unset upstream configurations we do not check return codes for the `git_config_set` functions. As those may indicate that we were unable to unset the respective configuration we may exit successfully without any error message while in fact the upstream configuration was not unset. Fix this by dying with an error message when we cannot unset the configuration. Signed-off-by: Patrick Steinhardt --- builtin/branch.c | 4 ++-- t/t3200-branch.sh | 7 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 3f6c825..0978287 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -791,10 +791,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix) die(_("Branch '%s' has no upstream information"), branch->name); strbuf_addf(&buf, "branch.%s.remote", branch->name); - git_config_set_multivar(buf.buf, NULL, NULL, 1); + git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1); strbuf_reset(&buf); strbuf_addf(&buf, "branch.%s.merge", branch->name); - git_config_set_multivar(buf.buf, NULL, NULL, 1); + git_config_set_multivar_or_die(buf.buf, NULL, NULL, 1); strbuf_release(&buf); } else if (argc > 0 && argc <= 2) { struct branch *branch = branch_get(argv[0]); diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index dd776b3..a897248 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -473,6 +473,13 @@ test_expect_success '--unset-upstream should fail if given a non-existent branch test_must_fail git branch --unset-upstream i-dont-exist ' +test_expect_success '--unset-upstream should fail if config is locked' ' + test_when_finished "rm -f .git/config.lock" && + git branch --set-upstream-to locked && + >.git/config.lock && + test_must_fail git branch --unset-upstream +' + test_expect_success 'test --unset-upstream on HEAD' ' git branch my14 && test_config branch.master.remote foo && -- 2.7.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
[PATCH v2 7/9] sequencer: handle config errors when saving opts
When we start picking a range of revisions we save the replay options that are required to restore state when interrupting and later continuing picking the revisions. However, we do not check the return values of the `git_config_set` functions, which may lead us to store incomplete information. As this may lead us to fail when trying to continue the sequence this error can be fatal. Fix this by dying immediately when we are unable to write back any replay option. Signed-off-by: Patrick Steinhardt --- sequencer.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sequencer.c b/sequencer.c index 8c58fa2..3c109b6 100644 --- a/sequencer.c +++ b/sequencer.c @@ -933,31 +933,31 @@ static void save_opts(struct replay_opts *opts) const char *opts_file = git_path_opts_file(); if (opts->no_commit) - git_config_set_in_file(opts_file, "options.no-commit", "true"); + git_config_set_in_file_or_die(opts_file, "options.no-commit", "true"); if (opts->edit) - git_config_set_in_file(opts_file, "options.edit", "true"); + git_config_set_in_file_or_die(opts_file, "options.edit", "true"); if (opts->signoff) - git_config_set_in_file(opts_file, "options.signoff", "true"); + git_config_set_in_file_or_die(opts_file, "options.signoff", "true"); if (opts->record_origin) - git_config_set_in_file(opts_file, "options.record-origin", "true"); + git_config_set_in_file_or_die(opts_file, "options.record-origin", "true"); if (opts->allow_ff) - git_config_set_in_file(opts_file, "options.allow-ff", "true"); + git_config_set_in_file_or_die(opts_file, "options.allow-ff", "true"); if (opts->mainline) { struct strbuf buf = STRBUF_INIT; strbuf_addf(&buf, "%d", opts->mainline); - git_config_set_in_file(opts_file, "options.mainline", buf.buf); + git_config_set_in_file_or_die(opts_file, "options.mainline", buf.buf); strbuf_release(&buf); } if (opts->strategy) - git_config_set_in_file(opts_file, "options.strategy", opts->strategy); + git_config_set_in_file_or_die(opts_file, "options.strategy", opts->strategy); if (opts->gpg_sign) - git_config_set_in_file(opts_file, "options.gpg-sign", opts->gpg_sign); + git_config_set_in_file_or_die(opts_file, "options.gpg-sign", opts->gpg_sign); if (opts->xopts) { int i; for (i = 0; i < opts->xopts_nr; i++) - git_config_set_multivar_in_file(opts_file, - "options.strategy-option", - opts->xopts[i], "^$", 0); + git_config_set_multivar_in_file_or_die(opts_file, + "options.strategy-option", + opts->xopts[i], "^$", 0); } } -- 2.7.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
[PATCH v2 0/9] Handle errors when setting configs
I've finally got around to producing version two of my previous patch to handle errors when setting configs. Back in September I've posted a single patch to handle errors when `install_branch_config` fails due to configuration failures [1]. Failure to write the configuration file may be caused by multiple conditions, but the most common one will surely be the case where the configuration is locked because of a leftover lock file or because another process is currently writing to it. We used to ignore those errors in many cases, possibly leading to inconsistently configured repositories. More often than not git even pretended that everything was fine and that the settings have been applied when indeed they were not. Version two of this patch is somewhat more involved in that I tried to track down all relevant places where we set configs without checking for error conditions. My current approach to most of those cases is to just die with an error message, this remains up to discussion though for the individual cases. [1]: $gmane/278544 Patrick Steinhardt (9): config: introduce set_or_die wrappers branch: return error code for install_branch_config remote: handle config errors in set_url clone: handle config errors in cmd_clone branch: handle config errors when unsetting upstream init-db: handle config errors sequencer: handle config errors when saving opts submodule--helper: handle config errors compat: die when unable to set core.precomposeunicode branch.c| 31 +-- branch.h| 3 ++- builtin/branch.c| 4 ++-- builtin/clone.c | 17 ++--- builtin/init-db.c | 20 ++-- builtin/remote.c| 11 ++- builtin/submodule--helper.c | 4 ++-- cache.h | 4 compat/precompose_utf8.c| 3 ++- config.c| 27 +++ sequencer.c | 22 +++--- t/t3200-branch.sh | 16 +++- t/t5505-remote.sh | 9 + transport.c | 11 ++- 14 files changed, 127 insertions(+), 55 deletions(-) -- 2.7.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
[PATCH v2 3/9] remote: handle config errors in set_url
The return codes for function calls that write the new URL configuration are not being checked in `set_url`. As the function is exposed to the user directly through `git remote set-url` we want to inform her that we were not able to complete the request and abort the program. Signed-off-by: Patrick Steinhardt --- builtin/remote.c | 11 ++- t/t5505-remote.sh | 9 + 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/builtin/remote.c b/builtin/remote.c index 2b2ff9b..8b78c3d 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -1583,11 +1583,12 @@ static int set_url(int argc, const char **argv) /* Special cases that add new entry. */ if ((!oldurl && !delete_mode) || add_mode) { if (add_mode) - git_config_set_multivar(name_buf.buf, newurl, - "^$", 0); + git_config_set_multivar_or_die(name_buf.buf, newurl, + "^$", 0); else - git_config_set(name_buf.buf, newurl); + git_config_set_or_die(name_buf.buf, newurl); strbuf_release(&name_buf); + return 0; } @@ -1608,9 +1609,9 @@ static int set_url(int argc, const char **argv) regfree(&old_regex); if (!delete_mode) - git_config_set_multivar(name_buf.buf, newurl, oldurl, 0); + git_config_set_multivar_or_die(name_buf.buf, newurl, oldurl, 0); else - git_config_set_multivar(name_buf.buf, NULL, oldurl, 1); + git_config_set_multivar_or_die(name_buf.buf, NULL, oldurl, 1); return 0; } diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 1a8e3b8..e019f27 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -932,6 +932,15 @@ test_expect_success 'get-url on new remote' ' echo foo | get_url_test --push --all someremote ' +test_expect_success 'remote set-url with locked config' ' + test_when_finished "rm -f .git/config.lock" && + git config --get-all remote.someremote.url >expect && + >.git/config.lock && + test_must_fail git remote set-url someremote baz && + git config --get-all remote.someremote.url >actual && + cmp expect actual +' + test_expect_success 'remote set-url bar' ' git remote set-url someremote bar && echo bar >expect && -- 2.7.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
[PATCH v2 2/9] branch: return error code for install_branch_config
The install_branch_config function may fail to write the configuration file due to locking. To accomodate for this scenario, introduce a return value which may be used to check if the function did finish correctly and adjust callers to use this error code. Signed-off-by: Patrick Steinhardt --- branch.c | 31 +-- branch.h | 3 ++- builtin/clone.c | 9 ++--- t/t3200-branch.sh | 9 - transport.c | 11 ++- 5 files changed, 43 insertions(+), 20 deletions(-) diff --git a/branch.c b/branch.c index 7ff3f20..7f35bcf 100644 --- a/branch.c +++ b/branch.c @@ -49,33 +49,38 @@ static int should_setup_rebase(const char *origin) return 0; } -void install_branch_config(int flag, const char *local, const char *origin, const char *remote) +int install_branch_config(int flag, const char *local, const char *origin, const char *remote) { const char *shortname = NULL; struct strbuf key = STRBUF_INIT; - int rebasing = should_setup_rebase(origin); + int ret = 0, rebasing = should_setup_rebase(origin); if (skip_prefix(remote, "refs/heads/", &shortname) && !strcmp(local, shortname) && !origin) { warning(_("Not setting branch %s as its own upstream."), local); - return; + return ret; } strbuf_addf(&key, "branch.%s.remote", local); - git_config_set(key.buf, origin ? origin : "."); + ret = git_config_set(key.buf, origin ? origin : "."); + if (ret) + goto out; strbuf_reset(&key); strbuf_addf(&key, "branch.%s.merge", local); - git_config_set(key.buf, remote); + ret = git_config_set(key.buf, remote); + if (ret) + goto out; if (rebasing) { strbuf_reset(&key); strbuf_addf(&key, "branch.%s.rebase", local); - git_config_set(key.buf, "true"); + ret = git_config_set(key.buf, "true"); + if (ret) + goto out; } - strbuf_release(&key); if (flag & BRANCH_CONFIG_VERBOSE) { if (shortname) { @@ -102,6 +107,10 @@ void install_branch_config(int flag, const char *local, const char *origin, cons local, remote); } } + +out: + strbuf_release(&key); + return ret; } /* @@ -134,8 +143,9 @@ static int setup_tracking(const char *new_ref, const char *orig_ref, return error(_("Not tracking: ambiguous information for ref %s"), orig_ref); - install_branch_config(config_flags, new_ref, tracking.remote, - tracking.src ? tracking.src : orig_ref); + if (install_branch_config(config_flags, new_ref, tracking.remote, + tracking.src ? tracking.src : orig_ref)) + return error(_("Could not install branch configuration")); free(tracking.src); return 0; @@ -295,7 +305,8 @@ void create_branch(const char *head, } if (real_ref && track) - setup_tracking(ref.buf + 11, real_ref, track, quiet); + if (setup_tracking(ref.buf + 11, real_ref, track, quiet) < 0) + die(_("Could not setup tracking branch")); strbuf_release(&ref); free(real_ref); diff --git a/branch.h b/branch.h index 58aa45f..78ad438 100644 --- a/branch.h +++ b/branch.h @@ -43,9 +43,10 @@ void remove_branch_state(void); /* * Configure local branch "local" as downstream to branch "remote" * from remote "origin". Used by git branch --set-upstream. + * Returns 0 on success. */ #define BRANCH_CONFIG_VERBOSE 01 -extern void install_branch_config(int flag, const char *local, const char *origin, const char *remote); +extern int install_branch_config(int flag, const char *local, const char *origin, const char *remote); /* * Read branch description diff --git a/builtin/clone.c b/builtin/clone.c index a7c8def..8b11650 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -648,6 +648,7 @@ static void update_head(const struct ref *our, const struct ref *remote, const char *msg) { const char *head; + if (our && skip_prefix(our->name, "refs/heads/", &head)) { /* Local default branch link */ if (create_symref("HEAD", our->name, NULL) < 0) @@ -655,7 +656,8 @@ static void update_head(const struct ref *our, const struct ref *remote, if (!option_bare) { update_ref(msg, "HEAD", our->old_oid.hash, NULL, 0, UPDATE_REFS_DIE_ON_ERR); - install_branch_config(0, head, option_origin, our->name); + if (install_branch_config(0, head, option_origin, our->name)) +
[PATCH v2 9/9] compat: die when unable to set core.precomposeunicode
When calling `git_config_set` to set 'core.precomposeunicode' we ignore the return value of the function, which may indicate that we were unable to write the value back to disk. As surrounding code is already dying when an error occurs we simply die and print an error message when we are unable to write the config value. Signed-off-by: Patrick Steinhardt --- compat/precompose_utf8.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c index 079070f..9ff1ebe 100644 --- a/compat/precompose_utf8.c +++ b/compat/precompose_utf8.c @@ -50,7 +50,8 @@ void probe_utf8_pathname_composition(void) close(output_fd); git_path_buf(&path, "%s", auml_nfd); precomposed_unicode = access(path.buf, R_OK) ? 0 : 1; - git_config_set("core.precomposeunicode", precomposed_unicode ? "true" : "false"); + git_config_set_or_die("core.precomposeunicode", + precomposed_unicode ? "true" : "false"); git_path_buf(&path, "%s", auml_nfc); if (unlink(path.buf)) die_errno(_("failed to unlink '%s'"), path.buf); -- 2.7.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
[PATCH v2 8/9] submodule--helper: handle config errors
When setting the 'core.worktree' option for a newly cloned submodule we ignore the return value of `git_config_set_in_file`. Instead, we want to inform the user that something has gone wrong by printing an error message and aborting the program. Signed-off-by: Patrick Steinhardt --- builtin/submodule--helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index f4c3eff..c7e1ea2 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -245,8 +245,8 @@ static int module_clone(int argc, const char **argv, const char *prefix) p = git_pathdup_submodule(path, "config"); if (!p) die(_("could not get submodule directory for '%s'"), path); - git_config_set_in_file(p, "core.worktree", - relative_path(sb.buf, sm_gitdir, &rel_path)); + git_config_set_in_file_or_die(p, "core.worktree", + relative_path(sb.buf, sm_gitdir, &rel_path)); strbuf_release(&sb); strbuf_release(&rel_path); free(sm_gitdir); -- 2.7.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
[PATCH v2 1/9] config: introduce set_or_die wrappers
A lot of call-sites for the existing family of `git_config_set` functions do not check for errors that may occur, e.g. the configuration file being locked. In many cases we simply want to die when such a situation arises. Introduce wrappers that will cause the program to die in those cases. Signed-off-by: Patrick Steinhardt --- cache.h | 4 config.c | 27 +++ 2 files changed, 31 insertions(+) diff --git a/cache.h b/cache.h index dfc459c..c1f844d 100644 --- a/cache.h +++ b/cache.h @@ -1508,11 +1508,15 @@ extern int git_config_maybe_bool(const char *, const char *); extern int git_config_string(const char **, const char *, const char *); extern int git_config_pathname(const char **, const char *, const char *); extern int git_config_set_in_file(const char *, const char *, const char *); +extern void git_config_set_in_file_or_die(const char *, const char *, const char *); extern int git_config_set(const char *, const char *); +extern void git_config_set_or_die(const char *, const char *); extern int git_config_parse_key(const char *, char **, int *); extern int git_config_key_is_valid(const char *key); extern int git_config_set_multivar(const char *, const char *, const char *, int); +extern void git_config_set_multivar_or_die(const char *, const char *, const char *, int); extern int git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int); +extern void git_config_set_multivar_in_file_or_die(const char *, const char *, const char *, const char *, int); extern int git_config_rename_section(const char *, const char *); extern int git_config_rename_section_in_file(const char *, const char *, const char *); extern const char *git_etc_gitconfig(void); diff --git a/config.c b/config.c index 86a5eb2..856f7d34 100644 --- a/config.c +++ b/config.c @@ -1831,11 +1831,22 @@ int git_config_set_in_file(const char *config_filename, return git_config_set_multivar_in_file(config_filename, key, value, NULL, 0); } +void git_config_set_in_file_or_die(const char *config_filename, + const char *key, const char *value) +{ + git_config_set_multivar_in_file_or_die(config_filename, key, value, NULL, 0); +} + int git_config_set(const char *key, const char *value) { return git_config_set_multivar(key, value, NULL, 0); } +void git_config_set_or_die(const char *key, const char *value) +{ + git_config_set_multivar_or_die(key, value, NULL, 0); +} + /* * Auxiliary function to sanity-check and split the key into the section * identifier and variable name. @@ -2179,6 +2190,15 @@ write_err_out: } +void git_config_set_multivar_in_file_or_die(const char *config_filename, + const char *key, const char *value, + const char *value_regex, int multi_replace) +{ + if (git_config_set_multivar_in_file(config_filename, key, value, + value_regex, multi_replace) < 0) + die(_("Could not set '%s' to '%s'"), key, value); +} + int git_config_set_multivar(const char *key, const char *value, const char *value_regex, int multi_replace) { @@ -2186,6 +2206,13 @@ int git_config_set_multivar(const char *key, const char *value, multi_replace); } +void git_config_set_multivar_or_die(const char *key, const char *value, + const char *value_regex, int multi_replace) +{ + git_config_set_multivar_in_file_or_die(NULL, key, value, value_regex, + multi_replace); +} + static int section_name_match (const char *buf, const char *name) { int i = 0, j = 0, dot = 0; -- 2.7.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
[PATCH v2 4/9] clone: handle config errors in cmd_clone
The clone command does not check for error codes returned by `git_config_set` functions. This may cause the user to end up with an inconsistent repository without any indication with what went wrong. Fix this problem by dying with an error message when we are unable to write the configuration files to disk. Signed-off-by: Patrick Steinhardt --- builtin/clone.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 8b11650..5f6f995 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -788,12 +788,12 @@ static void write_refspec_config(const char *src_ref_prefix, /* Configure the remote */ if (value.len) { strbuf_addf(&key, "remote.%s.fetch", option_origin); - git_config_set_multivar(key.buf, value.buf, "^$", 0); + git_config_set_multivar_or_die(key.buf, value.buf, "^$", 0); strbuf_reset(&key); if (option_mirror) { strbuf_addf(&key, "remote.%s.mirror", option_origin); - git_config_set(key.buf, "true"); + git_config_set_or_die(key.buf, "true"); strbuf_reset(&key); } } @@ -951,14 +951,14 @@ int cmd_clone(int argc, const char **argv, const char *prefix) src_ref_prefix = "refs/"; strbuf_addstr(&branch_top, src_ref_prefix); - git_config_set("core.bare", "true"); + git_config_set_or_die("core.bare", "true"); } else { strbuf_addf(&branch_top, "refs/remotes/%s/", option_origin); } strbuf_addf(&value, "+%s*:%s*", src_ref_prefix, branch_top.buf); strbuf_addf(&key, "remote.%s.url", option_origin); - git_config_set(key.buf, repo); + git_config_set_or_die(key.buf, repo); strbuf_reset(&key); if (option_reference.nr) -- 2.7.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 v3 00/20] Let Git's tests pass on Windows
Hi Eric, On Thu, 28 Jan 2016, Eric Sunshine wrote: > On Wed, Jan 27, 2016 at 11:19 AM, Johannes Schindelin > wrote: > > Relative to v2, this fixes stupid typos that made the tests fail on > > Linux, a stupid copy-paste error pointed out by Eric Sunshine, > > unnecessary 'printf ""' calls pointed out by Junio Hamano, and I now > > use `test_have_prereq !MINGW` consistently, as pointed out by both Eric > > and Junio. This time, I also send the patch series with the character > > set set (sic!) to UTF-8. Oh, and this time, I also made sure that the > > regression tests pass on Windows & Linux alike. > > For what it's worth, I ran the test suite on Mac OS X and FreeBSD, as > well, with this series applied and didn't run across any problems. I > also read through v3 and, other than the micro nit in patch 11/20, > didn't find anything upon which to comment. Thank you so much! I really appreciate your feedback, and I have a lot of respect for reviewers that go through a 19 or 20 strong patch series. Thanks! Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git for windows
Hi Andrey, on this mailing list, it is considered impolite to top-post, and it is also expected to reply-to-all unless there is a very good reason not to. Currently, you are trying to abuse me as a free help desk, which I do not appreciate. This time I am re-Cc:ing the mailing list and reply inline. On Thu, 28 Jan 2016, Andrey Chernyshov wrote: > The reason for you not seeing 2 emails I was replying to is that > initially it was in HTML format which looks like are not acceptable for > your mail server. So I hit 'reply' and converted to plan text. Okay, so you were lazy... ;-) > As for mcve I really have no idea what I could add. I'm not doing > anything special, just click Push in the GUI and get the error like > below. The idea of the "M" in "MCVE" is to make it easier for others to reproduce. For example, if you test only with PHPStorm and do not bother to test the plain command-line, then you are expecting others to install PHPStorm just to help you. You see, some people might take that as a perfect excuse not to help you at all! Count me in. And if you reproduce on the bare command-line, you do not even need to mention PHPStorm to begin with. But you do have to give a step-by-step list of what you did and what you expected to happen and what happened instead. You see, you really *want* to make it easy for others to help you. Generally a good resource is http://whathaveyoutried.com/ and I tried to come up with a separate list of good advice at https://github.com/git-for-windows/git/wiki/Issue-reporting-guidelines In your particular case, I would also recommend setting the environment variables GIT_TRACE=1 and GIT_CURL_VERBOSE=1 before re-trying and then pasting the entire text from the console into a reply (*after* seeing that nothing obvious sticks out). Ciao, Johannes -- To unsubscribe from this list: send the line "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: Starting on a microproject for GSoC
On 28 Jan 2016, at 01:40, Moritz Neeb wrote: > As the list of available microprojects 2016 is still to be created, I > might need your help in finding a project to work on. As Stefan already pointed out, working on something that scratches your (Git) itch is probably the best way to find a project. My recent itch was that I broke a test on Linux which I did not realize as I primarily work on OSX. As a solution for myself I suggested a TravisCI patch to the mailing list and it was accepted: https://travis-ci.org/git/git/branches I see a number of ways to improve the Git TravisCI integration: * install CVS on the build machines to run t94?? and t96?? tests * install SVN on the build machines to run t91?? tests * install Apache Web Server to run 5539, 5550, and 5561 * investigate if it is possible to run t1509 root worktree test * investigate if it is possible to add jgit to run t5310 * investigate why GIT_TEST_LONG=YesPlease does not work on TravisCI * investigate if we can use pylint to analyze the git-p4 Python code * investigate if we can trigger Coverity static code analysis for the Git master branch (hint: Stefan Beller already looked into this) https://scan.coverity.com/travis_ci I think all of these tasks can be done without deep Git knowledge. However, working with the tests is quite a good way to learn more about a complex project like Git. Cheers, Lars-- To unsubscribe from this list: send the line "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 00/20] Let Git's tests pass on Windows
On Wed, Jan 27, 2016 at 11:19 AM, Johannes Schindelin wrote: > Relative to v2, this fixes stupid typos that made the tests fail on > Linux, a stupid copy-paste error pointed out by Eric Sunshine, > unnecessary 'printf ""' calls pointed out by Junio Hamano, and I now > use `test_have_prereq !MINGW` consistently, as pointed out by both Eric > and Junio. This time, I also send the patch series with the character > set set (sic!) to UTF-8. Oh, and this time, I also made sure that the > regression tests pass on Windows & Linux alike. For what it's worth, I ran the test suite on Mac OS X and FreeBSD, as well, with this series applied and didn't run across any problems. I also read through v3 and, other than the micro nit in patch 11/20, didn't find anything upon which to comment. -- To unsubscribe from this list: send the line "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 11/20] tests: turn off git-daemon tests if FIFOs are not available
Hi Eric, On Thu, 28 Jan 2016, Eric Sunshine wrote: > On Wed, Jan 27, 2016 at 11:19 AM, Johannes Schindelin > wrote: > > The Git daemon tests create a FIFO first thing and will hang if said > > FIFO is not available. > > > > This is a problem with Git for Windows, where `mkfifo` is an MSYS2 > > program that leverages MSYS2's POSIX emulation layer, but > > `git-daemon.exe` is a MINGW program that has not the first clue about > > that POSIX emulation layer and therefore blinks twice when it sees > > MSYS2's emulated FIFOs and then just stares into space. > > > > This lets t5570-git-daemon.sh and t5811-proto-disable-git.sh pass. > > > > Signed-off-by: Stepan Kasal > > Signed-off-by: Johannes Schindelin > > --- > > diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh > > @@ -23,6 +23,11 @@ then > > +if ! test_have_prereq PIPE > > Maybe: > > if test_have_prereq !PIPE > > ? Darn. Of course I only looked for '! .*MINGW', but I should have looked for '! test_have_prereq' in the patches. Junio, could you kindly fix up locally if this is the only remaining issue of this patch series? Thanks, Dscho -- To unsubscribe from this list: send the line "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: Starting on a microproject for GSoC
Hi Moritz & Andrew, On Thu, 28 Jan 2016, Andrew Ardill wrote: > On 28 January 2016 at 11:40, Moritz Neeb wrote: > > I suppose just fixing/revising this would be kind > > of a too low hanging fruit? > > I am in no way qualified to speak to the majority of your post, but I > can't imagine anyone refusing your work because it was 'too low > hanging fruit'. I would agree that it is *not* a "too low hanging fruit". The thing is, it would be a really easy way to get into the groove, to understand how changes are expected to be contributed, what the process of iterating the patches/patch series is, and in what form the patches are preferred. Moritz, if you worry about this lil' project to be too little, why don't you just start with it and then tackle another one? That way, you will be already very familiar with the Git project (and the regulars will be familiar with you) when GSoC comes around. Ciao, Johannes -- To unsubscribe from this list: send the line "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 11/20] tests: turn off git-daemon tests if FIFOs are not available
On Wed, Jan 27, 2016 at 11:19 AM, Johannes Schindelin wrote: > The Git daemon tests create a FIFO first thing and will hang if said > FIFO is not available. > > This is a problem with Git for Windows, where `mkfifo` is an MSYS2 > program that leverages MSYS2's POSIX emulation layer, but > `git-daemon.exe` is a MINGW program that has not the first clue about > that POSIX emulation layer and therefore blinks twice when it sees > MSYS2's emulated FIFOs and then just stares into space. > > This lets t5570-git-daemon.sh and t5811-proto-disable-git.sh pass. > > Signed-off-by: Stepan Kasal > Signed-off-by: Johannes Schindelin > --- > diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh > @@ -23,6 +23,11 @@ then > +if ! test_have_prereq PIPE Maybe: if test_have_prereq !PIPE ? > +then > + test_skip_or_die $GIT_TEST_GIT_DAEMON "file system does not support > FIFOs" > +fi > + > LIB_GIT_DAEMON_PORT=${LIB_GIT_DAEMON_PORT-${this_test#t}} > > GIT_DAEMON_PID= > -- > 2.7.0.windows.1.7.g55a05c8 -- To unsubscribe from this list: send the line "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: GPL v2 authoritative answer on stored code as a derived work
Hi Philip, On Wed, 27 Jan 2016, Philip Oakley wrote: > From: "Junio C Hamano" > > Jonathan Smith writes: > > > > > It's pretty clear that code stored in a Git repository isn't > > > considered a derived work of Git, regardless of whether it is used > > > in a commercial context or otherwise. > > I'm guessing here, but I suspect that while its 'pretty clear' to Jonathan, > that he has met others who aren't so clear or trusting, and it's that > distrustful community that would need convincing. It is not so much distrust as something you could take to court, I guess, because an *authoritative* answer was asked for. Now, the question is a legal one, so it is pretty clear (;-)) to me that only a lawyer could give that answer. Having said that, I know of plenty of companies storing their proprietary code in Git repositories, and I would guess that they cleared that with their lawyers first. Jonathan, please do not take that as any indication that I try to give this answer: if you want an authoritative answer to your question, you really need to consider asking a lawyer. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git archive should use vendor extension in pax header
Hi Robert, [I am not going to re-Cc: the dropped email addresses; please note that it is pretty much frowned upon on this mailing list if you do not reply-to-all and might affect your conversation.] On Thu, 28 Jan 2016, f...@fuz.su wrote: > On Wed, Jan 27, 2016 at 09:31:15PM +0100, René Scharfe wrote: > > > Users can always go back to the original format. At least I don't > > expect this new format becoming the default too quickly. This is the most crucial issue here, as far as I am concerned: there are already tons of .zip files out there that were created by git archive, and there will inevitably be loads of tons more *having the current pax header format*. So tools wanting to deal with Git archives will have to handle those as well, i.e. do *precisely* as René suggested and use get-tar-commit-id. As such, the value of changing the format *now* is a bit like closing the barn's door after pretty much all of the horses left (except the old one that has a few troubles getting up in the morning but that is too nice to the kids to shoot). > Sure thing. If this is going to be implemented, I would add options > to choose what / what style of metadata to include. Why not put your money where your mouth is? I.e. get your head down into the code and come up with a patch (because otherwise it is unlikely that your idea will go anywhere)? Ciao, Johannes