Re: git rebase -i without altering the committer date

[Johannes Sixt]

Am 20.04.2016 um 23:47 schrieb Andreas Schwab:

Shaun Jackman  writes:


I'd like to insert a commit between two commits without changing
the committer date or author date of that commit or the subsequent
commits.


The easiest way to implement that is to add a graft to redirect the
parent of the second commit to the inserted commit, then use git
filter-branch to make the graft permanent.


This only inserts a new project state, but does not propagate the 
changes brought in by the new commit to the subsequent commits. This 
propagation of changes could also be done with filter-branch, but it may 
be difficult depending on circumstances.


-- 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: [PATCH 2/5] run-command: teach async threads to ignore SIGPIPE

[Jeff King]

On Thu, Apr 21, 2016 at 07:15:26AM +0200, Johannes Sixt wrote:

> Am 20.04.2016 um 00:49 schrieb Jeff King:
> >This is our first use of pthread_sigmask, and I think Windows will have
> >to come up with something for this in compat/. I don't know how SIGPIPE
> >works there at all, so it's possible that we can just turn this into a
> >noop. Worst case it could probably block SIGPIPE for the whole process.
> 
> There is no SIGPIPE on Windows. write() always returns EPIPE as if SIGPIPE
> was ignored.
> 
> We'll have to make pthread_sigmask() a no-op.

Great, thanks for clarifying. We can also #ifdef out the whole block
there if it's easier, but it looks like you already have noop
implementations for sigset, et al. So adding a noop pthread_sigmask()
should be enough.

-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 2/5] run-command: teach async threads to ignore SIGPIPE

[Johannes Sixt]

Am 20.04.2016 um 00:49 schrieb Jeff King:

This is our first use of pthread_sigmask, and I think Windows will have
to come up with something for this in compat/. I don't know how SIGPIPE
works there at all, so it's possible that we can just turn this into a
noop. Worst case it could probably block SIGPIPE for the whole process.


There is no SIGPIPE on Windows. write() always returns EPIPE as if 
SIGPIPE was ignored.


We'll have to make pthread_sigmask() a no-op.

-- 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: problems serving non-bare repos with submodules over http

[Yaroslav Halchenko]

NB Thank you for the lively discussion!

On Wed, 20 Apr 2016, Stefan Beller wrote:

> >> So currently the protocol doesn't allow to even specify the submodules
> >> directories.

> > Depends on what you exactly mean by "the protocol", but the
> > networking protocol is about accessing a single repository.  It is
> > up to you to decide where to go next after learning what you can
> > learn from the result, typically by following what appears in
> > the .gitmodules file.

> Right. But the .gitmodules file is not sufficient.

why?

> >...<

> I think on a hosting site they could even coexist when having the
> layout as above.

>  top.git/
>  top.git/refs/{heads,tags,...}/...
>  top.git/objects/...
>  sub.git/
>  sub.git/refs/{heads,tags,...}/...
>  sub.git/objects/...

>  # the following only exist in non bare:
>  top.git/modules/sub.git/
>  top.git/modules/sub.git/refs/{heads,tags,...}/...
>  top.git/modules/sub.git/objects/...

> The later files would be more reflective of what you *really*
> want if you clone from top.git.

may be there is no need for assumptions and .gitmodules should be
sufficient?

- absolute url in .gitmodules provides absolute URL/path to the
  submodule of interest, regardless either submodule is present in
  originating repository as updated submodule.  Either cloning it
  instead of original repository would be more efficient is already a
  heuristic which might fail miserably (may be I have a faster
  connection to the original repository pointed by the absolute
  url than to this particular repository)

- relative url in .gitmodules provides relative location to the location
  of the "top" repository, and that is only when that submodule "absolute"
  url should be resolved relative to the one of the "top" repository 

NB I will consider it a separate issue either relative paths
without '../' prefix are having any sense in bare repositories.

or have I missed the point?
-- 
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik
--
To unsubscribe from this list: send the line "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 09/12] worktree.c: test if branch being rebased in another worktree

[Duy Nguyen]

On Thu, Apr 21, 2016 at 1:04 AM, Junio C Hamano  wrote:
>> diff --git a/builtin/checkout.c b/builtin/checkout.c
>> index efcbd8f..6041718 100644
>> --- a/builtin/checkout.c
>> +++ b/builtin/checkout.c
>> @@ -,7 +,7 @@ static int checkout_branch(struct checkout_opts *opts,
>>   char *head_ref = resolve_refdup("HEAD", 0, sha1, );
>>   if (head_ref &&
>>   (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
>> - die_if_checked_out(new->path);
>> + die_if_checked_out(new->path, 1);
>>   free(head_ref);
>>   }
>
> So the idea is "if the branch is checked out (or "being worked on"
> even if technically the HEAD is detached, like with 'rebase')
> anywhere, callers of die-if-checked-out in general want to die; but
> for this caller, it is OK if the place the branch is checked out or
> being worked on is in this repository"?
>
> I understand die_if_checked_out() taking that "ignore this one" bit
> may be sensible, but I do not understand why find_shared_symref()
> needs to be told about it.  The change makes the meaning of the
> find_shared_symref() function unclear.  It used to mean "This
> symbolic ref cannot point at the same ref in different worktrees, so
> for a given pair of a symbolic ref and a concrete ref, there can be
> at most one worktree in which the symbolic ref points at that ref".
> That is already a mouthful.  As the worktree structure already have
> "Am I the current worktree?" bit, "ignore" logic can easily be done
> inside die_if_checked_out() and that would help find_shared_symref()
> stay simpler and more focused function, no?

That was the intention when I made find_shared_symref() return struct
worktree * instead of char *. I forget why I changed my mind and not
do so. The only case when find_shared_symref should do this is when a
ref is shared twice, then we need to ignore current worktree from
inside the loop. But that can't happen. Will move this
ignore-current-worktree test to die_if_checked_out().
-- 
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 v2 01/12] path.c: add git_common_path() and strbuf_git_common_path()

[Duy Nguyen]

On Thu, Apr 21, 2016 at 1:11 AM, Eric Sunshine  wrote:
> On Wed, Apr 20, 2016 at 9:24 AM, Nguyễn Thái Ngọc Duy  
> wrote:
>> diff --git a/path.c b/path.c
>> @@ -503,6 +503,35 @@ void strbuf_git_path_submodule(struct strbuf *buf, 
>> const char *path,
>> +const char *git_common_path(const char *fmt, ...)
>> +{
>> +   struct strbuf *pathname = get_pathname();
>> +   va_list args;
>> +   va_start(args, fmt);
>> +   do_git_common_path(pathname, fmt, args);
>> +   va_end(args);
>> +   return pathname->buf;
>
> Is the caller expected to free this value? If not, then shouldn't
> 'pathname' be static? If so, then perhaps strbuf_detach() would be
> clearer (and return 'char *' rather than 'const char *').

get_pathname() actually holds a ring of static buffer. So no, we don't
need static pathname, it can be a new buffer next time, mostly to to
make printf("%s %s", git_common_path(..), git_common_path(..)) work.
And no the caller is not supposed to free it, a little bit more
convenient.
-- 
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: problems serving non-bare repos with submodules over http

[Stefan Beller]

On Wed, Apr 20, 2016 at 2:27 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>>> I may be missing the subtleties, but if you are serving others from
>>> a non-bare repository with submodules, I do not think you would want
>>> to expose the in-tree version of the submodule in the first place.
>>
>> Well I would imagine that is the exact point.
>> If I was not trying to expose my state, I could ask you to
>> obtain your copy from $(git remote get-url origin) just as I did.
>
> That wasn't what I had in mind, but if the cloner cloned from your
> repository with a working tree, the cloner would discover submodules
> you use from your .gitmodules file, which would record the location
> you cloned them from, so something like that may come into the
> picture.  What I had in mind was more like this one you mentioned
> below:
>
>> $GIT_DIR_SUPER_PROJECT/modules/$MODULE_NAME
>> ...
>> Right instead of cloning $WORKTREE/sub/.git you rather want
>> $GITDIR/module/sub
>
>> So currently the protocol doesn't allow to even specify the submodules
>> directories.
>
> Depends on what you exactly mean by "the protocol", but the
> networking protocol is about accessing a single repository.  It is
> up to you to decide where to go next after learning what you can
> learn from the result, typically by following what appears in
> the .gitmodules file.

Right. But the .gitmodules file is not sufficient.

If I clone from a bare hosting location, the .gitmodules file
is the best we can do and the .gitmodules file works as intended.
But in the non bare I case I think we would want to get the submodule
from that location as well.

So in git clone (which calls out to git submodule update, which uses
submodule--helper update_clone for cloning submodules) we'd want to see

if remote was bare:
do as usual (obtain URL from .gitmodules file)
else
take URL=$NON_BARE_REMOTE/module/submodule



>
> The only special case is when .gitmodules file records the URL in a
> relative form, I would think.  Traditionally (i.e. when it was
> considered sane to clone only from bare repositories) I think people
> expected a layout like this:
>
> top.git/
> top.git/refs/{heads,tags,...}/...
> top.git/objects/...
> top.git/sub.git/
> top.git/sub.git/refs/{heads,tags,...}/...
> top.git/sub.git/objects/...

which could also be referred to as

  top

without the .git suffix as someone thought this was an optimization?

Relative paths for submodules I have seen so far (on github,
googlesource, eclipse)
start with a ../ such that we'd have

> top.git/
> top.git/refs/{heads,tags,...}/...
> top.git/objects/...
> sub.git/
> sub.git/refs/{heads,tags,...}/...
> sub.git/objects/...

and the .git suffix omission works as we only need to check for the last
for characters and not somewhere in between. The sub.git is a standalone
repository, and you cannot tell it is a submodule (except by its contents)

>
> and refer to ./sub.git from .gitmodules recorded in top.git.  It
> still would be norm for common distribution sites (i.e. the original
> place Yaroslav likely has cloned things from) to be bare, and with
> or without $GIT_DIR/modules/, the relative path of submodule seen
> by its superproject would (have to) be different between a bare and
> a non-bare repository.

I think on a hosting site they could even coexist when having the
layout as above.

 top.git/
 top.git/refs/{heads,tags,...}/...
 top.git/objects/...
 sub.git/
 sub.git/refs/{heads,tags,...}/...
 sub.git/objects/...

 # the following only exist in non bare:
 top.git/modules/sub.git/
 top.git/modules/sub.git/refs/{heads,tags,...}/...
 top.git/modules/sub.git/objects/...

The later files would be more reflective of what you *really*
want if you clone from top.git.

Traditionally (when cloning was done from bare repos only),
the .gitmodules file provides a very good way to indicate what
the intent of the superproject is as the recorded sha1 in the tree
doesn't tell you anything and tracking the remote for the submodule
out of tree is cumbersome, so an in tree solution makes perfect sense.

If we have a non bare repo, it is safe to assume that the cloner actually
meant to get the whole state from the remote (including submodules)?

I am trying to think of reasons why you would not want to get that copy
from the remote. One (weak) reason is that the submodule may be a
well known library, which you can obtain faster from a well known git
hosting site rather than $remote.

>
> I'd imagine that people could agree on a common layout like this
> even for a forest of bare repositories:
>
> top.git/
> top.git/refs/{heads,tags,...}/...
> top.git/objects/...
> top.git/modules/sub.git/
> 

Tcl/Tk now has a little 'c' style language..

[Philip Oakley]

The CodeProject news today mentioned the release of the 'L' (little) 
language which integrates with Tcl/Tk and compiles to Tcl/Tk  byte code.


This should be of interest to those looking at the Git Gui and Gitk code, in 
particular the examples showing code simplification (easire to 
read/understand).


The project page is at http://www.little-lang.org, with the original L paper 
(pdf) at http://www.little-lang.org/little.pdf and looks to have some solid 
documentation.


--

Philip 


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

Re: [PATCH v5 4/4] convert.c: ident + core.autocrlf didn't work

[Junio C Hamano]

tbo...@web.de writes:

>   if (ca.drv && (ca.drv->smudge || ca.drv->clean))
> - return filter;
> + return NULL;
>  
>   if (ca.ident)
>   filter = ident_filter(sha1);

We allocated an ident-filter here...

> - crlf_action = ca.crlf_action;
> -
> - if ((crlf_action == CRLF_BINARY) ||
> - crlf_action == CRLF_AUTO_INPUT ||
> - (crlf_action == CRLF_TEXT_INPUT))
> - filter = cascade_filter(filter, _filter_singleton);
> -
> - else if (output_eol(crlf_action) == EOL_CRLF &&
> -  !(crlf_action == CRLF_AUTO || crlf_action == CRLF_AUTO_CRLF))
> + if (output_eol(ca.crlf_action) == EOL_CRLF) {
> + if (ca.crlf_action == CRLF_AUTO || ca.crlf_action == 
> CRLF_AUTO_CRLF)
> + return NULL;

and then by returning NULL, we lost it.

>   filter = cascade_filter(filter, lf_to_crlf_filter());
> + } else
> + filter = cascade_filter(filter, _filter_singleton);
>  
>   return filter;
>  }
--
To unsubscribe from this list: send the line "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 0/5] fix deadlock in git-push

[Jeff King]

On Wed, Apr 20, 2016 at 02:17:16PM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > The first patch below fixes the deadlock. Unfortunately, it turns it
> > into a likely SIGPIPE death. Which is an improvement, but not ideal.
> >
> > Patches 2 and 3 address that by fixing the way we handle SIGPIPE in
> > async threads.
> >
> > Patches 4 and 5 are cleanups to earlier topics that are enabled by the
> > new SIGPIPE handling.
> >
> >   [1/5]: send-pack: close demux pipe before finishing async process
> >   [2/5]: run-command: teach async threads to ignore SIGPIPE
> >   [3/5]: send-pack: isolate sigpipe in demuxer thread
> >   [4/5]: fetch-pack: isolate sigpipe in demuxer thread
> >   [5/5]: t5504: drop sigpipe=ok from push tests
> 
> Thanks for a very well explained series.
> 
> We do not call finish_async (rather, we do not use async) from that
> many places, and from a cursory look this codepath is the only case
> where we may encounter this kind of deadlock (the ones in
> receive-pack is about relaying the error messages back to the other
> end over sideband multiplexing)?

Yeah, I checked the other demuxer in fetch-pack, but it does not have
any early returns like this (it just dies :) ).

It does not do an explicit close on demux.out, but I think it is
effectively closed when we hand it off to index-pack/unpack-objects via
cmd.in.

Arguably finish_async() should "close(demux.out)" itself, but that felt
like an ownership violation. Yes, that's how "struct async" passes out
the descriptor, but the caller is then expected to handle it, and
correct callers will typically have closed it themselves, handed it off
to a sub-process, etc. Closing it in finish_async() runs the risk that
we just call close() on a descriptor number that is either unattached,
or attached to some random other thing.

-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: git rebase -i without altering the committer date

[Andreas Schwab]

Shaun Jackman  writes:

> I'd like to insert a commit between two commits without changing the 
> committer date or author date of that commit or the subsequent commits. I'd 
> planned on using `git rebase -i` to insert the commit. I believe it retains 
> the author date, but changes the committer date to the current time. I've 
> seen the options `--committer-date-is-author-date` and `--ignore-date`, but I 
> don't believe either of those options does what I want. If no such option 
> currently exists to leave the committer and author date unchanged, is there 
> any chance that this functionality could please be implemented?

The easiest way to implement that is to add a graft to redirect the
parent of the second commit to the inserted commit, then use git
filter-branch to make the graft permanent.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "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: problems serving non-bare repos with submodules over http

[Junio C Hamano]

Stefan Beller  writes:

>> I may be missing the subtleties, but if you are serving others from
>> a non-bare repository with submodules, I do not think you would want
>> to expose the in-tree version of the submodule in the first place.
>
> Well I would imagine that is the exact point.
> If I was not trying to expose my state, I could ask you to
> obtain your copy from $(git remote get-url origin) just as I did.

That wasn't what I had in mind, but if the cloner cloned from your
repository with a working tree, the cloner would discover submodules
you use from your .gitmodules file, which would record the location
you cloned them from, so something like that may come into the
picture.  What I had in mind was more like this one you mentioned
below:

> $GIT_DIR_SUPER_PROJECT/modules/$MODULE_NAME
> ...
> Right instead of cloning $WORKTREE/sub/.git you rather want
> $GITDIR/module/sub

> So currently the protocol doesn't allow to even specify the submodules
> directories.

Depends on what you exactly mean by "the protocol", but the
networking protocol is about accessing a single repository.  It is
up to you to decide where to go next after learning what you can
learn from the result, typically by following what appears in
the .gitmodules file.

The only special case is when .gitmodules file records the URL in a
relative form, I would think.  Traditionally (i.e. when it was
considered sane to clone only from bare repositories) I think people
expected a layout like this:

top.git/
top.git/refs/{heads,tags,...}/...
top.git/objects/...
top.git/sub.git/
top.git/sub.git/refs/{heads,tags,...}/...
top.git/sub.git/objects/...

and refer to ./sub.git from .gitmodules recorded in top.git.  It
still would be norm for common distribution sites (i.e. the original
place Yaroslav likely has cloned things from) to be bare, and with
or without $GIT_DIR/modules/, the relative path of submodule seen
by its superproject would (have to) be different between a bare and
a non-bare repository.

I'd imagine that people could agree on a common layout like this
even for a forest of bare repositories:

top.git/
top.git/refs/{heads,tags,...}/...
top.git/objects/...
top.git/modules/sub.git/
top.git/modules/sub.git/refs/{heads,tags,...}/...
top.git/modules/sub.git/objects/...

which would probably make the "relative" relationship between the
supermodule and its submodules the same between bare and non-bare
repositories, but I didn't think it too deeply.
--
To unsubscribe from this list: send the line "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-rebase--merge: don't include absent parent as a base

[Ben Woosley]

It's helpful in the case where a bit of code has been detached from
its history by way of copying the files and starting a new repo, where
development continues. If you want to reunite the new history with the
prior history, while preferring the new history, you need to `rebase
-Xtheirs` the new branch onto the old.

Best,
Ben

On Wed, Apr 20, 2016 at 2:13 PM, Junio C Hamano  wrote:
> Ben Woosley  writes:
>
>> From: Ben Woosley 
>>
>> Absent this fix, attempts to rebase an orphan branch with --strategy 
>> recursive
>> will fail with:
>>
>> $ git rebase ORPHAN_TARGET_BASE -s recursive
>> First, rewinding head to replay your work on top of it...
>> fatal: Could not parse object 'ORPHAN_ROOT_SHA^'
>> Unknown exit code (128) from command: git-merge-recursive 
>> ORPHAN_ROOT_SHA^ -- HEAD ORPHAN_ROOT_SHA
>>
>> To fix, this will only include the rebase root's parent as a base if it 
>> exists,
>> so that in cases of rebasing an orphan branch, it is a simple two-way merge.
>>
>> Note the default rebase behavior does not fail:
>>
>> $ git rebase ORPHAN_TARGET_BASE
>> First, rewinding head to replay your work on top of it...
>> Applying: ORPHAN_ROOT_COMMIT_MSG
>> Using index info to reconstruct a base tree...
>>
>> Signed-off-by: Ben Woosley 
>> ---
>>  git-rebase--merge.sh| 4 +++-
>>  t/t3402-rebase-merge.sh | 9 +
>>  2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
>> index 2cc2a6d..8d43db9 100644
>> --- a/git-rebase--merge.sh
>> +++ b/git-rebase--merge.sh
>> @@ -67,7 +67,9 @@ call_merge () {
>>   GIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY
>>   fi
>>   test -z "$strategy" && strategy=recursive
>> - eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'
>> + # If cmt doesn't have a parent, don't include it as a base
>> + base=$(git rev-parse --verify --quiet $cmt^)
>> + eval 'git-merge-$strategy' $strategy_opts $base ' -- "$hd" "$cmt"'
>
> Makes sense to me.  It is not clear if such a merge without common
> ancestor is all that useful, but as it is mechanically possible,
> I do not see a reason to forbid it.
>
>>   rv=$?
>>   case "$rv" in
>>   0)
>> diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh
>> index 8f64505..488945e 100755
>> --- a/t/t3402-rebase-merge.sh
>> +++ b/t/t3402-rebase-merge.sh
>> @@ -85,6 +85,15 @@ test_expect_success 'rebase -Xtheirs' '
>>   ! grep 11 original
>>  '
>>
>> +test_expect_success 'rebase -Xtheirs from orphan' '
>> + git checkout --orphan orphan-conflicting master~2 &&
>> + echo "AB $T" >> original &&
>> + git commit -morphan-conflicting original &&
>> + git rebase -Xtheirs master &&
>> + grep AB original &&
>> + ! grep 11 original
>> +'
>> +
>>  test_expect_success 'merge and rebase should match' '
>>   git diff-tree -r test-rebase test-merge >difference &&
>>   if test -s difference
>>
>> --
>> https://github.com/git/git/pull/228
--
To unsubscribe from this list: send the line "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 rebase -i without altering the committer date

[Shaun Jackman]

On April 20, 2016 at 13:37:01, Junio C Hamano 
(gits...@pobox.com(mailto:gits...@pobox.com)) wrote:
> Shaun Jackman writes: 
> 
> > I'd like to insert a commit between two commits without changing 
> > the committer date or author date of that commit or the subsequent 
> > commits. I'd planned on using `git rebase -i` to insert the 
> > commit. I believe it retains the author date, but changes the 
> > committer date to the current time. I've seen the options 
> > `--committer-date-is-author-date` and `--ignore-date`, but I don't 
> > believe either of those options does what I want. If no such 
> > option currently exists to leave the committer and author date 
> > unchanged, is there any chance that this functionality could 
> > please be implemented? 
> 
> You can mark the commit as "edit", use "git commit --amend" when 
> "rebase -i" stops and gives control back to you, and say "rebase 
> --continue". That way, you can use your favourite trick to lie 
> about committer date (or identity or other aspects) when running 
> "git commit --amend" and its effect will be left in the resulting 
> history, I would think. 

Thanks for the suggestion, Junio. That would retain the committer date for the 
commit being inserted. I believe that the subsequent commits would have their 
committer date modified to the current time by the `git rebase --continue`.

Cheers,
Shaun



--
To unsubscribe from this list: send the line "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 0/5] fix deadlock in git-push

[Junio C Hamano]

Jeff King  writes:

> The first patch below fixes the deadlock. Unfortunately, it turns it
> into a likely SIGPIPE death. Which is an improvement, but not ideal.
>
> Patches 2 and 3 address that by fixing the way we handle SIGPIPE in
> async threads.
>
> Patches 4 and 5 are cleanups to earlier topics that are enabled by the
> new SIGPIPE handling.
>
>   [1/5]: send-pack: close demux pipe before finishing async process
>   [2/5]: run-command: teach async threads to ignore SIGPIPE
>   [3/5]: send-pack: isolate sigpipe in demuxer thread
>   [4/5]: fetch-pack: isolate sigpipe in demuxer thread
>   [5/5]: t5504: drop sigpipe=ok from push tests

Thanks for a very well explained series.

We do not call finish_async (rather, we do not use async) from that
many places, and from a cursory look this codepath is the only case
where we may encounter this kind of deadlock (the ones in
receive-pack is about relaying the error messages back to the other
end over sideband multiplexing)?

--
To unsubscribe from this list: send the line "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-rebase--merge: don't include absent parent as a base

[Junio C Hamano]

Ben Woosley  writes:

> From: Ben Woosley 
>
> Absent this fix, attempts to rebase an orphan branch with --strategy recursive
> will fail with:
>
> $ git rebase ORPHAN_TARGET_BASE -s recursive
> First, rewinding head to replay your work on top of it...
> fatal: Could not parse object 'ORPHAN_ROOT_SHA^'
> Unknown exit code (128) from command: git-merge-recursive 
> ORPHAN_ROOT_SHA^ -- HEAD ORPHAN_ROOT_SHA
>
> To fix, this will only include the rebase root's parent as a base if it 
> exists,
> so that in cases of rebasing an orphan branch, it is a simple two-way merge.
>
> Note the default rebase behavior does not fail:
>
> $ git rebase ORPHAN_TARGET_BASE
> First, rewinding head to replay your work on top of it...
> Applying: ORPHAN_ROOT_COMMIT_MSG
> Using index info to reconstruct a base tree...
>
> Signed-off-by: Ben Woosley 
> ---
>  git-rebase--merge.sh| 4 +++-
>  t/t3402-rebase-merge.sh | 9 +
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
> index 2cc2a6d..8d43db9 100644
> --- a/git-rebase--merge.sh
> +++ b/git-rebase--merge.sh
> @@ -67,7 +67,9 @@ call_merge () {
>   GIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY
>   fi
>   test -z "$strategy" && strategy=recursive
> - eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'
> + # If cmt doesn't have a parent, don't include it as a base
> + base=$(git rev-parse --verify --quiet $cmt^)
> + eval 'git-merge-$strategy' $strategy_opts $base ' -- "$hd" "$cmt"'

Makes sense to me.  It is not clear if such a merge without common
ancestor is all that useful, but as it is mechanically possible,
I do not see a reason to forbid it.

>   rv=$?
>   case "$rv" in
>   0)
> diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh
> index 8f64505..488945e 100755
> --- a/t/t3402-rebase-merge.sh
> +++ b/t/t3402-rebase-merge.sh
> @@ -85,6 +85,15 @@ test_expect_success 'rebase -Xtheirs' '
>   ! grep 11 original
>  '
>  
> +test_expect_success 'rebase -Xtheirs from orphan' '
> + git checkout --orphan orphan-conflicting master~2 &&
> + echo "AB $T" >> original &&
> + git commit -morphan-conflicting original &&
> + git rebase -Xtheirs master &&
> + grep AB original &&
> + ! grep 11 original
> +'
> +
>  test_expect_success 'merge and rebase should match' '
>   git diff-tree -r test-rebase test-merge >difference &&
>   if test -s difference
>
> --
> https://github.com/git/git/pull/228
--
To unsubscribe from this list: send the line "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] Update git-p4 to be compatible with git-lfs 1.2

[Ben Woosley]

Thanks! Some thoughts:

* One option on the Travis front would be to just test one combination
of the 1.1 build - e.g. linux + clang + 1.1, so you'll stay within the
5 parallel builds while also having some coverage on lfs 1.1.
* It might be risky to match on contentFile when looking for the
prefix - there could be expansions or other modifications applied by
git-lfs between input and output. I would think it a bit safer to just
match on the beginning of the line.
* Have you guys thought about splitting out git-p4? It seems like a
good candidate for an extension / add-on, and would remove the soft
git-lfs dependency.

Best,
Ben

On Wed, Apr 20, 2016 at 12:36 PM, Lars Schneider
 wrote:
>
>> On Wed, Apr 20, 2016 at 12:00 PM, Luke Diamand  wrote:
>>> On 20 April 2016 at 19:28, Ben Woosley  wrote:
 From: Ben Woosley 

 The git lfs pointer output was changed in:
 https://github.com/github/git-lfs/pull/1105

 This was causing Mac Travis runs to fail, as homebrew had updated to 1.2
 while Linux was pinned at 1.1 via GIT_LFS_VERSION.

 The travis builds against 1.1 and 1.2 both on linux. Mac can't do the same 
 as
 it takes the latest homebrew version regardless.
>>>
>>> Is this related to the very similar thread going on here:
>>>
>>> http://thread.gmane.org/gmane.comp.version-control.git/291917/focus=291926
>>>
>>> Thanks
>>> Luke
>
>
> On 20 Apr 2016, at 21:13, Ben Woosley  wrote:
>
>> Yep it's addressing the same problem - I developed this independently
>> after having only viewed the github repo:
>> https://github.com/git/git/pull/231
>>
>> Things I like about my patch:
>> 1) it maintains ongoing support for git-lfs 1.1 by retaining it in the 
>> travis CI
>> 2) it's a fairly minimal intervention into the existing behavior
>>
>> Lars how about adding my Travis changes to your patch?
>> Here's a look at the CI output: 
>> https://travis-ci.org/git/git/builds/124105972
>>
>> Best,
>> Ben
>
>
> Thanks a lot for your fix! It's great to see other people than
> me actually using this feature :)
>
> I already sent a v2 with LFS 1.x support, too:
> http://article.gmane.org/gmane.comp.version-control.git/291993
> Would you mind reviewing it, too?
> Sorry for the duplicated work :-(
>
> Your Travis changes are 100% correct and a very nice way to ensure we
> support Git LFS 1.1 and Git LFS 1.2. Unfortunately we run all other Git
> tests twice which increases the overall build time a lot (because we
> can only run 5 build jobs in parallel with the free Travis CI plan).
> I am not sure if testing an outdated LFS version justifies the increased
> build times...
>
> Best,
> Lars
>
> PS: Please see Junio's comment regarding top posting:
> http://article.gmane.org/gmane.comp.version-control.git/291932
>
>>>
>>>
>>>
 ---
 .travis.yml | 9 -
 git-p4.py   | 7 ++-
 2 files changed, 14 insertions(+), 2 deletions(-)

 diff --git a/.travis.yml b/.travis.yml
 index 78e433b..71510ee 100644
 --- a/.travis.yml
 +++ b/.travis.yml
 @@ -23,7 +23,6 @@ env:
   global:
 - DEVELOPER=1
 - P4_VERSION="15.2"
 -- GIT_LFS_VERSION="1.1.0"
 - DEFAULT_TEST_TARGET=prove
 - GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
 - GIT_TEST_OPTS="--verbose --tee"
 @@ -31,6 +30,14 @@ env:
 # t9810 occasionally fails on Travis CI OS X
 # t9816 occasionally fails with "TAP out of sequence errors" on Travis 
 CI OS X
 - GIT_SKIP_TESTS="t9810 t9816"
 +  matrix:
 +- GIT_LFS_VERSION="1.2.0"
 +- GIT_LFS_VERSION="1.1.0"
 +
 +matrix:
 +  exclude:
 +- os: osx
 +  env: GIT_LFS_VERSION="1.1.0"

 before_install:
   - >
 diff --git a/git-p4.py b/git-p4.py
 index 527d44b..6c06d17 100755
 --- a/git-p4.py
 +++ b/git-p4.py
 @@ -1064,7 +1064,12 @@ def generatePointer(self, contentFile):
 if pointerProcess.wait():
 os.remove(contentFile)
 die('git-lfs pointer command failed. Did you install the 
 extension?')
 -pointerContents = [i+'\n' for i in 
 pointerFile.split('\n')[2:][:-1]]
 +pointerLines = pointerFile.split('\n')
 +# In git-lfs < 1.2, the pointer output included some extraneous 
 information
 +# this was removed in https://github.com/github/git-lfs/pull/1105
 +if pointerLines[0].startswith('Git LFS pointer for'):
 +pointerLines = pointerLines[2:]
 +pointerContents = [i+'\n' for i in pointerLines[:-1]]
 oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
 localLargeFile = os.path.join(
 os.getcwd(),

 --
 https://github.com/git/git/pull/231
 --
 To unsubscribe from this list: send the line 

Re: problems serving non-bare repos with submodules over http

[Stefan Beller]

On Wed, Apr 20, 2016 at 12:51 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>>> 1. After cloning
>>>
>>> git clone http://localhost:8080/.git
>>>
>>>I cannot 'submodule update' the sub1 in the clone since its url after
>>>'submodule init' would be  http://localhost:8080/.git/sub1 .  If I 
>>> manually fix
>>>it up -- it seems to proceed normally since in original repository I have
>>>sub1/.git/ directory and not the "gitlink" for that submodule.
>>
>> So the expected URL would be  http://localhost:8080/sub1/.git ?
>>
>> I thought you could leave out the .git prefix, i.e. you can type
>>
>>  git clone http://localhost:8080
>>
>> and Git will recognize the missing .git and try that as well. The relative 
>> URL
>> would then be constructed as http://localhost:8080/sub1, which will use the
>> same mechanism to find the missing .git ending.
>
> I may be missing the subtleties, but if you are serving others from
> a non-bare repository with submodules, I do not think you would want
> to expose the in-tree version of the submodule in the first place.

Well I would imagine that is the exact point.
If I was not trying to expose my state, I could ask you to
obtain your copy from $(git remote get-url origin) just as I did.

I would imagine, if I have a problem with some repo I can tell my
coworker or others to get my copy to took into that exact state.
(Or I want to transfer state from workstation to laptop to
continue working)

Without submodules this workflow works. So I'd expect it
to work with submodules as well eventually. Also we probably don't
want to mix cloning the superproject from this non bare repo and
the generic submodule locations as the superproject may have
advanced submodule pointers to commits which are not present
in the generic submodule remotes.

So for the non-bare case I would really expect to be able to "copy"
the remote including submodules from that remote?

We could reason about only providing this for the superproject though
and not for submodules, i.e. cloning from the non bare submodule
could be not supported. (If you really want that non bare submodule,
you can still clone it manually from

$GIT_DIR_SUPER_PROJECT/modules/$MODULE_NAME



>
> These $submodule/.git files point via "gitdir:" to their real
> repository location, don't they?

Yes they do.

> And I would think that they are
> what you would want to expose to the outside world.  Your in-tree
> submodules may come and go as you checkout different branches in
> your working tree, but these copies at their real locations will
> stay.

Right instead of cloning $WORKTREE/sub/.git you rather want
$GITDIR/module/sub

(GITDIR and WORKTREE from the superprojects point of view)

The problem with a copy of a superproject including submodules is
the way cloning submodules work.

  1) clone the superproject
  2) for each gitlink in the tree, consult the .gitmodules file
  3) if we have a match in the .gitmodules file, clone from there

So currently the protocol doesn't allow to even specify the submodules
directories. In case the remote superproject is non bare in 1) the remote
would need to advertise the submodule repository URLS separately,
such that the cloning can be performed from those direct copies.


Thanks,
Stefan

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

Re: [PATCH v2 2/2] git-p4: fix Git LFS pointer parsing

[Junio C Hamano]

larsxschnei...@gmail.com writes:

> -pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
> -oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
> +
> +# Git LFS removed the preamble in the output of the 'pointer' command
> +# starting from version 1.2.0. Check for the preamble here to support
> +# earlier versions.
> +# c.f. 
> https://github.com/github/git-lfs/commit/da2935d9a739592bc775c98d8ef4df9c72ea3b43
> +preamble = 'Git LFS pointer for ' + contentFile + '\n\n'
> +if pointerFile.startswith(preamble):
> +pointerFile = pointerFile[len(preamble):]
> +
> +oidEntry = [i for i in pointerFile.split('\n') if 
> i.startswith('oid')]
> +oid = oidEntry[0].split(' ')[1].split(':')[1]
>  localLargeFile = os.path.join(
>  os.getcwd(),
>  '.git', 'lfs', 'objects', oid[:2], oid[2:4],
> @@ -1073,7 +1082,7 @@ class GitLFS(LargeFileSystem):
>  )
>  # LFS Spec states that pointer files should not have the executable 
> bit set.
>  gitMode = '100644'
> -return (gitMode, pointerContents, localLargeFile)
> +return (gitMode, pointerFile, localLargeFile)

It seems to me that you used to return pointerContents which is an
array of lines (each of which are LF terminated); the updated one
returns pointerFile which is a bare string with many lines.

Is that change intentional?  Does the difference matter to the
caller of this method?  Even if it doesn't, is it a good idea to
change it as part of this commit?

>  def pushFile(self, localLargeFile):
>  uploadProcess = subprocess.Popen(
> diff --git a/t/t9824-git-p4-git-lfs.sh b/t/t9824-git-p4-git-lfs.sh
> index 0b664a3..ca93ac8 100755
> --- a/t/t9824-git-p4-git-lfs.sh
> +++ b/t/t9824-git-p4-git-lfs.sh
> @@ -13,6 +13,10 @@ test_file_in_lfs () {
>   FILE="$1" &&
>   SIZE="$2" &&
>   EXPECTED_CONTENT="$3" &&
> + sed -n '1,1 p' "$FILE" | grep "^version " &&
> + sed -n '2,2 p' "$FILE" | grep "^oid " &&
> + sed -n '3,3 p' "$FILE" | grep "^size " &&
> + test_line_count = 3 "$FILE" &&
>   cat "$FILE" | grep "size $SIZE" &&
>   HASH=$(cat "$FILE" | grep "oid sha256:" | sed -e "s/oid sha256://g") &&
>   LFS_FILE=".git/lfs/objects/$(echo "$HASH" | cut -c1-2)/$(echo "$HASH" | 
> cut -c3-4)/$HASH" &&
--
To unsubscribe from this list: send the line "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/RFC 4/6] transport: add refspec list parameters to functions

[Jeff King]

On Wed, Apr 20, 2016 at 04:46:55PM -0400, David Turner wrote:

> As you note, it appears that git-daemon does sort-of have support for
> extra args -- see parse_host_arg.  So it wouldn't be hard to add
> something here. Unfortunately, current versions of git die on unknown
> args.  So this change would not be backwards-compatible.  We could put
> a decider on it so that clients would only try it when explicitly
> enabled.  Or we could have clients try it with, and in the event of an
> error, retry without.  Neither is ideal, but both are possible.

Right. This ends up being the same difficulty that the v2 protocol
encountered; how do you figure out what you can speak without resorting
to expensive fallbacks, when do you flip the switch, do you remember the
protocol you used last time with this server, etc.

Which isn't to say it's necessarily a bad thing. Maybe the path forward
instead of v2 is to shoe-horn this data into the pre-protocol
conversation, and go from there. The protocol accepts that "somehow" it
got some extra data from the transport layer, and acts on its uniformly.

> If I read this code correctly, git-over-ssh will pass through arbitrary
> arguments.  So this should be trivial.

It does if you are ssh-ing to a real shell-level account on the server,
but if you are using git-shell or some other wrapper to restrict clients
from running arbitrary commands, it will likely reject it.

-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/RFC 4/6] transport: add refspec list parameters to functions

[David Turner]

On Tue, 2016-04-19 at 21:17 -0400, Jeff King wrote:
> On Tue, Apr 19, 2016 at 07:43:11PM -0400, David Turner wrote:
> 
> > On Tue, 2016-04-19 at 19:22 -0400, Jeff King wrote:
> > > You can find previous discussion on the list, but I think the
> > > options
> > > basically are:
> > > 
> > >   1. Something like v2, where the client gets a chance to speak
> > > before
> > >  the advertisement.
> > > 
> > >   2. Some out-of-band way of getting values from the client to
> > > the
> > >  server (so maybe extra command-line arguments for git-over
> > > -ssh,
> > > and
> > >  maybe shoving something after the "\0" for git-daemon, and
> > > of
> > >  course extra parameters for HTTP).
> > > 
> > >   3. The client saying "stop spewing refs at me, I want to give
> > > you a
> > >  ref filter" asynchronously, and accepting a little spew at
> > > the
> > >  beginning of each conversation. That obviously only works
> > > for
> > > the
> > >  full-duplex transports, so you'd probably fall back to (2)
> > > for
> > >  http.
> > 
> > OK, so (2) seems like what I'm doing -- it just happens that I only
> > implemented it for one protocol.
> 
> Right. And I don't mind that approach _if_ we can figure out a way to
> do
> it for all protocols. But I think there are some complications with
> the
> other ones, which means that HTTP will have the ability to grow
> features
> the other protocols do not.

As you note, it appears that git-daemon does sort-of have support for
extra args -- see parse_host_arg.  So it wouldn't be hard to add
something here. Unfortunately, current versions of git die on unknown
args.  So this change would not be backwards-compatible.  We could put
a decider on it so that clients would only try it when explicitly
enabled.  Or we could have clients try it with, and in the event of an
error, retry without.  Neither is ideal, but both are possible.

If I read this code correctly, git-over-ssh will pass through arbitrary
arguments.  So this should be trivial.
--
To unsubscribe from this list: send the line "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: [PATCHv3 0/3] Implements the GIT_TRACE_CURL environment variable

[Jeff King]

On Wed, Apr 20, 2016 at 08:49:55PM +0100, Ramsay Jones wrote:

> > Strictly speaking 1 should come at the end for the same reason, as
> > setting GIT_TRACE_CURL after seeing that commit would not give users
> > anything new.
> 
> Yep, I was just about to send an email saying that the patches should
> be in the exact opposite order! (ie. 1->3 and 3->1) That is *if* you
> want to keep them as a series. I would squash them into one patch ...

I also wondered about simply squashing them. IMHO it does not help to
split documentation from the addition of a feature. It is not as if we
will take one over the other, and by putting them in the same patch you
do not have to justify one without the other.

> > Other than that, I didn't find anything blatantly wrong ;-).  Will
> > nitpick individual patches later but I expect that it would be
> > sufficient to locally tweak while queuing without rerolling.
> 
> I have one small issue ... 

Overall I'm pleased at the concept, though I find the output a little
funny in places.

Most of the "Send/Recv SSL data" chunks are just line noise. Do people
actually care about seeing them? I can conceive of a case where you are
debugging SSL-level stuff, but I feel like you might do better using
openssl s_client to do so, and not git. Should we stick to more
HTTP-level debugging?

For the actual data packets, the first line gets treated differently
than the rest, and you get:

16:33:38.164068 http.c:515  => Send header, 000167 bytes 
(0x00a7)
: GET /git/git/info/refs?service=git-upload-pack HTTP/1.1
16:33:38.164070 http.c:515  0039: Host: github.com
16:33:38.164072 http.c:515  004b: User-Agent: git/2.8.1.220.g9816fc6
...

for instance. Would it be saner to break the "Send header..." bit and
the first data line into separate trace outputs, and end up with
something more like:

16:33:38.164068 http.c:515  => Send header, 000167 bytes 
(0x00a7)
16:33:38.164069 http.c:515  : GET 
/git/git/info/refs?service=git-upload-pack HTTP/1.1
16:33:38.164070 http.c:515  0039: Host: github.com
16:33:38.164072 http.c:515  004b: User-Agent: git/2.8.1.220.g9816fc6

Or it might even be nice to prefix each line to indicate it is about
sending a header. That would make it much easier to grep for just
particular 

It might even be nice to prefix _all_ of the lines with some state
information, like "send header". That's more verbose, but makes it much
easier to pick out snippets with line-oriented tools like grep. I often
find myself doing that kind of thing, either to inspect a subset of the
output, or because I want to be able to pull out things like request
content verbatim so I can replay it.

One of my complaints with GIT_CURL_VERBOSE is that it puts your
credentials into the debugging output. Since it looks like we're
parsing through the data anyway, I wonder if we could auto-censor
Authorization headers by default (and then possibly output them if an
extra variable is given). That would make it safe to ask people to show
the output of GIT_CURL_TRACE on the list without having to explain
further.

-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: git rebase -i without altering the committer date

[Junio C Hamano]

Shaun Jackman  writes:

> I'd like to insert a commit between two commits without changing
> the committer date or author date of that commit or the subsequent
> commits. I'd planned on using `git rebase -i` to insert the
> commit. I believe it retains the author date, but changes the
> committer date to the current time. I've seen the options
> `--committer-date-is-author-date` and `--ignore-date`, but I don't
> believe either of those options does what I want. If no such
> option currently exists to leave the committer and author date
> unchanged, is there any chance that this functionality could
> please be implemented?

You can mark the commit as "edit", use "git commit --amend" when
"rebase -i" stops and gives control back to you, and say "rebase
--continue".  That way, you can use your favourite trick to lie
about committer date (or identity or other aspects) when running
"git commit --amend" and its effect will be left in the resulting
history, I would think.
--
To unsubscribe from this list: send the line "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: Is there a "git reset --keep || git reset --hard " alternative?

[Ævar Arnfjörð Bjarmason]

On Wed, Apr 20, 2016 at 9:03 PM, Junio C Hamano  wrote:
> Ævar Arnfjörð Bjarmason  writes:
>
>> $ ls -l INSTALL ; chmod 600 INSTALL ; git reset --hard @{u} ; ls -l 
>> INSTALL
>> -rw-r--r-- 1 avar avar 9147 Apr 20 17:11 INSTALL
>> HEAD is now at e6ac6e1 Fifth batch for post 2.8 cycle
>> -rw-r--r-- 1 avar avar 9147 Apr 20 17:12 INSTALL
>
> A quick question.  What happens when you did this instead?
>
> chmod 600 INSTALL
> git update-index --refresh
> git reset --hard
>
> Does it match what you want to see?
>
> The reason I ask is because I recall making a deliberate design
> decision around this area.

The --hard doesn't wipe away the chmod, makes sense:

$ ls -l INSTALL ; chmod 600 INSTALL ; git update-index --refresh;
git reset --hard @{u} ; ls -l INSTALL
-rw-r--r-- 1 avar avar 9147 Apr 20 22:27 INSTALL
HEAD is now at 05045e0 tracing
-rw--- 1 avar avar 9147 Apr 20 22:27 INSTALL
--
To unsubscribe from this list: send the line "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 rebase -i without altering the committer date

[Shaun Jackman]

I'd like to insert a commit between two commits without changing the committer 
date or author date of that commit or the subsequent commits. I'd planned on 
using `git rebase -i` to insert the commit. I believe it retains the author 
date, but changes the committer date to the current time. I've seen the options 
`--committer-date-is-author-date` and `--ignore-date`, but I don't believe 
either of those options does what I want. If no such option currently exists to 
leave the committer and author date unchanged, is there any chance that this 
functionality could please be implemented?

For a relevant SO question, see 
How to make a git rebase and keep the commit timestamp?
http://stackoverflow.com/questions/30790645/how-to-make-a-git-rebase-and-keep-the-commit-timestamp

Thanks, 
Shaun

-- 
http://sjackman.ca
--
To unsubscribe from this list: send the line "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: [PATCHv3 3/3] http.c: implements the GIT_TRACE_CURL environment variable

[Junio C Hamano]

On Wed, Apr 20, 2016 at 12:56 PM, Ramsay Jones
 wrote:
>>> +void curl_dump(const char *text, unsigned char *ptr, size_t size, char 
>>> nohex)
>>> +{
>>> +size_t i;
>>> +size_t w;
>
> As I said in a previous email, curl_dump() should be marked
> static here (and remove the declaration from http.h). Unless,
> of course, there are plans for future use/calls being added?

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

Re: [PATCH v2 2/2] git-p4: fix Git LFS pointer parsing

[Lars Schneider]

On 20 Apr 2016, at 20:30, Junio C Hamano  wrote:

> Sebastian Schuberth  writes:
> 
>> Why do we need to remove the preamble at all, if present?

Because I need the content of a valid Git LFS pointer file 
a few lines below:
https://github.com/larsxschneider/git/blob/5ee7601c49b6eaa9da5eb47db5cf12b5d8d672c9/git-p4.py#L1085

This pointer file content is then written to the Git repository 
instead of the actual file content.


>> If all we
>> want is the oid, we should simply only look at the line that starts
>> with that keyword, which would skip any preamble. Which is what you
>> already do here. However, I'd probably use .splitlines() instead of
>> .split('\n') and .startswith('oid ') (note the trailing space) instead
>> of .startswith('oid') to ensure "oid" is a separate word.
>> 
>> But then again, I wonder why there's so much split() logic involved in
>> extracting the oid. Couldn't we replace all of that with a regexp like
>> 
>> oid = re.search(r"^oid \w+:(\w+)", pointerFile, re.MULTILINE).group(1)
> 
> Yup, all of that is a very useful suggestion.  If we know how the
> piece of information we want is identified in the output,
> specifically looking for it would future-proof the code better, as
> it will not be affected by future change that adds unexpected cruft
> to the output we are reading from.

I agree that this is a better solution.

@Junio: Can you squash the patch below or do you prefer a v3?

Thanks,
Lars


diff --git a/git-p4.py b/git-p4.py
index ab52bc3..a0d529b 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1073,8 +1073,7 @@ class GitLFS(LargeFileSystem):
 if pointerFile.startswith(preamble):
 pointerFile = pointerFile[len(preamble):]

-oidEntry = [i for i in pointerFile.split('\n') if i.startswith('oid')]
-oid = oidEntry[0].split(' ')[1].split(':')[1]
+oid = re.search(r'^oid \w+:(\w+)', pointerFile, re.MULTILINE).group(1)
 localLargeFile = os.path.join(
 os.getcwd(),
 '.git', 'lfs', 'objects', oid[:2], oid[2: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: [PATCHv3 3/3] http.c: implements the GIT_TRACE_CURL environment variable

[Ramsay Jones]

On 20/04/16 19:53, Junio C Hamano wrote:
> Elia Pinto  writes:
> 
>> Implements the GIT_TRACE_CURL environment variable to allow a
> 
> s/Implements/Implement/; speak as if you are giving an order to the
> codebase to "be like so".
> 
>> greater degree of detail of GIT_CURL_VERBOSE, in particular
>> the complete transport header and all the data payload exchanged.
>> It might be useful if a particular situation could require a more
>> thorough debugging analysis.
>>
>> Helped-by: Torsten Bögershausen 
>> Helped-by: Ramsay Jones 
>> Helped-by: Junio C Hamano 
>> Helped-by: Eric Sunshine 
>> Helped-by: Jeff King 
>> Signed-off-by: Elia Pinto 
>> ---
>>  http.c | 101 
>> -
>>  http.h |   6 
>>  2 files changed, 106 insertions(+), 1 deletion(-)
>>
>> diff --git a/http.c b/http.c
>> index 4304b80..507c386 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -11,6 +11,7 @@
>>  #include "gettext.h"
>>  #include "transport.h"
>>  
>> +static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
>>  #if LIBCURL_VERSION_NUM >= 0x070a08
>>  long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
>>  #else
>> @@ -464,6 +465,100 @@ static void set_curl_keepalive(CURL *c)
>>  }
>>  #endif
>>  
>> +
> 
> No need for this extra blank line.
> 
>> +void curl_dump(const char *text, unsigned char *ptr, size_t size, char 
>> nohex)
>> +{
>> +size_t i;
>> +size_t w;

As I said in a previous email, curl_dump() should be marked
static here (and remove the declaration from http.h). Unless,
of course, there are plans for future use/calls being added?

ATB,
Ramsay Jones

--
To unsubscribe from this list: send the line "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/15] update-index: enable/disable watchman support

[David Turner]

On Wed, 2016-04-20 at 06:45 +0700, Duy Nguyen wrote:
> On Wed, Apr 20, 2016 at 6:28 AM, David Turner <
> dtur...@twopensource.com> wrote:
> > +   if (use_watchman > 0) {
> > +   the_index.last_update= xstrdup("");
> > +   the_index.cache_changed |= WATCHMAN_CHANGED;
> > +   } else if (!use_watchman) {
> > +   the_index.last_update= NULL;
> > +   the_index.cache_changed |= WATCHMAN_CHANGED;
> > +   }
> > +
> 
> We probably should warn people if index-helper is not built with
> watchman support, which makes this knob completely useless. If
> watchman fails to start, that's a separate problem..

Will warn.
--
To unsubscribe from this list: send the line "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: problems serving non-bare repos with submodules over http

[Junio C Hamano]

Stefan Beller  writes:

>> 1. After cloning
>>
>> git clone http://localhost:8080/.git
>>
>>I cannot 'submodule update' the sub1 in the clone since its url after
>>'submodule init' would be  http://localhost:8080/.git/sub1 .  If I 
>> manually fix
>>it up -- it seems to proceed normally since in original repository I have
>>sub1/.git/ directory and not the "gitlink" for that submodule.
>
> So the expected URL would be  http://localhost:8080/sub1/.git ?
>
> I thought you could leave out the .git prefix, i.e. you can type
>
>  git clone http://localhost:8080
>
> and Git will recognize the missing .git and try that as well. The relative URL
> would then be constructed as http://localhost:8080/sub1, which will use the
> same mechanism to find the missing .git ending.

I may be missing the subtleties, but if you are serving others from
a non-bare repository with submodules, I do not think you would want
to expose the in-tree version of the submodule in the first place.

These $submodule/.git files point via "gitdir:" to their real
repository location, don't they?  And I would think that they are
what you would want to expose to the outside world.  Your in-tree
submodules may come and go as you checkout different branches in
your working tree, but these copies at their real locations will
stay.


--
To unsubscribe from this list: send the line "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: [PATCHv3 0/3] Implements the GIT_TRACE_CURL environment variable

[Ramsay Jones]

On 20/04/16 19:41, Junio C Hamano wrote:
> Elia Pinto  writes:
> 
>> Elia Pinto (3):
>>   git.txt: document the new GIT_TRACE_CURL environment variable
>>   imap-send.c: introduce the GIT_TRACE_CURL enviroment variable
>>   http.c: implements the GIT_TRACE_CURL environment variable
> 
> I think 2 & 3 need to be swapped; otherwise 2 would refer to
> yet-to-be-invented curl_trace() function, and break the build
> without 3, no?
> 
> Strictly speaking 1 should come at the end for the same reason, as
> setting GIT_TRACE_CURL after seeing that commit would not give users
> anything new.

Yep, I was just about to send an email saying that the patches should
be in the exact opposite order! (ie. 1->3 and 3->1) That is *if* you
want to keep them as a series. I would squash them into one patch ...

> Other than that, I didn't find anything blatantly wrong ;-).  Will
> nitpick individual patches later but I expect that it would be
> sufficient to locally tweak while queuing without rerolling.

I have one small issue ... 

ATB,
Ramsay Jones


--
To unsubscribe from this list: send the line "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: problems serving non-bare repos with submodules over http

[Yaroslav Halchenko]

On Wed, 20 Apr 2016, Stefan Beller wrote:
> > I do realize that the situation is quite uncommon, partially I guess due
> > to git submodules mechanism flexibility and power on one hand and
> > under-use (imho) on the other, which leads to discovery of regressions
> > [e.g. 1] and corner cases as mine.

> Thanks for fixing the under-use and reporting bugs. :)

I am thrilled to help ;)

> > [1] http://thread.gmane.org/gmane.comp.version-control.git/288064
> > [2] http://www.onerussian.com/tmp/git-web-submodules.sh

> > My use case:  We are trying to serve a git repository with submodules
> > specified with relative paths over http from a simple web server.  With a 
> > demo
> > case and submodule specification [complete script to reproduce including the
> > webserver using python is at 2] such as

> > (git)hopa:/tmp/gitxxmsxYFO[master]git
> > $> tree
> > .
> > ├── f1
> > └── sub1
> > └── f2

> > $> cat .gitmodules
> > [submodule "sub1"]
> > path = sub1
> > url = ./sub1


> > 1. After cloning

> > git clone http://localhost:8080/.git

> >I cannot 'submodule update' the sub1 in the clone since its url after
> >'submodule init' would be  http://localhost:8080/.git/sub1 .  If I 
> > manually fix
> >it up -- it seems to proceed normally since in original repository I have
> >sub1/.git/ directory and not the "gitlink" for that submodule.

> So the expected URL would be  http://localhost:8080/sub1/.git ?

ATM, yes

> I thought you could leave out the .git prefix, i.e. you can type

>  git clone http://localhost:8080

> and Git will recognize the missing .git and try that as well. The relative URL
> would then be constructed as http://localhost:8080/sub1, which will use the
> same mechanism to find the missing .git ending.

[note1] Unfortunately it is not the case ATM (git version
2.8.1.369.geae769a, output is interspersed with log from the python's simple
http server):

$> git clone http://localhost:8080 xxx   
Cloning into 'xxx'... 
127.0.0.1 - - [20/Apr/2016 15:01:25] code 404, message File not found
127.0.0.1 - - [20/Apr/2016 15:01:25] "GET /info/refs?service=git-upload-pack 
HTTP/1.1" 404 -
fatal: repository 'http://localhost:8080/' not found


> > 2. If I serve the clone [2 demos that too] itself, there is no easy remedy 
> > at
> >all since sub1/.git is not a directory but a gitlink.

> Not sure I understand the second question.

If I serve via http a repository where sub1/.git is a "gitlink":

(git)hopa:/tmp/gitxxmsxYFO_[master]
$> cat sub1/.git 
gitdir: ../.git/modules/sub1

Such repository cannot be cloned:

(git)hopa:/tmp/gitxxmsxYFO_[master]git
$> git clone http://localhost:8080/sub1 /tmp/xxx
Cloning into '/tmp/xxx'...  
127.0.0.1 - - [20/Apr/2016 15:04:01] code 404, message File not found
127.0.0.1 - - [20/Apr/2016 15:04:01] "GET 
/sub1/info/refs?service=git-upload-pack HTTP/1.1" 404 -
fatal: repository 'http://localhost:8080/sub1/' not found

$> git clone http://localhost:8080/sub1/.git /tmp/xxx 
Cloning into '/tmp/xxx'...
127.0.0.1 - - [20/Apr/2016 15:04:06] code 404, message File not found
127.0.0.1 - - [20/Apr/2016 15:04:06] "GET 
/sub1/.git/info/refs?service=git-upload-pack HTTP/1.1" 404 -
fatal: repository 'http://localhost:8080/sub1/.git/' not found


> > N.B. I haven't approached nested submodules case yet in [2]

> > I wondered

> > a. could 'git clone' (probably actually some relevant helper used by fetch
> >etc) acquire ability to sense for URL/.git if URL itself doesn't point 
> > to a
> >usable git repository?

> So you mean in case of relative submodules, we need to take the parent
> url, and remove the ".git" at the end and try again if we cannot find
> the submodule?

that would be the a.2 which I have forgotten to outline ;)

in a.  I was suggesting what you have assumed [note 1 above] would be
happening (but doesn't) ATM: that /.git would be automagically sensed.

> > I think this could provide complete remedy for 1 since then relative 
> > urls
> > would be properly assembled, with similar 'sensing' for /.git for the 
> > final urls

> > I guess we could do it with rewrites/forwards on the "server side",
> > but it wouldn't be generally acceptable solution.

> > b. is there a better or already existing way to remedy my situation?

> > c. shouldn't "git clone" (or the relevant helper) be aware of remote
> >/.git possibly being a gitlink file within submodule?

> Oh. I think that non-bare repositories including submodules are not designed
> to be cloned, because they are for use in the file system.

Well -- that is the beauty of git being a distributed VCS, that non-bare repos
seems to be as nicely cloneable as bare ones. And in general it seems to work
with submodules as well, since they should be the "consistent"
philosophically... 

>  Even a local clone fails:

> # gerrit is a project I know which also has 

Re: [PATCH v5 02/15] read-cache: allow to keep mmap'd memory after reading

[David Turner]

On Wed, 2016-04-20 at 16:26 +0700, Duy Nguyen wrote:
> On Wed, Apr 20, 2016 at 6:27 AM, David Turner <
> dtur...@twopensource.com> wrote:
> > From: Nguyễn Thái Ngọc Duy 
> > 
> > Later, we will introduce git index-helper to share this memory with
> > other git processes.
> > 
> > Since the memory will be shared, it will never be unmapped
> > (although
> > the kernel may of course choose to page it out).
> 
> This is not entirely true. We do need to keep the mmap'd memory for
> longer, even after read_index_from() finishes. But we do not share
> the
> exact same address space to other processes (memcpy is used in
> index-helper.c:share_index()). We could even discard_index() at the
> end of share_index(), but I think we keep it anyway just in case
> another process asks, or when index is not updated.

Will reword.
--
To unsubscribe from this list: send the line "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 02/15] read-cache: allow to keep mmap'd memory after reading

[David Turner]

On Wed, 2016-04-20 at 11:01 +0200, Johannes Schindelin wrote:
> Hi Dave,
> 
> On Tue, 19 Apr 2016, David Turner wrote:
> 
> >  unmap:
> > +   istate->mmap = NULL;
> > munmap(mmap, mmap_size);
> > die("index file corrupt");
> >  }
> > [...]
> > @@ -1698,6 +1705,10 @@ int discard_index(struct index_state
> > *istate)
> > free(istate->cache);
> > istate->cache = NULL;
> > istate->cache_alloc = 0;
> > +   if (istate->keep_mmap && istate->mmap) {
> > +   munmap(istate->mmap, istate->mmap_size);
> > +   istate->mmap = NULL;
> > +   }
> > discard_split_index(istate);
> 
> Just curious: any reason why the first hunk munmap()s after resetting
> the
> field to NULL and the second hunk does it in the opposite order?

No idea.  Will change the first hunk.
--
To unsubscribe from this list: send the line "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/15] index-helper: new daemon for caching index and related stuff

[David Turner]

On Wed, 2016-04-20 at 14:17 +0200, Johannes Schindelin wrote:
> Hi Dave,
> 
> (apologies in advance if I may bring up anything that has been
> discussed
> in earlier iterations; I simply was too busy with the rebase--helper
> project to even look.)
> 
> On Tue, 19 Apr 2016, David Turner wrote:
> 
> > Shared memory is done by storing files in a per-repository
> > temporary
> > directory.  This is more portable than shm (which requires
> > posix-realtime and has various quirks on OS X).  It might even work
> > on
> > Windows, although this has not been tested. The shared memory
> > file's
> > name folows the template "shm--" where  is the
> 
> s/folows/follows/

Will fix, thanks.

> And: now that it is no longer shared memory, should we not do away
> with
> the "shm-" prefix?

Hm.  It's intended to be shared via MAP_SHARED.  So it is just a "disk
-backed" shared memory object instead of a POSIX shm object.

> > +   if (*new_mmap == MAP_FAILED) {
> > +   *new_mmap = NULL;
> > +   goto done;
> 
> Shouldn't we provide some sort of error message here?

We generally try to fail silently on index-helper failures -- the user
isn't necessarily expecting output at that point, and we can always
fall back to directly reading the index.

On the other hand, this error should pretty much never happen, so maybe
it's worth calling out?
--
To unsubscribe from this list: send the line "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] Update git-p4 to be compatible with git-lfs 1.2

[Lars Schneider]

> On Wed, Apr 20, 2016 at 12:00 PM, Luke Diamand  wrote:
>> On 20 April 2016 at 19:28, Ben Woosley  wrote:
>>> From: Ben Woosley 
>>> 
>>> The git lfs pointer output was changed in:
>>> https://github.com/github/git-lfs/pull/1105
>>> 
>>> This was causing Mac Travis runs to fail, as homebrew had updated to 1.2
>>> while Linux was pinned at 1.1 via GIT_LFS_VERSION.
>>> 
>>> The travis builds against 1.1 and 1.2 both on linux. Mac can't do the same 
>>> as
>>> it takes the latest homebrew version regardless.
>> 
>> Is this related to the very similar thread going on here:
>> 
>> http://thread.gmane.org/gmane.comp.version-control.git/291917/focus=291926
>> 
>> Thanks
>> Luke


On 20 Apr 2016, at 21:13, Ben Woosley  wrote:

> Yep it's addressing the same problem - I developed this independently
> after having only viewed the github repo:
> https://github.com/git/git/pull/231
> 
> Things I like about my patch:
> 1) it maintains ongoing support for git-lfs 1.1 by retaining it in the travis 
> CI
> 2) it's a fairly minimal intervention into the existing behavior
> 
> Lars how about adding my Travis changes to your patch?
> Here's a look at the CI output: https://travis-ci.org/git/git/builds/124105972
> 
> Best,
> Ben


Thanks a lot for your fix! It's great to see other people than
me actually using this feature :)

I already sent a v2 with LFS 1.x support, too:
http://article.gmane.org/gmane.comp.version-control.git/291993
Would you mind reviewing it, too?
Sorry for the duplicated work :-(

Your Travis changes are 100% correct and a very nice way to ensure we 
support Git LFS 1.1 and Git LFS 1.2. Unfortunately we run all other Git
tests twice which increases the overall build time a lot (because we
can only run 5 build jobs in parallel with the free Travis CI plan).
I am not sure if testing an outdated LFS version justifies the increased
build times...

Best,
Lars

PS: Please see Junio's comment regarding top posting:
http://article.gmane.org/gmane.comp.version-control.git/291932

>> 
>> 
>> 
>>> ---
>>> .travis.yml | 9 -
>>> git-p4.py   | 7 ++-
>>> 2 files changed, 14 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/.travis.yml b/.travis.yml
>>> index 78e433b..71510ee 100644
>>> --- a/.travis.yml
>>> +++ b/.travis.yml
>>> @@ -23,7 +23,6 @@ env:
>>>   global:
>>> - DEVELOPER=1
>>> - P4_VERSION="15.2"
>>> -- GIT_LFS_VERSION="1.1.0"
>>> - DEFAULT_TEST_TARGET=prove
>>> - GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
>>> - GIT_TEST_OPTS="--verbose --tee"
>>> @@ -31,6 +30,14 @@ env:
>>> # t9810 occasionally fails on Travis CI OS X
>>> # t9816 occasionally fails with "TAP out of sequence errors" on Travis 
>>> CI OS X
>>> - GIT_SKIP_TESTS="t9810 t9816"
>>> +  matrix:
>>> +- GIT_LFS_VERSION="1.2.0"
>>> +- GIT_LFS_VERSION="1.1.0"
>>> +
>>> +matrix:
>>> +  exclude:
>>> +- os: osx
>>> +  env: GIT_LFS_VERSION="1.1.0"
>>> 
>>> before_install:
>>>   - >
>>> diff --git a/git-p4.py b/git-p4.py
>>> index 527d44b..6c06d17 100755
>>> --- a/git-p4.py
>>> +++ b/git-p4.py
>>> @@ -1064,7 +1064,12 @@ def generatePointer(self, contentFile):
>>> if pointerProcess.wait():
>>> os.remove(contentFile)
>>> die('git-lfs pointer command failed. Did you install the 
>>> extension?')
>>> -pointerContents = [i+'\n' for i in 
>>> pointerFile.split('\n')[2:][:-1]]
>>> +pointerLines = pointerFile.split('\n')
>>> +# In git-lfs < 1.2, the pointer output included some extraneous 
>>> information
>>> +# this was removed in https://github.com/github/git-lfs/pull/1105
>>> +if pointerLines[0].startswith('Git LFS pointer for'):
>>> +pointerLines = pointerLines[2:]
>>> +pointerContents = [i+'\n' for i in pointerLines[:-1]]
>>> oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
>>> localLargeFile = os.path.join(
>>> os.getcwd(),
>>> 
>>> --
>>> https://github.com/git/git/pull/231
>>> --
>>> To unsubscribe from this list: send the line "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] Update git-p4 to be compatible with git-lfs 1.2

[Ben Woosley]

Yep it's addressing the same problem - I developed this independently
after having only viewed the github repo:
https://github.com/git/git/pull/231

Things I like about my patch:
1) it maintains ongoing support for git-lfs 1.1 by retaining it in the travis CI
2) it's a fairly minimal intervention into the existing behavior

Lars how about adding my Travis changes to your patch?
Here's a look at the CI output: https://travis-ci.org/git/git/builds/124105972

Best,
Ben

On Wed, Apr 20, 2016 at 12:00 PM, Luke Diamand  wrote:
> On 20 April 2016 at 19:28, Ben Woosley  wrote:
>> From: Ben Woosley 
>>
>> The git lfs pointer output was changed in:
>> https://github.com/github/git-lfs/pull/1105
>>
>> This was causing Mac Travis runs to fail, as homebrew had updated to 1.2
>> while Linux was pinned at 1.1 via GIT_LFS_VERSION.
>>
>> The travis builds against 1.1 and 1.2 both on linux. Mac can't do the same as
>> it takes the latest homebrew version regardless.
>
> Is this related to the very similar thread going on here:
>
> http://thread.gmane.org/gmane.comp.version-control.git/291917/focus=291926
>
> Thanks
> Luke
>
>
>
>> ---
>>  .travis.yml | 9 -
>>  git-p4.py   | 7 ++-
>>  2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/.travis.yml b/.travis.yml
>> index 78e433b..71510ee 100644
>> --- a/.travis.yml
>> +++ b/.travis.yml
>> @@ -23,7 +23,6 @@ env:
>>global:
>>  - DEVELOPER=1
>>  - P4_VERSION="15.2"
>> -- GIT_LFS_VERSION="1.1.0"
>>  - DEFAULT_TEST_TARGET=prove
>>  - GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
>>  - GIT_TEST_OPTS="--verbose --tee"
>> @@ -31,6 +30,14 @@ env:
>>  # t9810 occasionally fails on Travis CI OS X
>>  # t9816 occasionally fails with "TAP out of sequence errors" on Travis 
>> CI OS X
>>  - GIT_SKIP_TESTS="t9810 t9816"
>> +  matrix:
>> +- GIT_LFS_VERSION="1.2.0"
>> +- GIT_LFS_VERSION="1.1.0"
>> +
>> +matrix:
>> +  exclude:
>> +- os: osx
>> +  env: GIT_LFS_VERSION="1.1.0"
>>
>>  before_install:
>>- >
>> diff --git a/git-p4.py b/git-p4.py
>> index 527d44b..6c06d17 100755
>> --- a/git-p4.py
>> +++ b/git-p4.py
>> @@ -1064,7 +1064,12 @@ def generatePointer(self, contentFile):
>>  if pointerProcess.wait():
>>  os.remove(contentFile)
>>  die('git-lfs pointer command failed. Did you install the 
>> extension?')
>> -pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
>> +pointerLines = pointerFile.split('\n')
>> +# In git-lfs < 1.2, the pointer output included some extraneous 
>> information
>> +# this was removed in https://github.com/github/git-lfs/pull/1105
>> +if pointerLines[0].startswith('Git LFS pointer for'):
>> +pointerLines = pointerLines[2:]
>> +pointerContents = [i+'\n' for i in pointerLines[:-1]]
>>  oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
>>  localLargeFile = os.path.join(
>>  os.getcwd(),
>>
>> --
>> https://github.com/git/git/pull/231
>> --
>> To unsubscribe from this list: send the line "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: Is there a "git reset --keep || git reset --hard " alternative?

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  writes:

> $ ls -l INSTALL ; chmod 600 INSTALL ; git reset --hard @{u} ; ls -l 
> INSTALL
> -rw-r--r-- 1 avar avar 9147 Apr 20 17:11 INSTALL
> HEAD is now at e6ac6e1 Fifth batch for post 2.8 cycle
> -rw-r--r-- 1 avar avar 9147 Apr 20 17:12 INSTALL

A quick question.  What happens when you did this instead?

chmod 600 INSTALL
git update-index --refresh
git reset --hard

Does it match what you want to see?

The reason I ask is because I recall making a deliberate design
decision around this area.

--
To unsubscribe from this list: send the line "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] Update git-p4 to be compatible with git-lfs 1.2

[Luke Diamand]

On 20 April 2016 at 19:28, Ben Woosley  wrote:
> From: Ben Woosley 
>
> The git lfs pointer output was changed in:
> https://github.com/github/git-lfs/pull/1105
>
> This was causing Mac Travis runs to fail, as homebrew had updated to 1.2
> while Linux was pinned at 1.1 via GIT_LFS_VERSION.
>
> The travis builds against 1.1 and 1.2 both on linux. Mac can't do the same as
> it takes the latest homebrew version regardless.

Is this related to the very similar thread going on here:

http://thread.gmane.org/gmane.comp.version-control.git/291917/focus=291926

Thanks
Luke



> ---
>  .travis.yml | 9 -
>  git-p4.py   | 7 ++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 78e433b..71510ee 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -23,7 +23,6 @@ env:
>global:
>  - DEVELOPER=1
>  - P4_VERSION="15.2"
> -- GIT_LFS_VERSION="1.1.0"
>  - DEFAULT_TEST_TARGET=prove
>  - GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
>  - GIT_TEST_OPTS="--verbose --tee"
> @@ -31,6 +30,14 @@ env:
>  # t9810 occasionally fails on Travis CI OS X
>  # t9816 occasionally fails with "TAP out of sequence errors" on Travis 
> CI OS X
>  - GIT_SKIP_TESTS="t9810 t9816"
> +  matrix:
> +- GIT_LFS_VERSION="1.2.0"
> +- GIT_LFS_VERSION="1.1.0"
> +
> +matrix:
> +  exclude:
> +- os: osx
> +  env: GIT_LFS_VERSION="1.1.0"
>
>  before_install:
>- >
> diff --git a/git-p4.py b/git-p4.py
> index 527d44b..6c06d17 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1064,7 +1064,12 @@ def generatePointer(self, contentFile):
>  if pointerProcess.wait():
>  os.remove(contentFile)
>  die('git-lfs pointer command failed. Did you install the 
> extension?')
> -pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
> +pointerLines = pointerFile.split('\n')
> +# In git-lfs < 1.2, the pointer output included some extraneous 
> information
> +# this was removed in https://github.com/github/git-lfs/pull/1105
> +if pointerLines[0].startswith('Git LFS pointer for'):
> +pointerLines = pointerLines[2:]
> +pointerContents = [i+'\n' for i in pointerLines[:-1]]
>  oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
>  localLargeFile = os.path.join(
>  os.getcwd(),
>
> --
> https://github.com/git/git/pull/231
> --
> To unsubscribe from this list: send the line "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] Extend runtime prefix computation

[Michael Weiser]

Hi,

On Fri, Apr 15, 2016 at 09:43:29AM -0700, Junio C Hamano wrote:

> We tend to avoid system specific includes in individual *.c files.

> Perhaps implement platform specific bits in compat/?  E.g. each
> platform specific file in compat/ may implement and export the same
> git_extract_argv_path() function, perhaps, so that this file does
> not even need to know what platforms it supports?

On Mon, Apr 18, 2016 at 09:20:25AM +0200, Johannes Schindelin wrote:

> I have to admit that I am really, *really* skeptical. To me, it looks like
> this patch opens the door very wide to unintended consequences.

To make it short: I had received interest from another user for
relocatable git installations and an updated patch against current git
(based on my last post of the code to this list in 2012). This
confirmed to me that the use case is still valid. However, I do not see
myself in a position to iterate the code through the review and
improvement process that starting to unfold now. So if someone sometime
picks this up and carries on, I'd be happy. Otherwise please disregard.
-- 
Thanks,
Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 3/3] http.c: implements the GIT_TRACE_CURL environment variable

[Junio C Hamano]

Elia Pinto  writes:

> Implements the GIT_TRACE_CURL environment variable to allow a

s/Implements/Implement/; speak as if you are giving an order to the
codebase to "be like so".

> greater degree of detail of GIT_CURL_VERBOSE, in particular
> the complete transport header and all the data payload exchanged.
> It might be useful if a particular situation could require a more
> thorough debugging analysis.
>
> Helped-by: Torsten Bögershausen 
> Helped-by: Ramsay Jones 
> Helped-by: Junio C Hamano 
> Helped-by: Eric Sunshine 
> Helped-by: Jeff King 
> Signed-off-by: Elia Pinto 
> ---
>  http.c | 101 
> -
>  http.h |   6 
>  2 files changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/http.c b/http.c
> index 4304b80..507c386 100644
> --- a/http.c
> +++ b/http.c
> @@ -11,6 +11,7 @@
>  #include "gettext.h"
>  #include "transport.h"
>  
> +static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
>  #if LIBCURL_VERSION_NUM >= 0x070a08
>  long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
>  #else
> @@ -464,6 +465,100 @@ static void set_curl_keepalive(CURL *c)
>  }
>  #endif
>  
> +

No need for this extra blank line.

> +void curl_dump(const char *text, unsigned char *ptr, size_t size, char nohex)
> +{
> + size_t i;
> + size_t w;

Is it better to narrow the scope of 'w' to the "for (i)" loop?

> + struct strbuf out = STRBUF_INIT;
> +

No need for this extra blank line.

> + unsigned int width = 0x10;
> +
> + if (nohex)
> + /* without the hex output, we can fit more on screen */
> + width = 0x40;
> +
> + strbuf_addf(, "%s, %10.10ld bytes (0x%8.8lx)\n",
> + text, (long)size, (long)size);

Inconsistent indentation that uses only two HT, when existing
lines and other new lines in thsi patch align with HT with SP
to match "(" on the previous line.

> +
> + for (i = 0; i < size; i += width) {
> +
> + strbuf_addf(, "%4.4lx: ", (long)i);
> +
> + if (!nohex) {
> + /* hex not disabled, show it */

Unlike the previous "without the hex, we can fit more", this comment
is probably adds more noise than value.

> + for (w = 0; w < width; w++)
> + if (i + w < size)
> + strbuf_addf(, "%02x ", ptr[i + w]);
> + else
> + strbuf_addf(,"   ");
> + }
> +
> + for (w = 0; (w < width) && (i + w < size); w++) {
> + /* check for 0D0A; if found, skip past and start a new 
> line of output */
> + if (nohex && (i + w + 1 < size) && ptr[i + w] == '\r'
> + && ptr[i + w + 1] == '\n') {
> + i += (w + 2 - width);
> + break;
> + }
> + strbuf_addch(, (ptr[i + w] >= 0x20)
> + && (ptr[i + w] < 0x80) ? ptr[i + w] : '.');

Likewise.

> + /* check again for 0D0A, to avoid an extra \n if it's 
> at width */
> + if (nohex && (i + w + 2 < size)
> + && ptr[i + w + 1] == '\r'
> + && ptr[i + w + 2] == '\n') {
> + i += (w + 3 - width);
> + break;
> + }
> + }

This may only be the matter of taste, but I somehow found this "we
pretend to go width bytes at a time, unless there is a line-break in
which case we may fold before we hit width bytes" and need for
compensating with "w+3-width" here unnecessarily convoluted.  I
wonder if the code becomes clearer if insterad you said "we go one
line at a time, but we may fold the line if it is wider than width
bytes"?

> + strbuf_addch(, '\n');
> + trace_strbuf(_curl, );
> + strbuf_release();
> + }
> +}
> +
> +int curl_trace_want(void)
> +{
> + return trace_want(_curl);
> +}
> +
> +int curl_trace(CURL *handle, curl_infotype type,
> +  char *data, size_t size, void *userp)

Inconsistent indentation.

> +{
> + const char *text;
> + (void)handle;   /* prevent compiler warning */

What compiler warning?  Usually unused parameter (not unused
variable) is not something compilers warn against.

> + switch (type) {
> + case CURLINFO_TEXT:
> + trace_printf_key(_curl, "== Info: %s", data);
> + default:/* in case a new one is introduced to shock us 
> */

The comment bothers me.

What is the longer term plan for this function?  Do we expect to
ignore some type of data, or do we expect to show all new type of
data?  If the former, "we ignore unknown types by default" 

Re: [PATCHv3 0/3] Implements the GIT_TRACE_CURL environment variable

[Junio C Hamano]

Elia Pinto  writes:

> Elia Pinto (3):
>   git.txt: document the new GIT_TRACE_CURL environment variable
>   imap-send.c: introduce the GIT_TRACE_CURL enviroment variable
>   http.c: implements the GIT_TRACE_CURL environment variable

I think 2 & 3 need to be swapped; otherwise 2 would refer to
yet-to-be-invented curl_trace() function, and break the build
without 3, no?

Strictly speaking 1 should come at the end for the same reason, as
setting GIT_TRACE_CURL after seeing that commit would not give users
anything new.

Other than that, I didn't find anything blatantly wrong ;-).  Will
nitpick individual patches later but I expect that it would be
sufficient to locally tweak while queuing without rerolling.

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

Re: [PATCH v2 2/2] git-p4: fix Git LFS pointer parsing

[Junio C Hamano]

Sebastian Schuberth  writes:

> Why do we need to remove the preamble at all, if present? If all we
> want is the oid, we should simply only look at the line that starts
> with that keyword, which would skip any preamble. Which is what you
> already do here. However, I'd probably use .splitlines() instead of
> .split('\n') and .startswith('oid ') (note the trailing space) instead
> of .startswith('oid') to ensure "oid" is a separate word.
>
> But then again, I wonder why there's so much split() logic involved in
> extracting the oid. Couldn't we replace all of that with a regexp like
>
> oid = re.search(r"^oid \w+:(\w+)", pointerFile, re.MULTILINE).group(1)

Yup, all of that is a very useful suggestion.  If we know how the
piece of information we want is identified in the output,
specifically looking for it would future-proof the code better, as
it will not be affected by future change that adds unexpected cruft
to the output we are reading from.

--
To unsubscribe from this list: send the line "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] Update git-p4 to be compatible with git-lfs 1.2

[Ben Woosley]

From: Ben Woosley 

The git lfs pointer output was changed in:
https://github.com/github/git-lfs/pull/1105

This was causing Mac Travis runs to fail, as homebrew had updated to 1.2
while Linux was pinned at 1.1 via GIT_LFS_VERSION.

The travis builds against 1.1 and 1.2 both on linux. Mac can't do the same as
it takes the latest homebrew version regardless.
---
 .travis.yml | 9 -
 git-p4.py   | 7 ++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 78e433b..71510ee 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -23,7 +23,6 @@ env:
   global:
 - DEVELOPER=1
 - P4_VERSION="15.2"
-- GIT_LFS_VERSION="1.1.0"
 - DEFAULT_TEST_TARGET=prove
 - GIT_PROVE_OPTS="--timer --jobs 3 --state=failed,slow,save"
 - GIT_TEST_OPTS="--verbose --tee"
@@ -31,6 +30,14 @@ env:
 # t9810 occasionally fails on Travis CI OS X
 # t9816 occasionally fails with "TAP out of sequence errors" on Travis CI 
OS X
 - GIT_SKIP_TESTS="t9810 t9816"
+  matrix:
+- GIT_LFS_VERSION="1.2.0"
+- GIT_LFS_VERSION="1.1.0"
+
+matrix:
+  exclude:
+- os: osx
+  env: GIT_LFS_VERSION="1.1.0"
 
 before_install:
   - >
diff --git a/git-p4.py b/git-p4.py
index 527d44b..6c06d17 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1064,7 +1064,12 @@ def generatePointer(self, contentFile):
 if pointerProcess.wait():
 os.remove(contentFile)
 die('git-lfs pointer command failed. Did you install the 
extension?')
-pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
+pointerLines = pointerFile.split('\n')
+# In git-lfs < 1.2, the pointer output included some extraneous 
information
+# this was removed in https://github.com/github/git-lfs/pull/1105
+if pointerLines[0].startswith('Git LFS pointer for'):
+pointerLines = pointerLines[2:]
+pointerContents = [i+'\n' for i in pointerLines[:-1]]
 oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
 localLargeFile = os.path.join(
 os.getcwd(),

--
https://github.com/git/git/pull/231
--
To unsubscribe from this list: send the line "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] git-rebase--merge: don't include absent parent as a base

[Ben Woosley]

From: Ben Woosley 

Absent this fix, attempts to rebase an orphan branch with --strategy recursive
will fail with:

$ git rebase ORPHAN_TARGET_BASE -s recursive
First, rewinding head to replay your work on top of it...
fatal: Could not parse object 'ORPHAN_ROOT_SHA^'
Unknown exit code (128) from command: git-merge-recursive ORPHAN_ROOT_SHA^ 
-- HEAD ORPHAN_ROOT_SHA

To fix, this will only include the rebase root's parent as a base if it exists,
so that in cases of rebasing an orphan branch, it is a simple two-way merge.

Note the default rebase behavior does not fail:

$ git rebase ORPHAN_TARGET_BASE
First, rewinding head to replay your work on top of it...
Applying: ORPHAN_ROOT_COMMIT_MSG
Using index info to reconstruct a base tree...

Signed-off-by: Ben Woosley 
---
 git-rebase--merge.sh| 4 +++-
 t/t3402-rebase-merge.sh | 9 +
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 2cc2a6d..8d43db9 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -67,7 +67,9 @@ call_merge () {
GIT_MERGE_VERBOSITY=1 && export GIT_MERGE_VERBOSITY
fi
test -z "$strategy" && strategy=recursive
-   eval 'git-merge-$strategy' $strategy_opts '"$cmt^" -- "$hd" "$cmt"'
+   # If cmt doesn't have a parent, don't include it as a base
+   base=$(git rev-parse --verify --quiet $cmt^)
+   eval 'git-merge-$strategy' $strategy_opts $base ' -- "$hd" "$cmt"'
rv=$?
case "$rv" in
0)
diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh
index 8f64505..488945e 100755
--- a/t/t3402-rebase-merge.sh
+++ b/t/t3402-rebase-merge.sh
@@ -85,6 +85,15 @@ test_expect_success 'rebase -Xtheirs' '
! grep 11 original
 '
 
+test_expect_success 'rebase -Xtheirs from orphan' '
+   git checkout --orphan orphan-conflicting master~2 &&
+   echo "AB $T" >> original &&
+   git commit -morphan-conflicting original &&
+   git rebase -Xtheirs master &&
+   grep AB original &&
+   ! grep 11 original
+'
+
 test_expect_success 'merge and rebase should match' '
git diff-tree -r test-rebase test-merge >difference &&
if test -s difference

--
https://github.com/git/git/pull/228
--
To unsubscribe from this list: send the line "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/12] path.c: add git_common_path() and strbuf_git_common_path()

[Eric Sunshine]

On Wed, Apr 20, 2016 at 9:24 AM, Nguyễn Thái Ngọc Duy  wrote:
> diff --git a/path.c b/path.c
> @@ -503,6 +503,35 @@ void strbuf_git_path_submodule(struct strbuf *buf, const 
> char *path,
> +const char *git_common_path(const char *fmt, ...)
> +{
> +   struct strbuf *pathname = get_pathname();
> +   va_list args;
> +   va_start(args, fmt);
> +   do_git_common_path(pathname, fmt, args);
> +   va_end(args);
> +   return pathname->buf;

Is the caller expected to free this value? If not, then shouldn't
'pathname' be static? If so, then perhaps strbuf_detach() would be
clearer (and return 'char *' rather than 'const char *').

> +}
--
To unsubscribe from this list: send the line "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 00/12] fix checking out a being-rebased branch

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> Much happier with this version. This makes
>
>  - git checkout refuse if a branch is under rebase or bisect
>elsewhere
>
>  - git worktree add refuse if a branch is under rebase or bisect
>
>  - git branch -D refuse if a branch is under rebase or bisect.
>This applies for single worktree case as well.
>
>  - git branch -M refuse if a branch is under rebase or bisect
>(single worktree case as well)

I agree that this reads much nicer than v1, especially with the
abstraction around find_shared_symref() that returns the actual
worktree.

>   [01/12] path.c: add git_common_path() and strbuf_git_common_path()
>   [02/12] worktree.c: store "id" instead of "git_dir"
>   [03/12] worktree.c: make find_shared_symref() return struct worktree *
>   [04/12] worktree.c: mark current worktree
>   [05/12] path.c: refactor and add worktree_git_path()
>   [06/12] wt-status.c: split rebase detection out of wt_status_get_state()
>   [07/12] wt-status.c: make wt_status_check_rebase() work on any worktree
>   [08/12] worktree.c: avoid referencing to worktrees[i] multiple times
>   [09/12] worktree.c: test if branch being rebased in another worktree
>   [10/12] wt-status.c: split bisect detection out of wt_status_get_state()
>   [11/12] worktree.c: test if branch being bisected in another worktree
>   [12/12] branch: do not rename a branch under bisect or rebase
>
> Total 13 files changed, 335 insertions(+), 67 deletions(-)
--
To unsubscribe from this list: send the line "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 09/12] worktree.c: test if branch being rebased in another worktree

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> Subject: Re: [PATCH v2 09/12] worktree.c: test if branch being rebased in 
> another worktree

Lacks the verb?  Perhaps s/being/is/ is sufficient.

> This function find_shared_symref() is used in a couple places:
>
> 1) in builtin/branch.c: it's used to detect if a branch is checked out
>elsewhere and refuse to delete the branch.
>
> 2) in builtin/notes.c: it's used to detect if a note is being merged in
>another worktree
>
> 3) in branch.c, the function die_if_checked_out() is actually used by
>"git checkout" and "git worktree add" to see if a branch is already
>checked out elsewhere and refuse the operation.
>
> In cases 1 and 3, if a rebase is ongoing, "HEAD" will be in detached
> mode, find_shared_symref() fails to detect it and declares "no branch is
> checked out here", which is incorrect.

which is technically correct but is not what we want to check.

> This patch tightens the test. If the given symref is "HEAD", we try to
> detect if rebase is ongoing. If so return the branch being rebased. This
> makes checkout and branch delete operations safer because you can't
> checkout a branch being rebased in another place, or delete it.

Is rebase the only thing that tentatively detach before working on
the real branch?  It may be currently the case, but I would imagine
that we want to makefind-shared-symref to be responsible for
detecting new cases other than rebase that we may introduce in the
future, in which case we may want to leave come comment near the
function to describe that expectation.

> Special case for checkout. If the current branch is being rebased,
> git-rebase.sh may use "git checkout" to abort and return back to the
> original branch. The updated test in find_shared_symref() will prevent
> that and "git rebase --abort" will fail as a result.
> find_shared_symref() and die_if_checked_out() have to learn a new
> option ignore_current_worktree to loose the test a bit.

s/loose//

> +void die_if_checked_out(const char *branch, int ignore_current_worktree)
>  {
>   const struct worktree *wt;
>  
> - wt = find_shared_symref("HEAD", branch);
> + wt = find_shared_symref("HEAD", branch, ignore_current_worktree);
>   if (wt) {
>   skip_prefix(branch, "refs/heads/", );
>   die(_("'%s' is already checked out at '%s'"),
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index efcbd8f..6041718 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -,7 +,7 @@ static int checkout_branch(struct checkout_opts *opts,
>   char *head_ref = resolve_refdup("HEAD", 0, sha1, );
>   if (head_ref &&
>   (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
> - die_if_checked_out(new->path);
> + die_if_checked_out(new->path, 1);
>   free(head_ref);
>   }

So the idea is "if the branch is checked out (or "being worked on"
even if technically the HEAD is detached, like with 'rebase')
anywhere, callers of die-if-checked-out in general want to die; but
for this caller, it is OK if the place the branch is checked out or
being worked on is in this repository"?

I understand die_if_checked_out() taking that "ignore this one" bit
may be sensible, but I do not understand why find_shared_symref()
needs to be told about it.  The change makes the meaning of the
find_shared_symref() function unclear.  It used to mean "This
symbolic ref cannot point at the same ref in different worktrees, so
for a given pair of a symbolic ref and a concrete ref, there can be
at most one worktree in which the symbolic ref points at that ref".
That is already a mouthful.  As the worktree structure already have
"Am I the current worktree?" bit, "ignore" logic can easily be done
inside die_if_checked_out() and that would help find_shared_symref()
stay simpler and more focused function, no?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: bug: git submodule add fails when .git is a symlink

[Stefan Beller]

On Wed, Apr 20, 2016 at 9:41 AM, Stefan Beller  wrote:
> On Wed, Mar 2, 2016 at 12:49 AM, Jeff King  wrote:
>> On Tue, Mar 01, 2016 at 07:17:20PM -0400, Joey Hess wrote:
>>
>>> Junio C Hamano wrote:
>>> > A more pertinent question may be which version of Git did the above
>>> > ever work, I guess.  We fairly liberally chdir around and I do not
>>> > think we deliberately avoid assuming that "cd .git && cd .." might
>>> > not come back to the original directory, for example, so I wouldn't
>>> > be surprised if it never worked.
>>>
>>> IIRC git used symlinks for .git in submodules before version 1.7.8, so I
>>> guess that older versions supported that pretty well.
>>>
>>> This one case is the only time I've seen a symlink for .git present a
>>> problem so far.
>>
>> Fortunately you provided a simple reproduction case, so it is easy to
>> bisect. It did work in v1.7.8, and broke in d75219b (submodules: always
>> use a relative path from gitdir to work tree, 2012-03-04). Not
>> surprising, I guess. It presumably worked before only because we were
>> using absolute paths.
>
> So I was looking into this bug again, as it was linked from another bug 
> report.
>
> fatal: Could not chdir to '../../../sub': No such file or directory
>
> sounds like a path issue with the prefix thing.
>
> Using the " echo "gitdir: ../gitdir/.git" > .git" workaround does still work,
> I'll see if there is another way to fix it with actual links.

So I debugged into that using a test case

test_expect_success 'submodules are not confused by linked gitdir' '
git init gitdir &&
mkdir worktree &&
(
cd worktree &&
#echo "gitdir: ../gitdir/.git" >.git &&
ln -s ../gitdir/.git .git &&
test_pause &&
git submodule add ../ sub &&
test_pause
) &&
test_pause &&
git status
'

My observation: the error comes up in `git submodule add`,
which consists of 2 parts: the cloning and then the checkout.
The cloning works fine using the new "submodule--helper update-clone",
the checkout however breaks. So in your original test case you can go
into the submodule and run

git checkout -f -q

and you'll get the error message

fatal: Could not chdir to '../../../sub': No such file or directory

Looking at sub/.git:

$ cat .git
gitdir: ../.git/modules/sub
$ ls ../.git/modules/sub
HEAD  branches config description  hooks  info  logs objects
packed-refs  ref

so that gitlink of the submodule works just fine. The problem is `git checkout`
or any other core command (I tested with `git status`) doesn't like
the gitlink pointing to a directory which is symlinked.

I think the issue is a wrongly configured "core.worktree"
in gitdir/.git/modules/sub/config which contains the "../../../sub".

So I think the fix needs to be in the vicinity of
builtin/submodule--helper.c:module_clone
near the end of the function, where core.worktree is set.

Thanks,
Stefan


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

Is there a "git reset --keep || git reset --hard " alternative?

[Ævar Arnfjörð Bjarmason]

If you check out a git repository and chmod a checked-in file there
there away from git defaults then "git reset --hard" will re-chmod it.

The use-case for not having this happen is if you e.g. have some
inotify thing or a stat() loop monitoring changes to the files, and
you'd like them to fire on "real" updates, not just updates that were
introduced because something re-chmoded a file.

E.g. on current git.git master:

$ ls -l INSTALL ; chmod 600 INSTALL ; git reset --hard @{u} ; ls -l INSTALL
-rw-r--r-- 1 avar avar 9147 Apr 20 17:11 INSTALL
HEAD is now at e6ac6e1 Fifth batch for post 2.8 cycle
-rw-r--r-- 1 avar avar 9147 Apr 20 17:12 INSTALL

What I'd like is for the permissions not to be altered:

$ ls -l INSTALL ; chmod 600 INSTALL ; git reset --keep @{u} ; ls -l INSTALL
-rw-r--r-- 1 avar avar 9147 Apr 20 17:12 INSTALL
-rw--- 1 avar avar 9147 Apr 20 17:12 INSTALL

But I don't want this to happen:

$ echo "Blah" > INSTALL && git add INSTALL && git commit -m"blah"
[master d29463e] blah
 1 file changed, 1 insertion(+), 223 deletions(-)
 rewrite INSTALL (100%)
$ ls -l INSTALL ; chmod 600 INSTALL ; git reset --keep @{u} ; ls -l INSTALL
-rw--- 1 avar avar 5 Apr 20 17:14 INSTALL
error: Entry 'INSTALL' not uptodate. Cannot merge.
fatal: Could not reset index file to revision '@{u}'.
-rw--- 1 avar avar 5 Apr 20 17:14 INSTALL

Instead I want:

$ ls -l INSTALL ; chmod 600 INSTALL ; git reset --keep @{u} || git
reset --hard @{u}  ; ls -l INSTALL
-rw--- 1 avar avar 5 Apr 20 17:14 INSTALL
error: Entry 'INSTALL' not uptodate. Cannot merge.
fatal: Could not reset index file to revision '@{u}'.
HEAD is now at e6ac6e1 Fifth batch for post 2.8 cycle
-rw-r--r-- 1 avar avar 9147 Apr 20 17:15 INSTALL

And the expectation here is that I'll have something that does a chmod
after the reset happens, which is fine because we had a "real" change,
I just don't want the repo to keep having flip-flopping permissions
because I'd both like:

 * Local chmod to be respected
 * Actual file content changes to be wiped away by reset --hard

Is there another way to do this, or dare I say alternatively maybe we
could use another option to reset making it even more confusing :)
--
To unsubscribe from this list: send the line "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 03/12] worktree.c: make find_shared_symref() return struct worktree *

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> This gives the caller more information and they can answer things like,
> "is it the main worktree" or "is it the current worktree". The latter
> question is needed for the "checkout a rebase branch" case later.

That makes good sense.

> diff --git a/worktree.h b/worktree.h
> index 3198c8d..d71d7ec 100644
> --- a/worktree.h
> +++ b/worktree.h
> @@ -36,9 +36,10 @@ extern void free_worktrees(struct worktree **);
>  /*
>   * Check if a per-worktree symref points to a ref in the main worktree
>   * or any linked worktree, and return the path to the exising worktree
> - * if it is.  Returns NULL if there is no existing ref.  The caller is
> - * responsible for freeing the returned path.
> + * if it is. Returns NULL if there is no existing ref. The result
> + * may be destroyed by the next call.
>   */

To return and keep alive one worktree[] instance (i.e. "existing"),
the code holds onto the entire return value from get_worktrees(), if
I am not misreading it.  Typically you would have only a handful of
worktrees so this may not be an issue, though.

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

Re: bug: git submodule add fails when .git is a symlink

[Stefan Beller]

On Wed, Mar 2, 2016 at 12:49 AM, Jeff King  wrote:
> On Tue, Mar 01, 2016 at 07:17:20PM -0400, Joey Hess wrote:
>
>> Junio C Hamano wrote:
>> > A more pertinent question may be which version of Git did the above
>> > ever work, I guess.  We fairly liberally chdir around and I do not
>> > think we deliberately avoid assuming that "cd .git && cd .." might
>> > not come back to the original directory, for example, so I wouldn't
>> > be surprised if it never worked.
>>
>> IIRC git used symlinks for .git in submodules before version 1.7.8, so I
>> guess that older versions supported that pretty well.
>>
>> This one case is the only time I've seen a symlink for .git present a
>> problem so far.
>
> Fortunately you provided a simple reproduction case, so it is easy to
> bisect. It did work in v1.7.8, and broke in d75219b (submodules: always
> use a relative path from gitdir to work tree, 2012-03-04). Not
> surprising, I guess. It presumably worked before only because we were
> using absolute paths.

So I was looking into this bug again, as it was linked from another bug report.

fatal: Could not chdir to '../../../sub': No such file or directory

sounds like a path issue with the prefix thing.

Using the " echo "gitdir: ../gitdir/.git" > .git" workaround does still work,
I'll see if there is another way to fix it with actual links.


>
> -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
--
To unsubscribe from this list: send the line "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] git-p4: add P4 jobs to git commit message

[Jan Durovec]

> One thing I wondered about is whether this should be enabled by
> default or not. Long-time users of git-p4 might be a bit surprised to
> find their git commits suddenly gaining an extra Job: field.

I thought about that too when but then I realized that there's no
switch for the reverse direction either. I.e. when committing to p4
from git the commit messages are parsed and "Jobs: ..." section is
interpreted regardless of any switch, isn't it?

That's why I decided to keep this behaviour symmetrical. Otherwise I
would have reused the same switch that enables/disables job parsing
in git->p4 direction.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 3/3] http.c: implements the GIT_TRACE_CURL environment variable

[Elia Pinto]

Implements the GIT_TRACE_CURL environment variable to allow a
greater degree of detail of GIT_CURL_VERBOSE, in particular
the complete transport header and all the data payload exchanged.
It might be useful if a particular situation could require a more
thorough debugging analysis.

Helped-by: Torsten Bögershausen 
Helped-by: Ramsay Jones 
Helped-by: Junio C Hamano 
Helped-by: Eric Sunshine 
Helped-by: Jeff King 
Signed-off-by: Elia Pinto 
---
 http.c | 101 -
 http.h |   6 
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/http.c b/http.c
index 4304b80..507c386 100644
--- a/http.c
+++ b/http.c
@@ -11,6 +11,7 @@
 #include "gettext.h"
 #include "transport.h"
 
+static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
 #if LIBCURL_VERSION_NUM >= 0x070a08
 long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
 #else
@@ -464,6 +465,100 @@ static void set_curl_keepalive(CURL *c)
 }
 #endif
 
+
+void curl_dump(const char *text, unsigned char *ptr, size_t size, char nohex)
+{
+   size_t i;
+   size_t w;
+   struct strbuf out = STRBUF_INIT;
+
+   unsigned int width = 0x10;
+
+   if (nohex)
+   /* without the hex output, we can fit more on screen */
+   width = 0x40;
+
+   strbuf_addf(, "%s, %10.10ld bytes (0x%8.8lx)\n",
+   text, (long)size, (long)size);
+
+   for (i = 0; i < size; i += width) {
+
+   strbuf_addf(, "%4.4lx: ", (long)i);
+
+   if (!nohex) {
+   /* hex not disabled, show it */
+   for (w = 0; w < width; w++)
+   if (i + w < size)
+   strbuf_addf(, "%02x ", ptr[i + w]);
+   else
+   strbuf_addf(,"   ");
+   }
+
+   for (w = 0; (w < width) && (i + w < size); w++) {
+   /* check for 0D0A; if found, skip past and start a new 
line of output */
+   if (nohex && (i + w + 1 < size) && ptr[i + w] == '\r'
+   && ptr[i + w + 1] == '\n') {
+   i += (w + 2 - width);
+   break;
+   }
+   strbuf_addch(, (ptr[i + w] >= 0x20)
+   && (ptr[i + w] < 0x80) ? ptr[i + w] : '.');
+   /* check again for 0D0A, to avoid an extra \n if it's 
at width */
+   if (nohex && (i + w + 2 < size)
+   && ptr[i + w + 1] == '\r'
+   && ptr[i + w + 2] == '\n') {
+   i += (w + 3 - width);
+   break;
+   }
+   }
+   strbuf_addch(, '\n');
+   trace_strbuf(_curl, );
+   strbuf_release();
+   }
+}
+
+int curl_trace_want(void)
+{
+   return trace_want(_curl);
+}
+
+int curl_trace(CURL *handle, curl_infotype type,
+char *data, size_t size, void *userp)
+{
+   const char *text;
+   (void)handle;   /* prevent compiler warning */
+
+   switch (type) {
+   case CURLINFO_TEXT:
+   trace_printf_key(_curl, "== Info: %s", data);
+   default:/* in case a new one is introduced to shock us 
*/
+   return 0;
+
+   case CURLINFO_HEADER_OUT:
+   text = "=> Send header";
+   break;
+   case CURLINFO_DATA_OUT:
+   text = "=> Send data";
+   break;
+   case CURLINFO_SSL_DATA_OUT:
+   text = "=> Send SSL data";
+   break;
+   case CURLINFO_HEADER_IN:
+   text = "<= Recv header";
+   break;
+   case CURLINFO_DATA_IN:
+   text = "<= Recv data";
+   break;
+   case CURLINFO_SSL_DATA_IN:
+   text = "<= Recv SSL data";
+   break;
+   }
+
+   curl_dump(text, (unsigned char *)data, size, 1);
+   return 0;
+}
+
+
 static CURL *get_curl_handle(void)
 {
CURL *result = curl_easy_init();
@@ -563,7 +658,11 @@ static CURL *get_curl_handle(void)
"your curl version is too old (>= 7.19.4)");
 #endif
 
-   if (getenv("GIT_CURL_VERBOSE"))
+   if (curl_trace_want()) {
+   curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
+   curl_easy_setopt(result, CURLOPT_DEBUGFUNCTION, curl_trace);
+   curl_easy_setopt(result, CURLOPT_DEBUGDATA, NULL);
+   } else if (getenv("GIT_CURL_VERBOSE"))
curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
 
curl_easy_setopt(result, CURLOPT_USERAGENT,
diff --git a/http.h b/http.h
index 4ef4bbd..e452f6a 100644
--- a/http.h

[PATCHv3 1/3] git.txt: document the new GIT_TRACE_CURL environment variable

[Elia Pinto]

Describe the purpose of the GIT_TRACE_CURL environment variable.

Helped-by: Torsten Bögershausen 
Helped-by: Ramsay Jones 
Helped-by: Junio C Hamano 
Helped-by: Eric Sunshine 
Helped-by: Jeff King 
Signed-off-by: Elia Pinto 
---
 Documentation/git.txt | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 8afe349..958db0f 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -1075,6 +1075,14 @@ of clones and fetches.
cloning of shallow repositories.
See 'GIT_TRACE' for available trace output options.
 
+'GIT_TRACE_CURL'::
+   Enables a curl full trace dump of all incoming and outgoing data,
+   including descriptive information, of the git transport protocol.
+   This is similar to doing curl --trace-ascii on the command line.
+   This option overrides setting the GIT_CURL_VERBOSE environment
+   variable.
+   See 'GIT_TRACE' for available trace output options.
+
 'GIT_LITERAL_PATHSPECS'::
Setting this variable to `1` will cause Git to treat all
pathspecs literally, rather than as glob patterns. For example,
-- 
2.8.1.383.g31b84cc

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

[PATCHv3 0/3] Implements the GIT_TRACE_CURL environment variable

[Elia Pinto]

This is the third version but in reality is the complete rewriting of the 
patches discussed here
(here called V1)

$gmane/290520
$gmane/290521

*Changes from V2
($gmane/291868)

- fix garbage comment in http.c (i am very sorry for that)
- add final '.' to the commit message for imap-send.c and to other commit 
messages
- typofix double ; in http.c
- merge the nice cleanup and code refactoring of Ramsay Jones (Thank you very 
much !!)
- squash the previous commit 2/4

*Changes from V1

- introduced GIT_TRACE_CURL variable with its documentation
- changed the name of the temporary variable "i" in "w" in the helper routine
- used the c escape sequences instead of the hex equivalent
- dropped the previous GIT_DEBUG_CURL env var
- curl_dump and curl_trace factored out to a shared implementation
in http.c

Elia Pinto (3):
  git.txt: document the new GIT_TRACE_CURL environment variable
  imap-send.c: introduce the GIT_TRACE_CURL enviroment variable
  http.c: implements the GIT_TRACE_CURL environment variable

 Documentation/git.txt |   8 
 http.c| 101 +-
 http.h|   6 +++
 imap-send.c   |   6 +++
 4 files changed, 120 insertions(+), 1 deletion(-)

-- 
2.8.1.383.g31b84cc

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

[PATCHv3 2/3] imap-send.c: introduce the GIT_TRACE_CURL enviroment variable

[Elia Pinto]

Permit the use of the GIT_TRACE_CURL environment variable calling
the curl_trace and curl_dump http.c helper routine.

Helped-by: Torsten Bögershausen 
Helped-by: Ramsay Jones 
Helped-by: Junio C Hamano 
Helped-by: Eric Sunshine 
Helped-by: Jeff King 
Signed-off-by: Elia Pinto 
---
 imap-send.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/imap-send.c b/imap-send.c
index 938c691..61c6787 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1444,6 +1444,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc)
if (0 < verbosity || getenv("GIT_CURL_VERBOSE"))
curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
 
+   if (curl_trace_want()) {
+   curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
+   curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION, curl_trace);
+   curl_easy_setopt(curl, CURLOPT_DEBUGDATA, NULL);
+   }
+
return curl;
 }
 
-- 
2.8.1.383.g31b84cc

--
To unsubscribe from this list: send the line "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] xdiff: implement empty line chunk heuristic

[Jeff King]

On Wed, Apr 20, 2016 at 09:09:53AM -0700, Junio C Hamano wrote:

> "Michael S. Tsirkin"  writes:
> 
> > FWIW IIRC what that commit is about is ability to reorder the chunks in
> > a patch without changing patch-id. Not about keeping id stable across
> > git revisions.
> 
> OK, but "reorder the chunks" is not meant to stay to be the _ONLY_
> purpose for an option whose name is a broad "--[un]stable", but
> merely one (and only) possible cause of patch-id instability that
> happened to be noticed as an issue back then and was dealt with that
> commit, no?  In other words, the intent of the "--stable" feature is
> to give a stable ID that is not affected by random end-user settings
> (e.g. diff.orderfile) and if somebody invents a new configurable knob
> in the future, they are supposed to pay attention to the "--stable"
> feature or existing users who do use "--stable" will be broken, no?

I forgot that we added "--stable". Evne if it is not meant to be about
stability across versions, is there any reason _not_ to turn off
this heuristic for --stable (or for patch-ids in general)?

I guess maybe that creates some inconsistency between generating a
patch-id directly, and making one from a diff given on stdin (though I
don't know that we can promise much about the latter in the general
case; we can fix file ordering, but we don't have enough information to
tweak other aspects).

-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

Git gui - Commit message lost when changing from New Commit to Amend Last Commit

[Marcus Nascimento]

Hi,

When changing from New Commit to Amend Last Commit, the commit message
it discarded.
It could be appended to the previous commit message with a place
holder or something similar.

git-gui version 0.20.0.1
git version 2.8.0.rc3

Cheers,
--
Marcus Nascimento
--
To unsubscribe from this list: send the line "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: problems serving non-bare repos with submodules over http

[Stefan Beller]

On Wed, Apr 20, 2016 at 8:22 AM, Yaroslav Halchenko  wrote:
> Dear Git Folks,
>
> I do realize that the situation is quite uncommon, partially I guess due
> to git submodules mechanism flexibility and power on one hand and
> under-use (imho) on the other, which leads to discovery of regressions
> [e.g. 1] and corner cases as mine.

Thanks for fixing the under-use and reporting bugs. :)

>
> [1] http://thread.gmane.org/gmane.comp.version-control.git/288064
> [2] http://www.onerussian.com/tmp/git-web-submodules.sh
>
> My use case:  We are trying to serve a git repository with submodules
> specified with relative paths over http from a simple web server.  With a demo
> case and submodule specification [complete script to reproduce including the
> webserver using python is at 2] such as
>
> (git)hopa:/tmp/gitxxmsxYFO[master]git
> $> tree
> .
> ├── f1
> └── sub1
> └── f2
>
> $> cat .gitmodules
> [submodule "sub1"]
> path = sub1
> url = ./sub1
>
>
> 1. After cloning
>
> git clone http://localhost:8080/.git
>
>I cannot 'submodule update' the sub1 in the clone since its url after
>'submodule init' would be  http://localhost:8080/.git/sub1 .  If I 
> manually fix
>it up -- it seems to proceed normally since in original repository I have
>sub1/.git/ directory and not the "gitlink" for that submodule.

So the expected URL would be  http://localhost:8080/sub1/.git ?

I thought you could leave out the .git prefix, i.e. you can type

 git clone http://localhost:8080

and Git will recognize the missing .git and try that as well. The relative URL
would then be constructed as http://localhost:8080/sub1, which will use the
same mechanism to find the missing .git ending.

>
> 2. If I serve the clone [2 demos that too] itself, there is no easy remedy at
>all since sub1/.git is not a directory but a gitlink.

Not sure I understand the second question.

>
> N.B. I haven't approached nested submodules case yet in [2]
>
> I wondered
>
> a. could 'git clone' (probably actually some relevant helper used by fetch
>etc) acquire ability to sense for URL/.git if URL itself doesn't point to a
>usable git repository?

So you mean in case of relative submodules, we need to take the parent
url, and remove the ".git" at the end and try again if we cannot find
the submodule?

>
> I think this could provide complete remedy for 1 since then relative urls
> would be properly assembled, with similar 'sensing' for /.git for the 
> final urls
>
> I guess we could do it with rewrites/forwards on the "server side",
> but it wouldn't be generally acceptable solution.
>
> b. is there a better or already existing way to remedy my situation?
>
> c. shouldn't "git clone" (or the relevant helper) be aware of remote
>/.git possibly being a gitlink file within submodule?

Oh. I think that non-bare repositories including submodules are not designed
to be cloned, because they are for use in the file system. Even a
local clone fails:

# gerrit is a project I know which also has submodules:
git clone --recurse-submodules https://gerrit.googlesource.com/gerrit g1
git clone --recurse-submodules g1 g2
...
fatal: clone of '...' into submodule path '...' failed

So I think for cloning repositories you want to have each repository
as its own thing (bare or non bare).

The submodule mechanism is just a way to express a relation between
the reositories, it's like composing them together, but by that composition
it breaks the properties of each repository to be easily clonable.

I think we should fix that.

I guess the local clone case is 'easy' as you only need
to handle the link instead of directory thing correctly.

For the case you describe (cloning from a remote, whether it is http or ssh),
we would need to discuss security implications I would assume? It sounds
scary at first to follow a random git link to the outer space of the repository.
(A similar thing is that you cannot have symlinks in a git repository pointing
outside of it, IIRC? At least that was fishy.)

Thanks,
Stefan

>
>
> Thank you in advance for your thoughts and feedback on this case.
>
> P.S. Please maintain the CC list in replies.
> --
> Yaroslav O. Halchenko
> Center for Open Neuroscience http://centerforopenneuroscience.org
> Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
> Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
> WWW:   http://www.linkedin.com/in/yarik
> --
> To unsubscribe from this list: send the line "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] xdiff: implement empty line chunk heuristic

[Junio C Hamano]

"Michael S. Tsirkin"  writes:

> FWIW IIRC what that commit is about is ability to reorder the chunks in
> a patch without changing patch-id. Not about keeping id stable across
> git revisions.

OK, but "reorder the chunks" is not meant to stay to be the _ONLY_
purpose for an option whose name is a broad "--[un]stable", but
merely one (and only) possible cause of patch-id instability that
happened to be noticed as an issue back then and was dealt with that
commit, no?  In other words, the intent of the "--stable" feature is
to give a stable ID that is not affected by random end-user settings
(e.g. diff.orderfile) and if somebody invents a new configurable knob
in the future, they are supposed to pay attention to the "--stable"
feature or existing users who do use "--stable" will be broken, no?

I can still buy "--stable is not about stability across versions of
Git"--it makes our job easier ;-)  I just want to make sure that
"--stable is about stability inside a single version of Git that
patch ID for the same commit will stay the same and unaffected by
random end-user configuration knobs".

Which in turn would mean that we won't have to worry about this
option in patch-id as long as we remove the diff.compactionheuristic
configuration and command line option once the developers are done
experimenting with their heuristics code.

--
To unsubscribe from this list: send the line "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] git-p4: add P4 jobs to git commit message

[Junio C Hamano]

Luke Diamand  writes:

> One thing I wondered about is whether this should be enabled by
> default or not. Long-time users of git-p4 might be a bit surprised to
> find their git commits suddenly gaining an extra Job: field.

Ahh, I didn't even wonder about but that is not because I didn't
think it matters.

Does this change affect reproducibility of importing the history
from P4, doesn't it?  Would that be a problem?

How common is it to have the "extra" Job: thing in the history on P4
side?  If the answer to this question is "on rare occasions and only
when there is a very good reason to have 'jobs' associated with the
changelist", then the 'might be a bit surprised' brought by this
change can probably be explained away as "a fix to a (design) bug
that used to discard crucial information" that (unfortunately) have
to change the resulting Git object names.

--
To unsubscribe from this list: send the line "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 3/4] t0027: test cases for combined attributes

[Torsten Bögershausen]

>> Currently "* text=auto eol=lf" does the same as "* text eol=lf",
>> as the eol attribute overrides "text=auto", this will change in
>> future.
> Will change in future in what way?  In patch 5/4?
>
>
Yes, kind of.
I'm fighting to get the test passed under Windows,
and if this 4/4 could make it into pu, I don't need to
resend all of it.


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

problems serving non-bare repos with submodules over http

[Yaroslav Halchenko]

Dear Git Folks,

I do realize that the situation is quite uncommon, partially I guess due
to git submodules mechanism flexibility and power on one hand and
under-use (imho) on the other, which leads to discovery of regressions
[e.g. 1] and corner cases as mine.

[1] http://thread.gmane.org/gmane.comp.version-control.git/288064
[2] http://www.onerussian.com/tmp/git-web-submodules.sh

My use case:  We are trying to serve a git repository with submodules
specified with relative paths over http from a simple web server.  With a demo
case and submodule specification [complete script to reproduce including the
webserver using python is at 2] such as

(git)hopa:/tmp/gitxxmsxYFO[master]git
$> tree
.
├── f1
└── sub1
└── f2

$> cat .gitmodules
[submodule "sub1"]
path = sub1
url = ./sub1


1. After cloning 

git clone http://localhost:8080/.git

   I cannot 'submodule update' the sub1 in the clone since its url after
   'submodule init' would be  http://localhost:8080/.git/sub1 .  If I manually 
fix
   it up -- it seems to proceed normally since in original repository I have
   sub1/.git/ directory and not the "gitlink" for that submodule.

2. If I serve the clone [2 demos that too] itself, there is no easy remedy at
   all since sub1/.git is not a directory but a gitlink.

N.B. I haven't approached nested submodules case yet in [2]

I wondered

a. could 'git clone' (probably actually some relevant helper used by fetch
   etc) acquire ability to sense for URL/.git if URL itself doesn't point to a
   usable git repository?

I think this could provide complete remedy for 1 since then relative urls
would be properly assembled, with similar 'sensing' for /.git for the final 
urls

I guess we could do it with rewrites/forwards on the "server side",
but it wouldn't be generally acceptable solution.

b. is there a better or already existing way to remedy my situation?

c. shouldn't "git clone" (or the relevant helper) be aware of remote
   /.git possibly being a gitlink file within submodule?


Thank you in advance for your thoughts and feedback on this case.

P.S. Please maintain the CC list in replies.
-- 
Yaroslav O. Halchenko
Center for Open Neuroscience http://centerforopenneuroscience.org
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834   Fax: +1 (603) 646-1419
WWW:   http://www.linkedin.com/in/yarik
--
To unsubscribe from this list: send the line "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 v1 2/2] git-p4: fix Git LFS pointer parsing

[Sebastian Schuberth]

On Wed, Apr 20, 2016 at 5:30 PM, Junio C Hamano  wrote:

>> If clients rely on output targeted at human consumption it's not
>> surprising that these clients need to be adjusted from time to time.
>> What's troubling is not the change to git-lfs, but the very un-generic
>> way git-p4 is implemented.
>
> Sounds like the subcommand they are using is not meant for
> scripting?  What is the kosher way to get at the information they
> can use that is a supported interface for scripters?

The "pointer" subcommand indeed is listed under "Low level commands"
by "git lfs" (without any arguments), and as such it probably can be
considered for scripting use. However, before my fix in [1] the
subcommand was printing both output targeted at humans and output
targeted at scripts to stdout. After my fix, only output targeted at
script goes to stdout, and output targeted at humans goes to stderr.

[1] https://github.com/github/git-lfs/pull/1105

-- 
Sebastian Schuberth
--
To unsubscribe from this list: send the line "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] tag.c: remove extern keyword from function definition

[Santiago Torres]

On Wed, Apr 20, 2016 at 04:34:25PM +0100, Ramsay Jones wrote:
> 
> Signed-off-by: Ramsay Jones 
> ---
> 
> Hi Santiago,
> 
> If you need to re-roll your 'st/verify-tag' branch, could you
> please squash this into the relevant patch (commit c5213b40,
> "verify-tag: move tag verification code to tag.c", 19-04-2016).
> 
> Although not actually an error, it is very unusual for an extern
> keyword to be used on a function definition (it seems to be getting
> quite unusual on a function declaration these days!), which causes
> sparse to issue a warning.

Oh sure! I'm waiting to see if there are more reviews regarding this and
I'll re-roll. It's defintely unecessary to have the extern keyword
there.

Thanks for pointing it out!

-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

[PATCH] tag.c: remove extern keyword from function definition

[Ramsay Jones]

Signed-off-by: Ramsay Jones 
---

Hi Santiago,

If you need to re-roll your 'st/verify-tag' branch, could you
please squash this into the relevant patch (commit c5213b40,
"verify-tag: move tag verification code to tag.c", 19-04-2016).

Although not actually an error, it is very unusual for an extern
keyword to be used on a function definition (it seems to be getting
quite unusual on a function declaration these days!), which causes
sparse to issue a warning.

Thanks!

ATB,
Ramsay Jones

 tag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tag.c b/tag.c
index ace619a..93c6ae6 100644
--- a/tag.c
+++ b/tag.c
@@ -30,7 +30,7 @@ static int run_gpg_verify(const char *buf, unsigned long 
size, unsigned flags)
return ret;
 }
 
-extern int gpg_verify_tag(const unsigned char *sha1,
+int gpg_verify_tag(const unsigned char *sha1,
const char *name_to_report, unsigned flags)
 {
enum object_type type;
-- 
2.8.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 2/2] git-p4: fix Git LFS pointer parsing

[Junio C Hamano]

Sebastian Schuberth  writes:

> If clients rely on output targeted at human consumption it's not
> surprising that these clients need to be adjusted from time to time.
> What's troubling is not the change to git-lfs, but the very un-generic
> way git-p4 is implemented.

Sounds like the subcommand they are using is not meant for
scripting?  What is the kosher way to get at the information they
can use that is a supported interface for scripters?
--
To unsubscribe from this list: send the line "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] replace --edit: respect core.editor

[Junio C Hamano]

Jeff King  writes:

> On Wed, Apr 20, 2016 at 08:38:03AM +0200, Johannes Schindelin wrote:
>
>> We simply need to read the config, is all.
>> 
>> This fixes https://github.com/git-for-windows/git/issues/733
>> 
>> Signed-off-by: Johannes Schindelin 
>
> Looks good to me. Thanks.

Yup, I think the new placement is at the most logical place, just
before parse options.  I looked at other codepaths and I do not see
a reason why they shouldn't read the configuration variables.

Thanks, both.

--
To unsubscribe from this list: send the line "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] replace --edit: respect core.editor

[Junio C Hamano]

Johannes Schindelin  writes:

> Hi Junio,
>
> On Tue, 19 Apr 2016, Junio C Hamano wrote:
>
>> "git blame -L475,6 builtin/replace.c" points at b892bb45 (replace: add
>> --edit option, 2014-04-26) and the commit log message names two people
>> who can review this change, so that is what I am doing here.
>
> D'oh. Sorry.
> Dscho

Heh, no need to be sorry, that is what maintainers do.  If anything,
both of us need to thank them for commenting ;-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[ANNOUNCE] Git Rev News edition 14

[Christian Couder]

Hi everyone,

I'm happy announce that the 14th edition of Git Rev News is now published:

http://git.github.io/rev_news/2016/04/20/edition-14/

Thanks a lot to all the contributors and helpers, especially Johannes Sixt!

Enjoy,
Christian, Thomas and Nicola.
--
To unsubscribe from this list: send the line "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 06/12] wt-status.c: split rebase detection out of wt_status_get_state()

[Duy Nguyen]

On Wed, Apr 20, 2016 at 8:48 PM, Ramsay Jones
 wrote:
>
>
> On 20/04/16 14:24, Nguyễn Thái Ngọc Duy wrote:
>> worktree.c:find_shared_symref() later needs to know if a branch is being
>> rebased, and only rebased only. Split this code so it can be used
> ^
> Err ... what?

wt_status_get_state() detects more than rebase, it does bisect,
cherry-pick, detached head as well. I "only" too much there though.
-- 
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 v2 06/12] wt-status.c: split rebase detection out of wt_status_get_state()

[Ramsay Jones]

On 20/04/16 14:24, Nguyễn Thái Ngọc Duy wrote:
> worktree.c:find_shared_symref() later needs to know if a branch is being
> rebased, and only rebased only. Split this code so it can be used
^
Err ... what?

ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "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 03/12] worktree.c: make find_shared_symref() return struct worktree *

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

This gives the caller more information and they can answer things like,
"is it the main worktree" or "is it the current worktree". The latter
question is needed for the "checkout a rebase branch" case later.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 branch.c |  9 +
 builtin/branch.c |  8 
 builtin/notes.c  |  8 
 worktree.c   | 14 +-
 worktree.h   |  7 ---
 5 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/branch.c b/branch.c
index 0674a99..a84fb2c 100644
--- a/branch.c
+++ b/branch.c
@@ -336,12 +336,13 @@ void remove_branch_state(void)
 
 void die_if_checked_out(const char *branch)
 {
-   char *existing;
+   const struct worktree *wt;
 
-   existing = find_shared_symref("HEAD", branch);
-   if (existing) {
+   wt = find_shared_symref("HEAD", branch);
+   if (wt) {
skip_prefix(branch, "refs/heads/", );
-   die(_("'%s' is already checked out at '%s'"), branch, existing);
+   die(_("'%s' is already checked out at '%s'"),
+   branch, wt->path);
}
 }
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 0adba62..bcde87d 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -220,12 +220,12 @@ static int delete_branches(int argc, const char **argv, 
int force, int kinds,
name = mkpathdup(fmt, bname.buf);
 
if (kinds == FILTER_REFS_BRANCHES) {
-   char *worktree = find_shared_symref("HEAD", name);
-   if (worktree) {
+   const struct worktree *wt =
+   find_shared_symref("HEAD", name);
+   if (wt) {
error(_("Cannot delete branch '%s' "
"checked out at '%s'"),
- bname.buf, worktree);
-   free(worktree);
+ bname.buf, wt->path);
ret = 1;
continue;
}
diff --git a/builtin/notes.c b/builtin/notes.c
index 6fd058d..c65b59a 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -847,15 +847,15 @@ static int merge(int argc, const char **argv, const char 
*prefix)
update_ref(msg.buf, default_notes_ref(), result_sha1, NULL,
   0, UPDATE_REFS_DIE_ON_ERR);
else { /* Merge has unresolved conflicts */
-   char *existing;
+   const struct worktree *wt;
/* Update .git/NOTES_MERGE_PARTIAL with partial merge result */
update_ref(msg.buf, "NOTES_MERGE_PARTIAL", result_sha1, NULL,
   0, UPDATE_REFS_DIE_ON_ERR);
/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
-   existing = find_shared_symref("NOTES_MERGE_REF", 
default_notes_ref());
-   if (existing)
+   wt = find_shared_symref("NOTES_MERGE_REF", default_notes_ref());
+   if (wt)
die(_("A notes merge into %s is already in-progress at 
%s"),
-   default_notes_ref(), existing);
+   default_notes_ref(), wt->path);
if (create_symref("NOTES_MERGE_REF", default_notes_ref(), NULL))
die("Failed to store link to current notes ref (%s)",
default_notes_ref());
diff --git a/worktree.c b/worktree.c
index 5ae54f0..360ba41 100644
--- a/worktree.c
+++ b/worktree.c
@@ -191,14 +191,19 @@ const char *get_worktree_git_dir(const struct worktree 
*wt)
return git_common_path("worktrees/%s", wt->id);
 }
 
-char *find_shared_symref(const char *symref, const char *target)
+const struct worktree *find_shared_symref(const char *symref,
+ const char *target)
 {
-   char *existing = NULL;
+   const struct worktree *existing = NULL;
struct strbuf path = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
-   struct worktree **worktrees = get_worktrees();
+   static struct worktree **worktrees;
int i = 0;
 
+   if (worktrees)
+   free_worktrees(worktrees);
+   worktrees = get_worktrees();
+
for (i = 0; worktrees[i]; i++) {
strbuf_reset();
strbuf_reset();
@@ -211,14 +216,13 @@ char *find_shared_symref(const char *symref, const char 
*target)
}
 
if (!strcmp(sb.buf, target)) {
-   existing = xstrdup(worktrees[i]->path);
+   existing = worktrees[i];
break;
}
}
 
strbuf_release();
strbuf_release();
-   free_worktrees(worktrees);
 
return existing;
 }
diff --git a/worktree.h b/worktree.h
index 3198c8d..d71d7ec 100644
--- 

[PATCH v2 08/12] worktree.c: avoid referencing to worktrees[i] multiple times

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 worktree.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/worktree.c b/worktree.c
index 452f64a..b5ca78f 100644
--- a/worktree.c
+++ b/worktree.c
@@ -221,10 +221,12 @@ const struct worktree *find_shared_symref(const char 
*symref,
worktrees = get_worktrees();
 
for (i = 0; worktrees[i]; i++) {
+   struct worktree *wt = worktrees[i];
+
strbuf_reset();
strbuf_reset();
strbuf_addf(, "%s/%s",
-   get_worktree_git_dir(worktrees[i]),
+   get_worktree_git_dir(wt),
symref);
 
if (parse_ref(path.buf, , NULL)) {
@@ -232,7 +234,7 @@ const struct worktree *find_shared_symref(const char 
*symref,
}
 
if (!strcmp(sb.buf, target)) {
-   existing = worktrees[i];
+   existing = wt;
break;
}
}
-- 
2.8.0.rc0.210.gd302cd2

--
To unsubscribe from this list: send the line "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 11/12] worktree.c: test if branch being bisected in another worktree

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

Similar to the rebase case, we want to detect if "HEAD" in some worktree
is being bisected because

1) we do not want to checkout this branch in another worktree, after
   bisect is done it will want to go back to this branch

2) we do not want to delete the branch is either or git bisect will
   fail to return to the (long gone) branch

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 t/t2025-worktree-add.sh | 13 +
 worktree.c  | 19 +++
 2 files changed, 32 insertions(+)

diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index da54327..8f53944 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -263,4 +263,17 @@ test_expect_success 'check out from current worktree 
branch ok' '
)
 '
 
+test_expect_success 'checkout a branch under bisect' '
+   git worktree add under-bisect &&
+   (
+   cd under-bisect &&
+   git bisect start &&
+   git bisect bad &&
+   git bisect good HEAD~2 &&
+   git worktree list | grep "under-bisect.*detached HEAD" &&
+   test_must_fail git worktree add new-bisect under-bisect &&
+   ! test -d new-bisect
+   )
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index dc380a2..7b66071 100644
--- a/worktree.c
+++ b/worktree.c
@@ -226,6 +226,21 @@ static int is_worktree_being_rebased(const struct worktree 
*wt,
return found_rebase;
 }
 
+static int is_worktree_being_bisected(const struct worktree *wt,
+ const char *target)
+{
+   struct wt_status_state state;
+   int found_rebase;
+
+   memset(, 0, sizeof(state));
+   found_rebase = wt_status_check_bisect(wt, ) &&
+   state.branch &&
+   starts_with(target, "refs/heads/") &&
+   !strcmp(state.branch, target + strlen("refs/heads/"));
+   free(state.branch);
+   return found_rebase;
+}
+
 const struct worktree *find_shared_symref(const char *symref,
  const char *target,
  int ignore_current_worktree)
@@ -251,6 +266,10 @@ const struct worktree *find_shared_symref(const char 
*symref,
existing = wt;
break;
}
+   if (is_worktree_being_bisected(wt, target)) {
+   existing = wt;
+   break;
+   }
}
 
strbuf_reset();
-- 
2.8.0.rc0.210.gd302cd2

--
To unsubscribe from this list: send the line "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 12/12] branch: do not rename a branch under bisect or rebase

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

The branch name in that case could be saved in rebase's head_name or
bisect's BISECT_START files. Ideally we should try to update them as
well. But it's trickier (*). Let's play safe and see if the user
complains about inconveniences before doing that.

(*) If we do it, bisect and rebase need to provide an API to rename
branches. We can't do it in worktree.c or builtin/branch.c because
when other people change rebase/bisect code, they may not be aware of
this code and accidentally break it (e.g. rename the branch file, or
refer to the branch in new files). It's a lot more work.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 builtin/branch.c| 25 +
 t/t2025-worktree-add.sh |  8 
 worktree.c  |  8 
 worktree.h  |  3 +++
 4 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index bf91bbd..3a2eceb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -524,6 +524,29 @@ static void print_ref_list(struct ref_filter *filter, 
struct ref_sorting *sortin
ref_array_clear();
 }
 
+static void reject_rebase_or_bisect_branch(const char *target)
+{
+   struct worktree **worktrees = get_worktrees();
+   int i;
+
+   for (i = 0; worktrees[i]; i++) {
+   struct worktree *wt = worktrees[i];
+
+   if (!wt->is_detached)
+   continue;
+
+   if (is_worktree_being_rebased(wt, target))
+   die(_("Branch %s is being rebased at %s"),
+   target, wt->path);
+
+   if (is_worktree_being_bisected(wt, target))
+   die(_("Branch %s is being bisected at %s"),
+   target, wt->path);
+   }
+
+   free_worktrees(worktrees);
+}
+
 static void rename_branch(const char *oldname, const char *newname, int force)
 {
struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = 
STRBUF_INIT;
@@ -553,6 +576,8 @@ static void rename_branch(const char *oldname, const char 
*newname, int force)
 
validate_new_branchname(newname, , force, clobber_head_ok);
 
+   reject_rebase_or_bisect_branch(oldref.buf);
+
strbuf_addf(, "Branch: renamed %s to %s",
 oldref.buf, newref.buf);
 
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 8f53944..3a22fc5 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -254,6 +254,10 @@ test_expect_success 'not allow to delete a branch under 
rebase' '
)
 '
 
+test_expect_success 'rename a branch under rebase not allowed' '
+   test_must_fail git branch -M under-rebase rebase-with-new-name
+'
+
 test_expect_success 'check out from current worktree branch ok' '
(
cd under-rebase &&
@@ -276,4 +280,8 @@ test_expect_success 'checkout a branch under bisect' '
)
 '
 
+test_expect_success 'rename a branch under bisect not allowed' '
+   test_must_fail git branch -M under-bisect bisect-with-new-name
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index 7b66071..8a3d394 100644
--- a/worktree.c
+++ b/worktree.c
@@ -208,8 +208,8 @@ const char *get_worktree_git_dir(const struct worktree *wt)
return git_common_path("worktrees/%s", wt->id);
 }
 
-static int is_worktree_being_rebased(const struct worktree *wt,
-const char *target)
+int is_worktree_being_rebased(const struct worktree *wt,
+ const char *target)
 {
struct wt_status_state state;
int found_rebase;
@@ -226,8 +226,8 @@ static int is_worktree_being_rebased(const struct worktree 
*wt,
return found_rebase;
 }
 
-static int is_worktree_being_bisected(const struct worktree *wt,
- const char *target)
+int is_worktree_being_bisected(const struct worktree *wt,
+  const char *target)
 {
struct wt_status_state state;
int found_rebase;
diff --git a/worktree.h b/worktree.h
index fb9f5cc..d4a3534 100644
--- a/worktree.h
+++ b/worktree.h
@@ -44,6 +44,9 @@ extern const struct worktree *find_shared_symref(const char 
*symref,
 const char *target,
 int ignore_current_worktree);
 
+int is_worktree_being_rebased(const struct worktree *wt, const char *target);
+int is_worktree_being_bisected(const struct worktree *wt, const char *target);
+
 /*
  * Similar to git_path() and git_pathdup() but can produce paths for a
  * specified worktree instead of current one
-- 
2.8.0.rc0.210.gd302cd2

--
To unsubscribe from this list: send the line "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 04/12] worktree.c: mark current worktree

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 worktree.c | 18 +-
 worktree.h |  1 +
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/worktree.c b/worktree.c
index 360ba41..452f64a 100644
--- a/worktree.c
+++ b/worktree.c
@@ -2,6 +2,7 @@
 #include "refs.h"
 #include "strbuf.h"
 #include "worktree.h"
+#include "dir.h"
 
 void free_worktrees(struct worktree **worktrees)
 {
@@ -94,6 +95,7 @@ static struct worktree *get_main_worktree(void)
worktree->is_bare = is_bare;
worktree->head_ref = NULL;
worktree->is_detached = is_detached;
+   worktree->is_current = 0;
add_head_info(_ref, worktree);
 
 done:
@@ -138,6 +140,7 @@ static struct worktree *get_linked_worktree(const char *id)
worktree->is_bare = 0;
worktree->head_ref = NULL;
worktree->is_detached = is_detached;
+   worktree->is_current = 0;
add_head_info(_ref, worktree);
 
 done:
@@ -150,10 +153,11 @@ done:
 struct worktree **get_worktrees(void)
 {
struct worktree **list = NULL;
+   struct strbuf git_dir = STRBUF_INIT;
struct strbuf path = STRBUF_INIT;
DIR *dir;
struct dirent *d;
-   int counter = 0, alloc = 2;
+   int i, counter = 0, alloc = 2;
 
list = xmalloc(alloc * sizeof(struct worktree *));
 
@@ -178,6 +182,18 @@ struct worktree **get_worktrees(void)
}
ALLOC_GROW(list, counter + 1, alloc);
list[counter] = NULL;
+
+   strbuf_addstr(_dir, absolute_path(get_git_dir()));
+   for (i = 0; i < counter; i++) {
+   struct worktree *wt = list[i];
+   strbuf_addstr(, absolute_path(get_worktree_git_dir(wt)));
+   wt->is_current = !strcmp_icase(git_dir.buf, path.buf);
+   strbuf_reset();
+   if (wt->is_current)
+   break;
+   }
+   strbuf_release(_dir);
+   strbuf_release();
return list;
 }
 
diff --git a/worktree.h b/worktree.h
index d71d7ec..625fb8d 100644
--- a/worktree.h
+++ b/worktree.h
@@ -8,6 +8,7 @@ struct worktree {
unsigned char head_sha1[20];
int is_detached;
int is_bare;
+   int is_current;
 };
 
 /* Functions for acting on the information about worktrees. */
-- 
2.8.0.rc0.210.gd302cd2

--
To unsubscribe from this list: send the line "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 06/12] wt-status.c: split rebase detection out of wt_status_get_state()

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

worktree.c:find_shared_symref() later needs to know if a branch is being
rebased, and only rebased only. Split this code so it can be used
independently from other in-progress tests.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 wt-status.c | 23 +--
 wt-status.h |  1 +
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 1ea2ebe..35787ec 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1360,15 +1360,11 @@ static void wt_status_get_detached_from(struct 
wt_status_state *state)
strbuf_release();
 }
 
-void wt_status_get_state(struct wt_status_state *state,
-int get_detached_from)
+int wt_status_check_rebase(struct wt_status_state *state)
 {
struct stat st;
-   unsigned char sha1[20];
 
-   if (!stat(git_path_merge_head(), )) {
-   state->merge_in_progress = 1;
-   } else if (!stat(git_path("rebase-apply"), )) {
+   if (!stat(git_path("rebase-apply"), )) {
if (!stat(git_path("rebase-apply/applying"), )) {
state->am_in_progress = 1;
if (!stat(git_path("rebase-apply/patch"), ) && 
!st.st_size)
@@ -1385,6 +1381,21 @@ void wt_status_get_state(struct wt_status_state *state,
state->rebase_in_progress = 1;
state->branch = read_and_strip_branch("rebase-merge/head-name");
state->onto = read_and_strip_branch("rebase-merge/onto");
+   } else
+   return 0;
+   return 1;
+}
+
+void wt_status_get_state(struct wt_status_state *state,
+int get_detached_from)
+{
+   struct stat st;
+   unsigned char sha1[20];
+
+   if (!stat(git_path_merge_head(), )) {
+   state->merge_in_progress = 1;
+   } else if (wt_status_check_rebase(state)) {
+   /* all set */
} else if (!stat(git_path_cherry_pick_head(), ) &&
!get_sha1("CHERRY_PICK_HEAD", sha1)) {
state->cherry_pick_in_progress = 1;
diff --git a/wt-status.h b/wt-status.h
index c9b3b74..b398353 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -100,6 +100,7 @@ void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
 void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
+int wt_status_check_rebase(struct wt_status_state *state);
 
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
-- 
2.8.0.rc0.210.gd302cd2

--
To unsubscribe from this list: send the line "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 07/12] wt-status.c: make wt_status_check_rebase() work on any worktree

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

This is a preparation step for find_shared_symref() to detect if any
worktree is being rebased.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 wt-status.c | 33 -
 wt-status.h |  5 -
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 35787ec..2295682 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -15,6 +15,7 @@
 #include "column.h"
 #include "strbuf.h"
 #include "utf8.h"
+#include "worktree.h"
 
 static const char cut_line[] =
 " >8 \n";
@@ -1262,13 +1263,13 @@ static void show_bisect_in_progress(struct wt_status *s,
 /*
  * Extract branch information from rebase/bisect
  */
-static char *read_and_strip_branch(const char *path)
+static char *get_branch(const struct worktree *wt, const char *path)
 {
struct strbuf sb = STRBUF_INIT;
unsigned char sha1[20];
const char *branch_name;
 
-   if (strbuf_read_file(, git_path("%s", path), 0) <= 0)
+   if (strbuf_read_file(, worktree_git_path(wt, "%s", path), 0) <= 0)
goto got_nothing;
 
while (sb.len && sb.buf[sb.len - 1] == '\n')
@@ -1295,6 +1296,11 @@ got_nothing:
return NULL;
 }
 
+static char *read_and_strip_branch(const char *path)
+{
+   return get_branch(NULL, path);
+}
+
 struct grab_1st_switch_cbdata {
struct strbuf buf;
unsigned char nsha1[20];
@@ -1360,27 +1366,28 @@ static void wt_status_get_detached_from(struct 
wt_status_state *state)
strbuf_release();
 }
 
-int wt_status_check_rebase(struct wt_status_state *state)
+int wt_status_check_rebase(const struct worktree *wt,
+  struct wt_status_state *state)
 {
struct stat st;
 
-   if (!stat(git_path("rebase-apply"), )) {
-   if (!stat(git_path("rebase-apply/applying"), )) {
+   if (!stat(worktree_git_path(wt, "rebase-apply"), )) {
+   if (!stat(worktree_git_path(wt, "rebase-apply/applying"), )) 
{
state->am_in_progress = 1;
-   if (!stat(git_path("rebase-apply/patch"), ) && 
!st.st_size)
+   if (!stat(worktree_git_path(wt, "rebase-apply/patch"), 
) && !st.st_size)
state->am_empty_patch = 1;
} else {
state->rebase_in_progress = 1;
-   state->branch = 
read_and_strip_branch("rebase-apply/head-name");
-   state->onto = 
read_and_strip_branch("rebase-apply/onto");
+   state->branch = get_branch(wt, 
"rebase-apply/head-name");
+   state->onto = get_branch(wt, "rebase-apply/onto");
}
-   } else if (!stat(git_path("rebase-merge"), )) {
-   if (!stat(git_path("rebase-merge/interactive"), ))
+   } else if (!stat(worktree_git_path(wt, "rebase-merge"), )) {
+   if (!stat(worktree_git_path(wt, "rebase-merge/interactive"), 
))
state->rebase_interactive_in_progress = 1;
else
state->rebase_in_progress = 1;
-   state->branch = read_and_strip_branch("rebase-merge/head-name");
-   state->onto = read_and_strip_branch("rebase-merge/onto");
+   state->branch = get_branch(wt, "rebase-merge/head-name");
+   state->onto = get_branch(wt, "rebase-merge/onto");
} else
return 0;
return 1;
@@ -1394,7 +1401,7 @@ void wt_status_get_state(struct wt_status_state *state,
 
if (!stat(git_path_merge_head(), )) {
state->merge_in_progress = 1;
-   } else if (wt_status_check_rebase(state)) {
+   } else if (wt_status_check_rebase(NULL, state)) {
/* all set */
} else if (!stat(git_path_cherry_pick_head(), ) &&
!get_sha1("CHERRY_PICK_HEAD", sha1)) {
diff --git a/wt-status.h b/wt-status.h
index b398353..c4ddcad 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -6,6 +6,8 @@
 #include "color.h"
 #include "pathspec.h"
 
+struct worktree;
+
 enum color_wt_status {
WT_STATUS_HEADER = 0,
WT_STATUS_UPDATED,
@@ -100,7 +102,8 @@ void wt_status_prepare(struct wt_status *s);
 void wt_status_print(struct wt_status *s);
 void wt_status_collect(struct wt_status *s);
 void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
-int wt_status_check_rebase(struct wt_status_state *state);
+int wt_status_check_rebase(const struct worktree *wt,
+  struct wt_status_state *state);
 
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
-- 
2.8.0.rc0.210.gd302cd2

--
To unsubscribe from this list: send the line "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 09/12] worktree.c: test if branch being rebased in another worktree

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

This function find_shared_symref() is used in a couple places:

1) in builtin/branch.c: it's used to detect if a branch is checked out
   elsewhere and refuse to delete the branch.

2) in builtin/notes.c: it's used to detect if a note is being merged in
   another worktree

3) in branch.c, the function die_if_checked_out() is actually used by
   "git checkout" and "git worktree add" to see if a branch is already
   checked out elsewhere and refuse the operation.

In cases 1 and 3, if a rebase is ongoing, "HEAD" will be in detached
mode, find_shared_symref() fails to detect it and declares "no branch is
checked out here", which is incorrect.

This patch tightens the test. If the given symref is "HEAD", we try to
detect if rebase is ongoing. If so return the branch being rebased. This
makes checkout and branch delete operations safer because you can't
checkout a branch being rebased in another place, or delete it.

Special case for checkout. If the current branch is being rebased,
git-rebase.sh may use "git checkout" to abort and return back to the
original branch. The updated test in find_shared_symref() will prevent
that and "git rebase --abort" will fail as a result.
find_shared_symref() and die_if_checked_out() have to learn a new
option ignore_current_worktree to loose the test a bit.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 branch.c|  4 ++--
 branch.h|  2 +-
 builtin/branch.c|  2 +-
 builtin/checkout.c  |  2 +-
 builtin/notes.c |  2 +-
 builtin/worktree.c  |  4 ++--
 t/t2025-worktree-add.sh | 38 ++
 worktree.c  | 32 +++-
 worktree.h  |  3 ++-
 9 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/branch.c b/branch.c
index a84fb2c..8e323d3 100644
--- a/branch.c
+++ b/branch.c
@@ -334,11 +334,11 @@ void remove_branch_state(void)
unlink(git_path_squash_msg());
 }
 
-void die_if_checked_out(const char *branch)
+void die_if_checked_out(const char *branch, int ignore_current_worktree)
 {
const struct worktree *wt;
 
-   wt = find_shared_symref("HEAD", branch);
+   wt = find_shared_symref("HEAD", branch, ignore_current_worktree);
if (wt) {
skip_prefix(branch, "refs/heads/", );
die(_("'%s' is already checked out at '%s'"),
diff --git a/branch.h b/branch.h
index d69163d..b2f9649 100644
--- a/branch.h
+++ b/branch.h
@@ -58,7 +58,7 @@ extern int read_branch_desc(struct strbuf *, const char 
*branch_name);
  * worktree and die (with a message describing its checkout location) if
  * it is.
  */
-extern void die_if_checked_out(const char *branch);
+extern void die_if_checked_out(const char *branch, int 
ignore_current_worktree);
 
 /*
  * Update all per-worktree HEADs pointing at the old ref to point the new ref.
diff --git a/builtin/branch.c b/builtin/branch.c
index bcde87d..bf91bbd 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -221,7 +221,7 @@ static int delete_branches(int argc, const char **argv, int 
force, int kinds,
 
if (kinds == FILTER_REFS_BRANCHES) {
const struct worktree *wt =
-   find_shared_symref("HEAD", name);
+   find_shared_symref("HEAD", name, 0);
if (wt) {
error(_("Cannot delete branch '%s' "
"checked out at '%s'"),
diff --git a/builtin/checkout.c b/builtin/checkout.c
index efcbd8f..6041718 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -,7 +,7 @@ static int checkout_branch(struct checkout_opts *opts,
char *head_ref = resolve_refdup("HEAD", 0, sha1, );
if (head_ref &&
(!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))
-   die_if_checked_out(new->path);
+   die_if_checked_out(new->path, 1);
free(head_ref);
}
 
diff --git a/builtin/notes.c b/builtin/notes.c
index c65b59a..f154a69 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -852,7 +852,7 @@ static int merge(int argc, const char **argv, const char 
*prefix)
update_ref(msg.buf, "NOTES_MERGE_PARTIAL", result_sha1, NULL,
   0, UPDATE_REFS_DIE_ON_ERR);
/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
-   wt = find_shared_symref("NOTES_MERGE_REF", default_notes_ref());
+   wt = find_shared_symref("NOTES_MERGE_REF", default_notes_ref(), 
0);
if (wt)
die(_("A notes merge into %s is already in-progress at 
%s"),
default_notes_ref(), wt->path);
diff --git a/builtin/worktree.c b/builtin/worktree.c
index d8e3795..12c0af7 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -205,7 +205,7 @@ static int 

[PATCH v2 05/12] path.c: refactor and add worktree_git_path()

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

do_git_path(), which is the common code for all git_path* functions, is
modified to take a worktree struct and can produce paths for any
worktree.

worktree_git_path() is the first function that makes use of this. It can
be used to write code that can examine any worktree. For example,
wt_status_get_state() will be converted using this to take
am/rebase/... state of any worktree.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 path.c | 34 --
 worktree.h | 11 +++
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/path.c b/path.c
index 2ebb23d..c421d37 100644
--- a/path.c
+++ b/path.c
@@ -5,6 +5,7 @@
 #include "strbuf.h"
 #include "string-list.h"
 #include "dir.h"
+#include "worktree.h"
 
 static int get_st_mode_bits(const char *path, int *mode)
 {
@@ -383,10 +384,11 @@ static void adjust_git_path(struct strbuf *buf, int 
git_dir_len)
update_common_dir(buf, git_dir_len, NULL);
 }
 
-static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)
+static void do_git_path(const struct worktree *wt, struct strbuf *buf,
+   const char *fmt, va_list args)
 {
int gitdir_len;
-   strbuf_addstr(buf, get_git_dir());
+   strbuf_addstr(buf, get_worktree_git_dir(wt));
if (buf->len && !is_dir_sep(buf->buf[buf->len - 1]))
strbuf_addch(buf, '/');
gitdir_len = buf->len;
@@ -400,7 +402,7 @@ char *git_path_buf(struct strbuf *buf, const char *fmt, ...)
va_list args;
strbuf_reset(buf);
va_start(args, fmt);
-   do_git_path(buf, fmt, args);
+   do_git_path(NULL, buf, fmt, args);
va_end(args);
return buf->buf;
 }
@@ -409,7 +411,7 @@ void strbuf_git_path(struct strbuf *sb, const char *fmt, 
...)
 {
va_list args;
va_start(args, fmt);
-   do_git_path(sb, fmt, args);
+   do_git_path(NULL, sb, fmt, args);
va_end(args);
 }
 
@@ -418,7 +420,7 @@ const char *git_path(const char *fmt, ...)
struct strbuf *pathname = get_pathname();
va_list args;
va_start(args, fmt);
-   do_git_path(pathname, fmt, args);
+   do_git_path(NULL, pathname, fmt, args);
va_end(args);
return pathname->buf;
 }
@@ -428,7 +430,7 @@ char *git_pathdup(const char *fmt, ...)
struct strbuf path = STRBUF_INIT;
va_list args;
va_start(args, fmt);
-   do_git_path(, fmt, args);
+   do_git_path(NULL, , fmt, args);
va_end(args);
return strbuf_detach(, NULL);
 }
@@ -454,6 +456,26 @@ const char *mkpath(const char *fmt, ...)
return cleanup_path(pathname->buf);
 }
 
+const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)
+{
+   struct strbuf *pathname = get_pathname();
+   va_list args;
+   va_start(args, fmt);
+   do_git_path(wt, pathname, fmt, args);
+   va_end(args);
+   return pathname->buf;
+}
+
+char *worktree_git_pathdup(const struct worktree *wt, const char *fmt, ...)
+{
+   struct strbuf path = STRBUF_INIT;
+   va_list args;
+   va_start(args, fmt);
+   do_git_path(wt, , fmt, args);
+   va_end(args);
+   return strbuf_detach(, NULL);
+}
+
 static void do_submodule_path(struct strbuf *buf, const char *path,
  const char *fmt, va_list args)
 {
diff --git a/worktree.h b/worktree.h
index 625fb8d..9d2463e 100644
--- a/worktree.h
+++ b/worktree.h
@@ -43,4 +43,15 @@ extern void free_worktrees(struct worktree **);
 extern const struct worktree *find_shared_symref(const char *symref,
 const char *target);
 
+/*
+ * Similar to git_path() and git_pathdup() but can produce paths for a
+ * specified worktree instead of current one
+ */
+extern const char *worktree_git_path(const struct worktree *wt,
+const char *fmt, ...)
+   __attribute__((format (printf, 2, 3)));
+extern char *worktree_git_pathdup(const struct worktree *wt,
+ const char *fmt, ...)
+   __attribute__((format (printf, 2, 3)));
+
 #endif
-- 
2.8.0.rc0.210.gd302cd2

--
To unsubscribe from this list: send the line "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 10/12] wt-status.c: split bisect detection out of wt_status_get_state()

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

And make it work with any given worktree, in preparation for (again)
find_shared_symref(). read_and_strip_branch() is deleted because it's
no longer used.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 wt-status.c | 23 ++-
 wt-status.h |  2 ++
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index 2295682..36c85f8 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1296,11 +1296,6 @@ got_nothing:
return NULL;
 }
 
-static char *read_and_strip_branch(const char *path)
-{
-   return get_branch(NULL, path);
-}
-
 struct grab_1st_switch_cbdata {
struct strbuf buf;
unsigned char nsha1[20];
@@ -1393,6 +1388,19 @@ int wt_status_check_rebase(const struct worktree *wt,
return 1;
 }
 
+int wt_status_check_bisect(const struct worktree *wt,
+  struct wt_status_state *state)
+{
+   struct stat st;
+
+   if (!stat(worktree_git_path(wt, "BISECT_LOG"), )) {
+   state->bisect_in_progress = 1;
+   state->branch = get_branch(wt, "BISECT_START");
+   return 1;
+   }
+   return 0;
+}
+
 void wt_status_get_state(struct wt_status_state *state,
 int get_detached_from)
 {
@@ -1408,10 +1416,7 @@ void wt_status_get_state(struct wt_status_state *state,
state->cherry_pick_in_progress = 1;
hashcpy(state->cherry_pick_head_sha1, sha1);
}
-   if (!stat(git_path("BISECT_LOG"), )) {
-   state->bisect_in_progress = 1;
-   state->branch = read_and_strip_branch("BISECT_START");
-   }
+   wt_status_check_bisect(NULL, state);
if (!stat(git_path_revert_head(), ) &&
!get_sha1("REVERT_HEAD", sha1)) {
state->revert_in_progress = 1;
diff --git a/wt-status.h b/wt-status.h
index c4ddcad..2ca93f6 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -104,6 +104,8 @@ void wt_status_collect(struct wt_status *s);
 void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
 int wt_status_check_rebase(const struct worktree *wt,
   struct wt_status_state *state);
+int wt_status_check_bisect(const struct worktree *wt,
+  struct wt_status_state *state);
 
 void wt_shortstatus_print(struct wt_status *s);
 void wt_porcelain_print(struct wt_status *s);
-- 
2.8.0.rc0.210.gd302cd2

--
To unsubscribe from this list: send the line "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 00/12] fix checking out a being-rebased branch

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

Much happier with this version. This makes

 - git checkout refuse if a branch is under rebase or bisect
   elsewhere

 - git worktree add refuse if a branch is under rebase or bisect

 - git branch -D refuse if a branch is under rebase or bisect.
   This applies for single worktree case as well.

 - git branch -M refuse if a branch is under rebase or bisect
   (single worktree case as well)

  [01/12] path.c: add git_common_path() and strbuf_git_common_path()
  [02/12] worktree.c: store "id" instead of "git_dir"
  [03/12] worktree.c: make find_shared_symref() return struct worktree *
  [04/12] worktree.c: mark current worktree
  [05/12] path.c: refactor and add worktree_git_path()
  [06/12] wt-status.c: split rebase detection out of wt_status_get_state()
  [07/12] wt-status.c: make wt_status_check_rebase() work on any worktree
  [08/12] worktree.c: avoid referencing to worktrees[i] multiple times
  [09/12] worktree.c: test if branch being rebased in another worktree
  [10/12] wt-status.c: split bisect detection out of wt_status_get_state()
  [11/12] worktree.c: test if branch being bisected in another worktree
  [12/12] branch: do not rename a branch under bisect or rebase

Total 13 files changed, 335 insertions(+), 67 deletions(-)
--
To unsubscribe from this list: send the line "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 01/12] path.c: add git_common_path() and strbuf_git_common_path()

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

These are mostly convenient functions to reduce code duplication. Most
of the time, we should be able to get by with git_path() which handles
$GIT_COMMON_DIR internally. However there are a few cases where we need
to construct paths manually, for example some paths from a specific
worktree. These functions will enable that.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 cache.h |  3 +++
 path.c  | 29 +
 2 files changed, 32 insertions(+)

diff --git a/cache.h b/cache.h
index 2711048..c04a17f 100644
--- a/cache.h
+++ b/cache.h
@@ -799,11 +799,14 @@ extern void check_repository_format(void);
  */
 extern const char *mkpath(const char *fmt, ...) __attribute__((format (printf, 
1, 2)));
 extern const char *git_path(const char *fmt, ...) __attribute__((format 
(printf, 1, 2)));
+extern const char *git_common_path(const char *fmt, ...) __attribute__((format 
(printf, 1, 2)));
 
 extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
__attribute__((format (printf, 3, 4)));
 extern void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
+extern void strbuf_git_common_path(struct strbuf *sb, const char *fmt, ...)
+   __attribute__((format (printf, 2, 3)));
 extern char *git_path_buf(struct strbuf *buf, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
 extern void strbuf_git_path_submodule(struct strbuf *sb, const char *path,
diff --git a/path.c b/path.c
index bbaea5a..2ebb23d 100644
--- a/path.c
+++ b/path.c
@@ -503,6 +503,35 @@ void strbuf_git_path_submodule(struct strbuf *buf, const 
char *path,
va_end(args);
 }
 
+static void do_git_common_path(struct strbuf *buf,
+  const char *fmt,
+  va_list args)
+{
+   strbuf_addstr(buf, get_git_common_dir());
+   if (buf->len && !is_dir_sep(buf->buf[buf->len - 1]))
+   strbuf_addch(buf, '/');
+   strbuf_vaddf(buf, fmt, args);
+   strbuf_cleanup_path(buf);
+}
+
+const char *git_common_path(const char *fmt, ...)
+{
+   struct strbuf *pathname = get_pathname();
+   va_list args;
+   va_start(args, fmt);
+   do_git_common_path(pathname, fmt, args);
+   va_end(args);
+   return pathname->buf;
+}
+
+void strbuf_git_common_path(struct strbuf *sb, const char *fmt, ...)
+{
+   va_list args;
+   va_start(args, fmt);
+   do_git_common_path(sb, fmt, args);
+   va_end(args);
+}
+
 int validate_headref(const char *path)
 {
struct stat st;
-- 
2.8.0.rc0.210.gd302cd2

--
To unsubscribe from this list: send the line "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 02/12] worktree.c: store "id" instead of "git_dir"

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

We can reconstruct git_dir from id quite easily. It's a bit hackier to
do the reverse.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 branch.c   |  3 ++-
 worktree.c | 31 ++-
 worktree.h |  8 +++-
 3 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/branch.c b/branch.c
index 4162443..0674a99 100644
--- a/branch.c
+++ b/branch.c
@@ -357,7 +357,8 @@ int replace_each_worktree_head_symref(const char *oldref, 
const char *newref)
if (strcmp(oldref, worktrees[i]->head_ref))
continue;
 
-   if (set_worktree_head_symref(worktrees[i]->git_dir, newref)) {
+   if (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),
+newref)) {
ret = -1;
error(_("HEAD of working tree %s is not updated"),
  worktrees[i]->path);
diff --git a/worktree.c b/worktree.c
index 6181a66..5ae54f0 100644
--- a/worktree.c
+++ b/worktree.c
@@ -9,7 +9,7 @@ void free_worktrees(struct worktree **worktrees)
 
for (i = 0; worktrees[i]; i++) {
free(worktrees[i]->path);
-   free(worktrees[i]->git_dir);
+   free(worktrees[i]->id);
free(worktrees[i]->head_ref);
free(worktrees[i]);
}
@@ -74,13 +74,11 @@ static struct worktree *get_main_worktree(void)
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
struct strbuf worktree_path = STRBUF_INIT;
-   struct strbuf gitdir = STRBUF_INIT;
struct strbuf head_ref = STRBUF_INIT;
int is_bare = 0;
int is_detached = 0;
 
-   strbuf_addf(, "%s", absolute_path(get_git_common_dir()));
-   strbuf_addbuf(_path, );
+   strbuf_addstr(_path, absolute_path(get_git_common_dir()));
is_bare = !strbuf_strip_suffix(_path, "/.git");
if (is_bare)
strbuf_strip_suffix(_path, "/.");
@@ -92,7 +90,7 @@ static struct worktree *get_main_worktree(void)
 
worktree = xmalloc(sizeof(struct worktree));
worktree->path = strbuf_detach(_path, NULL);
-   worktree->git_dir = strbuf_detach(, NULL);
+   worktree->id = NULL;
worktree->is_bare = is_bare;
worktree->head_ref = NULL;
worktree->is_detached = is_detached;
@@ -100,7 +98,6 @@ static struct worktree *get_main_worktree(void)
 
 done:
strbuf_release();
-   strbuf_release();
strbuf_release(_path);
strbuf_release(_ref);
return worktree;
@@ -111,16 +108,13 @@ static struct worktree *get_linked_worktree(const char 
*id)
struct worktree *worktree = NULL;
struct strbuf path = STRBUF_INIT;
struct strbuf worktree_path = STRBUF_INIT;
-   struct strbuf gitdir = STRBUF_INIT;
struct strbuf head_ref = STRBUF_INIT;
int is_detached = 0;
 
if (!id)
die("Missing linked worktree name");
 
-   strbuf_addf(, "%s/worktrees/%s",
-   absolute_path(get_git_common_dir()), id);
-   strbuf_addf(, "%s/gitdir", gitdir.buf);
+   strbuf_git_common_path(, "worktrees/%s/gitdir", id);
if (strbuf_read_file(_path, path.buf, 0) <= 0)
/* invalid gitdir file */
goto done;
@@ -140,7 +134,7 @@ static struct worktree *get_linked_worktree(const char *id)
 
worktree = xmalloc(sizeof(struct worktree));
worktree->path = strbuf_detach(_path, NULL);
-   worktree->git_dir = strbuf_detach(, NULL);
+   worktree->id = xstrdup(id);
worktree->is_bare = 0;
worktree->head_ref = NULL;
worktree->is_detached = is_detached;
@@ -148,7 +142,6 @@ static struct worktree *get_linked_worktree(const char *id)
 
 done:
strbuf_release();
-   strbuf_release();
strbuf_release(_path);
strbuf_release(_ref);
return worktree;
@@ -188,6 +181,16 @@ struct worktree **get_worktrees(void)
return list;
 }
 
+const char *get_worktree_git_dir(const struct worktree *wt)
+{
+   if (!wt)
+   return get_git_dir();
+   else if (!wt->id)
+   return get_git_common_dir();
+   else
+   return git_common_path("worktrees/%s", wt->id);
+}
+
 char *find_shared_symref(const char *symref, const char *target)
 {
char *existing = NULL;
@@ -199,7 +202,9 @@ char *find_shared_symref(const char *symref, const char 
*target)
for (i = 0; worktrees[i]; i++) {
strbuf_reset();
strbuf_reset();
-   strbuf_addf(, "%s/%s", worktrees[i]->git_dir, symref);
+   strbuf_addf(, "%s/%s",
+   get_worktree_git_dir(worktrees[i]),
+   symref);
 
if (parse_ref(path.buf, , NULL)) {
continue;
diff --git a/worktree.h b/worktree.h
index b4b3dda..3198c8d 100644
--- 

Re: [PATCH 2/2] xdiff: implement empty line chunk heuristic

[Michael S. Tsirkin]

On Tue, Apr 19, 2016 at 04:07:35PM -0700, Junio C Hamano wrote:
> Jacob Keller  writes:
> 
> > On Tue, Apr 19, 2016 at 10:06 AM, Jeff King  wrote:
> >> On Tue, Apr 19, 2016 at 08:17:38AM -0700, Stefan Beller wrote:
> >>
> >>> On Mon, Apr 18, 2016 at 10:03 PM, Jeff King  wrote:
> >>>
> >>> > I guess this will invalidate old patch-ids, but there's not much to be
> >>> > done about that.
> >>>
> >>> What do you mean by that? (What consequences do you imagine?)
> >>> I think diffs with any kind of heuristic can still be applied, no?
> >>
> >> I mean that if you save any old patch-ids from "git patch-id", they
> >> won't match up when compared with new versions of git. We can probably
> >> ignore it, though. This isn't the first time that patch-ids might have
> >> changed, and I think the advice is already that one should not count on
> >> them to be stable in the long term.
> >>
> >> -Peff
> >
> > Plus they'll be stable within a version of Git, it's only recorded
> > patch ids that change, which hopefully isn't done very much if at all.
> >
> > Thanks,
> > Jake
> 
> Some people, like those who did things like 30e12b92 (patch-id: make
> it stable against hunk reordering, 2014-04-27), _may_ care.
> 

FWIW IIRC what that commit is about is ability to reorder the chunks in
a patch without changing patch-id. Not about keeping id stable across
git revisions.

-- 
MST
--
To unsubscribe from this list: send the line "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 16/15] Add tracing to measure where most of the time is spent

[Duy Nguyen]

On Wed, Apr 20, 2016 at 7:28 PM, Johannes Schindelin
 wrote:
> Hi Duy,
>
> On Wed, 20 Apr 2016, Nguyễn Thái Ngọc Duy wrote:
>
>> All the known heavy code blocks are measured (except object database
>> access). This should help identify if an optimization is effective or
>> not. An unoptimized git-status would give something like below (92% of
>> time is accounted). To sum up the effort of making git scale better:
>>
>>  - read cache line is being addressed by index-helper
>>  - preload/refresh index by watchman
>>  - read packed refs by lmdb backend
>>  - diff-index by rebuilding cache-tree
>>  - read directory by untracked cache and watchman
>>  - write index by split index
>>  - name hash potientially by index-helper
>>
>> read-cache.c:2075 performance: 0.004058570 s: read cache .../index
>> preload-index.c:104   performance: 0.004419864 s: preload index
>> read-cache.c:1265 performance: 0.000185224 s: refresh index
>> refs/files-backend.c:1100 performance: 0.001935788 s: read packed refs
>> diff-lib.c:240performance: 0.000144132 s: diff-files
>> diff-lib.c:506performance: 0.013592000 s: diff-index
>> name-hash.c:128   performance: 0.000614177 s: initialize name hash
>> dir.c:2030performance: 0.015814103 s: read directory
>> read-cache.c:2565 performance: 0.004052343 s: write index, changed 
>> mask = 2
>> trace.c:420   performance: 0.048365509 s: git command: './git' 
>> 'status'
>
> Thank you for doing this! It will be *highly* valuable to get the
> performance on Windows where I want it to be, too.

Just to be clear, these are unoptimized numbers, no untracked cache,
no split index, no index helper, and on a tiny repository that is
git.git. It's mostly to show the percentage of time spent in each
phase. We probably can do similar measurement and record it in the
watchman patch (can't wait for lmdb to be here :).
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 03/15] index-helper: new daemon for caching index and related stuff

[Duy Nguyen]

On Wed, Apr 20, 2016 at 7:17 PM, Johannes Schindelin
 wrote:
>> We keep this daemon's logic as thin as possible. The "brain" stays in
>> git. So the daemon can read and validate stuff, but that's all it's
>> allowed to do. It does not add/create new information. It doesn't even
>> accept direct updates from git.
>
> I like this design. For now. Later, I really think we should add the
> ability to update the index via the index-helper.

Later we do. For watchman, index-helper creates and shares a piece of
information retrieved from watchman, which has to be combined back to
the index by git process. But it's still not a _direct_ index update.

> I am thinking about a
> method similar to watchman where a separate process (that may use the
> inotify syscall on Linux, or tap into the NTFS journal on Windows) tells
> the index-helper specifically which paths to look at, so that nobody ever
> has to look at any files that were not modified at all.

Am I missing something here? I thought watchman could already run on
Windows. We benefit from watchman, which was originally written for
Mercurial. If there's a better way to do inotify equivalent for
Windows, is it possible to do it in watchman so Mercurial can benefit
from it too?

On Wed, Apr 20, 2016 at 7:19 PM, Johannes Schindelin
 wrote:
>> On Wed, Apr 20, 2016 at 6:27 AM, David Turner  
>> wrote:
>> > Shared memory is done by storing files in a per-repository temporary
>> > directory.  This is more portable than shm (which requires
>> > posix-realtime and has various quirks on OS X).  It might even work on
>> > Windows, although this has not been tested.
>>
>> There's another option, but I'm not sure if it's too clever/tricky to
>> do. Anyway, on *nix we can send file descriptors over unix socket [2],
>> then mmap them back to access content.
>
> This sounds too clever to me ;-)

Well. At least we have considered everything (that I'm aware of).
-- 
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 16/15] Add tracing to measure where most of the time is spent

[Johannes Schindelin]

Hi Duy,

On Wed, 20 Apr 2016, Nguyễn Thái Ngọc Duy wrote:

> All the known heavy code blocks are measured (except object database
> access). This should help identify if an optimization is effective or
> not. An unoptimized git-status would give something like below (92% of
> time is accounted). To sum up the effort of making git scale better:
> 
>  - read cache line is being addressed by index-helper
>  - preload/refresh index by watchman
>  - read packed refs by lmdb backend
>  - diff-index by rebuilding cache-tree
>  - read directory by untracked cache and watchman
>  - write index by split index
>  - name hash potientially by index-helper
> 
> read-cache.c:2075 performance: 0.004058570 s: read cache .../index
> preload-index.c:104   performance: 0.004419864 s: preload index
> read-cache.c:1265 performance: 0.000185224 s: refresh index
> refs/files-backend.c:1100 performance: 0.001935788 s: read packed refs
> diff-lib.c:240performance: 0.000144132 s: diff-files
> diff-lib.c:506performance: 0.013592000 s: diff-index
> name-hash.c:128   performance: 0.000614177 s: initialize name hash
> dir.c:2030performance: 0.015814103 s: read directory
> read-cache.c:2565 performance: 0.004052343 s: write index, changed 
> mask = 2
> trace.c:420   performance: 0.048365509 s: git command: './git' 
> 'status'

Thank you for doing this! It will be *highly* valuable to get the
performance on Windows where I want it to be, too.

Ciao,
Dscho

Re: [PATCH v5 03/15] index-helper: new daemon for caching index and related stuff

[Johannes Schindelin]

Hi Duy,

On Wed, 20 Apr 2016, Duy Nguyen wrote:

> On Wed, Apr 20, 2016 at 6:27 AM, David Turner  
> wrote:
> > Shared memory is done by storing files in a per-repository temporary
> > directory.  This is more portable than shm (which requires
> > posix-realtime and has various quirks on OS X).  It might even work on
> > Windows, although this has not been tested.
> 
> There's another option, but I'm not sure if it's too clever/tricky to
> do. Anyway, on *nix we can send file descriptors over unix socket [2],
> then mmap them back to access content.

This sounds too clever to me ;-)

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 v5 03/15] index-helper: new daemon for caching index and related stuff

[Johannes Schindelin]

Hi Dave,

(apologies in advance if I may bring up anything that has been discussed
in earlier iterations; I simply was too busy with the rebase--helper
project to even look.)

On Tue, 19 Apr 2016, David Turner wrote:

> Shared memory is done by storing files in a per-repository temporary
> directory.  This is more portable than shm (which requires
> posix-realtime and has various quirks on OS X).  It might even work on
> Windows, although this has not been tested. The shared memory file's
> name folows the template "shm--" where  is the

s/folows/follows/

And: now that it is no longer shared memory, should we not do away with
the "shm-" prefix?

> trailing SHA-1 of the index file.  is "index" for cached index
> files (and might later be "name-hash" for name-hash cache). If such
> shared memory exists, it contains the same index content as on
> disk. The content is already validated by the daemon and git won't
> validate it again (except comparing the trailing SHA-1s).

Excellent. I was worrying about the penalty of validating.

> We keep this daemon's logic as thin as possible. The "brain" stays in
> git. So the daemon can read and validate stuff, but that's all it's
> allowed to do. It does not add/create new information. It doesn't even
> accept direct updates from git.

I like this design. For now. Later, I really think we should add the
ability to update the index via the index-helper. I am thinking about a
method similar to watchman where a separate process (that may use the
inotify syscall on Linux, or tap into the NTFS journal on Windows) tells
the index-helper specifically which paths to look at, so that nobody ever
has to look at any files that were not modified at all.

> + if (*new_mmap == MAP_FAILED) {
> + *new_mmap = NULL;
> + goto done;

Shouldn't we provide some sort of error message here?

> + }
> + madvise(new_mmap, size, MADV_WILLNEED);

I guess I'll need to add support for that to compat/win32mmap.c (most
likely using PrefetchVirtualMemory() when available, i.e. Windows >= 8, see
https://msdn.microsoft.com/en-us/library/windows/desktop/hh780543.aspx :-))

Likewise, I think it will be easy to use bidirectional named pipes on
Windows to accommodate for the lack of Unix domain sockets...

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] gitweb: apply fallback encoding before highlight

[Shin Kojima]

Some multi-byte character encodings (such as Shift_JIS and GBK) have
characters whose final bytes is an ASCII '\' (0x5c), and they
will be displayed as funny-characters even if $fallback_encoding is
correct.  This is because `highlight` command always expects UTF-8
encoded strings from STDIN.

$ echo 'my $v = "申";' | highlight --syntax perl | w3m -T text/html -dump
my $v = "申";

$ echo 'my $v = "申";' | iconv -f UTF-8 -t Shift_JIS | highlight \
--syntax perl | iconv -f Shift_JIS -t UTF-8 | w3m -T text/html -dump

iconv: (stdin):9:135: cannot convert
my $v = "

This patch prepare git blob objects to be encoded into UTF-8 before
highlighting in the manner of `to_utf8` subroutine.
---
 gitweb/gitweb.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 05d7910..2fddf75 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3935,6 +3935,9 @@ sub run_highlighter {
 
close $fd;
open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
+ quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', 
'-pse',
+   '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',
+   '--', "-fe=$fallback_encoding")." | ".
  quote_command($highlight_bin).
  " --replace-tabs=8 --fragment --syntax $syntax |"
or die_error(500, "Couldn't open file or run syntax 
highlighter");
-- 
2.8.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

[PATCH 16/15] Add tracing to measure where most of the time is spent

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

All the known heavy code blocks are measured (except object database
access). This should help identify if an optimization is effective or
not. An unoptimized git-status would give something like below (92% of
time is accounted). To sum up the effort of making git scale better:

 - read cache line is being addressed by index-helper
 - preload/refresh index by watchman
 - read packed refs by lmdb backend
 - diff-index by rebuilding cache-tree
 - read directory by untracked cache and watchman
 - write index by split index
 - name hash potientially by index-helper

read-cache.c:2075 performance: 0.004058570 s: read cache .../index
preload-index.c:104   performance: 0.004419864 s: preload index
read-cache.c:1265 performance: 0.000185224 s: refresh index
refs/files-backend.c:1100 performance: 0.001935788 s: read packed refs
diff-lib.c:240performance: 0.000144132 s: diff-files
diff-lib.c:506performance: 0.013592000 s: diff-index
name-hash.c:128   performance: 0.000614177 s: initialize name hash
dir.c:2030performance: 0.015814103 s: read directory
read-cache.c:2565 performance: 0.004052343 s: write index, changed mask 
= 2
trace.c:420   performance: 0.048365509 s: git command: './git' 
'status'

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 This is worth merging, I think.

 diff-lib.c   |  4 
 dir.c|  2 ++
 name-hash.c  |  2 ++
 preload-index.c  |  2 ++
 read-cache.c | 11 +++
 refs/files-backend.c |  2 ++
 6 files changed, 23 insertions(+)

diff --git a/diff-lib.c b/diff-lib.c
index bc49c70..7af7f9a 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -90,6 +90,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
int diff_unmerged_stage = revs->max_count;
unsigned ce_option = ((option & DIFF_RACY_IS_MODIFIED)
  ? CE_MATCH_RACY_IS_DIRTY : 0);
+   uint64_t start = getnanotime();
 
diff_set_mnemonic_prefix(>diffopt, "i/", "w/");
 
@@ -236,6 +237,7 @@ int run_diff_files(struct rev_info *revs, unsigned int 
option)
}
diffcore_std(>diffopt);
diff_flush(>diffopt);
+   trace_performance_since(start, "diff-files");
return 0;
 }
 
@@ -491,6 +493,7 @@ static int diff_cache(struct rev_info *revs,
 int run_diff_index(struct rev_info *revs, int cached)
 {
struct object_array_entry *ent;
+   uint64_t start = getnanotime();
 
ent = revs->pending.objects;
if (diff_cache(revs, ent->item->oid.hash, ent->name, cached))
@@ -500,6 +503,7 @@ int run_diff_index(struct rev_info *revs, int cached)
diffcore_fix_diff_index(>diffopt);
diffcore_std(>diffopt);
diff_flush(>diffopt);
+   trace_performance_since(start, "diff-index");
return 0;
 }
 
diff --git a/dir.c b/dir.c
index 91003e5..1ebf9fe 100644
--- a/dir.c
+++ b/dir.c
@@ -1991,6 +1991,7 @@ int read_directory(struct dir_struct *dir, const char 
*path, int len, const stru
 {
struct path_simplify *simplify;
struct untracked_cache_dir *untracked;
+   uint64_t start = getnanotime();
 
/*
 * Check out create_simplify()
@@ -2026,6 +2027,7 @@ int read_directory(struct dir_struct *dir, const char 
*path, int len, const stru
free_simplify(simplify);
qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);
qsort(dir->ignored, dir->ignored_nr, sizeof(struct dir_entry *), 
cmp_name);
+   trace_performance_since(start, "read directory %.*s", len, path);
if (dir->untracked) {
static struct trace_key trace_untracked_stats = 
TRACE_KEY_INIT(UNTRACKED_STATS);
trace_printf_key(_untracked_stats,
diff --git a/name-hash.c b/name-hash.c
index 6d9f23e..b3966d8 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -115,6 +115,7 @@ static int cache_entry_cmp(const struct cache_entry *ce1,
 static void lazy_init_name_hash(struct index_state *istate)
 {
int nr;
+   uint64_t start = getnanotime();
 
if (istate->name_hash_initialized)
return;
@@ -124,6 +125,7 @@ static void lazy_init_name_hash(struct index_state *istate)
for (nr = 0; nr < istate->cache_nr; nr++)
hash_index_entry(istate, istate->cache[nr]);
istate->name_hash_initialized = 1;
+   trace_performance_since(start, "initialize name hash");
 }
 
 void add_name_hash(struct index_state *istate, struct cache_entry *ce)
diff --git a/preload-index.c b/preload-index.c
index c1fe3a3..7bb4809 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -72,6 +72,7 @@ static void preload_index(struct index_state *index,
 {
int threads, i, work, offset;
struct thread_data data[MAX_PARALLEL];
+   uint64_t start = getnanotime();
 
if (!core_preload_index)
return;
@@ -100,6 +101,7 @@ static void preload_index(struct index_state *index,
   

Re: [PATCH v5 09/15] index-helper: use watchman to avoid refreshing index with lstat()

[Duy Nguyen]

On Wed, Apr 20, 2016 at 8:01 AM, David Turner  wrote:
> On Wed, 2016-04-20 at 07:15 +0700, Duy Nguyen wrote:
>> Continuing my comment from the --use-watchman patch about watchman
>> not
>> being supported...
>>
>> On Wed, Apr 20, 2016 at 6:28 AM, David Turner <
>> dtur...@twopensource.com> wrote:
>> > +static int poke_and_wait_for_reply(int fd)
>> > +{
>> > +   struct strbuf buf = STRBUF_INIT;
>> > +   struct strbuf reply = STRBUF_INIT;
>> > +   int ret = -1;
>> > +   fd_set fds;
>> > +   struct timeval timeout;
>> > +
>> > +   timeout.tv_usec = 0;
>> > +   timeout.tv_sec = 1;
>> > +
>> > +   if (fd < 0)
>> > +   return -1;
>> > +
>> > +   strbuf_addf(, "poke %d", getpid());
>> > +   if (write_in_full(fd, buf.buf, buf.len + 1) != buf.len + 1)
>> > +   goto done_poke;
>> > +
>> > +   /* Now wait for a reply */
>> > +   FD_ZERO();
>> > +   FD_SET(fd, );
>> > +   if (select(fd + 1, , NULL, NULL, ) == 0)
>> > +   /* No reply, giving up */
>> > +   goto done_poke;
>> > +
>> > +   if (strbuf_getwholeline_fd(, fd, 0))
>> > +   goto done_poke;
>> > +
>> > +   if (!starts_with(reply.buf, "OK"))
>> > +   goto done_poke;
>>
>> ... while we could simply check USE_WATCHMAN macro and reject in
>> update-index, a better solution is sending "poke %d watchman" and
>> returning "OK watchman" (vs "OK") when watchman is supported and
>> active. If the user already requests watchman and index-helper
>> returns
>> just "OK" then we can warn the user the reason of possible
>> performance
>> degradation. It's related to the error reporting, but I don't think
>> you can send straight errors over unix socket. It's possible but it's
>> a bit more complicated.
>
> Do you mean that we should do this here?  Or in update-index -
> -watchman?  If the former, I agree.  If the latter, I'm not sure; maybe
> you'll be setting up your index before you've started the index helper?

Here is better than update-index because we can't know what
index-helper is capable of (the USE_WATCHMAN macro is more like a
suggestion)

>> > +static void refresh_by_watchman(struct index_state *istate)
>> > +{
>> > +   void *shm = NULL;
>> > +   int length;
>> > +   int i;
>> > +   struct stat st;
>> > +   int fd = -1;
>> > +   const char *path = git_path("shm-watchman-%s-%"PRIuMAX,
>> > +   sha1_to_hex(istate->sha1),
>> > +   (uintmax_t)getpid());
>> > +
>> > +   fd = open(path, O_RDONLY);
>> > +   if (fd < 0)
>> > +   return;
>> > +
>> > +   /*
>> > +* This watchman data is just for us -- no need to keep it
>> > +* around once we've got it open.
>> > +*/
>> > +   unlink(path);
>>
>> This will not play well when multiple processes read and refresh the
>> index at the same time.
>
> Multiple processes will have different pids, right?  And the pid is
> included in the filename.  Am I missing something?

Ahhh! I thought that pid was index-helper's. Silly me.

>> Now that I think of it, with watchman backing us, we probably should
>> just do nothing in update_index_if_able() (or write_locked_index()
>> when we know only stat info is changed) when watchman is active. The
>> purpose of update_index_if_able() is to avoid costly refresh, but we
>> can already avoid that with watchman. And updating big index files is
>> always costly (even though it should cost less with split-index).
>
> That sounds like a change we could make in a separate series.  It's not
> a bad idea, but if our goal is to get the basic version out, we should
> start there.

Agreed. More optimizations can always wait till later. We just need a
good foundation first.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 06/15] index-helper: add --detach

[Duy Nguyen]

On Wed, Apr 20, 2016 at 8:04 AM, David Turner  wrote:
> On Wed, 2016-04-20 at 06:50 +0700, Duy Nguyen wrote:
>> On Wed, Apr 20, 2016 at 6:28 AM, David Turner <
>> dtur...@twopensource.com> wrote:
>> > @@ -317,6 +320,8 @@ int main(int argc, char **argv)
>> > if (fd < 0)
>> > die_errno(_("could not set up index-helper
>> > socket"));
>> >
>> > +   if (detach && daemonize())
>> > +   die_errno(_("unable to detach"));
>>
>> At the least, I think we need to redirect both stdout and stderr to a
>> file, so we can catch errors. The watchman patch uses warning() to
>> report errors, for example. And there is always a chance of die().
>>
>> Then we need to report the errors back. I faced the same problem with
>> daemonizing git-gc, but I'm not sure if we can do exactly the same
>> here like in commit 329e6e8 (gc: save log from daemonized gc --auto
>> and print it next time - 2015-09-19)
>
> I'll add in code to log errors.  I'm not sure where it would make sense
> to report the errors.  Generally, for errors during a client operation,
> we would like to report them to the client, but the client might have
> already disconnected.  I guess in that case it's OK if they just go to
> the log?  The client could warn on a timeout while waiting for index
> -helper and direct people to the log.

Yeah if the client already disconnects, we have no way but saving the
errors somewhere. index-helper can pick up from the log and report to
the next client, if you want to keep it simple. If we're building a
more complicated protocol on top of unix socket, I suggest you use
pkt-line to wrap/unwrap messages. Bonus point, we can trace what's
sending/receiving with GIT_TRACE_PACKET.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 02/15] read-cache: allow to keep mmap'd memory after reading

[Duy Nguyen]

On Wed, Apr 20, 2016 at 6:27 AM, David Turner  wrote:
> From: Nguyễn Thái Ngọc Duy 
>
> Later, we will introduce git index-helper to share this memory with
> other git processes.
>
> Since the memory will be shared, it will never be unmapped (although
> the kernel may of course choose to page it out).

This is not entirely true. We do need to keep the mmap'd memory for
longer, even after read_index_from() finishes. But we do not share the
exact same address space to other processes (memcpy is used in
index-helper.c:share_index()). We could even discard_index() at the
end of share_index(), but I think we keep it anyway just in case
another process asks, or when index is not updated.
-- 
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

Silent failure to add Windows-style paths in Cygwin Git

[Adam Dinwoodie]

If I attempt to `git add` an extant file specified using a Windows-style
path on Cygwin Git, this doesn't add the file, and produces no error
message:

$ pwd  # As seen by Cygwin
/cygdrive/c/tmp

$ cygpath -aw .  # As seen by Windows
C:\tmp

$ git init
Initialized empty Git repository in /cygdrive/c/tmp/.git/

$ git add 'c:\tmp\file' || echo non-zero exit code  # Errors out as expected
fatal: pathspec 'c:\tmp\file' did not match any files
non-zero exit code

$ touch file

$ git add 'c:\tmp\file' || echo non-zero exit code  # No error this time...

$ git status  # ...even though the file didn't get added
On branch master

Initial commit

Untracked files:
  (use "git add ..." to include in what will be committed)

file

nothing added to commit but untracked files present (use "git add" to track)

I wouldn't expect adding the file to actually succeed, but I would
expect it to either succeed or produce an error, rather than silently
failing.

Experimentation shows I get the same behaviour for 'c:\tmp\file',
'c:/tmp/file' and 'subdir\file'.  I'm seeing this on v2.8.0; the
downstream report says the same behaviour occurs on v2.7.4[0], and I've
also seen what appears to be the same behaviour on a v2.0.5 build I
produced to check.

Adam

[0]: https://cygwin.com/ml/cygwin/2016-04/msg00474.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 v5 02/15] read-cache: allow to keep mmap'd memory after reading

[Johannes Schindelin]

Hi Dave,

On Tue, 19 Apr 2016, David Turner wrote:

>  unmap:
> + istate->mmap = NULL;
>   munmap(mmap, mmap_size);
>   die("index file corrupt");
>  }
> [...]
> @@ -1698,6 +1705,10 @@ int discard_index(struct index_state *istate)
>   free(istate->cache);
>   istate->cache = NULL;
>   istate->cache_alloc = 0;
> + if (istate->keep_mmap && istate->mmap) {
> + munmap(istate->mmap, istate->mmap_size);
> + istate->mmap = NULL;
> + }
>   discard_split_index(istate);

Just curious: any reason why the first hunk munmap()s after resetting the
field to NULL and the second hunk does it in the opposite order?

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 2/2] git-p4: fix Git LFS pointer parsing

[Sebastian Schuberth]

On Wed, Apr 20, 2016 at 10:10 AM,   wrote:

> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1064,8 +1064,17 @@ class GitLFS(LargeFileSystem):
>  if pointerProcess.wait():
>  os.remove(contentFile)
>  die('git-lfs pointer command failed. Did you install the 
> extension?')
> -pointerContents = [i+'\n' for i in pointerFile.split('\n')[2:][:-1]]
> -oid = pointerContents[1].split(' ')[1].split(':')[1][:-1]
> +
> +# Git LFS removed the preamble in the output of the 'pointer' command

git-lfs did not remove the output. It simply goes to stderr instead of
stdout now. That said, could a fix simply be to capture both stdout
and sterr? If the output to the streams remain interleaved it should
look exactly like before.

> +# starting from version 1.2.0. Check for the preamble here to support
> +# earlier versions.
> +# c.f. 
> https://github.com/github/git-lfs/commit/da2935d9a739592bc775c98d8ef4df9c72ea3b43
> +preamble = 'Git LFS pointer for ' + contentFile + '\n\n'
> +if pointerFile.startswith(preamble):
> +pointerFile = pointerFile[len(preamble):]
> +
> +oidEntry = [i for i in pointerFile.split('\n') if 
> i.startswith('oid')]
> +oid = oidEntry[0].split(' ')[1].split(':')[1]

Why do we need to remove the preamble at all, if present? If all we
want is the oid, we should simply only look at the line that starts
with that keyword, which would skip any preamble. Which is what you
already do here. However, I'd probably use .splitlines() instead of
.split('\n') and .startswith('oid ') (note the trailing space) instead
of .startswith('oid') to ensure "oid" is a separate word.

But then again, I wonder why there's so much split() logic involved in
extracting the oid. Couldn't we replace all of that with a regexp like

oid = re.search(r"^oid \w+:(\w+)", pointerFile, re.MULTILINE).group(1)

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