search

rpm-software-management / spec-cleaner

spec-cleaner
BSD 3-Clause "New" or "Revised" License
28 stars 34 forks source link

Group settings in %if sections are handled incorrectly #44

Closed weberhofer closed 9 years ago

weberhofer commented 9 years ago

The following block in a spec file...

%if 0%{?suse_version} Group: Productivity/Networking/Email/Servers %else Group: System Environment/Daemons %endif

...results into the incorrect block:

Group: System Environment/Daemons %if 0%{?suse_version} %else %endif

IMHO it should not be touched at all.

scarabeusiv commented 9 years ago

It is not exactly bad, it just has logic that there is supposed to be only one Group definition (and later on finds empty conditional it has nothing to work for :)), and as it is not used anywhere in application except rpm info I was too lazy to tweak that. Other that are one definition only are Url and something else I don't remember.

weberhofer commented 9 years ago

In this particular case the disadvantange is that a package that is cross-distribution build-able looses it's (correct) group-assignements as soon as it is sent to factory. I do not see any problem setting only one URL, too.

scarabeusiv commented 9 years ago

Actually I have problem. It seems the minimal case can't reproduce it with current version. Could you point me to full spec it broke so I can put it into spec-cleaner testsuite?

weberhofer commented 9 years ago

Thank you for having a look at it; I append it here as I can not attach it:

#
# spec file for package qmailadmin
#
# Copyright (c) 2015 SUSE LINUX GmbH, Nuernberg, Germany.
#
# All modifications and additions to the file contributed by third parties
# remain the property of their copyright owners, unless otherwise agreed
# upon. The license for this file, and modifications and additions to the
# file, is the same license as for the pristine package itself (unless the
# license for the pristine package is not an Open Source License, in which
# case the license is the MIT License). An "Open Source License" is a
# license that conforms to the Open Source Definition (Version 1.9)
# published by the Open Source Initiative.

# Please submit bugfixes or comments via http://bugs.opensuse.org/
#

%define     sname       qmailadmin

%if 0%{?spambox:1}
Name:       qmailadmin-spambox
Provides:   qmailadmin = %{version}
Summary:    Web Administration for qmail/vpopmail with Spambox suuport
%else
Name:       qmailadmin
Conflicts:  qmailadmin-spambox
Summary:    Web Administration for qmail/vpopmail
%endif
License:        GPL-2.0
%if 0%{?suse_version}
Group:          Productivity/Networking/Email/Servers
%else
Group:          Networking/Other
%endif
Version:        1.2.15
Release:        0
Url:            http://www.inter7.com/qmailadmin
Source0:        qmailadmin-%{version}.tar.bz2
Source1:        help.tar.bz2
Source2:        qmailadmin-rpmlintrc
Source3:        qmailadmin.permissions
Source4:        qmailadmin.conf
Patch0:         qmailadmin_lang_de.diff
Patch1:         mailinglist.c.ezmlm7.patch
Patch2:         header.html.patch

BuildRoot:      %{_tmppath}/%{name}-%{version}-build

Requires:       ezmlm-idx
Requires:       maildrop
Requires:       qmail
Requires:       qmail-autorespond
Requires:       vpopmail
BuildRequires:  ezmlm-idx
BuildRequires:  qmail-skel-devel
BuildRequires:  vpopmail-devel

Obsoletes:      qmailadmin-toaster < %{version}
Provides:       qmailadmin-toaster = %{version}

%define     tdir %{_datadir}/toaster
%define     vdir /home/vpopmail
%define     qdir /var/qmail

%if 0%{?suse_version}
Requires:       apache2
PreReq:         permissions
BuildRequires:  apache2
BuildRequires:  automake
%define     apache_confd    %{_sysconfdir}/apache2/conf.d
%else
%define     apache_confd    %{_sysconfdir}/httpd/conf.d
%endif

%if 0%{?rhel_version} || 0%{?centos_version} || 0%{?fedora_version}
Requires:       httpd
BuildRequires:  httpd
%endif

%if 0%{?mandriva_version}
Requires:       apache-base
BuildRequires:  apache-base
%endif

%description
QmailAdmin is a free software package that provides a web interface
for managing a  qmail  system with virtual domains. This version is
for use with the vpopmail program. It provides admin for
adding/deleting users, Aliases, Forwards, Mailing lists and
Autoresponders.

%prep
%setup -q -n %{sname}-%{version}
%patch0
%patch1
%patch2

cp %{SOURCE1} help.tar.bz2
tar xfj help.tar.bz2
rm -f help.tar.bz2

#dont change owner to allow builds without root
%{__sed} -i -e 's/-o @vpopuser@//g ; s/-g @vpopgroup@//g' Makefile.am

%build
%define cflags %(echo %{optflags} | sed -e 's/$/ -fPIE/' )
%define ldflags %(echo %{optflags} | sed -e 's/$/ -pie/' )

export CFLAGS="%{cflags}"
export LDFLAGS="%{ldflags}"

%{__aclocal}
%{__autoconf}
%{__autoheader}
%{__automake}
%{configure} \
 --datadir=%{_datadir}/%{sname} \
 --enable-htmllibdir=%{_datadir}/%{sname} \
 --enable-htmldir=%{_datadir}/%{sname} \
 --enable-cgibindir=%{_datadir}/%{sname} \
 --enable-imageurl=/%{sname}/images \
 --enable-imagedir=%{_datadir}/%{sname}/images \
 --enable-cgipath=/qmailadmin/index.cgi \
 --enable-vpopmaildir=%{vdir} \
 --enable-qmaildir=%{qdir} \
 --enable-ezmlmdir=%{_bindir} \
 --enable-autoresponder-path=%{_bindir} \
 --enable-vpopuser=vpopmail \
 --enable-vpopgroup=vchkpw \
 --enable-maxusersperpage=12 \
 --enable-maxaliasesperpage=12 \
 --enable-modify-quota=y \
 --enable-ezmlm-mysql=n \
 --enable-help=y \
%if 0%{?spambox:1}
 --enable-modify-spam \
 --enable-spam-command="|/var/qmail/bin/preline /usr/bin/maildrop -A 'Content-Filter: maildrop-toaster' /etc/mail/mailfilter" \
%endif
--enable-domain-autofill=n

%{__make}

%install
mkdir -p %{buildroot}%{_sysconfdir}/permissions.d
install -m644 %{SOURCE2} %{buildroot}%{_sysconfdir}/permissions.d/qmailadmin

install -d %{buildroot}%{_datadir}/%{sname}
%{__make} DESTDIR=%{buildroot} install

pushd %{buildroot}%{_datadir}/%{sname}
  ln -s qmailadmin index.cgi
popd

# module to be inserted into toaster-web-admin
echo "

<!-- qmailadmin.module -->
<tr>
    <td align=\"right\" width=\"47%\">Edit Users, mailing lists, forwarders</td>
    <td width=\"6%\"></td>
    <td align=\"left\" width=47%\"><input type=\"button\" value=\"%{name}-%{version}\" class=\"inputs\" onClick=\"location.href='/qmailadmin';\"></td>
</tr>
<!-- qmailadmin.module -->

" > %{buildroot}%{_datadir}/%{sname}/%{sname}.module

# apache configuration file
mkdir -p %{buildroot}/%{apache_confd}
install -m644 %{SOURCE4} %{buildroot}/%{apache_confd}/%{sname}.conf
sed -e 's|QMAILADMINDIR|%{_datadir}/%{sname}|' -i %{buildroot}/%{apache_confd}/%{sname}.conf

# Install online help
install -d %{buildroot}%{_datadir}/%{sname}/images/help
cp -R $RPM_BUILD_DIR/%{sname}-%{version}/help/* %{buildroot}%{_datadir}/%{sname}/images/help/

%triggerin -- control-panel-toaster
# Insert into toaster-web-admin
if [ -d %{tdir}/include ] ; then
    ln -fs %{_datadir}/%{sname}/%{sname}.module %{tdir}/include
fi

%triggerun -- control-panel-toaster
# Delete from toaster-web-admin
if [ -L %{tdir}/include/%{sname}.module ] ; then
    rm %{tdir}/include/%{sname}.module
fi

%post

%clean
[ -d %{buildroot} ] && %{__rm} -rf %{buildroot}

%files
%defattr(0644,root,root,0755)
%config %{_sysconfdir}/permissions.d/%{sname}
%dir %{_datadir}/%{sname}
%{_datadir}/%{sname}/html
%{_datadir}/%{sname}/lang
%{_datadir}/%{sname}/images

%{_datadir}/%{sname}/%{sname}.module

%doc AUTHORS BUGS ChangeLog COPYING FAQ INSTALL NEWS README* TODO TRANSLATORS

%config(noreplace) %{apache_confd}/%{sname}.conf

%verify(not mode) %attr(0755,vpopmail,vchkpw) %{_datadir}/%{sname}/index.cgi
%verify(not mode) %attr(6755,vpopmail,vchkpw) %{_datadir}/%{sname}/qmailadmin

%changelog
scarabeusiv commented 9 years ago

Haum it seems working properly for me. What version of spec-cleaner you have?

Here goes the output:

# spec file for package doublegroup                                                                                                                                                                                                                          
#                                                                                                                                                                                                                                                            
# Copyright (c) 2015 SUSE LINUX GmbH, Nuernberg, Germany.                                                                                                                                                                                                    
#                                                                                                                                                                                                                                                            
# All modifications and additions to the file contributed by third parties                                                                                                                                                                                   
# remain the property of their copyright owners, unless otherwise agreed                                                                                                                                                                                     
# upon. The license for this file, and modifications and additions to the                                                                                                                                                                                    
# file, is the same license as for the pristine package itself (unless the                                                                                                                                                                                   
# license for the pristine package is not an Open Source License, in which                                                                                                                                                                                   
# case the license is the MIT License). An "Open Source License" is a                                                                                                                                                                                        
# license that conforms to the Open Source Definition (Version 1.9)                                                                                                                                                                                          
# published by the Open Source Initiative.                                                                                                                                                                                                                   

# Please submit bugfixes or comments via http://bugs.opensuse.org/                                                                                                                                                                                           
#                                                                                                                                                                                                                                                            

%define         sname           qmailadmin
%define         tdir %{_datadir}/toaster
%define         vdir /home/vpopmail
%define         qdir %{_localstatedir}/qmail
%if 0%{?suse_version}
%define         apache_confd    %{_sysconfdir}/apache2/conf.d
BuildRequires:  apache2
BuildRequires:  automake
Requires:       apache2
# FIXME: use proper Requires(pre/post/preun/...)
PreReq:         permissions
%else
%define         apache_confd    %{_sysconfdir}/httpd/conf.d
%endif
Version:        1.2.15
Release:        0
License:        GPL-2.0
Url:            http://www.inter7.com/qmailadmin
Source0:        qmailadmin-%{version}.tar.bz2
Source1:        help.tar.bz2
Source2:        qmailadmin-rpmlintrc
Source3:        qmailadmin.permissions
Source4:        qmailadmin.conf
Patch0:         qmailadmin_lang_de.diff
Patch1:         mailinglist.c.ezmlm7.patch
Patch2:         header.html.patch
BuildRequires:  ezmlm-idx
BuildRequires:  qmail-skel-devel
BuildRequires:  vpopmail-devel
Requires:       ezmlm-idx
Requires:       maildrop
Requires:       qmail
Requires:       qmail-autorespond
Requires:       vpopmail
Obsoletes:      qmailadmin-toaster < %{version}
Provides:       qmailadmin-toaster = %{version}
BuildRoot:      %{_tmppath}/%{name}-%{version}-build
%if 0%{?spambox:1}
Name:           qmailadmin-spambox
Summary:        Web Administration for qmail/vpopmail with Spambox suuport
Provides:       qmailadmin = %{version}
%else
Name:           qmailadmin
Summary:        Web Administration for qmail/vpopmail
Conflicts:      qmailadmin-spambox
%endif
%if 0%{?suse_version}
Group:          Productivity/Networking/Email/Servers
%else
Group:          Networking/Other
%endif
%if 0%{?rhel_version} || 0%{?centos_version} || 0%{?fedora_version}
BuildRequires:  httpd
Requires:       httpd
%endif
%if 0%{?mandriva_version}
BuildRequires:  apache-base
Requires:       apache-base
%endif

%description
QmailAdmin is a free software package that provides a web interface
for managing a  qmail  system with virtual domains. This version is
for use with the vpopmail program. It provides admin for
adding/deleting users, Aliases, Forwards, Mailing lists and
Autoresponders.

%prep
%setup -q -n %{sname}-%{version}
%patch0
%patch1
%patch2

cp %{SOURCE1} help.tar.bz2
tar xfj help.tar.bz2
rm -f help.tar.bz2

#dont change owner to allow builds without root
sed -i -e 's/-o @vpopuser@//g ; s/-g @vpopgroup@//g' Makefile.am

%build
%define cflags %(echo %{optflags} | sed -e 's/$/ -fPIE/' )
%define ldflags %(echo %{optflags} | sed -e 's/$/ -pie/' )

export CFLAGS="%{cflags}"
export LDFLAGS="%{ldflags}"

aclocal
autoconf
autoheader
automake
%configure \
 --datadir=%{_datadir}/%{sname} \
 --enable-htmllibdir=%{_datadir}/%{sname} \
 --enable-htmldir=%{_datadir}/%{sname} \
 --enable-cgibindir=%{_datadir}/%{sname} \
 --enable-imageurl=/%{sname}/images \
 --enable-imagedir=%{_datadir}/%{sname}/images \
 --enable-cgipath=/qmailadmin/index.cgi \
 --enable-vpopmaildir=%{vdir} \
 --enable-qmaildir=%{qdir} \
 --enable-ezmlmdir=%{_bindir} \
 --enable-autoresponder-path=%{_bindir} \
 --enable-vpopuser=vpopmail \
 --enable-vpopgroup=vchkpw \
 --enable-maxusersperpage=12 \
 --enable-maxaliasesperpage=12 \
 --enable-modify-quota=y \
 --enable-ezmlm-mysql=n \
 --enable-help=y \
%if 0%{?spambox:1}
 --enable-modify-spam \
 --enable-spam-command="|%{_localstatedir}/qmail/bin/preline %{_bindir}/maildrop -A 'Content-Filter: maildrop-toaster' %{_sysconfdir}/mail/mailfilter" \
%endif
--enable-domain-autofill=n

make %{?_smp_mflags}

%install
mkdir -p %{buildroot}%{_sysconfdir}/permissions.d
install -m644 %{SOURCE2} %{buildroot}%{_sysconfdir}/permissions.d/qmailadmin

install -d %{buildroot}%{_datadir}/%{sname}
make DESTDIR=%{buildroot} install %{?_smp_mflags}

pushd %{buildroot}%{_datadir}/%{sname}
  ln -s qmailadmin index.cgi
popd

# module to be inserted into toaster-web-admin
echo "

<!-- qmailadmin.module -->
<tr>
        <td align=\"right\" width=\"47%\">Edit Users, mailing lists, forwarders</td>
        <td width=\"6%\"></td>
        <td align=\"left\" width=47%\"><input type=\"button\" value=\"%{name}-%{version}\" class=\"inputs\" onClick=\"location.href='/qmailadmin';\"></td>
</tr>
<!-- qmailadmin.module -->

" > %{buildroot}%{_datadir}/%{sname}/%{sname}.module

# apache configuration file
mkdir -p %{buildroot}/%{apache_confd}
install -m644 %{SOURCE4} %{buildroot}/%{apache_confd}/%{sname}.conf
sed -e 's|QMAILADMINDIR|%{_datadir}/%{sname}|' -i %{buildroot}/%{apache_confd}/%{sname}.conf

# Install online help
install -d %{buildroot}%{_datadir}/%{sname}/images/help
cp -R $RPM_BUILD_DIR/%{sname}-%{version}/help/* %{buildroot}%{_datadir}/%{sname}/images/help/

%triggerin -- control-panel-toaster
# Insert into toaster-web-admin
if [ -d %{tdir}/include ] ; then
        ln -fs %{_datadir}/%{sname}/%{sname}.module %{tdir}/include
fi

%triggerun -- control-panel-toaster
# Delete from toaster-web-admin
if [ -L %{tdir}/include/%{sname}.module ] ; then
        rm %{tdir}/include/%{sname}.module
fi

%post

%files
%defattr(0644,root,root,0755)
%config %{_sysconfdir}/permissions.d/%{sname}
%dir %{_datadir}/%{sname}
%{_datadir}/%{sname}/html
%{_datadir}/%{sname}/lang
%{_datadir}/%{sname}/images

%{_datadir}/%{sname}/%{sname}.module

%doc AUTHORS BUGS ChangeLog COPYING FAQ INSTALL NEWS README* TODO TRANSLATORS

%config(noreplace) %{apache_confd}/%{sname}.conf

%verify(not mode) %attr(0755,vpopmail,vchkpw) %{_datadir}/%{sname}/index.cgi
%verify(not mode) %attr(6755,vpopmail,vchkpw) %{_datadir}/%{sname}/qmailadmin

%changelog
weberhofer commented 9 years ago

I have obs-service-format_spec_file-20150121-4.1 installed.

osc service localrun format_spec_file

results in

%if 0%{?spambox:1}
Name:           qmailadmin-spambox
Provides:       qmailadmin = %{version}
Summary:        Web Administration for qmail/vpopmail with Spambox suuport
License:        GPL-2.0
Group:          Networking/Other
%else
Name:           qmailadmin
Conflicts:      qmailadmin-spambox
Summary:        Web Administration for qmail/vpopmail
License:        GPL-2.0
Group:          Networking/Other
%endif
%if 0%{?suse_version}
%else
%endif
scarabeusiv commented 9 years ago

Spec-cleaner != format_spec_file :) Spec-cleaner is "osc service localrun clean_spec_file"

You want to report the issue here: https://github.com/openSUSE/obs-service-format_spec_file

coolo commented 9 years ago

Check https://build.opensuse.org/request/show/287873 how pcp solves the "different tags on different platforms" without making a complete mess out of the spec file.

The way you're doing it will basically unmaintainable as soon as a 3rd option arrives.

weberhofer commented 9 years ago

Ok, so I should set/use variables; I'll try it. I'm a bit surprised, that there are two spec-file-formatters more or doing the same thing, I was always thinking the spec-cleaner is the basis for the forma-spec-file service.

coolo commented 9 years ago

spec-cleaner is format-spec-file-NG :)

weberhofer commented 9 years ago

Cool. Is there any plan when the NG will be expected? Following the mailinglists, there are some more people not that satisfied with the current behaviour of modifying specs.

scarabeusiv commented 9 years ago

Well I always ask for more testers, so start using the spec-cleaner and let us know if it breaks something for you. The biggest problem is that spec-cleaner is more "intrusive" and adds FIXME where it encounters something wrong, so it is not automatic-cleaner but semi-automatic with requirement of quick review.

weberhofer commented 9 years ago

I'll test and will report issues if I found some. Thx!