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

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

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

To unsubscribe from this discussion, e-mail: [[email protected]].