Re: Building git 2.4.5 on AIX 6.1 problems

2015-07-11 Thread Jeff King
On Fri, Jul 10, 2015 at 11:57:28PM -0700, Junio C Hamano wrote:

> > So it is obviously a structure or variable that is being used so does anyone
> > know the library that this is defined in?
> 
> Most of our objects should be found in libgit.a (you can see it on your
> command line) that our Makefile builds.
> 
> Now, it has been more than a decade since I last had to deal with a
> system that needs this the last time, but perhaps AIX linker needs the
> archives explicitly prepared with ranlib(1)? Just a shot in the dark...

Ugh, you are giving me flashbacks. ;)

We use the "s" flag to "ar", which is supposed to do the same thing as
ranlib. But I guess it is possible that the system ar does not respect
that flag. Certainly running ranlib manually would be an interesting
experiment.

If the system has GNU ar on it as well as an AIX ar, it might be
interesting to point $(AR) at the former. E.g.:

  make clean
  make AR=gar

(where "gar" is whatever GNU ar is called on the system).

-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 10/16] worktree: make branch creation distinct from worktree population

2015-07-11 Thread Eric Sunshine
On Sat, Jul 11, 2015 at 11:14 PM, Eric Sunshine  wrote:
> On Sat, Jul 11, 2015 at 11:10 PM, Eric Sunshine  
> wrote:
>> On Sat, Jul 11, 2015 at 10:48 PM, Duy Nguyen  wrote:
>>> The "shouldn't affect" is potentially a problem.If the current
>>> 'worktree add' process caches something (in ref handling, for example)
>>> that the 'git branch' process changes, then we may need to invalidate
>>> cache in 'worktree add' process after run_command(). I guess it's ok
>>> in this case since all we do is run_command(), we do not lookup refs
>>> or anything else afterwards.
>>
>> With this patch series applied, the code effectively does this:
>>
>> branch = ...
>> if (create_new_branch) {
>> exec "git branch newbranch branch"
>> branch = newbranch;
>> }
>> if (ref_exists(branch) && !detach)
>> exec "git symbolic-ref HEAD branch"
>> else
>> exec "git update-ref HEAD $(git rev-parse branch)"
>> exec "git reset --hard"
>>
>> So, if I understand your concern correctly, then you are worried that,
>> following the git-branch invocation, ref_exists() could return the
>> wrong answer with a pluggable ref-backend since it might be answering
>> based upon stale information. Is that what you mean? If so, I can see
>> how that it could be an issue. (As far as I can tell, the current
>> file-based backend doesn't have a problem with this since it's hitting
>> the filesystem directly to answer the ref_exists() question.)
>
> I meant for this final sentence to end like this:
>
>  ...to answer the ref_exists() question, but it still seems
>  fragile since some future change could introduce caching.

In this case, it's easy enough to side-step the issue since there's no
need to call ref_exists() if the new branch was created successfully
(since we know it exists). The logic would effectively become this:

branch = ...
if (create_new_branch) {
exec "git branch newbranch branch"
exec "git symbolic-ref HEAD newbranch"
} else if (ref_exists(branch) && !detach)
exec "git symbolic-ref HEAD branch"
else
exec "git update-ref HEAD $(git rev-parse branch)"
exec "git reset --hard"
--
To unsubscribe from this list: send the line "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 10/16] worktree: make branch creation distinct from worktree population

2015-07-11 Thread Eric Sunshine
On Sat, Jul 11, 2015 at 11:10 PM, Eric Sunshine  wrote:
> On Sat, Jul 11, 2015 at 10:48 PM, Duy Nguyen  wrote:
>> The "shouldn't affect" is potentially a problem.If the current
>> 'worktree add' process caches something (in ref handling, for example)
>> that the 'git branch' process changes, then we may need to invalidate
>> cache in 'worktree add' process after run_command(). I guess it's ok
>> in this case since all we do is run_command(), we do not lookup refs
>> or anything else afterwards.
>
> With this patch series applied, the code effectively does this:
>
> branch = ...
> if (create_new_branch) {
> exec "git branch newbranch branch"
> branch = newbranch;
> }
> if (ref_exists(branch) && !detach)
> exec "git symbolic-ref HEAD branch"
> else
> exec "git update-ref HEAD $(git rev-parse branch)"
> exec "git reset --hard"
>
> So, if I understand your concern correctly, then you are worried that,
> following the git-branch invocation, ref_exists() could return the
> wrong answer with a pluggable ref-backend since it might be answering
> based upon stale information. Is that what you mean? If so, I can see
> how that it could be an issue. (As far as I can tell, the current
> file-based backend doesn't have a problem with this since it's hitting
> the filesystem directly to answer the ref_exists() question.)

I meant for this final sentence to end like this:

 ...to answer the ref_exists() question, but it still seems
 fragile since some future change could introduce caching.
--
To unsubscribe from this list: send the line "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 10/16] worktree: make branch creation distinct from worktree population

2015-07-11 Thread Eric Sunshine
On Sat, Jul 11, 2015 at 10:48 PM, Duy Nguyen  wrote:
> On Sun, Jul 12, 2015 at 9:36 AM, Eric Sunshine  
> wrote:
>> On Sat, Jul 11, 2015 at 9:20 PM, Duy Nguyen  wrote:
>>> On Sat, Jul 11, 2015 at 7:05 AM, Eric Sunshine  
>>> wrote:
 The plan is eventually to populate the new worktree via "git reset
 --hard" rather than "git checkout". Thus, rather than piggybacking on
 git-checkout's -b/-B ability to create a new branch at checkout time,
 git-worktree will need to do so itself.
>>>
>>> I don't know much about ref handling code (especially after the big
>>> transaction update), so i may be wrong, but do we need to invalidate
>>> some caches in refs.c after this? The same for update-ref in the other
>>> patch. We may need to re-read the index after 'reset --hard' too if we
>>> ever need to do touch the index after that (unlikely though in the
>>> case of 'worktree add')
>>
>> I'm not sure I understand. Are you talking about this patch's
>> implementation or a possible future change which uses the C API rather
>> than git-branch?
>>
>> If you're talking about this patch, then I don't think we need to do
>> anything more, as the "git branch" and "git reset --hard" invocations
>> are separate process invocations which shouldn't affect the current
>> worktree or the current "git worktree add" process.
>
> The "shouldn't affect" is potentially a problem.If the current
> 'worktree add' process caches something (in ref handling, for example)
> that the 'git branch' process changes, then we may need to invalidate
> cache in 'worktree add' process after run_command(). I guess it's ok
> in this case since all we do is run_command(), we do not lookup refs
> or anything else afterwards.

With this patch series applied, the code effectively does this:

branch = ...
if (create_new_branch) {
exec "git branch newbranch branch"
branch = newbranch;
}
if (ref_exists(branch) && !detach)
exec "git symbolic-ref HEAD branch"
else
exec "git update-ref HEAD $(git rev-parse branch)"
exec "git reset --hard"

So, if I understand your concern correctly, then you are worried that,
following the git-branch invocation, ref_exists() could return the
wrong answer with a pluggable ref-backend since it might be answering
based upon stale information. Is that what you mean? If so, I can see
how that it could be an issue. (As far as I can tell, the current
file-based backend doesn't have a problem with this since it's hitting
the filesystem directly to answer the ref_exists() question.)
--
To unsubscribe from this list: send the line "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 10/16] worktree: make branch creation distinct from worktree population

2015-07-11 Thread Duy Nguyen
On Sun, Jul 12, 2015 at 9:36 AM, Eric Sunshine  wrote:
> On Sat, Jul 11, 2015 at 9:20 PM, Duy Nguyen  wrote:
>> On Sat, Jul 11, 2015 at 7:05 AM, Eric Sunshine  
>> wrote:
>>> The plan is eventually to populate the new worktree via "git reset
>>> --hard" rather than "git checkout". Thus, rather than piggybacking on
>>> git-checkout's -b/-B ability to create a new branch at checkout time,
>>> git-worktree will need to do so itself.
>>>
>>> Signed-off-by: Eric Sunshine 
>>> ---
>>>
>>> I considered calling branch-related API, such as create_branch(),
>>> directly, however, git-branch provides extra value which may be useful
>>> in the future (such as when --track and --orphan get added to
>>> git-worktree), so it seemed wise to re-use git-branch's implementation
>>> rather than duplicating the functionality. If this proves the wrong
>>> choice, then the series can either be re-rolled or follow-on patches can
>>> address the issue.
>>
>> I don't know much about ref handling code (especially after the big
>> transaction update), so i may be wrong, but do we need to invalidate
>> some caches in refs.c after this? The same for update-ref in the other
>> patch. We may need to re-read the index after 'reset --hard' too if we
>> ever need to do touch the index after that (unlikely though in the
>> case of 'worktree add')
>
> I'm not sure I understand. Are you talking about this patch's
> implementation or a possible future change which uses the C API rather
> than git-branch?
>
> If you're talking about this patch, then I don't think we need to do
> anything more, as the "git branch" and "git reset --hard" invocations
> are separate process invocations which shouldn't affect the current
> worktree or the current "git worktree add" process.

The "shouldn't affect" is potentially a problem.If the current
'worktree add' process caches something (in ref handling, for example)
that the 'git branch' process changes, then we may need to invalidate
cache in 'worktree add' process after run_command(). I guess it's ok
in this case since all we do is run_command(), we do not lookup refs
or anything else afterwards.
-- 
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 10/16] worktree: make branch creation distinct from worktree population

2015-07-11 Thread Eric Sunshine
On Sat, Jul 11, 2015 at 9:20 PM, Duy Nguyen  wrote:
> On Sat, Jul 11, 2015 at 7:05 AM, Eric Sunshine  
> wrote:
>> The plan is eventually to populate the new worktree via "git reset
>> --hard" rather than "git checkout". Thus, rather than piggybacking on
>> git-checkout's -b/-B ability to create a new branch at checkout time,
>> git-worktree will need to do so itself.
>>
>> Signed-off-by: Eric Sunshine 
>> ---
>>
>> I considered calling branch-related API, such as create_branch(),
>> directly, however, git-branch provides extra value which may be useful
>> in the future (such as when --track and --orphan get added to
>> git-worktree), so it seemed wise to re-use git-branch's implementation
>> rather than duplicating the functionality. If this proves the wrong
>> choice, then the series can either be re-rolled or follow-on patches can
>> address the issue.
>
> I don't know much about ref handling code (especially after the big
> transaction update), so i may be wrong, but do we need to invalidate
> some caches in refs.c after this? The same for update-ref in the other
> patch. We may need to re-read the index after 'reset --hard' too if we
> ever need to do touch the index after that (unlikely though in the
> case of 'worktree add')

I'm not sure I understand. Are you talking about this patch's
implementation or a possible future change which uses the C API rather
than git-branch?

If you're talking about this patch, then I don't think we need to do
anything more, as the "git branch" and "git reset --hard" invocations
are separate process invocations which shouldn't affect the current
worktree or the current "git worktree add" process.

If you're talking about a future patch switching over to the C API,
and avoiding run_command(), then perhaps (but I haven't investigated
that avenue thoroughly enough to answer).

Or, am I totally misunderstanding you?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option

2015-07-11 Thread Duy Nguyen
On Sat, Jul 11, 2015 at 7:05 PM, Karthik Nayak  wrote:
> On Fri, Jul 10, 2015 at 9:50 PM, Junio C Hamano  wrote:
>>
>> This may be enough to support the various existing formats that are
>> offered by "git branch" and/or "git tag", but I do not think if this
>> is the right approach in the longer term, or if we are painting
>> ourselves in a corner we cannot cleanly get out of later [*1*].
>> Will the "refname" stay to be the only thing that may want alignment
>> padding appended in the future?  Will it stay true that we want to
>> align only to the left?  Etc., etc.
>>
>> Cc'ed Duy as %< in the pretty-format was his invention at around
>> a5752342 (pretty: support padding placeholders, %< %> and %><,
>> 2013-04-19).
>>
>
> I kinda had the same though, my only justification was that it was only being
> internally used. I'll have another look if as to see if I can make it
> universal somehow.
> Let's see what Duy has to suggest.

I guess if you can have multiple arguments after ':' in an atom, then
you have wiggle room for future. But it looks like you only accept one
argument after ':'.. (I only checked the version on 'pu'). Having an
"alignment atom" to augment the real one (like %< changes the behavior
of the next placeholder), could also work, but it adds dependency
between atoms, something I don't think ref-filter.c is ready for.

Another thing, the atom value is also used for sorting. When used for
sorting, I think these padding spaces should not be generated or it
may confuse the sort algorithm. Left alignment may be ok, right or
center alignment (in future?), not  so much. Perhaps we should do the
padding in a separate phase, outside populate_value(). If you go this
route, having separate atoms for alignment works better: you don't
have to parse them in populate_value() which is for actual values, and
you can handle dependency easily (I think).

By the way, please consider adding _() back to translatable strings,
usually those die() or warn(), or "[ahead %s]"... In the last case,
because you don't really know how long the string is after
translation, avoid hard coding buffer size (to 40).
-- 
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 10/16] worktree: make branch creation distinct from worktree population

2015-07-11 Thread Duy Nguyen
On Sat, Jul 11, 2015 at 7:05 AM, Eric Sunshine  wrote:
> The plan is eventually to populate the new worktree via "git reset
> --hard" rather than "git checkout". Thus, rather than piggybacking on
> git-checkout's -b/-B ability to create a new branch at checkout time,
> git-worktree will need to do so itself.
>
> Signed-off-by: Eric Sunshine 
> ---
>
> I considered calling branch-related API, such as create_branch(),
> directly, however, git-branch provides extra value which may be useful
> in the future (such as when --track and --orphan get added to
> git-worktree), so it seemed wise to re-use git-branch's implementation
> rather than duplicating the functionality. If this proves the wrong
> choice, then the series can either be re-rolled or follow-on patches can
> address the issue.

I don't know much about ref handling code (especially after the big
transaction update), so i may be wrong, but do we need to invalidate
some caches in refs.c after this? The same for update-ref in the other
patch. We may need to re-read the index after 'reset --hard' too if we
ever need to do touch the index after that (unlikely though in the
case of 'worktree add')
-- 
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: Git Smart HTTP with HTTP/2.0

2015-07-11 Thread Shawn Pearce
On Sat, Jul 11, 2015 at 11:26 AM, Ilari Liusvaara
 wrote:
> On Sat, Jul 11, 2015 at 10:23:09AM -0700, Shawn Pearce wrote:
>>
>> > Websockets over HTTP/2 (a.k.a. "websockets2") has not been defined yet.
>> > With Websockets(1), it would probably already be possible to tunnel the
>> > native git smart transport protocol over it. Probably not worth it.
>>
>> Another option is to tunnel using gRPC (grpc.io). libcurl probably
>> can't do this. And linking grpc.io library into git-core is crazy. So
>> its probably a non-starter. But food for thought.
>
> Wouldn't it link into git-remote-http (and on the server side, one
> could use pipes to talk to git)?
>
> But supporting websockets in git-remote-http could get annoying,
> especially for wss:// (https://). Dunno how bad gRPC would be.

We wrote it as git-remote-$THING, invoked with $THING:// URLs. And
git-remote-$THING just implements the "connect" helper protocol. Its
much simpiler than git-remote-http.

Maybe its done that way in git-core as http2:// aka git-remote-http2.

Or the git-remote-http helper connects to the remote system and tries
to guess if it supports Git on HTTP/2 before responding to the
capabilities request from transport code. If yes, it replies with
connect, if no, it does the current fetch and push protocol.
--
To unsubscribe from this list: send the line "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] check_and_freshen_file: fix reversed success-check

2015-07-11 Thread X H

Le 10/07/2015 0:48, Jeff King a écrit :

On Thu, Jul 09, 2015 at 10:51:50PM +0200, Johannes Sixt wrote:


Ah! That code is less than a year old. When I began to adopt a workflow
requiring force-pushes lately, I wondered why I haven't seen these
failures earlier, because I did do force pushes in the past, but not
that frequently. I thought that I had just been lucky. But this would
explain it.


And, in fact, with this patch these particular failures are gone! Thank you
so much!


Great, thanks for testing. You can temper your appreciation by noticing
that I introduced the bug in the first place. ;)

-Peff


Hi,

Thank you for the patch. I hope it will solve the problem and permit to 
have a second user using the same repository.


How are the permission handled, is it git that is asking to create a 
file read only or rw on the remote or is it the environment with umask 
ans so on that decides it, or Windows when the drive is mounted with noacl?

--
To unsubscribe from this list: send the line "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 an hackers. Get out of my account. I know who you are.

2015-07-11 Thread Latrelle R. Davidson
--
To unsubscribe from this list: send the line "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 Smart HTTP with HTTP/2.0

2015-07-11 Thread Ilari Liusvaara
On Sat, Jul 11, 2015 at 10:23:09AM -0700, Shawn Pearce wrote:
> On Sat, Jul 11, 2015 at 12:00 AM, Ilari Liusvaara
>  wrote:
> > On Sat, Jul 11, 2015 at 11:10:48AM +0800, ForceCharlie wrote:
> >
> >> Frequently used Git developers often feel Git HTTP protocol is not
> >> satisfactory, slow and unstable.This is because the HTTP protocol itself
> >> decides
> >
> > Note that there are already two versions of HTTP transport, the old "dumb"
> > one and the newer "smart" one.
> >
> > The smart one is difficult to speed up (due to nature of the negotiations),
> > but usually is pretty reliable (the efficiency isn't horrible).
> 
> The negotiation in smart-HTTP actually has some bad corner cases. Each
> round of negotiation requires a new POST resupplying all previously
> agreed upon SHA-1s, and a batch of new SHA-1s. We have observed many
> rounds where this POST is MiBs in size because the peers can't quite
> agree and have to keep digging through history.

Oh yeah that... Well, that is artifact of HTTP semantics.

> > Now, the old "dumb" protocol is pretty unreliable and slow. HTTP/2 probably
> > can't do anything with the reliability problems, but probably could improve
> > the speed a bit.
> >
> > Websockets over HTTP/2 (a.k.a. "websockets2") has not been defined yet.
> > With Websockets(1), it would probably already be possible to tunnel the
> > native git smart transport protocol over it. Probably not worth it.
> 
> Another option is to tunnel using gRPC (grpc.io). libcurl probably
> can't do this. And linking grpc.io library into git-core is crazy. So
> its probably a non-starter. But food for thought.

Wouldn't it link into git-remote-http (and on the server side, one
could use pipes to talk to git)?

But supporting websockets in git-remote-http could get annoying,
especially for wss:// (https://). Dunno how bad gRPC would be.



-Ilari
--
To unsubscribe from this list: send the line "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 Smart HTTP with HTTP/2.0

2015-07-11 Thread Shawn Pearce
On Sat, Jul 11, 2015 at 12:00 AM, Ilari Liusvaara
 wrote:
> On Sat, Jul 11, 2015 at 11:10:48AM +0800, ForceCharlie wrote:
>> As we known, HTTP/2.0 has been released. All Git-Smart-HTTP are currently
>> implemented using HTTP/1.1.
>
> Nit: It is HTTP/2.
>
>> Frequently used Git developers often feel Git HTTP protocol is not
>> satisfactory, slow and unstable.This is because the HTTP protocol itself
>> decides
>
> Note that there are already two versions of HTTP transport, the old "dumb"
> one and the newer "smart" one.
>
> The smart one is difficult to speed up (due to nature of the negotiations),
> but usually is pretty reliable (the efficiency isn't horrible).

The negotiation in smart-HTTP actually has some bad corner cases. Each
round of negotiation requires a new POST resupplying all previously
agreed upon SHA-1s, and a batch of new SHA-1s. We have observed many
rounds where this POST is MiBs in size because the peers can't quite
agree and have to keep digging through history.

The native protocol on git:// and SSH is not as bad. Negotiation is
still many rounds, but these are pipelined and each round does not
need to repeat the prior round, as the server has a single stream and
is saving state.

> Now, the old "dumb" protocol is pretty unreliable and slow. HTTP/2 probably
> can't do anything with the reliability problems, but probably could improve
> the speed a bit.
>
> Websockets over HTTP/2 (a.k.a. "websockets2") has not been defined yet.
> With Websockets(1), it would probably already be possible to tunnel the
> native git smart transport protocol over it. Probably not worth it.

Another option is to tunnel using gRPC (grpc.io). libcurl probably
can't do this. And linking grpc.io library into git-core is crazy. So
its probably a non-starter. But food for thought.

But, at $DAY_JOB we tunnel the native bidirectional protocol in
grpc.io's predecessor, and it works quite well for 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


[PATCH] diff: parse ws-error-highlight option more strictly

2015-07-11 Thread René Scharfe
Check if a matched token is followed by a delimiter before advancing the
pointer arg.  This avoids accepting composite words like "allnew" or
"defaultcontext".

Signed-off-by: Rene Scharfe 
---
 diff.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index 87b16d5..0f17ec5 100644
--- a/diff.c
+++ b/diff.c
@@ -3653,7 +3653,12 @@ static void enable_patch_output(int *fmt) {
 
 static int parse_one_token(const char **arg, const char *token)
 {
-   return skip_prefix(*arg, token, arg) && (!**arg || **arg == ',');
+   const char *rest;
+   if (skip_prefix(*arg, token, &rest) && (!*rest || *rest == ',')) {
+   *arg = rest;
+   return 1;
+   }
+   return 0;
 }
 
 static int parse_ws_error_highlight(struct diff_options *opt, const char *arg)
-- 
2.4.4

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


Re: [PATCH v2 05/10] ref-filter: add option to match literal pattern

2015-07-11 Thread Karthik Nayak
On Sat, Jul 11, 2015 at 2:56 PM, Matthieu Moy
 wrote:
> Karthik Nayak  writes:
>
>> On Fri, Jul 10, 2015 at 10:13 PM, Junio C Hamano  wrote:
>>> Karthik Nayak  writes:
>>>
 Since 'ref-filter' only has an option to match path names
 add an option for regular pattern matching.
>>>
>>> There is nothing "regular" about the pattern matching you are
>>> adding.
>>>
>>> Everywhere else we use patterns on refs we call wildmatch(), which
>>> is an enhanced implementation of fnmatch(3), and you are doing the
>>> same in this new codepath.
>>>
>>> Just drop that word from here (and if you said something similar in
>>> the documentation, drop "regular" ffrom there as well).  It would
>>> invite confusion with regular expression matching, which we will not
>>> do for refs.
>>
>> Ok, will do. Thanks
>
> Just dropping "regular" leads to a strange sentence, since the path name
> match is also a kind of pattern-matching. I'd write
>
> Since 'ref-filter' only has an option to match path names, add an option
> for plain fnmatch pattern-matching.
>

Thanks for the heads up :)

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


Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option

2015-07-11 Thread Karthik Nayak
On Fri, Jul 10, 2015 at 9:50 PM, Junio C Hamano  wrote:
>
> This may be enough to support the various existing formats that are
> offered by "git branch" and/or "git tag", but I do not think if this
> is the right approach in the longer term, or if we are painting
> ourselves in a corner we cannot cleanly get out of later [*1*].
> Will the "refname" stay to be the only thing that may want alignment
> padding appended in the future?  Will it stay true that we want to
> align only to the left?  Etc., etc.
>
> Cc'ed Duy as %< in the pretty-format was his invention at around
> a5752342 (pretty: support padding placeholders, %< %> and %><,
> 2013-04-19).
>

I kinda had the same though, my only justification was that it was only being
internally used. I'll have another look if as to see if I can make it
universal somehow.
Let's see what Duy has to suggest.

>
> When adding a new thing to an existing list, we prefer to append it
> at the end of the list, if there is no other reason not to do so
> (e.g. "the existing list is sorted in this order, and the new
> location was chosen to fit the new item to honor the existing
> ordering rule" is a valid reason to put it at the beginning, if the
> existing sorting rule dictates that the new thing must come at the
> beginning).
>

my bad, will change it!

>
> In newer code, we would want to avoid atoi() so that "foo:shortalign=1z"
> that is a typo of "12" can be caught as an error.  Either strtol_i()
> or strtoul_ui() may be better (we would need to adjust it further
> when Michael decides to resurrect his numparse thing that has been
> in the stalled bin for quite a while, though).
>

Will have a look, thanks :)

>
> What should happen when the display column width of the string is
> wider?  If a user wants to align the refs that are usually usually
> short start the next thing at the 8th column, which should she use?
>
> "%(refname:shorta=7) %(next item)"
> "%(refname:shorta=8)%(next item)"
>

Both your examples would start the next item at the 8th column
(starting with 0),
the only difference being :
Case 1: when the refname is 8 columns wide
"%(refname:shorta=7) %(next item)": would give us eight columns of
refname + space + next item
"%(refname:shorta=8)%(next item)": would give us eight columns of
refname + next item
Case 2: when the refname < 8 columns wide
Both would give: upto 7 columns of refname + space + next item.

Thanks for the suggestions :)

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


Re: [PATCH v2 01/10] ref-filter: add %(refname:shortalign=X) option

2015-07-11 Thread Matthieu Moy
Karthik Nayak  writes:

> On Thu, Jul 9, 2015 at 6:28 PM, Matthieu Moy
>  wrote:
>
>> I think this would deserve a test and documentation. Even though your
>> motivation is for an internal implementation, some users may want to use
>> the feature in 'git for-each-ref --format=...'.
>>
>
> I didn't want to include documentation as this is mostly for internal use,
> but will add with tests.

You need it for internal use, but it's still available to the users, and
may actually turn out to be useful to users.

Actually, you are rewritting tag and branch based on the internal C API,
but it should be possible to write similar commands based on the
command-line interface and by looking at the documentation. It's part of
the philosophy of Git ("toolkit design") to have plumbing commands that
allow writting high-level commands through scripting, or custom commands
through aliases.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 05/10] ref-filter: add option to match literal pattern

2015-07-11 Thread Matthieu Moy
Karthik Nayak  writes:

> On Fri, Jul 10, 2015 at 10:13 PM, Junio C Hamano  wrote:
>> Karthik Nayak  writes:
>>
>>> Since 'ref-filter' only has an option to match path names
>>> add an option for regular pattern matching.
>>
>> There is nothing "regular" about the pattern matching you are
>> adding.
>>
>> Everywhere else we use patterns on refs we call wildmatch(), which
>> is an enhanced implementation of fnmatch(3), and you are doing the
>> same in this new codepath.
>>
>> Just drop that word from here (and if you said something similar in
>> the documentation, drop "regular" ffrom there as well).  It would
>> invite confusion with regular expression matching, which we will not
>> do for refs.
>
> Ok, will do. Thanks

Just dropping "regular" leads to a strange sentence, since the path name
match is also a kind of pattern-matching. I'd write

Since 'ref-filter' only has an option to match path names, add an option
for plain fnmatch pattern-matching.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jul 2015, #03; Fri, 10)

2015-07-11 Thread Johannes Schindelin
Hi Junio,

On 2015-07-11 00:45, Junio C Hamano wrote:

> * et/http-proxyauth (2015-06-29) 1 commit
>   (merged to 'next' on 2015-07-09 at cf80874)
>  + http: always use any proxy auth method available
> 
>  We used to ask libCURL to use the most secure authentication method
>  available when talking to an HTTP proxy only when we were told to
>  talk to one via configuration variables.  We now ask libCURL to
>  always use the most secure authentication method, because the user
>  can tell libCURL to use an HTTP proxy via an environment variable
>  without using configuration variables.
> 
>  An extra set of eyes appreciated, but I think this is ready.

I believe so, yes, and I further believe that this would also have helped a Git 
for Windows user: https://github.com/git-for-windows/git/issues/234

Therefore, I am very much in favor of merging this to 'master'.

> * jc/fsck-retire-require-eoh (2015-06-28) 1 commit
>   (merged to 'next' on 2015-07-09 at dbc292b)
>  + fsck: it is OK for a tag and a commit to lack the body
> 
>  A fix to a minor regression to "git fsck" in v2.2 era that started
>  complaining about a body-less tag object when it lacks a separator
>  empty line after its header to separate it with a non-existent body.

Thanks for cleaning up after me!

> * pt/pull-builtin (2015-06-18) 19 commits
>   (merged to 'next' on 2015-07-10 at 07b1794)
>  + pull: remove redirection to git-pull.sh
>  + pull --rebase: error on no merge candidate cases
>  + pull --rebase: exit early when the working directory is dirty
>  + pull: configure --rebase via branch..rebase or pull.rebase
>  + pull: teach git pull about --rebase
>  + pull: set reflog message
>  + pull: implement pulling into an unborn branch
>  + pull: fast-forward working tree if head is updated
>  + pull: check if in unresolved merge state
>  + pull: support pull.ff config
>  + pull: error on no merge candidates
>  + pull: pass git-fetch's options to git-fetch
>  + pull: pass git-merge's options to git-merge
>  + pull: pass verbosity, --progress flags to fetch and merge
>  + pull: implement fetch + merge
>  + pull: implement skeletal builtin pull
>  + argv-array: implement argv_array_pushv()
>  + parse-options-cb: implement parse_opt_passthru_argv()
>  + parse-options-cb: implement parse_opt_passthru()
>  (this branch is used by pt/am-builtin.)
> 
>  Reimplement 'git pull' in C.
> 
>  Will merge to 'master'.

Yeah!

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


Re: [PATCH] git-filter-branch.sh: clarify an error message

2015-07-11 Thread Johannes Schindelin
Hi Chris,

On 2015-07-11 04:04, Chris Jones wrote:
> How does "You did not provide a valid range of commits.  
> At least one tip in your range must be a ref." sound?

Maybe a bit shorter, as well as less accusing (which "You did not...!" may 
sound):

filter-branch requires a valid commit range.

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


Re: [PATCH v2 7/9] grep/pcre: support utf-8

2015-07-11 Thread Plamen Totev
Nguyễn Thái Ngọc Duy  writes:
> 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 parse the string 
> correctly before folding case. 

> if (opt->ignore_case) { 
> p->pcre_tables = pcre_maketables(); 
> + if (is_utf8_locale()) 
> + options |= PCRE_UTF8; 
> options |= PCRE_CASELESS; 
> } 

We need to set the PCRE_UTF8 flag in all cases when the locale is UTF-8
not only when the search is case insensitive.
Otherwise pcre threats the encoding as single byte and if the regex contains
quantifiers it will not work as expected. The quantifier will try to match the
second byte of the multi-byte symbol instead of the whole symbol.

For example lets have file that contains the string

TILRAUN: HALLÓÓÓ HEIMUR!

the following command

git grep -P "HALLÓ{3}"

will not match the file while 

git grep -P "HAL{2}ÓÓÓ"

will. That's because the L symbol is a single byte.

Regards,
Plamen Totev

--
To unsubscribe from this list: send the line "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 14/16] worktree: avoid resolving HEAD unnecessarily

2015-07-11 Thread Eric Sunshine
Now that git-worktree sets HEAD explicitly to its final value via either
git-symbolic-ref or git-update-ref, rather than relying upon
git-checkout to do so, the "hack" for pacifying is_git_directory() with
a temporary HEAD, though still necessary, can be simplified.

Since the real HEAD is now populated with its proper final value, the
value of the temporary HEAD truly no longer matters, and any value which
looks like an object ID is good enough to satisfy is_git_directory().
Therefore, just set the temporary HEAD to a literal value rather than
going through the effort of resolving the current branch's HEAD.

Signed-off-by: Eric Sunshine 
---
 builtin/worktree.c | 17 +
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 94c1701..9101a3c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -191,7 +191,6 @@ static int add_worktree(const char *path, const char 
*refname,
int counter = 0, len, ret;
struct strbuf symref = STRBUF_INIT;
struct commit *commit = NULL;
-   unsigned char rev[20];
 
if (file_exists(path) && !is_empty_dir(path))
die(_("'%s' already exists"), path);
@@ -249,20 +248,14 @@ static int add_worktree(const char *path, const char 
*refname,
   real_path(get_git_common_dir()), name);
/*
 * This is to keep resolve_ref() happy. We need a valid HEAD
-* or is_git_directory() will reject the directory. Moreover, HEAD
-* in the new worktree must resolve to the same value as HEAD in
-* the current tree since the command invoked to populate the new
-* worktree will be handed the branch/ref specified by the user.
-* For instance, if the user asks for the new worktree to be based
-* at HEAD~5, then the resolved HEAD~5 in the new worktree must
-* match the resolved HEAD~5 in the current tree in order to match
-* the user's expectation.
+* or is_git_directory() will reject the directory. Any value which
+* looks like an object ID will do since it will be immediately
+* replaced by the symbolic-ref or update-ref invocation in the new
+* worktree.
 */
-   if (!resolve_ref_unsafe("HEAD", 0, rev, NULL))
-   die(_("unable to resolve HEAD"));
strbuf_reset(&sb);
strbuf_addf(&sb, "%s/HEAD", sb_repo.buf);
-   write_file(sb.buf, 1, "%s\n", sha1_to_hex(rev));
+   write_file(sb.buf, 1, "\n");
strbuf_reset(&sb);
strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
write_file(sb.buf, 1, "../..\n");
-- 
2.5.0.rc1.201.ga12d9f8

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


[PATCH 11/16] worktree: add_worktree: construct worktree-population command locally

2015-07-11 Thread Eric Sunshine
The caller of add_worktree() provides it with a command to invoke to
populate the new worktree. This was a useful abstraction during the
conversion of "git checkout --to" functionality to "git worktree add"
since git-checkout and git-worktree constructed the population command
differently. However, now that "git checkout --to" has been retired, and
add_worktree() has access to the options given to "worktree add", this
extra indirection is no longer useful, and makes the code a bit more
convoluted than it needs to be.

Moreover, the eventual goal is for git-worktree to populate the new
worktree via "git reset --hard" rather than "git checkout", and to do
so, it may need to invoke other commands in addition to the
worktree-population command, so it will be doing command construction
itself anyhow. Therefore, relocate construction of the
worktree-population command from add() to add_worktree().

Signed-off-by: Eric Sunshine 
---
 builtin/worktree.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 9be1c74..e04a6d1 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -178,7 +178,7 @@ static const char *worktree_basename(const char *path, int 
*olen)
return name;
 }
 
-static int add_worktree(const char *path, const char **child_argv,
+static int add_worktree(const char *path, const char *refname,
const struct add_opts *opts)
 {
struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;
@@ -260,7 +260,12 @@ static int add_worktree(const char *path, const char 
**child_argv,
setenv(GIT_WORK_TREE_ENVIRONMENT, path, 1);
memset(&cp, 0, sizeof(cp));
cp.git_cmd = 1;
-   cp.argv = child_argv;
+   argv_array_push(&cp.args, "checkout");
+   if (opts->force)
+   argv_array_push(&cp.args, "--ignore-other-worktrees");
+   if (opts->detach)
+   argv_array_push(&cp.args, "--detach");
+   argv_array_push(&cp.args, refname);
ret = run_command(&cp);
if (!ret) {
is_junk = 0;
@@ -283,7 +288,6 @@ static int add(int ac, const char **av, const char *prefix)
struct add_opts opts;
const char *new_branch_force = NULL;
const char *path, *branch;
-   struct argv_array cmd = ARGV_ARRAY_INIT;
struct option options[] = {
OPT__FORCE(&opts.force, N_("checkout  even if already 
checked out in other worktree")),
OPT_STRING('b', NULL, &opts.new_branch, N_("branch"),
@@ -328,14 +332,7 @@ static int add(int ac, const char **av, const char *prefix)
branch = opts.new_branch;
}
 
-   argv_array_push(&cmd, "checkout");
-   if (opts.force)
-   argv_array_push(&cmd, "--ignore-other-worktrees");
-   if (opts.detach)
-   argv_array_push(&cmd, "--detach");
-   argv_array_push(&cmd, branch);
-
-   return add_worktree(path, cmd.argv, &opts);
+   return add_worktree(path, branch, &opts);
 }
 
 int cmd_worktree(int ac, const char **av, const char *prefix)
-- 
2.5.0.rc1.201.ga12d9f8

--
To unsubscribe from this list: send the line "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 04/16] checkout: die_if_checked_out: simplify strbuf management

2015-07-11 Thread Eric Sunshine
There is no reason to keep the strbuf active long after its last use.
By releasing it as early as possible, resource management is simplified
and there is less worry about future changes resulting in a leak.

Signed-off-by: Eric Sunshine 
---
 builtin/checkout.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index fc8bd79..ee33a20 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -924,17 +924,16 @@ static void die_if_checked_out(struct branch_info *new)
check_linked_checkout(new, NULL);
 
strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
-   if ((dir = opendir(path.buf)) == NULL) {
-   strbuf_release(&path);
+   dir = opendir(path.buf);
+   strbuf_release(&path);
+   if (!dir)
return;
-   }
 
while ((d = readdir(dir)) != NULL) {
if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
continue;
check_linked_checkout(new, d->d_name);
}
-   strbuf_release(&path);
closedir(dir);
 }
 
-- 
2.5.0.rc1.201.ga12d9f8

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


[PATCH 03/16] checkout: improve die_if_checked_out() robustness

2015-07-11 Thread Eric Sunshine
die_if_checked_out() is intended to check if the branch about to be
checked out is already checked out either in the main worktree or in a
linked worktree. However, if .git/worktrees directory does not exist,
then it never bothers checking the main worktree, even though the
specified branch might indeed be checked out there, which is fragile
behavior.

This hasn't been a problem in practice since the current implementation
of "git worktree add" (and, earlier, "git checkout --to") always creates
.git/worktrees before die_if_checked_out() is called by the child "git
checkout" invocation which populates the new worktree.

However, git-worktree will eventually be retrofitted to populate the new
worktree via "git reset --hard" rather than "git checkout", and it will
want to use die_if_checked_out() to perform this check as well. But,
the reliance upon order of operations (creating .git/worktrees before
checking if a branch is already checked out) is fragile, and, as a
general function, callers should not be expected to abide by this
undocumented and unwarranted restriction. Therefore, make
die_if_checked_out() more robust by checking the main worktree whether
.git/worktrees exists or not.

Signed-off-by: Eric Sunshine 
---
 builtin/checkout.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index e75fb5e..fc8bd79 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -916,12 +916,6 @@ static void die_if_checked_out(struct branch_info *new)
DIR *dir;
struct dirent *d;
 
-   strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
-   if ((dir = opendir(path.buf)) == NULL) {
-   strbuf_release(&path);
-   return;
-   }
-
/*
 * $GIT_COMMON_DIR/HEAD is practically outside
 * $GIT_DIR so resolve_ref_unsafe() won't work (it
@@ -929,6 +923,12 @@ static void die_if_checked_out(struct branch_info *new)
 */
check_linked_checkout(new, NULL);
 
+   strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
+   if ((dir = opendir(path.buf)) == NULL) {
+   strbuf_release(&path);
+   return;
+   }
+
while ((d = readdir(dir)) != NULL) {
if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
continue;
-- 
2.5.0.rc1.201.ga12d9f8

--
To unsubscribe from this list: send the line "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 15/16] worktree: populate via "git reset --hard" rather than "git checkout"

2015-07-11 Thread Eric Sunshine
Now that git-worktree handles all functionality (--force, --detach,
-b/-B) previously delegated to git-checkout, actual population of the
new worktree can be accomplished more directly and more lightweight with
"git reset --hard" rather than "git checkout".

Signed-off-by: Eric Sunshine 
---
 builtin/worktree.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 9101a3c..51c57bc 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -262,7 +262,6 @@ static int add_worktree(const char *path, const char 
*refname,
 
fprintf_ln(stderr, _("Enter %s (identifier %s)"), path, name);
 
-   setenv("GIT_CHECKOUT_NEW_WORKTREE", "1", 1);
setenv(GIT_DIR_ENVIRONMENT, sb_git.buf, 1);
setenv(GIT_WORK_TREE_ENVIRONMENT, path, 1);
memset(&cp, 0, sizeof(cp));
@@ -280,7 +279,7 @@ static int add_worktree(const char *path, const char 
*refname,
 
cp.argv = NULL;
argv_array_clear(&cp.args);
-   argv_array_push(&cp.args, "checkout");
+   argv_array_pushl(&cp.args, "reset", "--hard", NULL);
ret = run_command(&cp);
if (!ret) {
is_junk = 0;
-- 
2.5.0.rc1.201.ga12d9f8

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


[PATCH 06/16] branch: publish die_if_checked_out()

2015-07-11 Thread Eric Sunshine
git-worktree will eventually be retrofitted to populate the new worktree
via "git reset --hard" rather than "git checkout", at which time it will
need to be able to detect if the branch is already checked out
elsewhere, rather than relying upon git-branch to make this
determination, so publish die_if_checked_out().

Signed-off-by: Eric Sunshine 
---
 branch.c   | 65 ++
 branch.h   |  7 ++
 builtin/checkout.c | 65 --
 3 files changed, 72 insertions(+), 65 deletions(-)

diff --git a/branch.c b/branch.c
index 4bab55a..7b8b9a3 100644
--- a/branch.c
+++ b/branch.c
@@ -309,3 +309,68 @@ void remove_branch_state(void)
unlink(git_path("MERGE_MODE"));
unlink(git_path("SQUASH_MSG"));
 }
+
+static void check_linked_checkout(const char *branch, const char *id)
+{
+   struct strbuf sb = STRBUF_INIT;
+   struct strbuf path = STRBUF_INIT;
+   struct strbuf gitdir = STRBUF_INIT;
+   const char *start, *end;
+
+   if (id)
+   strbuf_addf(&path, "%s/worktrees/%s/HEAD", 
get_git_common_dir(), id);
+   else
+   strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
+
+   if (strbuf_read_file(&sb, path.buf, 0) < 0 ||
+   !skip_prefix(sb.buf, "ref:", &start))
+   goto done;
+   while (isspace(*start))
+   start++;
+   end = start;
+   while (*end && !isspace(*end))
+   end++;
+   if (strncmp(start, branch, end - start) || branch[end - start] != '\0')
+   goto done;
+   if (id) {
+   strbuf_reset(&path);
+   strbuf_addf(&path, "%s/worktrees/%s/gitdir", 
get_git_common_dir(), id);
+   if (strbuf_read_file(&gitdir, path.buf, 0) <= 0)
+   goto done;
+   strbuf_rtrim(&gitdir);
+   } else
+   strbuf_addstr(&gitdir, get_git_common_dir());
+   skip_prefix(branch, "refs/heads/", &branch);
+   die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
+done:
+   strbuf_release(&path);
+   strbuf_release(&sb);
+   strbuf_release(&gitdir);
+}
+
+void die_if_checked_out(const char *branch)
+{
+   struct strbuf path = STRBUF_INIT;
+   DIR *dir;
+   struct dirent *d;
+
+   /*
+* $GIT_COMMON_DIR/HEAD is practically outside
+* $GIT_DIR so resolve_ref_unsafe() won't work (it
+* uses git_path). Parse the ref ourselves.
+*/
+   check_linked_checkout(branch, NULL);
+
+   strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
+   dir = opendir(path.buf);
+   strbuf_release(&path);
+   if (!dir)
+   return;
+
+   while ((d = readdir(dir)) != NULL) {
+   if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
+   continue;
+   check_linked_checkout(branch, d->d_name);
+   }
+   closedir(dir);
+}
diff --git a/branch.h b/branch.h
index 64173ab..58aa45f 100644
--- a/branch.h
+++ b/branch.h
@@ -52,4 +52,11 @@ extern void install_branch_config(int flag, const char 
*local, const char *origi
  */
 extern int read_branch_desc(struct strbuf *, const char *branch_name);
 
+/*
+ * Check if a branch is checked out in the main worktree or any linked
+ * worktree and die (with a message describing its checkout location) if
+ * it is.
+ */
+extern void die_if_checked_out(const char *branch);
+
 #endif
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 673daa0..4ae895c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -873,71 +873,6 @@ static const char *unique_tracking_name(const char *name, 
unsigned char *sha1)
return NULL;
 }
 
-static void check_linked_checkout(const char *branch, const char *id)
-{
-   struct strbuf sb = STRBUF_INIT;
-   struct strbuf path = STRBUF_INIT;
-   struct strbuf gitdir = STRBUF_INIT;
-   const char *start, *end;
-
-   if (id)
-   strbuf_addf(&path, "%s/worktrees/%s/HEAD", 
get_git_common_dir(), id);
-   else
-   strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
-
-   if (strbuf_read_file(&sb, path.buf, 0) < 0 ||
-   !skip_prefix(sb.buf, "ref:", &start))
-   goto done;
-   while (isspace(*start))
-   start++;
-   end = start;
-   while (*end && !isspace(*end))
-   end++;
-   if (strncmp(start, branch, end - start) || branch[end - start] != '\0')
-   goto done;
-   if (id) {
-   strbuf_reset(&path);
-   strbuf_addf(&path, "%s/worktrees/%s/gitdir", 
get_git_common_dir(), id);
-   if (strbuf_read_file(&gitdir, path.buf, 0) <= 0)
-   goto done;
-   strbuf_rtrim(&gitdir);
-   } else
-   strbuf_addstr(&gitdir, get_git_common_dir());
-   skip_prefix(branch, "refs/heads/", &branch);
-   die(

[PATCH 01/16] checkout: avoid resolving HEAD unnecessarily

2015-07-11 Thread Eric Sunshine
When --ignore-other-worktree is specified, we unconditionally skip the
check to see if the requested branch is already checked out in a linked
worktree. Since we know that we will be skipping that check, there is no
need to resolve HEAD in order to detect other conditions under which we
may skip the check.

Signed-off-by: Eric Sunshine 
---
 builtin/checkout.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5754554..75f90a9 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1145,13 +1145,13 @@ static int checkout_branch(struct checkout_opts *opts,
die(_("Cannot switch branch to a non-commit '%s'"),
new->name);
 
-   if (new->path && !opts->force_detach && !opts->new_branch) {
+   if (new->path && !opts->force_detach && !opts->new_branch &&
+   !opts->ignore_other_worktrees) {
unsigned char sha1[20];
int flag;
char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
if (head_ref &&
-   (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)) &&
-   !opts->ignore_other_worktrees)
+   (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
check_linked_checkouts(new);
free(head_ref);
}
-- 
2.5.0.rc1.201.ga12d9f8

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


git@vger.kernel.org

2015-07-11 Thread René Scharfe

Am 10.07.2015 um 22:50 schrieb Jeff King:

Thanks, this definitely is a problem, but we already have a fix in the
sb/p5310-and-chain topic. I thought that had been merged-up, but it
looks like it is only in "next" right now.


All the better.  And I see it's in master now.

René

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


[PATCH 09/16] worktree: make --detach mutually exclusive with -b/-B

2015-07-11 Thread Eric Sunshine
Be consistent with git-checkout which disallows this (not particularly
meaningful) combination.

Signed-off-by: Eric Sunshine 
---
 builtin/worktree.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 253382a..cd06bf5 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -296,8 +296,8 @@ static int add(int ac, const char **av, const char *prefix)
 
memset(&opts, 0, sizeof(opts));
ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
-   if (opts.new_branch && new_branch_force)
-   die(_("-b and -B are mutually exclusive"));
+   if (!!opts.detach + !!opts.new_branch + !!new_branch_force > 1)
+   die(_("-b, -B, and --detach are mutually exclusive"));
if (ac < 1 || ac > 2)
usage_with_options(worktree_usage, options);
 
-- 
2.5.0.rc1.201.ga12d9f8

--
To unsubscribe from this list: send the line "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 05/16] checkout: generalize die_if_checked_out() branch name argument

2015-07-11 Thread Eric Sunshine
The plan is to publish die_if_checked_out() so that callers other than
git-checkout can take advantage of it, however, those callers won't have
access to git-checkout's "struct branch_info". Therefore, change it to
accept the full name of the branch as a simple string instead.

While here, also give the argument a more meaningful name ("branch"
instead of "new").

Signed-off-by: Eric Sunshine 
---
 builtin/checkout.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index ee33a20..673daa0 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -873,7 +873,7 @@ static const char *unique_tracking_name(const char *name, 
unsigned char *sha1)
return NULL;
 }
 
-static void check_linked_checkout(struct branch_info *new, const char *id)
+static void check_linked_checkout(const char *branch, const char *id)
 {
struct strbuf sb = STRBUF_INIT;
struct strbuf path = STRBUF_INIT;
@@ -893,7 +893,7 @@ static void check_linked_checkout(struct branch_info *new, 
const char *id)
end = start;
while (*end && !isspace(*end))
end++;
-   if (strncmp(start, new->path, end - start) || new->path[end - start] != 
'\0')
+   if (strncmp(start, branch, end - start) || branch[end - start] != '\0')
goto done;
if (id) {
strbuf_reset(&path);
@@ -903,14 +903,15 @@ static void check_linked_checkout(struct branch_info 
*new, const char *id)
strbuf_rtrim(&gitdir);
} else
strbuf_addstr(&gitdir, get_git_common_dir());
-   die(_("'%s' is already checked out at '%s'"), new->name, gitdir.buf);
+   skip_prefix(branch, "refs/heads/", &branch);
+   die(_("'%s' is already checked out at '%s'"), branch, gitdir.buf);
 done:
strbuf_release(&path);
strbuf_release(&sb);
strbuf_release(&gitdir);
 }
 
-static void die_if_checked_out(struct branch_info *new)
+static void die_if_checked_out(const char *branch)
 {
struct strbuf path = STRBUF_INIT;
DIR *dir;
@@ -921,7 +922,7 @@ static void die_if_checked_out(struct branch_info *new)
 * $GIT_DIR so resolve_ref_unsafe() won't work (it
 * uses git_path). Parse the ref ourselves.
 */
-   check_linked_checkout(new, NULL);
+   check_linked_checkout(branch, NULL);
 
strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
dir = opendir(path.buf);
@@ -932,7 +933,7 @@ static void die_if_checked_out(struct branch_info *new)
while ((d = readdir(dir)) != NULL) {
if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
continue;
-   check_linked_checkout(new, d->d_name);
+   check_linked_checkout(branch, d->d_name);
}
closedir(dir);
 }
@@ -1151,7 +1152,7 @@ static int checkout_branch(struct checkout_opts *opts,
char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
if (head_ref &&
(!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
-   die_if_checked_out(new);
+   die_if_checked_out(new->path);
free(head_ref);
}
 
-- 
2.5.0.rc1.201.ga12d9f8

--
To unsubscribe from this list: send the line "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 00/16] worktree: use "git reset --hard" to populate worktree

2015-07-11 Thread Eric Sunshine
This is a follow-on series to [1], which migrated "git checkout --to"
functionality to "git worktree add". That series continued using "git
checkout" for the initial population of the new worktree, which required
git-checkout to have too intimate knowledge that it was operating in a
newly created worktree.

This series eliminates git-checkout from the picture by instead
employing "git reset --hard"[2] to populate the new worktree initially.

It is built atop 1eb07d8 (worktree: add: auto-vivify new branch when
 is omitted, 2015-07-06), currently in 'next', which is
es/worktree-add except for the final patch (which retires
--ignore-other-worktrees) since the intention[3] was to drop that patch.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/273415
[2]: http://thread.gmane.org/gmane.comp.version-control.git/273415/focus=273454
[3]: http://thread.gmane.org/gmane.comp.version-control.git/273415/focus=273585

Eric Sunshine (16):
  checkout: avoid resolving HEAD unnecessarily
  checkout: name check_linked_checkouts() more meaningfully
  checkout: improve die_if_checked_out() robustness
  checkout: die_if_checked_out: simplify strbuf management
  checkout: generalize die_if_checked_out() branch name argument
  branch: publish die_if_checked_out()
  worktree: simplify new branch (-b/-B) option checking
  worktree: introduce options container
  worktree: make --detach mutually exclusive with -b/-B
  worktree: make branch creation distinct from worktree population
  worktree: add_worktree: construct worktree-population command locally
  worktree: detect branch symref/detach and error conditions locally
  worktree: make setup of new HEAD distinct from worktree population
  worktree: avoid resolving HEAD unnecessarily
  worktree: populate via "git reset --hard" rather than "git checkout"
  checkout: drop intimate knowledge of new worktree initial population

 branch.c   |  65 +++
 branch.h   |   7 
 builtin/checkout.c |  82 +++
 builtin/worktree.c | 110 +++--
 4 files changed, 151 insertions(+), 113 deletions(-)

-- 
2.5.0.rc1.201.ga12d9f8

--
To unsubscribe from this list: send the line "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 16/16] checkout: drop intimate knowledge of new worktree initial population

2015-07-11 Thread Eric Sunshine
Now that git-worktree no longer relies upon git-checkout to perform
initial population of the new worktree, git-checkout no longer needs
intimate knowledge that it may be working on a newly created worktree.
Therefore, drop 'new_worktree_mode' and the private
GIT_CHECKOUT_NEW_WORKTREE environment variable by which git-worktree
communicated to git-checkout that it was being invoked to populate a new
worktree.

This reverts the remaining changes to checkout.c by 529fef2 (checkout:
support checking out into a new working directory, 2014-11-30).

Signed-off-by: Eric Sunshine 
---
 builtin/checkout.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 4ae895c..02d78ba 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -48,8 +48,6 @@ struct checkout_opts {
const char *prefix;
struct pathspec pathspec;
struct tree *source_tree;
-
-   int new_worktree_mode;
 };
 
 static int post_checkout_hook(struct commit *old, struct commit *new,
@@ -491,7 +489,7 @@ static int merge_working_tree(const struct checkout_opts 
*opts,
topts.dir->flags |= DIR_SHOW_IGNORED;
setup_standard_excludes(topts.dir);
}
-   tree = parse_tree_indirect(old->commit && 
!opts->new_worktree_mode ?
+   tree = parse_tree_indirect(old->commit ?
   old->commit->object.sha1 :
   EMPTY_TREE_SHA1_BIN);
init_tree_desc(&trees[0], tree->buffer, tree->size);
@@ -807,8 +805,7 @@ static int switch_branches(const struct checkout_opts *opts,
return ret;
}
 
-   if (!opts->quiet && !old.path && old.commit &&
-   new->commit != old.commit && !opts->new_worktree_mode)
+   if (!opts->quiet && !old.path && old.commit && new->commit != 
old.commit)
orphaned_commit_warning(old.commit, new->commit);
 
update_refs_for_switch(opts, &old, new);
@@ -1151,8 +1148,6 @@ int cmd_checkout(int argc, const char **argv, const char 
*prefix)
argc = parse_options(argc, argv, prefix, options, checkout_usage,
 PARSE_OPT_KEEP_DASHDASH);
 
-   opts.new_worktree_mode = getenv("GIT_CHECKOUT_NEW_WORKTREE") != NULL;
-
if (conflict_style) {
opts.merge = 1; /* implied */
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
-- 
2.5.0.rc1.201.ga12d9f8

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


[PATCH 10/16] worktree: make branch creation distinct from worktree population

2015-07-11 Thread Eric Sunshine
The plan is eventually to populate the new worktree via "git reset
--hard" rather than "git checkout". Thus, rather than piggybacking on
git-checkout's -b/-B ability to create a new branch at checkout time,
git-worktree will need to do so itself.

Signed-off-by: Eric Sunshine 
---

I considered calling branch-related API, such as create_branch(),
directly, however, git-branch provides extra value which may be useful
in the future (such as when --track and --orphan get added to
git-worktree), so it seemed wise to re-use git-branch's implementation
rather than duplicating the functionality. If this proves the wrong
choice, then the series can either be re-rolled or follow-on patches can
address the issue.

 builtin/worktree.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index cd06bf5..9be1c74 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -314,12 +314,23 @@ static int add(int ac, const char **av, const char 
*prefix)
opts.new_branch = xstrndup(s, n);
}
 
+   if (opts.new_branch) {
+   struct child_process cp;
+   memset(&cp, 0, sizeof(cp));
+   cp.git_cmd = 1;
+   argv_array_push(&cp.args, "branch");
+   if (opts.force_new_branch)
+   argv_array_push(&cp.args, "--force");
+   argv_array_push(&cp.args, opts.new_branch);
+   argv_array_push(&cp.args, branch);
+   if (run_command(&cp))
+   return -1;
+   branch = opts.new_branch;
+   }
+
argv_array_push(&cmd, "checkout");
if (opts.force)
argv_array_push(&cmd, "--ignore-other-worktrees");
-   if (opts.new_branch)
-   argv_array_pushl(&cmd, opts.force_new_branch ? "-B" : "-b",
-opts.new_branch, NULL);
if (opts.detach)
argv_array_push(&cmd, "--detach");
argv_array_push(&cmd, branch);
-- 
2.5.0.rc1.201.ga12d9f8

--
To unsubscribe from this list: send the line "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 Smart HTTP with HTTP/2.0

2015-07-11 Thread Ilari Liusvaara
On Sat, Jul 11, 2015 at 11:10:48AM +0800, ForceCharlie wrote:
> As we known, HTTP/2.0 has been released. All Git-Smart-HTTP are currently
> implemented using HTTP/1.1.

Nit: It is HTTP/2.
 
> Frequently used Git developers often feel Git HTTP protocol is not
> satisfactory, slow and unstable.This is because the HTTP protocol itself
> decides

Note that there are already two versions of HTTP transport, the old "dumb"
one and the newer "smart" one.

The smart one is difficult to speed up (due to nature of the negotiations),
but usually is pretty reliable (the efficiency isn't horrible).

Now, the old "dumb" protocol is pretty unreliable and slow. HTTP/2 probably
can't do anything with the reliability problems, but probably could improve
the speed a bit.

Websockets over HTTP/2 (a.k.a. "websockets2") has not been defined yet.
With Websockets(1), it would probably already be possible to tunnel the
native git smart transport protocol over it. Probably not worth it.

> When HTTP/2.0 is published. We might be able to git developers jointly,
> based on HTTP/2.0 Git-Smart-HTTP service and client support.
> HTTP/2.0: https://tools.ietf.org/html/rfc7540

Well, it is published already.
 

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


[PATCH 12/16] worktree: detect branch symref/detach and error conditions locally

2015-07-11 Thread Eric Sunshine
The eventual goal is for git-worktree to populate the new worktree via
"git reset --hard" rather than "git checkout". As a consequence,
git-worktree will no longer be able to rely upon git-branch to determine
the state of the worktree (detached or branch symref), or to check for
error conditions, such as the requested branch already checked out
elsewhere, or an invalid reference. Therefore, imbue git-worktree with
the intelligence to determine a branch symref or detached state locally,
and to perform error checking on its own.

Signed-off-by: Eric Sunshine 
---

This patch is somewhat RFC since I spent a lot of time browsing the
branch- and ref-related APIs figuring out how to determine if the
provided refname was a branch name or some other (detached) reference.
I'm not at all sure that my use of strbuf_check_branch_ref(),
ref_exists(), and lookup_commit_reference_by_name() is the best
approach, or even appropriate, although it seems to work as desired.

 builtin/worktree.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index e04a6d1..babdef1 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -3,6 +3,8 @@
 #include "dir.h"
 #include "parse-options.h"
 #include "argv-array.h"
+#include "branch.h"
+#include "refs.h"
 #include "run-command.h"
 #include "sigchain.h"
 
@@ -187,11 +189,23 @@ static int add_worktree(const char *path, const char 
*refname,
struct stat st;
struct child_process cp;
int counter = 0, len, ret;
+   struct strbuf symref = STRBUF_INIT;
+   struct commit *commit = NULL;
unsigned char rev[20];
 
if (file_exists(path) && !is_empty_dir(path))
die(_("'%s' already exists"), path);
 
+   if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&
+   ref_exists(symref.buf)) {
+   if (!opts->force)
+   die_if_checked_out(symref.buf);
+   } else {
+   commit = lookup_commit_reference_by_name(refname);
+   if (!commit)
+   die(_("invalid reference: %s"), refname);
+   }
+
name = worktree_basename(path, &len);
strbuf_addstr(&sb_repo,
  git_path("worktrees/%.*s", (int)(path + len - name), 
name));
@@ -278,6 +292,7 @@ static int add_worktree(const char *path, const char 
*refname,
strbuf_addf(&sb, "%s/locked", sb_repo.buf);
unlink_or_warn(sb.buf);
strbuf_release(&sb);
+   strbuf_release(&symref);
strbuf_release(&sb_repo);
strbuf_release(&sb_git);
return ret;
-- 
2.5.0.rc1.201.ga12d9f8

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


[PATCH 02/16] checkout: name check_linked_checkouts() more meaningfully

2015-07-11 Thread Eric Sunshine
check_linked_checkouts() doesn't just "check" linked checkouts for
"something"; specifically, it aborts the operation if the branch about
to be checked out is already checked out elsewhere. Therefore, rename it
to die_if_checked_out() to give a better indication of its function.
The more meaningful name will be particularly important when this
function is later published for use by other callers.

Signed-off-by: Eric Sunshine 
---
 builtin/checkout.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 75f90a9..e75fb5e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -910,7 +910,7 @@ done:
strbuf_release(&gitdir);
 }
 
-static void check_linked_checkouts(struct branch_info *new)
+static void die_if_checked_out(struct branch_info *new)
 {
struct strbuf path = STRBUF_INIT;
DIR *dir;
@@ -1152,7 +1152,7 @@ static int checkout_branch(struct checkout_opts *opts,
char *head_ref = resolve_refdup("HEAD", 0, sha1, &flag);
if (head_ref &&
(!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
-   check_linked_checkouts(new);
+   die_if_checked_out(new);
free(head_ref);
}
 
-- 
2.5.0.rc1.201.ga12d9f8

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