Looks good to me from my quick skip through; I did notice another
missing indent on line 60 of install-sfw (no need review it)
paul
Amanda Waite wrote:
> Thanks, I've addressed all of your comments TMPDIR has become
> TMP_PROTO_DIR to avoid classes with the build environment and I've used
> it throughout ('cept for DESTDIR in the make install target).
>
> Update webrev at: http://cr.opensolaris.org/~tekgrrl/FastCGI/
>
> Thanks
>
> Amanda
>
>
>
> Paul Cunningham wrote:
>> Amanda,
>>
>> Mainly looks good, but see below ..
>>
>> Paul
>>
>> Amanda Waite wrote:
>>>
>>> Changes at http://cr.opensolaris.org/~tekgrrl/FastCGI/ see inline for
>>> comments.
>>>
>>>
>>> Paul Cunningham wrote:
>>
>> .. cut ...
>>
>>>> 2. usr/src/lib/fastcgi/Makefile.sfw
>>
>> .. cut ..
>>
>>>> Lines ...
>>>> 88 $(CCSMAKE) install
>>>> 94 $(CCSMAKE) install
>>>> why are these not installing into the ws proto area?
>>>
>>> Because of libtool. Believe me this is the only way to get libtool to
>>> behave correctly
>>
>> Okay I believe you :-)
>> I guess it must mangle something during the 'make install' stage
>>
>>>> Why not use the same name for TMPDIR and DESTDIR
>>>> (its confusing) throughout ... (and into install-sfw*)
>>>
>>> Different things mus confuse different people, I found it confusing
>>> when I made them the same. You're the voice of reason though so I've
>>> changed it.
>>
>> That's better :-) But maybe TMPDIR would have been a better throughout
>> (except the 'DESTDIR=' passed to 'make install'). Hope I'm not
>> confusing you.
>>
>>>>
>>>> 4. pkg copyright files (SUNW*/copyright)
>>>> You probably need to add the source owner copyright lines
>>>> at the top at the top of this, ie. those extracted from
>>>> the src code files in the unpacked tarballs.
>>>
>>> I've done this but if I'm going to ask others to do the same I'd
>>> really like to know where this instruction came from, is it
>>> documented somewhere? It does kind of make sense, but then I'm not a
>>> lawyer.
>>
>> Does SUNWfcgi-doc/copyright also need them? or some different ones
>>
>>
>> ..cut ..
>>
>> Also ...
>> 1. usr/src/lib/fastcgi/install-sfw
>> You may want to indent line ...
>> 107 _install N include/${file} ${INCDIR}/${file} 0444
>> for the do-done.
>>
>
--
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Business Unit
