Re: [Patch] improvement of "make rpm"

[Akio Takebe <takebe_akio-+CUm20s59erQFUHtdCDX3A@public.gmane.org>]

Hi, Avi

Sorry for my late responce, and thank you for your review.

>> --- kvm-49.orig/kvm.spec	2007-10-29 02:14:57.000000000 +0900
>> +++ kvm-49/kvm.spec	2007-10-31 23:24:05.000000000 +0900
>> @@ -1,3 +1,12 @@
>> +%define Distribution %(rpm -q -qf /etc/redhat-release --qf '%{name}' | 
>> cut -d - 
>> -f 1)
>> +%define os_release %(rpm -q --qf '%%{version}' /etc/redhat-release)
>> +%define bindir /usr/bin
>> +%define bin %{bindir}/kvm
>> +%define initdir /etc/init.d
>> +%define confdir /etc/kvm
>> +%define utilsdir /etc/kvm/utils
>> +%define _prebuilt %{?prebuilt:1}%{!?prebuilt:0}
>> +
>>   
>
>Why are you moving this around?  It makes the patch harder to review 
>because you can't immediately see if the code has changed.
>
It was for cleanup. I'll put it back.


>>  Name:           kvm
>>  Version:        0.0
>>  Release:        0
>> @@ -11,20 +20,8 @@ BuildRoot:      %{_tmppath}/%{name}-%{ve
>>  ExclusiveArch:  i386 x86_64
>>  
>>  Requires:	kvm-kmod bridge-utils
>> -
>> -%define Distribution %(cat /etc/redhat-release | awk '{ print $1}' | tr [
>> A-Z] 
>> [a-z])
>> -%define os_release %(rpm -q --qf '%%{version}' %{Distribution}-release)
>> -
>> -%if %{Distribution} == "fedora" && %{os_release} == 5 || %{Distribution} 
>> == 
>> "centos" && %{os_release} == 4
>> -BuildRequires: compat-gcc-32
>> -%else
>> -BuildRequires: compat-gcc-34
>> -%endif
>> -
>>  BuildRequires:  SDL-devel zlib-devel alsa-lib-devel
>>  
>> -%define _prebuilt %{?prebuilt:1}%{!?prebuilt:0}
>> -
>>  %if !%{_prebuilt}
>>  Source0: kvm.tar.gz
>>  Source1: user.tar.gz
>> @@ -32,15 +29,32 @@ Source2: kernel.tar.gz
>>  Source3: scripts.tar.gz
>>  Source4: Makefile
>>  Source5: configure
>> -Source5: kvm_stat
>> +Source6: kvm_stat
>>  %endif
>>  
>>  %description
>>  The Kernel Virtual Machine provides a virtualization enviroment for 
>> processors
>>  with hardware support for virtualization: Intel's VT and AMD's AMD-V.
>>  
>> -%prep
>> +%if %([ x"%{Distribution}" = x"fedora" -a x"%{os_release}" = x"5" ] && 
>> echo 1 |
>> | echo 0)
>> +%define fedora5 1
>> +%endif
>>   
>
>Why not use regular rpm conditionals instead of calling the shell?
>
The regular rpm conditionals are poor facilities.
Shell has better parsing facilities.
The original checking fedora5 and centos4 doesn't work on RHEL5.
So I changed the checking with shell.

>Can you add an explicit test for RHEL 5?
OK, I'll add it.

Best Reagards,

Akio Takebe


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/