Re: [PATCH v2 2/4] git-p4: create new method gitRunHook
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 6, 2020, 18:26 UTC
- Message-ID
- <xmqq8slffu7y.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <9cf8790e-6d13-4653-cddc-f5328f22b0df@gmail.com>
Ben Keene <seraphire@gmail.com> writes:
Show 13 quoted lines
> On 2/5/2020 4:42 PM, Junio C Hamano wrote:
>> Ben Keene <seraphire@gmail.com> writes:
>>
>>>>> + hooks_path = os.path.join(os.environ.get("GIT_DIR", ".git"), "hooks")
>>>> This assumes that the process when his function is called (by the
>>>> way, even though the title of the patch uses the word "method", this
>>>> is not a method but a function, no?), it is always at the top level
>>>> of the working tree. Is that a good assumption? I don't know the
>>>> code well, so "yes it is good because a very early thing we do is to
>>>> go up to the top" is a good answer.
>>> ...
> As best as I understand the mechanics, and I haven't examined the
> source code of git, this is just experimental,...As I already said that I do not know the code well, it is useless for you to also speculate. One of us must read the code before speaking further ;-)
I just scanned "def main()", and it seems that it always goes to the top-level of the working tree by doing chdir(cdup), where cdup is learned from "git rev-parse --show-cdup", i.e. "tell me how to chdir up to the top-level of the working tree".
I am assuming that nobody runs "git p4" in a bare repository, so under that assumption, I think it would be safe to say that we can assume we are always at the top. Also, GIT_DIR is exported from there, so it probably is a good idea to make sure that the run-hook helper just uses os.environ.get("GIT_DIR") and barfs if the environ is not set (i.e. there is something funny going on) without pretending that it is prepared to deal with such a case, which is what the "[, default]" parameter to .get method is doing.
I.e.
hooks_path = os.path.join(os.environ["GIT_DIR"], "hooks")
> Does this prompt the need to search the hierarchy if we don't find > the directory?
No, we just saw that it is done early in "def main()". It is done by "rev-parse --git-dir" and "rev-parse --show-cdup".