Vector,
See below ....
Vector Li wrote:
>
> Thanks so much for your review and comments.
That's okay :-)
>> 3. usr/src/cmd/mpg123/install-sfw
>> You could have put these dirs ...
>> 43 [[ -d ${MPG123LIBDIR} ]] || mkdir -m 0755 ${MPG123LIBDIR}
>> 44 [[ -d ${MPG123LIBDIR} ]] || mkdir -m 0755 ${PKGCONFIGDIR}
>> in Targetdirs
>>
> sorry, not sure what you mean.
> Does what you mean look like this?
> Targedirs="${MPG123LIBDIR} ${MPG123LIBDIR}"
> for i in $Targedirs; do
> [[ -d $i ]] || mkdir -m 0755 $i
> done
>
> Please correct me and I will update the codes, thx!
What I meant was that, the directories ${MPG123LIBDIR} & ${PKGCONFIGDIR}
could be put into the file Targetdirs ..
"http://src.opensolaris.org/source/xref/sfw/usr/src/Targetdirs";
and the above lines removed. I hope that is clearer.
>> Do these need the 777 ...
>> 64 _install L libmpg123.so.0.11.2 libmpg123.so.0 777
>> 65 _install L libmpg123.so.0.11.2 libmpg123.so 777
>>
> I have taken a look at the files under proto/root_i386/usr/lib,
> the mod of all soft link files is 0777.
> e.g.
> lrwxrwxrwx 1 hl198248 staff 9 Feb 16 00:40 libz.so -> libz.so.1
> So I keep their mod 777 here.
but the macro _install for the L option only requires the _type & _src
options see ...
"http://src.opensolaris.org/source/xref/sfw/usr/src/tools/install.subr";.
> Please take a look again, thanks!
> webrev at:
> http://cr.opensolaris.org/~vector/mpg123/
other than the above, everything else looks okay to me (on my quick skip
through).
Paul
--
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Business Unit
