> > 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? >
i think you are wrog here, as i wrote on the comment, ONLY an (open)SUSE box will take care of it, any other distro will consider it like a regular comment on the spec file, so no demages, only advantages. >> 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 > i agree here, but in the same way, my change has been wrote as a comment, so again, no real changes on spec file. Rreally, if i would like to have both we can easly workaround like that: %if 0%suse_version Group: SUSE-GROUP %else Group: RPM-GROUP %endif > 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. > so here we agree, the best way will be to have Source0: http://......./lx...@[email protected] where the url will be changed with the url of the upstream stable released tarball. (better a tar.bz2 than a tar.gz to made src.rpm smaller) >> 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. > that sound like a news for me, actually on what i know, rpm do not automatically export RPM_OPT_FLAGS (rpmlint infact perform a check on that) in particular that needs to be checked package by package, any way, lxdm now use only "-g -O2" that are inside RPM_OPT_FLAGS (to clarify that to non-rpm-packagers: RPM_OPT_FLAGS: "-fmessage-length=0 -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables -fasynchronous-unwind-tables -g" ) >> +# %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 >> + i really believe here on that change, also because using "%define _prefix /new/prefix" will allow to only change on line... example: we know that (where default prefix is /usr): %_datadir = %_prefix/share --> /usr/share %_bindir = %_prefix/bin --> /usr/bin if we use "%define _prefix /usr/local": %_datadir = %_prefix/share --> /usr/local/share %_bindir = %_prefix/bin --> /usr/local/bin in particular we edit only one line. Passing --prefix=/usr/local instead we'll need to change ALL the %files section of the spec file. >> +# 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. > agree here, in particular any way, my change is still a comment, so no real changes on spec file >> %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. > again that's a news for me, %makeinstall is a short form for the following (from default /usr/lib/rpm/macros) : %makeinstall \ make \\\ prefix=%{?buildroot:%{buildroot}}%{_prefix} \\\ exec_prefix=%{?buildroot:%{buildroot}}%{_exec_prefix} \\\ bindir=%{?buildroot:%{buildroot}}%{_bindir} \\\ sbindir=%{?buildroot:%{buildroot}}%{_sbindir} \\\ sysconfdir=%{?buildroot:%{buildroot}}%{_sysconfdir} \\\ datadir=%{?buildroot:%{buildroot}}%{_datadir} \\\ includedir=%{?buildroot:%{buildroot}}%{_includedir} \\\ libdir=%{?buildroot:%{buildroot}}%{_libdir} \\\ libexecdir=%{?buildroot:%{buildroot}}%{_libexecdir} \\\ localstatedir=%{?buildroot:%{buildroot}}%{_localstatedir} \\\ sharedstatedir=%{?buildroot:%{buildroot}}%{_sharedstatedir} \\\ mandir=%{?buildroot:%{buildroot}}%{_mandir} \\\ infodir=%{?buildroot:%{buildroot}}%{_infodir} \\\ install on other words %makeinstall = make DESTDIR=${RPM_BUILD_ROOT} install if i'm wrong here, please let me know. >> +# 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. > so we agree here. >> %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. > i do not agree here. Xsession is not a config file, like lxdm.conf. in particular it defines new (and actually iproved upate by update) setting to allow to succesfull start an xsession if default xinitrc script is not found. anyway, following rpm direction, only NON executable config files should be added as %config. instead i think that %config %{_sysconfdir}/lxdm/lxdm.conf should be %config(noreplace) %{_sysconfdir}/lxdm/lxdm.conf >> >> Andrea > > Ok, let's try to sort out on which changes we all can agree and commit > them to our packages. > > Kind regards, > Christoph > i love to exachange our toughts so we can both became better packagers. Regars Andrea -- ------------------------------------------ Andrea Florio QSI International School of Brindisi Sys Admin openSUSE-Education Administrator openSUSE Official Member (anubisg1) Email: [email protected] Packman Packaging Team Email: [email protected] Web: http://packman.links2linux.org/ Cell: +39-328-7365667 ------------------------------------------
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ 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
