Attached is a combined diff of the files.
Comments to the inline comments are inline....

Doug

Laszlo (Laca) Peter wrote:
> Hi Doug,
>
> Looks good generally.  Some comments inline:
>
>   
>> Index: dbus.spec
>> ===================================================================
>> --- dbus.spec   (revision 10631)
>> +++ dbus.spec   (working copy)
>> @@ -20,6 +20,7 @@
>>  URL:          http://www.freedesktop.org/wiki/Software_2fdbus
>>  #owner:yippi date:2006-11-23 type:bug bugzilla:8391
>>  Patch1:       dbus-01-dbus-launch.diff
>> +Patch2:       dbus-02-have-atomic.diff
>>     
>
> You need to add patch comments.  I'll publish the patch comment
> spec later today.  In this case it'll be
>
> #owner:drdoug type:bug date:2007-02-23 bugzilla:xxxx
>
> and please file a bug in bugzilla.freedesktop.org
>   
Done.
>   
>> +./configure --prefix=%{_prefix}                                \
>> +            --includedir=%{_includedir}                \
>> +            --sysconfdir=%{_sysconfdir}                \
>> +            --libexecdir=%{_libexecdir}/%{_arch64}     \
>>     
>
> Hmm... what goes into libexecdir?  Is it something we need
> both 32-bit and 64-bit variants of?
>   
libexecdir removed. Not used....
>   
>> +            --libdir=%{_libdir}/%{_arch64}             \
>> +            --bindir=%{_bindir}/%{_arch64}             \
>>     
>
> Make sure we don't ship the 64-bit binaries, unless they are
> actually useful.
>   
Can't think why the 64-bit binaries would be useful. Thus removed...

>> +            --localstatedir=%{_localstatedir}          \
>> +            --with-dbus-user=root                      \
>> +            --with-dbus-daemondir=/usr/lib/%{_arch64}  \
>>     
>
> This looks wrong.  We want the 64-bit libs to talk to the
> same (32-bit) daemon.
>   
No problems. Fixed.
>> @@ -116,6 +170,11 @@
>>  %{_libdir}/libdbus*.so*
>>  %{_datadir}/man/*
>>  %{_datadir}/dbus-1/*
>> +%ifos solaris
>> +%ifarch amd64 sparcv9
>> +%{_libdir}/%{_arch64}/libdbus*.so*
>> +%endif
>> +%endif
>>  
>>  %files devel
>>  %defattr(-, root, root)
>> @@ -124,8 +183,17 @@
>>  %{_libdir}/dbus-1.0/*
>>  %{_libdir}/pkgconfig/*
>>  %{_libdir}/python?.?/vendor-packages/*
>> +%ifos solaris
>> +%ifarch amd64 sparcv9
>> +%{_libdir}/%{_arch64}/dbus-1.0/*
>> +%{_libdir}/%{_arch64}/pkgconfig/*
>> +%endif
>> +%endif
>>     
>
> The %files lists of the linux spec files are not used in the
> Solaris builds (only the ones in the SUNW spec files are), so
> no need to update these. 
>   
 Cool. Less work needed next time :)

> We don't ship .la files, they are evil.  Please delete them.
>   
Done.

Doug
-------------- next part --------------
A non-text attachment was scrubbed...
Name: combined.diff
Type: text/x-patch
Size: 12474 bytes
Desc: not available
URL: 
<http://mail.opensolaris.org/pipermail/jds-review/attachments/20070224/ec202e33/attachment.bin>

Reply via email to