Duy Nguyen <pclo...@gmail.com> writes:

> On Wed, Feb 26, 2014 at 5:44 AM, Junio C Hamano <gits...@pobox.com> wrote:
>> Nguyễn Thái Ngọc Duy  <pclo...@gmail.com> writes:
>>
>>> git_path() soon understands the path given to it. Some paths "abc" may
>>> become "def" while other "ghi" may become "ijk". We don't want
>>> git_path() to interfere with .lock path construction. Concatenate
>>> ".lock" after the path has been resolved by git_path() so if "abc"
>>> becomes "def", we'll have "def.lock", not "ijk".
>>
>> Hmph.  I am not sure if the above is readable (or if I am reading it
>> correctly).
>
> Definitely not now that I have had my break from the series and reread it.
>
>> If "abc" becomes "def", it would take deliberate work to make
>> "abc.lock" into "ijk", and it would be natural to expect that
>> "abc.lock" would become "def.lock" without any fancy trick, no?
>
> A better explanation may be, while many paths are not converted by
> git_path() ("abc" -> "abc"), some of them will be based on the given
> path ("def" -> "ghi"). Giving path def.lock to git_path() may confuse
> it and make it believe def.lock should not be converted because the
> signature is "def.lock" not "def". But we want the lock file to have
> the same base name with the locked file (e.g. "ghi.lock", not
> "def.lock"). So it's best to append ".lock" after git_path() has done
> its conversion.
>
> The alternative is teach git_path about ".lock", but I don't really
> want to put more logic down there.

I think your attempt to sound generic by using "abc", "def",
etc. backfired and made the description only obscure.  It would have
been immediately understandable if it were more like this:

        Among pathnames in $GIT_DIR, e.g. "index" or "packed-refs",
        we want to automatically and silently map some of them to
        the $GIT_DIR of the repository we are borrowing from via
        $GIT_COMMON_DIR mechanism.  When we formulate the pathname
        for its lockfile, we want it to be in the same location as
        its final destination.  "index" is not shared and needs to
        remain in the borrowing repository, while "packed-refs" is
        shared and needs to go to the borrowed repository.

        git_path() could be taught about the ".lock" suffix and map
        "index.lock" and "packed-refs.lock" the same way their
        basenames are mapped, but instead the caller can help by
        asking where the basename (e.g. "index") is mapped to
        git_path() and then appending ".lock" after the mapping is
        done.

Thanks for an explanation.  With that understanding, the patch makes
sense.

        
--
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

Reply via email to