Am Mittwoch, den 23.09.2009, 01:59 +0200 schrieb Andrea Florio: > Hi developers, i saw you added a spec file in lxdm source code. > That's a greate news and i hope to be albe to help on that way > (i am the maintainer of the official openSUSE lxde repo). > > Regariding the lxdm spec file any way i have some fix on that file. > since i cannot commit (and i have no reason to ask commit right > because i'm not a developer) i'm going to attach a diff file for the > spec you just provided, I hope that will be accepted (it's verbosly > commented) >
Hi Andrea, thanks for your work. You found a couple of issues and made some valid points. However I consider some of your changes a step into the wrong direction, because they are SUSE-specific, but a spec included in the source tarball should be generic. Distro's can still have their customized specs, but they should not be upstream. So please, please, nobody commit this changes without further discussion. > --- lxdm.spec.in 2009-09-23 01:45:11.000000000 +0200 > +++ new.lxdm.spec 2009-09-23 01:46:24.000000000 +0200 > @@ -1,11 +1,23 @@ > +# on SuSE based distros that's allow compilation > +# without root rights, on other distros that's just > +# a comment, you should keep it. > +# norootforbuild This tag is for the SUSE buildsys, so it should not be in the generic spec. BTW: Packages should *never* be built as root. Can you tell me one good reason to do so? > Name: lxdm > Version: @PACKAGE_VERSION@ > Release: 1 > Summary: Light weight X11 display manager > License: GPL > +# On SuSE that's not a valid RPM group, > +# that MAY change on different distros (not sure) > +# SuSE group is commented > +#Group: System/GUI/Others > + > Group: User Interface/X Here SUSE is the only Distro to not follow the rpm upstream and use different groups. "User Interface/X" is upstream, see http://rpm.org/gitweb?p=rpm.git;a=blob_plain;f=GROUPS BTW: IMO it should be "User Interface/Desktops" as we consider LXDE a desktop environment. > URL: http://lxde.sourceforge.net/ > -Source0: https://lxde.svn.sourceforge.net/svnroot/lxde/trunk/lxdm/ > +# the source should be an archive... > +#Source0: https://lxde.svn.sourceforge.net/svnroot/lxde/trunk/lxdm/ > +Source0: lxdm.tar.bz2 As long as it's only available from SVN we need to stick with that, but of course referencing the svn URL as source wont work. > BuildRoot: %{_tmppath}/%{name}-%{version}-%{release} > > @@ -18,13 +30,41 @@ > %setup -q > > %build > -%configure --prefix=/usr > +# always export $RPM_OPT_FLAGS > + > +export CFLAGS="$RPM_OPT_FLAGS" > +export CXXFLAGS="$RPM_OPT_FLAGS" This should be done by rpm, so it's not necessary to do it in the spec. In fact, you could accidentially overwrite flags set by the %configure macro. > +# %configure macro already pass "--prefix=%_prefix" > +# where prefix is /usr, if the user needs a different > +# _prefix that should be exported on spec file like: > +# %define _prefix /new/prefix > +# %configure --prefix=/usr > + > +%configure > + > +# a make macro should be used here > +# even a "job based" make if possible > +# %__make %{?jobs:-j%{jobs}} > + > +%__make I agree we should use macro if possible, but for a generic spec file we never know how they are defined on different systems. So a simple "make" is less specific and more secure. > %install > -rm -rf $RPM_BUILD_ROOT > -make DESTDIR=${RPM_BUILD_ROOT} install > +# Never delete $RPM_BUILD_ROOT without re-make it > +# that operation is already made by rpm itsealf > +# and manual intervention may breake the build > +# rm -rf $RPM_BUILD_ROOT > + > +# better tu use %makeinstall macro > +# make DESTDIR=${RPM_BUILD_ROOT} install > + > +%makeinstall Why use %makeinstall??? %makeinstall has a couple of downsides such as overwriting variables set by %configure, breaking libtool archives and so on. Please don't use it. > +# That's only required if translations are available > +# (.mo/.po/.gmo/ecc) files available, otherwise > +# that's not needed > > -%find_lang %{name} > +#%find_lang %{name} > > %clean > rm -rf $RPM_BUILD_ROOT > @@ -37,10 +77,13 @@ > > %postun > > -%files -f %{name}.lang > +# exactly as "%find_lang" macro > +#%files -f %{name}.lang > +%files Right, currently there are no locales and running find_lang will break the build. > %defattr (-,root,root-) > %doc AUTHORS NEWS README COPYING INSTALL Changelog > %dir %{_datadir}/lxdm/ > +%dir %{_sysconfdir}/lxdm/ > %{_bindir}/lxdm > %{_bindir}/lxdm-greeter-gtk > %{_sysconfdir}/lxdm/Xsession The last line should be %config(noreplace) %{_sysconfdir}/lxdm/Xsession to make sure custom configs don't get overwritten by package updates. > > Andrea Ok, let's try to sort out on which changes we all can agree and commit them to our packages. Kind regards, Christoph ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference _______________________________________________ Lxde-list mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/lxde-list
