Re: [Quilt-dev] dirname

[Jean Delvare]

Hi John,

> In order to workaround the msys bug [1] [2], Andreas suggested that
> msys users could set QUILT_PC to ".pc/" instead of ".pc".  This works
> well except in fold, where mkdir (line 95) does the wrong thing when
> the dirname removes all consecutive slashes [3], resulting in a path
> like .pcpatch1.diff/dir instead of .pc/patch1.diff/dir.  Backing that
> patch out fixes this problem, but I suspect more changes are required
> before dirname does the right thing.
> (...)
> 3. 
> http://cvs.sv.nongnu.org/viewcvs/quilt/quilt/scripts/Attic/patchfns.in?r1=1.9&r2=1.10

This change looks wrong to me in the first place. Removing all double
slashes from a path doesn't make any sense. I would guess that the
original intent was to replace double slashes with single slashes.
Unfortunately, there is no log message for this change so this is only
a guess. Andreas?

We should either revert that change, or fix the code to do the right
thing.

> As an aside, dirname isn't heavily used, or used in a performance
> critical loop (afaics) -- running the entire test suite invokes it 148
> times.  Is it worth keeping in patchfns ?  If it exist for
> compatibility reasons, can I write a few tests and move it to
> compat/dirname.in?

The reason why they were created in the first place isn't clearly
documented, but I indeed suppose that this was for compatibility.
Andreas?

I second your point on performance, basename and dirname being internal
functions or external programs really makes no (measurable) difference.

It it probably worth noting that /usr/bin/dirname does _not_ "simplify"
double slashes. Now, the quilt function didn't either (not properly at
least) so it's probably not a required feature after all. The test
suite doesn't cover this case.

Moving these functions to the compatibility layer and only using them
where needed is OK with me.

-- 
Jean Delvare