On 2013-09-17 21.32, Johannes Sixt wrote:
> Am 17.09.2013 10:24, schrieb Jiang Xin:
>> I have checked the behavior of UNC path on Windows (msysGit):
>> * I can cd to a UNC path:
>>     cd //server1/share1/path
>> * can cd to other share:
>>     cd ../../share2/path
>> * and can cd to other server's share:
>>     cd ../../../server2/share/path
>> That means relative_path(path1, path2) support UNC paths out of the box.
>> We only need to check both path1 and path2 are UNC paths, or both not.
> Your tests are flawed. You issued the commands in bash, which (or rather
> MSYS) does everything for you that you need to make it work. But in
> reality it does not, because the system cannot apply .. to //server/share:
> $ git ls-remote //srv/public/../repos/his/setups.git
> fatal: '//srv/public/../repos/his/setups.git' does not appear to be a
> git repository
> fatal: Could not read from remote repository.
> Please make sure you have the correct access rights
> and the repository exists.
> even though the repository (and //srv/public, let me assure) exists:
> $ git ls-remote //srv/repos/his/setups.git
> bea489b0611a72c41f133343fdccbd3e2b9f80b5        HEAD
> ...
> The situation does not change with your latest round (v3).
> Please let me suggest not to scratch where there is no itch. ;) Your
> round v2 was good enough.
> If you really want to check UNC paths, then you must compare two path
> components after the the double-slash, not just one.
> Furthermore, you should audit all code that references
> is_absolute_path(), relative_path(), normalize_path_copy(), and possibly
> a few others whether the functions or call sites need improvement.
> That's worth a separate patch.
> -- Hannes

I tend to agree here.
The V2 patch fixed a regression.
This should be one commit on its own:
(1) Make separate commits for logically separate changes.

Fixing a bug is a good thing, thanks for working on this,

The support for UNC paths is a new feature, and this deserves a seperate commit.

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