GIT 2.7.0 is listed on Software Informer

[Kasey Bloome]

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] attempt connects in parallel for IPv6-capable builds

[Torsten Bögershausen]

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

[Jeff King]

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 04/10] grep/icase: avoid kwsset on literal non-ascii strings

[Eric Sunshine]

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 05/10] grep/icase: avoid kwsset when -F is specified

[Eric Sunshine]

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(, p->pattern);
> +   err = regcomp(>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();
> +   if (err) {
> +   char errbuf[1024];
> +   regerror(err, >regexp, errbuf, 1024);

I guess this was copy/pasted from compile_regexp(), but for this new
code, perhaps: s/1024/sizeof(errbuf)/

> +   regfree(>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] attempt connects in parallel for IPv6-capable builds

[Johannes Sixt]

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

Re: Bugs in git filter-branch (git replace related)

[Jeff King]

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 v5 04/10] grep/icase: avoid kwsset on literal non-ascii strings

[Eric Sunshine]

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: [PATCH v5 10/10] diffcore-pickaxe: support case insensitive match on non-ascii

[Eric Sunshine]

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 v3 4/6] worktree: new config file hierarchy

[Junio C Hamano]

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

Re: Starting on a microproject for GSoC

[Junio C Hamano]

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

[Matthias Aßhauer]

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: [PATCHv4 1/2] submodule: port resolve_relative_url from shell to C

[Junio C Hamano]

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();
> + strbuf_addf(, "./%s", remoteurl);
> + free(remoteurl);
> + remoteurl = strbuf_detach(, 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(, is_relative);
> + } else if (starts_with_dot_slash(url))
> + url += 2;
> + else
> + break;
> + }
> + strbuf_reset();
> + strbuf_addf(, "%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();
> +
> + 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] convert: legitimately disable clean/smudge filter with an empty override

[Junio C Hamano]

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

fast-import fails in read-only tree

[Stefan Monnier]

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: [PATCH v3] push --force-with-lease: Fix ref status reporting

[Junio C Hamano]

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"

[Junio C Hamano]

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(_tree.args, "read-tree");
> + argv_array_pushf(_tree.args, "--index-output=%s", tmp_indexfile);
> + argv_array_pushl(_tree.args, "-m", i_tree, NULL);
> +
> + argv_array_pushl(, "diff", "--name-only", "-z", "HEAD", "--",
> + NULL);
> +
> + argv_array_pushl(_index.args, "update-index", "--add",
> + "--remove", NULL);
> +
> + argv_array_push(_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 

Re: [PATCHv4 1/2] submodule: port resolve_relative_url from shell to C

[Stefan Beller]

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();
>> + strbuf_addf(, "./%s", remoteurl);
>> + free(remoteurl);
>> + remoteurl = strbuf_detach(, 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(, is_relative);
>> + } else if (starts_with_dot_slash(url))
>> + url += 2;
>> + else
>> + break;
>> + }
>> + strbuf_reset();
>> + strbuf_addf(, "%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();
>> +
>> + 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

[PATCH] pass transport verbosity down to git_connect

[Eric Wong]

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, ,
 for_push ? REF_NORMAL : 0,
 >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, _tmp, 0,
 NULL, >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, _refs, REF_NORMAL,
 NULL, >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

[PATCH v3] push --force-with-lease: Fix ref status reporting

[Andrew Wheeler]

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(>old_oid, >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(>old_oid)) {
+   if (!reject_reason && !ref->deletion && 
!is_null_oid(>old_oid)) {
if (starts_with(ref->name, "refs/tags/"))
reject_reason = 
REF_STATUS_REJECT_ALREADY_EXISTS;
else if (!has_object_file(>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 2/2] stash: use "stash--helper"

[Matthias Asshauer]

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

[PATCH 1/2] stash--helper: implement "git stash--helper"

[Matthias Asshauer]

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(_tree.args, "read-tree");
+   argv_array_pushf(_tree.args, "--index-output=%s", tmp_indexfile);
+   argv_array_pushl(_tree.args, "-m", i_tree, NULL);
+
+   argv_array_pushl(, "diff", "--name-only", "-z", "HEAD", "--",
+   NULL);
+
+   argv_array_pushl(_index.args, "update-index", "--add",
+   "--remove", NULL);
+
+   argv_array_push(_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 = 

Re: [PATCH 2/2] stash: use "stash--helper"

[Stefan Beller]

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

Re: [PATCH 2/2] stash: use "stash--helper"

[Stefan Beller]

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: [RFC] tag-ref and tag object binding

[Santiago Torres]

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: [PATCH v3 11/20] tests: turn off git-daemon tests if FIFOs are not available

[Junio C Hamano]

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

[Junio C Hamano]

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

[PATCH v2] push --force-with-lease: Fix ref status reporting

[Andrew Wheeler]

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(>old_oid, >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(>old_oid)) {
+   if (!reject_reason && !ref->deletion && 
!is_null_oid(>old_oid)) {
if (starts_with(ref->name, "refs/tags/"))
reject_reason = 
REF_STATUS_REJECT_ALREADY_EXISTS;
else if (!has_object_file(>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: [RFC] tag-ref and tag object binding

[Santiago Torres]

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 

Re: [PATCH v2] push --force-with-lease: Fix ref status reporting

[Andrew Wheeler]

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(>old_oid, >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(>old_oid)) {
> +   if (!reject_reason && !ref->deletion && 
> !is_null_oid(>old_oid)) {
> if (starts_with(ref->name, "refs/tags/"))
> reject_reason = 
> REF_STATUS_REJECT_ALREADY_EXISTS;
> else if (!has_object_file(>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

AW: [PATCH 2/2] stash: use "stash--helper"

[Matthias Aßhauer]

> 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: [PATCH v3 00/20] Let Git's tests pass on Windows

[Junio C Hamano]

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 2/2] stash: use "stash--helper"

[Roberto Tyley]

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] attempt connects in parallel for IPv6-capable builds

[Junio C Hamano]

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(, );
>   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(_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(_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 

Re: [PATCH] convert: legitimately disable clean/smudge filter with an empty override

[Junio C Hamano]

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 v5 00/10] Fix icase grep on non-ascii

[Junio C Hamano]

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

[Eric Wong]

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] attempt connects in parallel for IPv6-capable builds

[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.
--
To unsubscribe from this list: send the line "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

[Junio C Hamano]

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, ,
>for_push ? REF_NORMAL : 0,
>>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, _tmp, 0,
>NULL, >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, _refs, REF_NORMAL,
>NULL, >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] pass transport verbosity down to git_connect

[Jeff King]

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 v3 00/20] Let Git's tests pass on Windows

[Eric Sunshine]

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: git for windows

[Johannes Schindelin]

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: [PATCH v3 11/20] tests: turn off git-daemon tests if FIFOs are not available

[Johannes Schindelin]

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

[Lars Schneider]

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] convert: legitimately disable clean/smudge filter with an empty override

[Lars Schneider]

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

[Johannes Schindelin]

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

Re: [PATCH v3 11/20] tests: turn off git-daemon tests if FIFOs are not available

[Eric Sunshine]

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: Starting on a microproject for GSoC

[Johannes Schindelin]

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 00/20] Let Git's tests pass on Windows

[Johannes Schindelin]

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: GPL v2 authoritative answer on stored code as a derived work

[Johannes Schindelin]

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

[PATCH v2 3/9] remote: handle config errors in set_url

[Patrick Steinhardt]

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(_buf);
+
return 0;
}
 
@@ -1608,9 +1609,9 @@ static int set_url(int argc, const char **argv)
regfree(_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

[Patrick Steinhardt]

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/", )
&& !strcmp(local, shortname)
&& !origin) {
warning(_("Not setting branch %s as its own upstream."),
local);
-   return;
+   return ret;
}
 
strbuf_addf(, "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();
strbuf_addf(, "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();
strbuf_addf(, "branch.%s.rebase", local);
-   git_config_set(key.buf, "true");
+   ret = git_config_set(key.buf, "true");
+   if (ret)
+   goto out;
}
-   strbuf_release();
 
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();
+   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();
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/", )) {
/* 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

[Patrick Steinhardt]

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(, "%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(, "%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 6/9] init-db: handle config errors

[Patrick Steinhardt]

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(, "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(, "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

[Patrick Steinhardt]

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(, "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();
strbuf_addf(, "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();
} 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

[Patrick Steinhardt]

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(, "%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();
}
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 8/9] submodule--helper: handle config errors

[Patrick Steinhardt]

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, _path));
+   git_config_set_in_file_or_die(p, "core.worktree",
+ relative_path(sb.buf, sm_gitdir, 
_path));
strbuf_release();
strbuf_release(_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

[Patrick Steinhardt]

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

[Patrick Steinhardt]

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(, "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();
 
if (option_mirror) {
strbuf_addf(, "remote.%s.mirror", 
option_origin);
-   git_config_set(key.buf, "true");
+   git_config_set_or_die(key.buf, "true");
strbuf_reset();
}
}
@@ -951,14 +951,14 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
src_ref_prefix = "refs/";
strbuf_addstr(_top, src_ref_prefix);
 
-   git_config_set("core.bare", "true");
+   git_config_set_or_die("core.bare", "true");
} else {
strbuf_addf(_top, "refs/remotes/%s/", option_origin);
}
 
strbuf_addf(, "+%s*:%s*", src_ref_prefix, branch_top.buf);
strbuf_addf(, "remote.%s.url", option_origin);
-   git_config_set(key.buf, repo);
+   git_config_set_or_die(key.buf, repo);
strbuf_reset();
 
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

[PATCH v2 0/9] Handle errors when setting configs

[Patrick Steinhardt]

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

Re: git archive should use vendor extension in pax header

[fuz]

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

Re: [PATCH v3 1/6] worktree: new repo extension to manage worktree behaviors

[Duy Nguyen]

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

[Duy Nguyen]

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

Re: Starting on a microproject for GSoC

[Moritz Neeb]

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

[PATCH] attempt connects in parallel for IPv6-capable builds

[Eric Wong]

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(, );
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(_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(_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,
+   , );
+   if (rc != 0)
+   die_errno("getsockopt errno=%s\n",
+ 

[PATCH v5 07/10] gettext: add is_utf8_locale()

[Nguyễn Thái Ngọc Duy]

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 v5 09/10] diffcore-pickaxe: "share" regex error handling code

[Nguyễn Thái Ngọc Duy]

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(, needle, cflags);
-   if (err) {
-   /* The POSIX.2 people are surely sick */
-   char errbuf[1024];
-   regerror(err, , errbuf, 1024);
-   regfree();
-   die("invalid regex: %s", errbuf);
-   }
regexp = 
} 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, , errbuf, 1024);
+   regfree();
+   die("invalid regex: %s", errbuf);
+   }
 
/* Might want to warn when both S and G are on; I don't care... */
pickaxe(_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 05/10] grep/icase: avoid kwsset when -F is specified

[Nguyễn Thái Ngọc Duy]

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(, p->pattern);
+   err = regcomp(>regexp, sb.buf, opt->regflags & ~REG_EXTENDED);
+   if (opt->debug)
+   fprintf(stderr, "fixed%s\n", sb.buf);
+   strbuf_release();
+   if (err) {
+   char errbuf[1024];
+   regerror(err, >regexp, errbuf, 1024);
+   regfree(>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: 

[PATCH v5 10/10] diffcore-pickaxe: support case insensitive match on non-ascii

[Nguyễn Thái Ngọc Duy]

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(, needle, cflags);
regexp = 
+   } 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(, needle);
+   err = regcomp(, sb.buf, cflags);
+   strbuf_release();
+   regexp = 
} 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

[Nguyễn Thái Ngọc Duy]

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, , ,
-   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 08/10] grep/pcre: support utf-8

[Nguyễn Thái Ngọc Duy]

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

[Nguyễn Thái Ngọc Duy]

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

[Nguyễn Thái Ngọc Duy]

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

[Nguyễn Thái Ngọc Duy]

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(, 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(, pat, flags))
die("failed regcomp() for pattern '%s'", pat);
-   if (regexec(, str, 1, m, 0))
-   die("no match of pattern '%s' to string '%s'", pat, str);
+   if (regexec(, 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

[Nguyễn Thái Ngọc Duy]

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

[Nguyễn Thái Ngọc Duy]

-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();
-- 
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] mergetool: reorder vim/gvim buffers in three-way diffs

[Dickson Wong]

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]

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

Re: [PATCH] blame: display a more helpful error message if the file was deleted

[Alex Henrie]

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

[Alex Henrie]

`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

NEW ARRIVALS, CISCO,CPU's,Memory, LAPTOP AND AVAYA

[Laison Computech Inc]

 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

[Junio C Hamano]

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

Re: [PATCH] pass transport verbosity down to git_connect

[Jeff King]

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

[PATCH] stripspace: Call U+0020 a "space" instead of a "blank"

[Alex Henrie]

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

Bugs in git filter-branch (git replace related)

[Anatoly Borodin]

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 v5 03/10] test-regex: expose full regcomp() to the command line

[Eric Sunshine]

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(, 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(, pat, flags))
>   die("failed regcomp() for pattern '%s'", pat);
> - if (regexec(, str, 1, m, 0))
> - die("no match of pattern '%s' to string '%s'", pat, str);
> + if (regexec(, 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