Re: [Quilt-dev] dirname

[Andreas Gruenbacher]

Jean,

On Thursday, 25. May 2006 10:25, Jean Delvare wrote:
> 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?

Kind of, yes. I tried to get rid of all slashes at the end of paths, but both 
${x//\/\//\/} and ${x%%/} fail that.

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

I have fixed the code. (Sorry this message is delayed, it got stuck on my 
drafts folder.)

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

I suppose it was used a lot more frequently in older versions. It was really 
meant for performance originally.

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

It's not a big deal either way; I don't mind.

Andreas