From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH 03/16] pkg: add recipe for RPM Date: Wed, 02 Apr 2014 11:01:49 +0200 Message-ID: <4232273.mks7Rollq1@xps13> References: <1391529271-24606-1-git-send-email-thomas.monjalon@6wind.com> <1391529271-24606-4-git-send-email-thomas.monjalon@6wind.com> <20140224165208.GF27503@x220.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev-VfR2kkLFssw@public.gmane.org To: Chris Wright Return-path: In-Reply-To: <20140224165208.GF27503-SwUeJysX96B82hYKe6nXyg@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" Hello, Sorry for the long delay. 2014-02-24 08:52, Chris Wright: > > pkg/rpm.spec | 143 > > This should be dpdk.spec Actually it should be dpdk-core.spec. Since it is a file hosted in the project, is it mandatory to have such naming? Could you explain why? When building it with "rpmbuild -ta dpdk.tar.gz", the .spec name has no importance. > > +ExclusiveArch: i686, x86_64 > > +%define target %{_arch}-default-linuxapp-gcc > > What is this for? Multi-targets builds are supported in DPDK Makefiles. So the target must be explicited when building. Is it OK to define such variable here? > > +%description > > +Dummy main package. Make only subpackages. > > I think the core package should have the .so, the -devel package w/ > headers, and no -static package. Including a .a file is discouraged in > Fedora OK > > +%package core-runtime > > perhaps just 'runtime' [...] > > +%package core-devel > > name this devel No, dpdk-core is a name to differentiate the main component and the extensions such as dpdk-memnic. So we should have dpdk-core and dpdk-core-devel. Do you agree? > > +# debuginfo packaging is broken > > +%define debug_package %{nil} > > Can you add a better comment why it's broken? Yes, I think it's due to direct use of ld linker. > > +make O=%{target} T=%{target} config > > Did you try to update the build system to output into $RPM_BUILD_ROOT > directly? It's much preferred compared to the cp/rmdir below Probably we could change the build system but it should be another patch. > > +%install > > +rm -rf %{buildroot} > > +make O=%{target} DESTDIR=%{destdir} > > Why make again (IOW, why not in %build section)? This one is for installing files only. Isn't it the role of %install? > > +mv %{destdir}/%{target}/kmod/*.ko %{buildroot}%{moddir} > > Normally I'd use install command (-d will create directories) Yes but here the strategy is to dispatch some files and keep the remaining ones in %{datadir}. Thanks for the review -- Thomas