[asterisk-dev] [Code Review]: Allow spaces in standard paths and DESTDIR

[Tilghman Lesher]

> On July 21, 2011, 10:57 a.m., Kevin Fleming wrote:
> > /branches/1.8/Makefile, lines 607-610
> > <https://reviewboard.asterisk.org/r/1326/diff/1/?file=17574#file17574line607>
> >
> >     Why is this needed?

Can be removed.  It was part of my debugging.


> On July 21, 2011, 10:57 a.m., Kevin Fleming wrote:
> > /branches/1.8/Makefile, lines 611-613
> > <https://reviewboard.asterisk.org/r/1326/diff/1/?file=17574#file17574line611>
> >
> >     Is there a POSTINSTALL to clean up these symlinks?

There isn't, but we can.


> On July 21, 2011, 10:57 a.m., Kevin Fleming wrote:
> > /branches/1.8/Makefile, lines 717-720
> > <https://reviewboard.asterisk.org/r/1326/diff/1/?file=17574#file17574line717>
> >
> >     A number of unquoted references to DESTDIR here.

I'll get those in the next revision.


> On July 21, 2011, 10:57 a.m., Kevin Fleming wrote:
> > /branches/1.8/Makefile, line 515
> > <https://reviewboard.asterisk.org/r/1326/diff/1/?file=17574#file17574line515>
> >
> >     In my testing on the previous patch, this sort of thing did not work. $(wildcard) assumes that spaces in its arguments separate arguments, and as a result the function does not return the expected results. This needs to be *thoroughly* tested.

This one is fun.  Basically, what happens is that notdir trims everything before the final slash in each space separated argument.  The intended result is to obtain a list of headers with the leading path removed.  So what this does is add the path itself to the part to filter out, which if it contains a space, evaluates to two parts to filter out of the result.

For example, where ASTHEADERDIR=/Library/Application Support/Asterisk/Headers, filter-out additionally removes "Application" and "Headers".  Note that all legitimate files in this directory are named with a .h suffix.

I've tested this with both one space and two spaces in the path (one in DESTDIR, one in ASTHEADERDIR), and it works as expected.


- Tilghman


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1326/#review3909
-----------------------------------------------------------


On July 20, 2011, 7:40 p.m., Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1326/
> -----------------------------------------------------------
> 
> (Updated July 20, 2011, 7:40 p.m.)
> 
> 
> Review request for Asterisk Developers, Russell Bryant and Kevin Fleming.
> 
> 
> Summary
> -------
> 
> Support for spaces in standard paths was recently reverted, because spaces in DESTDIR were not supported.  This patch implements changes allowing spaces within both standard application paths (standard on Mac OS X), as well as within DESTDIR.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/Makefile 328881 
>   /branches/1.8/Makefile.moddir_rules 328881 
>   /branches/1.8/agi/Makefile 328881 
>   /branches/1.8/configure UNKNOWN 
>   /branches/1.8/configure.ac 328881 
>   /branches/1.8/include/asterisk/autoconfig.h.in 328881 
>   /branches/1.8/makeopts.in 328881 
>   /branches/1.8/sounds/Makefile 328881 
>   /branches/1.8/utils/Makefile 328881 
> 
> Diff: https://reviewboard.asterisk.org/r/1326/diff
> 
> 
> Testing
> -------
> 
> Verified that the install routines run correctly when there are spaces in both standard application paths, as well as in DESTDIR.
> 
> 
> Thanks,
> 
> Tilghman
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110721/fea14d25/attachment-0001.htm>