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