Thanks Paul, I'll tidy up the indent before putback.
Any other reviewers out there? http://cr.opensolaris.org/~tekgrrl/FastCGI/
Amanda
Paul Cunningham wrote:
> 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.
>>>
>>
>
