Git development
 help / color / mirror / Atom feed
* [PATCH v2] rev-list-options.txt: update --all about HEAD
From: Nguyễn Thái Ngọc Duy @ 2017-02-08  6:06 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170207133850.14056-1-pclouds@gmail.com>

This is the document patch for f0298cf1c6 (revision walker: include a
detached HEAD in --all - 2009-01-16).

Even though that commit is about detached HEAD, as Jeff pointed out,
always adding HEAD in that case may have subtle differences with
--source or --exclude. So the document mentions nothing about the
detached-ness.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 v2 drops "detached".

 Documentation/rev-list-options.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 5da7cf5a8d..a02f7324c0 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -133,8 +133,8 @@ parents) and `--max-parents=-1` (negative numbers denote no upper limit).
 	for all following revision specifiers, up to the next `--not`.
 
 --all::
-	Pretend as if all the refs in `refs/` are listed on the
-	command line as '<commit>'.
+	Pretend as if all the refs in `refs/`, along with `HEAD`, are
+	listed on the command line as '<commit>'.
 
 --branches[=<pattern>]::
 	Pretend as if all the refs in `refs/heads` are listed
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* Re: [PATCH] dir: avoid allocation in fill_directory()
From: Duy Nguyen @ 2017-02-08  6:22 UTC (permalink / raw)
  To: René Scharfe; +Cc: Brandon Williams, Git List
In-Reply-To: <73ec9cc7-8a86-8ba9-90fd-a954fa031820@web.de>

On Wed, Feb 8, 2017 at 5:04 AM, René Scharfe <l.s.r@web.de> wrote:
> Pass the match member of the first pathspec item directly to
> read_directory() instead of using common_prefix() to duplicate it first,
> thus avoiding memory duplication, strlen(3) and free(3).

How about killing common_prefix()? There are two other callers in
ls-files.c and commit.c and it looks safe to do (but I didn't look
very hard).

> diff --git a/dir.c b/dir.c
> index 65c3e681b8..4541f9e146 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -174,20 +174,19 @@ char *common_prefix(const struct pathspec *pathspec)
>
>  int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
>  {
> -       char *prefix;
> +       const char *prefix;
>         size_t prefix_len;
>
>         /*
>          * Calculate common prefix for the pathspec, and
>          * use that to optimize the directory walk
>          */
> -       prefix = common_prefix(pathspec);
> -       prefix_len = prefix ? strlen(prefix) : 0;
> +       prefix_len = common_prefix_len(pathspec);
> +       prefix = prefix_len ? pathspec->items[0].match : "";

There's a subtle difference. Before the patch, prefix[prefix_len] is
NUL. After the patch, it's not always true. If some code (incorrectly)
depends on that, this patch exposes it. I had a look inside
read_directory() though and it looks like no such code exists. So, all
good.

>
>         /* Read the directory and prune it */
>         read_directory(dir, prefix, prefix_len, pathspec);
>
> -       free(prefix);
>         return prefix_len;
>  }
-- 
Duy

^ permalink raw reply

* Re: Request re git status
From: Duy Nguyen @ 2017-02-08  6:13 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Samuel Lijin, Phil Hord, Ron Pero, Git
In-Reply-To: <CA+P7+xoZHOtURfbBbHHTpC3DsGxaGOVToqmW5wTg2EniRpL-Cg@mail.gmail.com>

On Wed, Feb 8, 2017 at 2:18 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> Personally, I think that the fact that Git forces the user to think
> about it in terms of "oh I have to fetch" instead of that happening
> automatically, it helps teach the model to the user. If it happened in
> the background then the user might not be confronted with the
> distributed nature of the tool.

I agree. But I think there is some room for improvement. Do we know
when the last fetch of the relevant upstream is? If we do, and if it's
been "a while" (configurable), then we should make a note suggesting
fetching again in git-status.

This is not exactly my own idea. Gentoo's portage (i.e. friends with
apt-get, yum... if you're not familiar) also has this explicit "fetch"
operation, which is called sync. If you haven't sync'd in a while and
try to install new package, you get a friendly message (that helps me
a couple times).
-- 
Duy

^ permalink raw reply

* Re: The --name-only option for git log does not play nicely with --graph
From: Duy Nguyen @ 2017-02-08  6:24 UTC (permalink / raw)
  To: Davide Del Vento; +Cc: Git Mailing List
In-Reply-To: <CAMh-zaPdSGaDvQSiWx0p7zUmfDAFDWUyHkY4BTs=j85Ue65XnA@mail.gmail.com>

On Wed, Feb 8, 2017 at 6:11 AM, Davide Del Vento <ddvento@ucar.edu> wrote:
> `git log --graph  --name-only` works fine, but the name is not
> properly indented as it is for `git log --graph  --name-status`
>
> I tested this in git v1.9.1 the only one I have access at the moment

Confirmed still happens on master. --stat plays nicely with --graph
though, so it's probably just some small fixes somewhere in diff*.c.
-- 
Duy

^ permalink raw reply

* Re: Fwd: Possibly nicer pathspec syntax?
From: Duy Nguyen @ 2017-02-08  6:39 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.20.1702072112160.25002@i7.lan>

On Wed, Feb 8, 2017 at 12:12 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Two-patch series to follow.

glossary-content.txt update for both patches would be nice.
-- 
Duy

^ permalink raw reply

* Re: Request re git status
From: Jacob Keller @ 2017-02-08  6:30 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Samuel Lijin, Phil Hord, Ron Pero, Git
In-Reply-To: <CACsJy8CWJvwSnmdpMg=atu+M8=4ksrTAYTgZyF5U7JnOCnPAkg@mail.gmail.com>

On Tue, Feb 7, 2017 at 10:13 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Wed, Feb 8, 2017 at 2:18 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> Personally, I think that the fact that Git forces the user to think
>> about it in terms of "oh I have to fetch" instead of that happening
>> automatically, it helps teach the model to the user. If it happened in
>> the background then the user might not be confronted with the
>> distributed nature of the tool.
>
> I agree. But I think there is some room for improvement. Do we know
> when the last fetch of the relevant upstream is? If we do, and if it's
> been "a while" (configurable), then we should make a note suggesting
> fetching again in git-status.
>
> This is not exactly my own idea. Gentoo's portage (i.e. friends with
> apt-get, yum... if you're not familiar) also has this explicit "fetch"
> operation, which is called sync. If you haven't sync'd in a while and
> try to install new package, you get a friendly message (that helps me
> a couple times).
> --
> Duy

That seems reasonable.

Thanks,
Jake

^ permalink raw reply

* Re: "disabling bitmap writing, as some objects are not being packed"?
From: Duy Nguyen @ 2017-02-08  6:45 UTC (permalink / raw)
  To: David Turner; +Cc: Junio C Hamano, Git Mailing List, Jeff King
In-Reply-To: <1486515795.1938.45.camel@novalis.org>

On Wed, Feb 8, 2017 at 8:03 AM, David Turner <novalis@novalis.org> wrote:
> On Sat, 2016-12-17 at 14:50 +0700, Duy Nguyen wrote:
>> And we can't grep for fatal errors anyway. The problem that led to
>> 329e6e8794 was this line
>>
>>     warning: There are too many unreachable loose objects; run 'git
>> prune' to remove them.
>>
>> which is not fatal.
>
> So, speaking of that message, I noticed that our git servers were
> getting slow again and found that message in gc.log.
>
> I propose to make auto gc not write that message either. Any objections?

Does that really help? auto gc would run more often, but unreachable
loose objects are still present and potentially make your servers
slow? Should these servers run periodic and explicit gc/prune?
-- 
Duy

^ permalink raw reply

* Re: Request re git status
From: Samuel Lijin @ 2017-02-08  7:44 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Duy Nguyen, Phil Hord, Ron Pero, Git
In-Reply-To: <CA+P7+xqg9b+49P6bO65E0q4a87BkNL76XGiUjcMg+UmDcU=WPg@mail.gmail.com>

On Wed, Feb 8, 2017 at 12:30 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Tue, Feb 7, 2017 at 10:13 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Wed, Feb 8, 2017 at 2:18 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>>> Personally, I think that the fact that Git forces the user to think
>>> about it in terms of "oh I have to fetch" instead of that happening
>>> automatically, it helps teach the model to the user. If it happened in
>>> the background then the user might not be confronted with the
>>> distributed nature of the tool.
>>
>> I agree. But I think there is some room for improvement. Do we know
>> when the last fetch of the relevant upstream is? If we do, and if it's
>> been "a while" (configurable), then we should make a note suggesting
>> fetching again in git-status.
>>
>> This is not exactly my own idea. Gentoo's portage (i.e. friends with
>> apt-get, yum... if you're not familiar) also has this explicit "fetch"
>> operation, which is called sync. If you haven't sync'd in a while and
>> try to install new package, you get a friendly message (that helps me
>> a couple times).
>> --
>> Duy

Arch's pacman -S sync operation also has the -y flag, which updates
the local package databases, and can be used in conjunction with the
-u upgrade flag to upgrade repositories.

> That seems reasonable.
>
> Thanks,
> Jake

To be clear, I'm not advocating changing the *default* behavior of git
status; I agree that it wouldn't make sense. And although personally I
constantly update remotes manually (to the point where I abhor using
pull), I do think there's room to add an option to "fetch the
remote-tracking branch" to git status.

^ permalink raw reply

* Re: "disabling bitmap writing, as some objects are not being packed"?
From: David Turner @ 2017-02-08  8:24 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Junio C Hamano, Git Mailing List, Jeff King
In-Reply-To: <CACsJy8C81+D=UG4pZ4e+URQqKRCPG=5bLiCHbGCQamvE-2y2MQ@mail.gmail.com>

On Wed, 2017-02-08 at 13:45 +0700, Duy Nguyen wrote:
> On Wed, Feb 8, 2017 at 8:03 AM, David Turner <novalis@novalis.org> wrote:
> > On Sat, 2016-12-17 at 14:50 +0700, Duy Nguyen wrote:
> >> And we can't grep for fatal errors anyway. The problem that led to
> >> 329e6e8794 was this line
> >>
> >>     warning: There are too many unreachable loose objects; run 'git
> >> prune' to remove them.
> >>
> >> which is not fatal.
> >
> > So, speaking of that message, I noticed that our git servers were
> > getting slow again and found that message in gc.log.
> >
> > I propose to make auto gc not write that message either. Any objections?
> 
> Does that really help? auto gc would run more often, but unreachable
> loose objects are still present and potentially make your servers
> slow? Should these servers run periodic and explicit gc/prune?

At least pack files wouldn't accumulate.  This is the major cause of
slowdown, since each pack file must be checked for each object.

(And, also, maybe those unreachable loose objects are too new to get
gc'd, but if we retry next week, we'll gc them).



^ permalink raw reply

* Re: "disabling bitmap writing, as some objects are not being packed"?
From: Duy Nguyen @ 2017-02-08  8:37 UTC (permalink / raw)
  To: David Turner; +Cc: Junio C Hamano, Git Mailing List, Jeff King
In-Reply-To: <1486542299.1938.47.camel@novalis.org>

On Wed, Feb 8, 2017 at 3:24 PM, David Turner <novalis@novalis.org> wrote:
> On Wed, 2017-02-08 at 13:45 +0700, Duy Nguyen wrote:
>> On Wed, Feb 8, 2017 at 8:03 AM, David Turner <novalis@novalis.org> wrote:
>> > On Sat, 2016-12-17 at 14:50 +0700, Duy Nguyen wrote:
>> >> And we can't grep for fatal errors anyway. The problem that led to
>> >> 329e6e8794 was this line
>> >>
>> >>     warning: There are too many unreachable loose objects; run 'git
>> >> prune' to remove them.
>> >>
>> >> which is not fatal.
>> >
>> > So, speaking of that message, I noticed that our git servers were
>> > getting slow again and found that message in gc.log.
>> >
>> > I propose to make auto gc not write that message either. Any objections?
>>
>> Does that really help? auto gc would run more often, but unreachable
>> loose objects are still present and potentially make your servers
>> slow? Should these servers run periodic and explicit gc/prune?
>
> At least pack files wouldn't accumulate.  This is the major cause of
> slowdown, since each pack file must be checked for each object.
>
> (And, also, maybe those unreachable loose objects are too new to get
> gc'd, but if we retry next week, we'll gc them).

I was about to suggest a config option that lets you run auto gc
unconditionally, which, I think, is better than suppressing the
message. Then I found gc.autoDetach. If you set it to false globally,
I think you'll get the behavior you want.

On second thought, perhaps gc.autoDetach should default to false if
there's no tty, since its main point it to stop breaking interactive
usage. That would make the server side happy (no tty there).
-- 
Duy

^ permalink raw reply

* [PATCH] refs-internal.c: make files_log_ref_write() static
From: Nguyễn Thái Ngọc Duy @ 2017-02-08  9:45 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

Created in 5f3c3a4e6f (files_log_ref_write: new function - 2015-11-10)
but probably never used outside refs-internal.c

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/files-backend.c | 3 +++
 refs/refs-internal.h | 4 ----
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index c041d4ba21..6a0bf94bf1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -165,6 +165,9 @@ static struct ref_entry *create_dir_entry(struct files_ref_store *ref_store,
 					  const char *dirname, size_t len,
 					  int incomplete);
 static void add_entry_to_dir(struct ref_dir *dir, struct ref_entry *entry);
+static int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
+			       const unsigned char *new_sha1, const char *msg,
+			       int flags, struct strbuf *err);
 
 static struct ref_dir *get_ref_dir(struct ref_entry *entry)
 {
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 25444cf5b0..4c9215d33f 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -220,10 +220,6 @@ struct ref_transaction {
 	enum ref_transaction_state state;
 };
 
-int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
-			const unsigned char *new_sha1, const char *msg,
-			int flags, struct strbuf *err);
-
 /*
  * Check for entries in extras that are within the specified
  * directory, where dirname is a reference directory name including
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* Re: Non-zero exit code without error
From: Christian Couder @ 2017-02-08 10:07 UTC (permalink / raw)
  To: Serdar Sahin; +Cc: git
In-Reply-To: <CAL7ZE5xYVM6=C+SJLJ2HMFZ2gvuduw8p0UnS0RnBaXibj0mgDw@mail.gmail.com>

Hi,

On Tue, Feb 7, 2017 at 12:27 PM, Serdar Sahin <serdar@peakgames.net> wrote:
> Hi,
>
> When we execute the following lines, the exit code is 1, but it is
> unclear what is the reason of this exit code. Do you have any idea?
>
> git clone --mirror --depth 50 --no-single-branch
> git@github.hede.com:Casual/hodo-server.git

First, could you tell us the git version you are using on the client
and on the server, and if this a new problem with newer versions?
Also is the repos accessible publicly or is it possible to reproduce
on another repo?
And what happens using other protocols like HTTP/S?

> Cloning into bare repository 'hodo-server.git'...
> remote: Counting objects: 3371, done.
> remote: Compressing objects: 100% (1219/1219), done.
> remote: Total 3371 (delta 2344), reused 2971 (delta 2098), pack-reused 0
> Receiving objects: 100% (3371/3371), 56.77 MiB | 2.18 MiB/s, done.
> Resolving deltas: 100% (2344/2344), done.
>
> echo $?
> 0
>
> cd hodo-server.git/
>
> GIT_CURL_VERBOSE=1 GIT_TRACE=1  git fetch --depth 50 origin
> cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee
> 14:12:35.215889 git.c:350               trace: built-in: git 'fetch'
> '--depth' '50' 'origin' 'cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee'
> 14:12:35.217273 run-command.c:336       trace: run_command: 'ssh'
> 'git@github.hede.com' 'git-upload-pack '\''Casual/hodo-server.git'\'''
> 14:12:37.301122 run-command.c:336       trace: run_command: 'gc' '--auto'
> 14:12:37.301866 exec_cmd.c:189          trace: exec: 'git' 'gc' '--auto'
> 14:12:37.304473 git.c:350               trace: built-in: git 'gc' '--auto'
>
> echo $?
> 1

What happens if you just run 'git gc --auto' after that?

^ permalink raw reply

* Bug with fixup and autosquash
From: Ashutosh Bapat @ 2017-02-08 10:10 UTC (permalink / raw)
  To: git

Hi Git-developers,
First of all thanks for developing this wonderful scm tool. It's sooo versatile.

I have been using git rebase heavily these days and seem to have found a bug.

If there are two commit messages which have same prefix e.g.
yyyyyy This is prefix
xxxxxx This is prefix and message

xxxxxx comitted before yyyyyy

Now I commit a fixup to yyyyyy using git commit --fixup yyyyyy
zzzzzz fixup! This is prefix

When I run git rebase -i --autosquash, the script it shows me looks like
pick xxxxxx This is prefix and message
fixup zzzzzz fixup! This is prefix
pick yyyyyy This is prefix

I think the correct order is
pick xxxxxx This is prefix and message
pick yyyyyy This is prefix
fixup zzzzzz fixup! This is prefix

Is that right?

I am using
[ubuntu]git --version
git version 1.7.9.5

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

^ permalink raw reply

* Re: subtree merging fails
From: Stavros Liaskos @ 2017-02-08 10:42 UTC (permalink / raw)
  To: David Aguilar; +Cc: Samuel Lijin, git@vger.kernel.org
In-Reply-To: <20170207184437.c6uuuxcmhi434vbc@gmail.com>

@Samuel Lijin
I tried now and I get:
"merge: branch_name
- not something we can merge".
Maybe that is something easy to fix but currently I am using a
workaround script so I am not putting any more effort at this at the
moment.

@David Aguilar
That's true but the trailing slash is already there. This commands
looks promising. An update would be GREAT!

FYI that's the script I am using to address this problem:

#!/bin/bash

function initial {
  if git remote | grep -q lisa_remote
  then
    echo "Subtree delete & update"
    git checkout lisa_branch
    git pull
    git checkout master
    git merge --squash -s subtree --no-commit lisa_branch
    git merge --squash --allow-unrelated-histories -s subtree
--no-commit lisa_branch
  else
    echo "Add subtree"
    git remote add lisa_remote git@xxxxxxxx:lisa/lisa.git
    git fetch lisa_remote
    git checkout -b lisa_branch lisa_remote/master
    git checkout master
    git read-tree --prefix=lisaSubTree/ -u lisa_branch
    gitrm
    git rm --cached -r lisaSubTree/.gitignore
    git rm --cached -r lisaSubTree/*
  fi
}
initial



On Tue, Feb 7, 2017 at 7:44 PM, David Aguilar <davvid@gmail.com> wrote:
> On Tue, Feb 07, 2017 at 08:59:06AM -0600, Samuel Lijin wrote:
>> Have you tried using (without -s subtree) -X subtree=path/to/add/subtree/at?
>>
>> From the man page:
>>
>>           subtree[=<path>]
>>                This option is a more advanced form of subtree
>> strategy, where the strategy
>>                makes a guess on how two trees must be shifted to match
>> with each other when
>>                merging. Instead, the specified path is prefixed (or
>> stripped from the
>>                beginning) to make the shape of two trees to match.
>
> I'm not 100% certain, but it's highly likely that the subtree=<prefix>
> argument needs to include a trailing slash "/" in the prefix,
> otherwise files will be named e.g. "fooREADME" instead of
> "foo/README" when prefix=foo.
>
> These days I would steer users towards the "git-subtree" command in
> contrib/ so that users don't need to deal with these details.  It
> handles all of this stuff for you.
>
> https://github.com/git/git/blob/master/contrib/subtree/git-subtree.txt
>
> https://github.com/git/git/tree/master/contrib/subtree
>
> Updating the progit book to also mention git-subtree, in addition to the
> low-level methods, would probably be a good user-centric change.
> --
> David

^ permalink raw reply

* Re: Non-zero exit code without error
From: Serdar Sahin @ 2017-02-08 10:26 UTC (permalink / raw)
  To: Christian Couder; +Cc: git
In-Reply-To: <CAP8UFD18Sbqo-_ZVyYTJtwNaRc8bFSd0KEYQ1oRH7-G+xnJTJg@mail.gmail.com>

Hi Christian,


We are using a private repo (Github Enterprise). Let me give you the
details you requested.


On Git Server: git version 2.6.5.1574.g5e6a493

On my client: git version 2.10.1 (Apple Git-78)


I’ve tried to reproduce it with public repos, but couldn’t do so. If I
could get an error/log output, that would be sufficient.


I am also including the full output below. (also git gc)


MacOSX:test serdar$ git clone --mirror --depth 50 --no-single-branch
git@git.privateserver.com:Casual/code_repository.git

Cloning into bare repository 'code_repository.git'...

remote: Counting objects: 3362, done.

remote: Compressing objects: 100% (1214/1214), done.

remote: Total 3362 (delta 2335), reused 2968 (delta 2094), pack-reused 0

Receiving objects: 100% (3362/3362), 56.77 MiB | 1.83 MiB/s, done.

Resolving deltas: 100% (2335/2335), done.

MacOSX:test serdar$ cd code_repository.git/

MacOSX:code_repository.git serdar$ GIT_CURL_VERBOSE=1 GIT_TRACE=1  git
fetch --depth 50 origin cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee

13:23:15.648337 git.c:350               trace: built-in: git 'fetch'
'--depth' '50' 'origin' 'cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee'

13:23:15.651127 run-command.c:336       trace: run_command: 'ssh'
'git@git.privateserver.com' 'git-upload-pack
'\''Casual/code_repository.git'\'''

13:23:17.750015 run-command.c:336       trace: run_command: 'gc' '--auto'

13:23:17.750829 exec_cmd.c:189          trace: exec: 'git' 'gc' '--auto'

13:23:17.753983 git.c:350               trace: built-in: git 'gc' '--auto'

MacOSX:code_repository.git serdar$ echo $?

1

MacOSX:code_repository.git serdar$ GIT_CURL_VERBOSE=1 GIT_TRACE=1 git gc --auto

13:23:45.899038 git.c:350               trace: built-in: git 'gc' '--auto'

MacOSX:code_repository.git serdar$ echo $?

0

MacOSX:code_repository.git serdar$

On Wed, Feb 8, 2017 at 1:07 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> Hi,
>
> On Tue, Feb 7, 2017 at 12:27 PM, Serdar Sahin <serdar@peakgames.net> wrote:
>> Hi,
>>
>> When we execute the following lines, the exit code is 1, but it is
>> unclear what is the reason of this exit code. Do you have any idea?
>>
>> git clone --mirror --depth 50 --no-single-branch
>> git@github.hede.com:Casual/hodo-server.git
>
> First, could you tell us the git version you are using on the client
> and on the server, and if this a new problem with newer versions?
> Also is the repos accessible publicly or is it possible to reproduce
> on another repo?
> And what happens using other protocols like HTTP/S?
>
>> Cloning into bare repository 'hodo-server.git'...
>> remote: Counting objects: 3371, done.
>> remote: Compressing objects: 100% (1219/1219), done.
>> remote: Total 3371 (delta 2344), reused 2971 (delta 2098), pack-reused 0
>> Receiving objects: 100% (3371/3371), 56.77 MiB | 2.18 MiB/s, done.
>> Resolving deltas: 100% (2344/2344), done.
>>
>> echo $?
>> 0
>>
>> cd hodo-server.git/
>>
>> GIT_CURL_VERBOSE=1 GIT_TRACE=1  git fetch --depth 50 origin
>> cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee
>> 14:12:35.215889 git.c:350               trace: built-in: git 'fetch'
>> '--depth' '50' 'origin' 'cc086c96cdffe5c1ac78e6139a7a4b79e7c821ee'
>> 14:12:35.217273 run-command.c:336       trace: run_command: 'ssh'
>> 'git@github.hede.com' 'git-upload-pack '\''Casual/hodo-server.git'\'''
>> 14:12:37.301122 run-command.c:336       trace: run_command: 'gc' '--auto'
>> 14:12:37.301866 exec_cmd.c:189          trace: exec: 'git' 'gc' '--auto'
>> 14:12:37.304473 git.c:350               trace: built-in: git 'gc' '--auto'
>>
>> echo $?
>> 1
>
> What happens if you just run 'git gc --auto' after that?

^ permalink raw reply

* [PATCH RFC 0/2] Kill manual ref parsing code in worktree.c
From: Nguyễn Thái Ngọc Duy @ 2017-02-08 11:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Michael Haggerty,
	Nguyễn Thái Ngọc Duy

A hundred years ago I added this code because a "standalone" ref
parsing code was not available from refs.c and the file was going through
some heavy changes that refactoring its ref parsing code out was not
an option. I promised to kill this parse_ref() eventually. I'm
fulfilling it today (or soon).

I would really like to you double check the approach I'm using here
(using submodule interface for accessing refs from another worktree)
since that may be the way forward to fix the "gc losing objects" in
multi worktrees. I've given it lots of thoughts in the last 24 hours.
Still can't find any fundamental flaw...

Nguyễn Thái Ngọc Duy (2):
  refs.c: add resolve_ref_submodule()
  worktree.c: use submodule interface to access refs from another worktree

 branch.c   |  3 +-
 refs.c     | 20 +++++++++----
 refs.h     |  3 ++
 worktree.c | 99 +++++++++++++++-----------------------------------------------
 worktree.h |  2 +-
 5 files changed, 44 insertions(+), 83 deletions(-)

-- 
2.11.0.157.gd943d85


^ permalink raw reply

* [PATCH 1/2] refs.c: add resolve_ref_submodule()
From: Nguyễn Thái Ngọc Duy @ 2017-02-08 11:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Michael Haggerty,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170208113144.8201-1-pclouds@gmail.com>

This is basically the extended version of resolve_gitlink_ref() where we
have access to more info from the underlying resolve_ref_recursively() call.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs.c | 20 ++++++++++++++------
 refs.h |  3 +++
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index cd36b64ed9..02e35d83f3 100644
--- a/refs.c
+++ b/refs.c
@@ -1325,18 +1325,18 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
 				       resolve_flags, sha1, flags);
 }
 
-int resolve_gitlink_ref(const char *submodule, const char *refname,
-			unsigned char *sha1)
+const char *resolve_ref_submodule(const char *submodule, const char *refname,
+				  int resolve_flags, unsigned char *sha1,
+				  int *flags)
 {
 	size_t len = strlen(submodule);
 	struct ref_store *refs;
-	int flags;
 
 	while (len && submodule[len - 1] == '/')
 		len--;
 
 	if (!len)
-		return -1;
+		return NULL;
 
 	if (submodule[len]) {
 		/* We need to strip off one or more trailing slashes */
@@ -1349,9 +1349,17 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,
 	}
 
 	if (!refs)
-		return -1;
+		return NULL;
+
+	return resolve_ref_recursively(refs, refname, resolve_flags, sha1, flags);
+}
+
+int resolve_gitlink_ref(const char *submodule, const char *refname,
+			unsigned char *sha1)
+{
+	int flags;
 
-	if (!resolve_ref_recursively(refs, refname, 0, sha1, &flags) ||
+	if (!resolve_ref_submodule(submodule, refname, 0, sha1, &flags) ||
 	    is_null_sha1(sha1))
 		return -1;
 	return 0;
diff --git a/refs.h b/refs.h
index 9fbff90e79..74542468d8 100644
--- a/refs.h
+++ b/refs.h
@@ -88,6 +88,9 @@ int peel_ref(const char *refname, unsigned char *sha1);
  */
 int resolve_gitlink_ref(const char *submodule, const char *refname,
 			unsigned char *sha1);
+const char *resolve_ref_submodule(const char *submodule, const char *refname,
+				  int resolve_flags, unsigned char *sha1,
+				  int *flags);
 
 /*
  * Return true iff abbrev_name is a possible abbreviation for
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* [PATCH 0/2] Demonstrate a critical worktree/gc bug
From: Johannes Schindelin @ 2017-02-08 12:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

... and a half-working workaround for the auto-gc case.

This patch series really is just another attempt to prod Duy into fixing
this instead of dabbling with shiny new toys ;-)

Changes since "v0":

- split out the test case, both to make it easier for Duy to integrate
  it into the patch series that fixes the bug, as well as to provide a
  little more, very gentle pressure to demonstrate the severity of this
  problem.


Johannes Schindelin (2):
  worktree: demonstrate data lost to auto-gc
  worktree: demonstrate a half-working workaround for auto-gc

 t/t6500-gc.sh | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)


base-commit: 6e3a7b3398559305c7a239a42e447c21a8f39ff8
Published-As: https://github.com/dscho/git/releases/tag/gc-worktree-v1
Fetch-It-Via: git fetch https://github.com/dscho/git gc-worktree-v1

-- 
2.11.1.windows.1


^ permalink raw reply

* [PATCH] rev-parse --git-path: fix output when running in a subdirectory
From: Johannes Schindelin @ 2017-02-08 12:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

In addition to making git_path() aware of certain file names that need
to be handled differently e.g. when running in worktrees, the commit
557bd833bb (git_path(): be aware of file relocation in $GIT_DIR,
2014-11-30) also snuck in a new option for `git rev-parse`:
`--git-path`.

On the face of it, there is no obvious bug in that commit's diff: it
faithfully calls git_path() on the argument and prints it out, i.e. `git
rev-parse --git-path <filename>` has the same precise behavior as
calling `git_path("<filename>")` in C.

The problem lies deeper, much deeper. In hindsight (which is always
unfair), implementing the .git/ directory discovery in
`setup_git_directory()` by changing the working directory may have
allowed us to avoid passing around a struct that contains information
about the current repository, but it bought us many, many problems.

In this case, when being called in a subdirectory, `git rev-parse`
changes the working directory to the top-level directory before calling
`git_path()`. In the new working directory, the result is correct. But
in the working directory of the calling script, it is incorrect.

Example: when calling `git rev-parse --git-path HEAD` in, say, the
Documentation/ subdirectory of Git's own source code, the string
`.git/HEAD` is printed.

Side note: that bug is hidden when running in a subdirectory of a
worktree that was added by the `git worktree` command: in that case, the
(correct) absolute path of the `HEAD` file is printed.

In the interest of time, this patch does not go the "correct" route to
introduce a struct with repository information (and removing global
state in the process), instead this patch chooses to detect when the
command was called in a subdirectory and forces the result to be an
absolute path.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/git-path-in-subdir-v1
Fetch-It-Via: git fetch https://github.com/dscho/git git-path-in-subdir-v1

 builtin/rev-parse.c   | 6 +++++-
 t/t0060-path-utils.sh | 2 ++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index ff13e59e1d..f9d5762bf2 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -597,9 +597,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 		}
 
 		if (!strcmp(arg, "--git-path")) {
+			const char *path;
 			if (!argv[i + 1])
 				die("--git-path requires an argument");
-			puts(git_path("%s", argv[i + 1]));
+			path = git_path("%s", argv[i + 1]);
+			if (prefix && !is_absolute_path(path))
+				path = real_path(path);
+			puts(path);
 			i++;
 			continue;
 		}
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 444b5a4df8..790584fcc5 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -271,6 +271,8 @@ relative_path "<null>"		"<empty>"	./
 relative_path "<null>"		"<null>"	./
 relative_path "<null>"		/foo/a/b	./
 
+test_git_path "mkdir sub && cd sub && test_when_finished cd .. &&" \
+	foo "$(pwd)/.git/foo"
 test_git_path A=B                info/grafts .git/info/grafts
 test_git_path GIT_GRAFT_FILE=foo info/grafts foo
 test_git_path GIT_GRAFT_FILE=foo info/////grafts foo

base-commit: 6e3a7b3398559305c7a239a42e447c21a8f39ff8
-- 
2.11.1.windows.1

^ permalink raw reply related

* [PATCH 1/2] worktree: demonstrate data lost to auto-gc
From: Johannes Schindelin @ 2017-02-08 12:17 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <cover.1486556262.git.johannes.schindelin@gmx.de>

When gc --auto is called in the presence of worktrees, it fails to take
*all* reflogs into account when trying to retain reachable objects, and as
a consequence data integrity goes pretty much to hell.

This patch provides a test case in the hopes that this bug gets fixed,
after an initial attempt has stalled for eight months already.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t6500-gc.sh | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index 1762dfa6a3..e24a4fb611 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -67,5 +67,44 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre
 	test_line_count = 2 new # There is one new pack and its .idx
 '
 
+trigger_auto_gc () {
+	# This is unfortunately very, very ugly
+	gdir="$(git rev-parse --git-common-dir)" &&
+	mkdir -p "$gdir"/objects/17 &&
+	touch "$gdir"/objects/17/17171717171717171717171717171717171717 &&
+	touch "$gdir"/objects/17/17171717171717171717171717171717171718 &&
+	git -c gc.auto=1 -c gc.pruneexpire=now -c gc.autodetach=0 gc --auto
+}
+
+test_expect_failure 'gc respects refs/reflogs in all worktrees' '
+	test_commit something &&
+	git worktree add worktree &&
+	(
+		cd worktree &&
+		git checkout --detach &&
+		# avoid implicit tagging of test_commit
+		echo 1 >something.t &&
+		test_tick &&
+		git commit -m worktree-reflog something.t &&
+		git rev-parse --verify HEAD >../commit-reflog &&
+		echo 2 >something.t &&
+		test_tick &&
+		git commit -m worktree-ref something.t &&
+		git rev-parse --verify HEAD >../commit-ref
+	) &&
+	trigger_auto_gc &&
+	git rev-parse --verify $(cat commit-ref)^{commit} &&
+	git rev-parse --verify $(cat commit-reflog)^{commit} &&
+
+	# Now, add a reflog in the top-level dir and verify that `git gc` in
+	# the worktree does not purge that
+	git checkout --detach &&
+	echo 3 >something.t &&
+	test_tick &&
+	git commit -m commondir-reflog something.t &&
+	git rev-parse --verify HEAD >commondir-reflog &&
+	(cd worktree && trigger_auto_gc) &&
+	git rev-parse --verify $(cat commondir-reflog)^{commit}
+'
 
 test_done
-- 
2.11.1.windows.1



^ permalink raw reply related

* [PATCH 2/2] worktree: demonstrate a half-working workaround for auto-gc
From: Johannes Schindelin @ 2017-02-08 12:18 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <cover.1486556262.git.johannes.schindelin@gmx.de>

While worktrees are marked "experimental", there is really no alternative
to using them when trying to work on multiple patch series in parallel.

Sadly, there is still a bug that causes irretrievable data loss caused by
worktree's incomplete handling of reflogs (and of the multiple indices of
the various worktrees), as this developer can testify a dozen times over.

In the --auto case, we can install a hook that runs before-hand,
accumulates all the worktree-specific refs and reflogs and installs them
into a very special reflog in the common refs namespace. The only
purpose of this stunt is to let gc pick up on those refs and reflogs, of
course, and *not* ignore them.

Unfortunately, this still does not address the "git gc" case, but
hopefully a real fix will be here some time soon.

Also, if there are simply too many loose objects for Git's liking, it will
kick off auto-gc (including the hook, which takes a couple of seconds to
run) that subsequently only says that it won't do anything, so the next
Git operation will kick off another auto-gc (including the multi-second
hook run).

Needless to say: this patch is not actually intended for inclusion, but
instead tries to demonstrate the pain and the dear need for a real bug fix
(and not another 8 months of buggy Git).

So here is hoping to a timely fix. Cheers.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t6500-gc.sh | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index e24a4fb611..2f1e52825d 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -67,6 +67,30 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre
 	test_line_count = 2 new # There is one new pack and its .idx
 '
 
+test_expect_success 'install pre-auto-gc hook for worktrees' '
+	mkdir -p .git/hooks &&
+	write_script .git/hooks/pre-auto-gc <<-\EOF
+	echo "Preserving refs/reflogs of worktrees" >&2 &&
+	dir="$(git rev-parse --git-common-dir)" &&
+	refsdir="$dir/logs/refs" &&
+	rm -f "$refsdir"/preserve &&
+	ident="$(GIT_COMMITTER_DATE= git var GIT_COMMITTER_IDENT)" &&
+	(
+		find "$dir"/logs "$dir"/worktrees/*/logs \
+			-type f -exec cat {} \; |
+		cut -d" " -f1
+		find "$dir"/HEAD "$dir"/worktrees/*/HEAD "$dir"/refs \
+			"$dir"/worktrees/*/refs -type f -exec cat {} \; |
+		grep -v "^ref:"
+	) 2>/dev/null |
+	sort |
+	uniq |
+	sed "s/.*/& & $ident	dummy/" >"$dir"/preserve &&
+	mkdir -p "$refsdir" &&
+	mv "$dir"/preserve "$refsdir"/
+	EOF
+'
+
 trigger_auto_gc () {
 	# This is unfortunately very, very ugly
 	gdir="$(git rev-parse --git-common-dir)" &&
@@ -76,7 +100,7 @@ trigger_auto_gc () {
 	git -c gc.auto=1 -c gc.pruneexpire=now -c gc.autodetach=0 gc --auto
 }
 
-test_expect_failure 'gc respects refs/reflogs in all worktrees' '
+test_expect_success 'gc respects refs/reflogs in all worktrees' '
 	test_commit something &&
 	git worktree add worktree &&
 	(
-- 
2.11.1.windows.1

^ permalink raw reply related

* [PATCH 2/2] worktree.c: use submodule interface to access refs from another worktree
From: Nguyễn Thái Ngọc Duy @ 2017-02-08 11:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Michael Haggerty,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <20170208113144.8201-1-pclouds@gmail.com>

The patch itself is relatively simple: manual parsing code is replaced
with a call to resolve_ref_submodule(). The manual parsing code must die
because only refs/files-backend.c should do that. Why submodule here is
a more interesting question.

From an outside look, any .git/worktrees/foo is seen as a "normal"
repository. You can set GIT_DIR to it and have access to everything,
even shared things that are not literally inside that directory, like
object db or shared refs.

On top of that, linked worktrees point to those directories with ".git"
files. These two make a linked worktree's path "X" a "submodule" (*) (**)
because X/.git is a file that points to a repository somewhere.

As such, we can just linked worktree's path as a submodule. We just need
to make sure they are unique because they are used to lookup submodule
refs store.

Main worktree is a a bit trickier. If we stand at a linked worktree, we
may still need to peek into main worktree's HEAD, for example. We can
treat main worktree's path as submodule as well since git_path_submodule()
can tolerate ".git" dirs, in addition to ".git" files.

The constraint is, if main worktree is X, then the git repo must be at
X/.git. If the user separates .git repo far away and tell git to point
to it via GIT_DIR or something else, then the "main worktree as submodule"
trick fails. Within multiple worktree context, I think we can limit
support to "standard" layout, at least for now.

(*) The differences in sharing object database and refs between
submodules and linked worktrees don't really matter in this context.

(**) At this point, we may want to rename refs *_submodule API to
something more neutral, maybe s/_submodule/_remote/

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 branch.c   |  3 +-
 worktree.c | 99 +++++++++++++++-----------------------------------------------
 worktree.h |  2 +-
 3 files changed, 27 insertions(+), 77 deletions(-)

diff --git a/branch.c b/branch.c
index b955d4f316..db5843718f 100644
--- a/branch.c
+++ b/branch.c
@@ -354,7 +354,8 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref)
 	for (i = 0; worktrees[i]; i++) {
 		if (worktrees[i]->is_detached)
 			continue;
-		if (strcmp(oldref, worktrees[i]->head_ref))
+		if (worktrees[i]->head_ref &&
+		    strcmp(oldref, worktrees[i]->head_ref))
 			continue;
 
 		if (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),
diff --git a/worktree.c b/worktree.c
index d633761575..25e5bc9a3e 100644
--- a/worktree.c
+++ b/worktree.c
@@ -19,54 +19,24 @@ void free_worktrees(struct worktree **worktrees)
 	free (worktrees);
 }
 
-/*
- * read 'path_to_ref' into 'ref'.  Also if is_detached is not NULL,
- * set is_detached to 1 (0) if the ref is detached (is not detached).
- *
- * $GIT_COMMON_DIR/$symref (e.g. HEAD) is practically outside $GIT_DIR so
- * for linked worktrees, `resolve_ref_unsafe()` won't work (it uses
- * git_path). Parse the ref ourselves.
- *
- * return -1 if the ref is not a proper ref, 0 otherwise (success)
- */
-static int parse_ref(char *path_to_ref, struct strbuf *ref, int *is_detached)
-{
-	if (is_detached)
-		*is_detached = 0;
-	if (!strbuf_readlink(ref, path_to_ref, 0)) {
-		/* HEAD is symbolic link */
-		if (!starts_with(ref->buf, "refs/") ||
-				check_refname_format(ref->buf, 0))
-			return -1;
-	} else if (strbuf_read_file(ref, path_to_ref, 0) >= 0) {
-		/* textual symref or detached */
-		if (!starts_with(ref->buf, "ref:")) {
-			if (is_detached)
-				*is_detached = 1;
-		} else {
-			strbuf_remove(ref, 0, strlen("ref:"));
-			strbuf_trim(ref);
-			if (check_refname_format(ref->buf, 0))
-				return -1;
-		}
-	} else
-		return -1;
-	return 0;
-}
-
 /**
- * Add the head_sha1 and head_ref (if not detached) to the given worktree
+ * Update head_sha1, head_ref and is_detached of the given worktree
  */
-static void add_head_info(struct strbuf *head_ref, struct worktree *worktree)
+static void add_head_info(struct worktree *wt)
 {
-	if (head_ref->len) {
-		if (worktree->is_detached) {
-			get_sha1_hex(head_ref->buf, worktree->head_sha1);
-		} else {
-			resolve_ref_unsafe(head_ref->buf, 0, worktree->head_sha1, NULL);
-			worktree->head_ref = strbuf_detach(head_ref, NULL);
-		}
-	}
+	int flags;
+	const char *target;
+
+	target = resolve_ref_submodule(wt->path, "HEAD",
+				       RESOLVE_REF_READING,
+				       wt->head_sha1, &flags);
+	if (!target)
+		return;
+
+	if (flags & REF_ISSYMREF)
+		wt->head_ref = xstrdup(target);
+	else
+		wt->is_detached = 1;
 }
 
 /**
@@ -77,9 +47,7 @@ static struct worktree *get_main_worktree(void)
 	struct worktree *worktree = NULL;
 	struct strbuf path = STRBUF_INIT;
 	struct strbuf worktree_path = STRBUF_INIT;
-	struct strbuf head_ref = STRBUF_INIT;
 	int is_bare = 0;
-	int is_detached = 0;
 
 	strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
 	is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
@@ -91,13 +59,10 @@ static struct worktree *get_main_worktree(void)
 	worktree = xcalloc(1, sizeof(*worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
 	worktree->is_bare = is_bare;
-	worktree->is_detached = is_detached;
-	if (!parse_ref(path.buf, &head_ref, &is_detached))
-		add_head_info(&head_ref, worktree);
+	add_head_info(worktree);
 
 	strbuf_release(&path);
 	strbuf_release(&worktree_path);
-	strbuf_release(&head_ref);
 	return worktree;
 }
 
@@ -106,8 +71,6 @@ 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 head_ref = STRBUF_INIT;
-	int is_detached = 0;
 
 	if (!id)
 		die("Missing linked worktree name");
@@ -127,19 +90,14 @@ static struct worktree *get_linked_worktree(const char *id)
 	strbuf_reset(&path);
 	strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
 
-	if (parse_ref(path.buf, &head_ref, &is_detached) < 0)
-		goto done;
-
 	worktree = xcalloc(1, sizeof(*worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
 	worktree->id = xstrdup(id);
-	worktree->is_detached = is_detached;
-	add_head_info(&head_ref, worktree);
+	add_head_info(worktree);
 
 done:
 	strbuf_release(&path);
 	strbuf_release(&worktree_path);
-	strbuf_release(&head_ref);
 	return worktree;
 }
 
@@ -334,8 +292,6 @@ const struct worktree *find_shared_symref(const char *symref,
 					  const char *target)
 {
 	const struct worktree *existing = NULL;
-	struct strbuf path = STRBUF_INIT;
-	struct strbuf sb = STRBUF_INIT;
 	static struct worktree **worktrees;
 	int i = 0;
 
@@ -345,6 +301,10 @@ const struct worktree *find_shared_symref(const char *symref,
 
 	for (i = 0; worktrees[i]; i++) {
 		struct worktree *wt = worktrees[i];
+		const char *symref_target;
+		unsigned char sha1[20];
+		int flags;
+
 		if (wt->is_bare)
 			continue;
 
@@ -359,25 +319,14 @@ const struct worktree *find_shared_symref(const char *symref,
 			}
 		}
 
-		strbuf_reset(&path);
-		strbuf_reset(&sb);
-		strbuf_addf(&path, "%s/%s",
-			    get_worktree_git_dir(wt),
-			    symref);
-
-		if (parse_ref(path.buf, &sb, NULL)) {
-			continue;
-		}
-
-		if (!strcmp(sb.buf, target)) {
+		symref_target = resolve_ref_submodule(wt->path, symref, 0,
+						      sha1, &flags);
+		if ((flags & REF_ISSYMREF) && !strcmp(symref_target, target)) {
 			existing = wt;
 			break;
 		}
 	}
 
-	strbuf_release(&path);
-	strbuf_release(&sb);
-
 	return existing;
 }
 
diff --git a/worktree.h b/worktree.h
index 6bfb985203..5ea5e503fb 100644
--- a/worktree.h
+++ b/worktree.h
@@ -4,7 +4,7 @@
 struct worktree {
 	char *path;
 	char *id;
-	char *head_ref;
+	char *head_ref;		/* NULL if HEAD is broken or detached */
 	char *lock_reason;	/* internal use */
 	unsigned char head_sha1[20];
 	int is_detached;
-- 
2.11.0.157.gd943d85


^ permalink raw reply related

* Re: [PATCH 0/2] Demonstrate a critical worktree/gc bug
From: Duy Nguyen @ 2017-02-08 12:28 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <cover.1486556262.git.johannes.schindelin@gmx.de>

On Wed, Feb 8, 2017 at 7:17 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> ... and a half-working workaround for the auto-gc case.
>
> This patch series really is just another attempt to prod Duy into fixing
> this instead of dabbling with shiny new toys ;-)

FYI work is ongoing [1] [2]. If you want it even faster, go do it yourself.

[1] https://github.com/pclouds/git/commits/prune-in-worktrees-2
[2] https://public-inbox.org/git/20170208113144.8201-1-pclouds@gmail.com/
-- 
Duy

^ permalink raw reply

* Re: [PATCH 2/2] pathspec: don't error out on all-exclusionary pathspec patterns
From: Cornelius Weig @ 2017-02-08 13:23 UTC (permalink / raw)
  To: Linus Torvalds, Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.20.1702072113380.25002@i7.lan>

Again, as Duy pointed out this should be documented.

How about something like this:

diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index f127fe9..781cde3 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -387,7 +387,9 @@ Glob magic is incompatible with literal magic.
 exclude;;
        After a path matches any non-exclude pathspec, it will be run
        through all exclude pathspec (magic signature: `!` or `^`). If it
-       matches, the path is ignored.
+       matches, the path is ignored. If only exclude pathspec are given,
+       the exclusion is applied to the result set as if invoked without any
+       pathspec.
 --
 
 [[def_parent]]parent::

^ permalink raw reply related

* Re: [PATCH 1/2] pathspec magic: add '^' as alias for '!'
From: Cornelius Weig @ 2017-02-08 13:23 UTC (permalink / raw)
  To: Linus Torvalds, Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.20.1702072113040.25002@i7.lan>

As Duy pointed out, the glossary needs an update too.

For this one, the cange can be minimal I think:

diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index 8ad29e6..f127fe9 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -386,7 +386,7 @@ Glob magic is incompatible with literal magic.
 
 exclude;;
        After a path matches any non-exclude pathspec, it will be run
-       through all exclude pathspec (magic signature: `!`). If it
+       through all exclude pathspec (magic signature: `!` or `^`). If it
        matches, the path is ignored.
 --
 

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox