Hi Paul, Marcel, all,

thanks for the review.

Here's an updated version with patch level 50,
(including another fix for sparc only) and with
the points below answered (see inline please):

   http://cr.opensolaris.org/~hnhn/bash_6780158.pl50/

Any comments are welcome.

Paul Cunningham wrote:
> This mainly looks okay to me, but see below for a few comments ...
> 
> Paul
> 
> Jan Hnatek wrote:
>>
>> please review my change to bring SUNWbash to the latest
>> official patch level in order to fix 6780158.
>>
>> Webrev:
>> http://cr.opensolaris.org/~hnhn/bash_6780158.pl49/
> 
> 1. usr/src/cmd/bash/Makefile.sfw
>    Line 22, change copyright year
Done.

> 
>    Line ..
>      28 VER=bash-3.2
>    you could change this to extract the info from the
>    METADATA file
Done.

> 
>    Patch level, you might want to add that into the METADATA in the
>    COMMENTS: field
Added to METADATA.

> 
>    Line ..
>     45    @find . -name core -exec rm -f {} \;
>    is this actually needed, if not delete it.
Removed.

> 
>    You could do these also ..
>     Roland Mainz wrote:
>     > use "env - ..." and not "env ..." in the Makefiles
>     > to make sure "configure"&&"make" only see the env
>     > variables they should really get
>     > (and not pick-up any random env variable).
>     > Use either $(SHELL) or /usr/bin/bash for "configure" calls
>     > (so we know which one is used and "configure" doesn't pick
>     > one itself)
Added, merged the env variables to $(TARGET_ENV), added
$(PATCH_LEVEL), since it's needed.

> 
>    You also could do ...
> 
>    Line ..
>     64             ./configure --prefix=/usr \
>    use the predefined value for prefix from Makefile.master,
>    eg ...
>       ./configure --prefix=$(CFGPREFIX) \
Sure.

> 
>    Define TARBALL=, and then use the now built in stuff to
>    unpack the tarball and apply the patches (see cups)
Thanks. There's a couple of renames and minor adjustments
to 2 patches, but the Makefile simplification is significant.

Regards,
hnhn

-- 
Jan Hnatek
jan.hnatek at sun.com




Reply via email to