Re: [viewvc-dev] r2674 (Fix annotate views using urllib.pathname2url) breaks annotate on windows

Jens Peters <[email protected]> Wed, 04 Jan 2012 21:13:51 +0100
Newsgroups gmane.comp.version-control.cvs.viewcvs.devel
Message-ID <[email protected]>
------=_Part_25470_2127544190.1325708088316
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

On 3-1-2012 22:11, Jens Peters wrote:
> On Tuesday 03 January 2012 15:03:20 C. Michael Pilato wrote:
>> On 01/03/2012 02:02 PM, Jens Peters wrote:
>>> On 3-1-2012 16:24, C. Michael Pilato wrote:
>>>> On 12/30/2011 05:08 AM, Jens Peters wrote:
>>>>> Hello there,
>>>>>
>>>>> While testing the current 1.1 branch on a windows box I could not
>>>>> see
>>>>> annotations. After some investigation it looks like r2675/r2674 is
>>>>> the
>>>>> reason. On windows the rootpath looks like '/d:/svn/repos'.
>>>>> Obviously
>>>>> urllib.pathname2url does not like a path segment like that.
>>>>>
>>>>> Unfortunately I don't know beter solution to your original issue :(
>>>>>
>>>>> Regards,
>>>>> Jens
>>>> Thanks for the report, Jens!  Man, I feel like I'm playing
>>>> whack-a-mole with that section of code...
>>>>
>>>> I've taken yet another crack it in r2707 (r2708 on the 1.1.x branch).
>>>> Could you give the code a spin in your Windows environment?
>>>>
>>>> Obviously I'd like annotations to work *at all* in Windows.  But I'm
>>>> also trying to ensure that they work when either or both of the
>>>> "on-disk path to the repository" and "versioned file path inside the
>>>> repository" have URI-unsafe characters (spaces, for example).
>>>>
>>>> Thanks!
>>> Hello Mike,
>>>
>>> Not yet, but applying a patch like attached made ik work.
>>>
>>> 'drive' already contains the ':' so no need to concatenate it manually.
>> Ah, right.  I botched that, for sure.
>>
>>> What
>>> do you had in mind with os.path.abspath, it concatenate the current
>>> working directory with the rootpath, thus in my case adding D:\svn as
>>> prefix again>
>>> :)
>> I'm assuming that Windows folks can/will configure their rootpaths in any of
>> several ways:
>>
>>     Path\\To\\Root (driveless, relative path)
>>     C:Path\\To\\Root (has a drive, but still relative)
>>     \\Path\\To\\Root (driveless, sorta-kinda-absolute)
>>     C:\\Path\\To\\Root (absolute path)
>>
>> And of course, they might be using '/' separators instead of '\\' for all of
>> those forms, too, just as you are.
>>
>> On Unix, of course, you can have
>>
>>     Path/To/Root (relative)
>>     /Path/To/Root (absolute)
>>
>> Now, as the relative path case is probably just begging for trouble (what's
>> it relative to?), maybe ViewVC should just run os.path.isabs() on those
>> suckers when parsing the configuration, and croak on (or ignore)
>> non-absolute rootpaths.  That would narrow things down to:
>>
>>     /Path/To/Root
>>     \\Path\\To\\Root
>>     C:/Path/To/Root
>>     C:\\Path\\To\\Root
>>
>>> May be os.path.normpath is what you want (Just based on documentation, I
>>> don't have any experience with it)?
>> os.path.normpath will normalize a path (removing "." and ".." components,
>> empty components, etc.), but won't ensure that it is absolute (which you
>> really must have for a path-to-URL transformation).  As I noted above
>> though, it might just make more sense overall to mandate that root paths be
>> absolute.
> Ah, I never even consired that relative paths could or should work ;). I would
> fully agree that only absolute paths should be allowed and anything that is
> just kind of absolute or less should result in an exception.
>
> Regarding my initial patch, just revert from normpath to abspath and do
> abspath before splitdrive should do it.
>
> Regards,
> Jens

Hello,

Thanks for committing r2710. The attached patch resolves the path issue 
for me. What do you think?

Regards,
Jens

------------------------------------------------------
http://viewvc.tigris.org/ds/viewMessage.do?dsForumId=4251&dsMessageId=2905999

To unsubscribe from this discussion, e-mail: [[email protected]].
------=_Part_25470_2127544190.1325708088316
Content-Type: text/plain; name=svn_repos.py.patch
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename=svn_repos.py.patch

SW5kZXg6IGxpYi92Y2xpYi9zdm4vc3ZuX3JlcG9zLnB5DQo9PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09DQotLS0gbGliL3Zj
bGliL3N2bi9zdm5fcmVwb3MucHkJKHJldmlzaW9uIDI3MTIpDQorKysgbGliL3ZjbGliL3N2bi9z
dm5fcmVwb3MucHkJKHdvcmtpbmcgY29weSkNCkBAIC0xMDgsOCArMTA4LDggQEANCiAKIAogZGVm
IF9yb290cGF0aDJ1cmwocm9vdHBhdGgsIHBhdGgpOgorICByb290cGF0aCA9IG9zLnBhdGguYWJz
cGF0aChyb290cGF0aCkKICAgZHJpdmUsIHJvb3RwYXRoID0gb3MucGF0aC5zcGxpdGRyaXZlKHJv
b3RwYXRoKQotICByb290cGF0aCA9IG9zLnBhdGguYWJzcGF0aChyb290cGF0aCkKICAgaWYgb3Mu
c2VwICE9ICcvJzoKICAgICByb290cGF0aCA9IHN0cmluZy5yZXBsYWNlKHJvb3RwYXRoLCBvcy5z
ZXAsICcvJykKICAgcm9vdHBhdGggPSB1cmxsaWIucXVvdGUocm9vdHBhdGgpCg==
------=_Part_25470_2127544190.1325708088316--