Re: [PATCH v2 2/5] enter_repo: avoid duplicating logic, use is_git_directory() instead

2015-09-21 Thread Junio C Hamano
Duy Nguyen  writes:

> I know. I sent the re-roll before receiving this. I think I still
> haven't mentioned the impact on remote case. Another update coming,
> maybe next weekend.

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/5] enter_repo: avoid duplicating logic, use is_git_directory() instead

2015-09-14 Thread Duy Nguyen
On Sun, Sep 13, 2015 at 8:04 AM, Junio C Hamano  wrote:
> Duy Nguyen  writes:
>
>> On Mon, Sep 7, 2015 at 11:33 PM, Junio C Hamano  wrote:
>>> Nguyễn Thái Ngọc Duy   writes:
>>>
 Signed-off-by: Nguyễn Thái Ngọc Duy 
 ---
>>>
>>> The cover letter talks about "local clone", and in this entire
>>> series, I saw new tests only for the local case, but doesn't this
>>> and the next change also affect the case where a Git daemon or a
>>> upload-pack process is serving the remote repository?
>>>
>>> And if so, how is that case affected?
>>
>> People who serve .git-dir repos should not be affected (I think we
>> have enough test cases covering that). People can serve .git-file
>> repos as well, which is sort of tested in the local clone test case
>> because upload-pack is involved for providing remote refs, I think.
>
> Unfortunately, the above is still not unclear to me.
>
> Was serving from a linked repository working without these five
> patches, i.e. was the local case the only one that was broken and
> needed fixing with these five patches?  If so, the log message
> should mention that (i.e. "remote case was working OK but local was
> broken because ...; change this and that to make local one work as
> well").  If the remote case also was broken and fixed by these five
> patches, then that is also worth mentioning the same way.
>
> I didn't ask you to explain it to me in the first place in a
> response.  The review comment pointed out that the proposed log
> message was unclear and those who will be reading "git log" output
> need clearer description.

I know. I sent the re-roll before receiving this. I think I still
haven't mentioned the impact on remote case. Another update coming,
maybe next weekend.
-- 
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 2/5] enter_repo: avoid duplicating logic, use is_git_directory() instead

2015-09-12 Thread Junio C Hamano
Duy Nguyen  writes:

> On Mon, Sep 7, 2015 at 11:33 PM, Junio C Hamano  wrote:
>> Nguyễn Thái Ngọc Duy   writes:
>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>>> ---
>>
>> The cover letter talks about "local clone", and in this entire
>> series, I saw new tests only for the local case, but doesn't this
>> and the next change also affect the case where a Git daemon or a
>> upload-pack process is serving the remote repository?
>>
>> And if so, how is that case affected?
>
> People who serve .git-dir repos should not be affected (I think we
> have enough test cases covering that). People can serve .git-file
> repos as well, which is sort of tested in the local clone test case
> because upload-pack is involved for providing remote refs, I think.

Unfortunately, the above is still not unclear to me.

Was serving from a linked repository working without these five
patches, i.e. was the local case the only one that was broken and
needed fixing with these five patches?  If so, the log message
should mention that (i.e. "remote case was working OK but local was
broken because ...; change this and that to make local one work as
well").  If the remote case also was broken and fixed by these five
patches, then that is also worth mentioning the same way.

I didn't ask you to explain it to me in the first place in a
response.  The review comment pointed out that the proposed log
message was unclear and those who will be reading "git log" output
need clearer description.

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/5] enter_repo: avoid duplicating logic, use is_git_directory() instead

2015-09-12 Thread Duy Nguyen
On Mon, Sep 7, 2015 at 11:33 PM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>
> The cover letter talks about "local clone", and in this entire
> series, I saw new tests only for the local case, but doesn't this
> and the next change also affect the case where a Git daemon or a
> upload-pack process is serving the remote repository?
>
> And if so, how is that case affected?

People who serve .git-dir repos should not be affected (I think we
have enough test cases covering that). People can serve .git-file
repos as well, which is sort of tested in the local clone test case
because upload-pack is involved for providing remote refs, I think.
--
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 2/5] enter_repo: avoid duplicating logic, use is_git_directory() instead

2015-09-07 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---

The cover letter talks about "local clone", and in this entire
series, I saw new tests only for the local case, but doesn't this
and the next change also affect the case where a Git daemon or a
upload-pack process is serving the remote repository?

And if so, how is that case affected?

>  path.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/path.c b/path.c
> index a536ee3..7340e11 100644
> --- a/path.c
> +++ b/path.c
> @@ -441,8 +441,7 @@ const char *enter_repo(const char *path, int strict)
>   else if (chdir(path))
>   return NULL;
>  
> - if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
> - validate_headref("HEAD") == 0) {
> + if (is_git_directory(".")) {
>   set_git_dir(".");
>   check_repository_format();
>   return path;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 2/5] enter_repo: avoid duplicating logic, use is_git_directory() instead

2015-08-21 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 path.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/path.c b/path.c
index a536ee3..7340e11 100644
--- a/path.c
+++ b/path.c
@@ -441,8 +441,7 @@ const char *enter_repo(const char *path, int strict)
else if (chdir(path))
return NULL;
 
-   if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
-   validate_headref("HEAD") == 0) {
+   if (is_git_directory(".")) {
set_git_dir(".");
check_repository_format();
return path;
-- 
2.3.0.rc1.137.g477eb31

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