Some help(I'm not a sponsor, please find a potential sponsor via:

https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group):

1. Please drop these:

#
# rpm spec for xtrabackup
#

and 


###
### eof
###


As far as I can tell you, these are useless and they make the spec jumbled

2. Release: 1

Please read carefully when you want to build a package for Fedora as the first time:

https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Using_the_.25.7B.3Fdist.7D_Tag

3. http://www.percona.com/redir/downloads/XtraBackup/LATEST/source/percona-xtrabackup-2.1.5.tar.gz

-->

http://www.percona.com/redir/downloads/XtraBackup/LATEST/source/%{name}-%{version}.tar.gz

So next time when you want to update the package, you just need to modify the version tag and it will be changed automatically.

4. Please remove these obsoleted lines for ~EPEL5:

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

[ "%{buildroot}" != '/' ] && rm -rf %{buildroot}

%defattr(-,root,root)

%clean

5. Please sort your spec in a nice order, in brief, please move %changelog section to the bottom of every spec.

6. I don't know why you had this:

%define __os_install_post /usr/lib/rpm/brp-compress

I don't think we need it anymore now.

7. install -m 755

Please see:

https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps

and fix.

8. We don't allow this:

AutoReqProv: no

Please tell us the reason, we can help.

(In reply to Christopher Meng from comment #1)
> Some help(I'm not a sponsor, please find a potential sponsor via:
> 
> https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group):

Thanks so much for the review!

> 1. Please drop these:
> 
> #
> # rpm spec for xtrabackup
> #
> 
> and 
> 
> 
> ###
> ### eof
> ###
> 
> 
> As far as I can tell you, these are useless and they make the spec jumbled

Done.

> 2. Release: 1
> 
> Please read carefully when you want to build a package for Fedora as the
> first time:
> 
> https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Using_the_.25.7B.
> 3Fdist.7D_Tag

done, thanks!


> 3.
> http://www.percona.com/redir/downloads/XtraBackup/LATEST/source/percona-
> xtrabackup-2.1.5.tar.gz
> 
> -->
> 
> http://www.percona.com/redir/downloads/XtraBackup/LATEST/source/%{name}-
> %{version}.tar.gz
> 
> So next time when you want to update the package, you just need to modify
> the version tag and it will be changed automatically.

Done.

> 4. Please remove these obsoleted lines for ~EPEL5:
> 
> BuildRoot: %{_tmppath}/%{name}-%{version}-root
> 
> [ "%{buildroot}" != '/' ] && rm -rf %{buildroot}
> 
> %defattr(-,root,root)
> 
> %clean

Done. Would this affect building for EPEL for CentOS/RHEL5?

> 5. Please sort your spec in a nice order, in brief, please move %changelog
> section to the bottom of every spec.

Sounds like a good idea, done.
 
> 6. I don't know why you had this:
> 
> %define __os_install_post /usr/lib/rpm/brp-compress
> 
> I don't think we need it anymore now.

It seems like we added it many years ago, but the debuginfo packages take care of this now. Removed.

> 
> 7. install -m 755
> 
> Please see:
> 
> https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps
> 
> and fix.

done

> 8. We don't allow this:
> 
> AutoReqProv: no
> 
> Please tell us the reason, we can help.

Removed. This was due to a (now mostly obsolete) test infrastructure we had. I've solved it by just not packaging it.


Updated spec file: https://flamingspork.com/junk/percona-xtrabackup-2.1.5-fedora/percona-xtrabackup.spec

Updated SRPM: https://flamingspork.com/junk/percona-xtrabackup-2.1.5-fedora/percona-xtrabackup-2.1.5-1.fc19.src.rpm

(give the SRPM a few minutes, it's uploading)

(In reply to Stewart Smith from comment #2)
> > 4. Please remove these obsoleted lines for ~EPEL5:
> > 
> > BuildRoot: %{_tmppath}/%{name}-%{version}-root
> > 
> > [ "%{buildroot}" != '/' ] && rm -rf %{buildroot}
> > 
> > %defattr(-,root,root)
> > 
> > %clean
> 
> Done. Would this affect building for EPEL for CentOS/RHEL5?

Yes, it will affect.

From Fedora 10/RHEL6 we don't need these but on RHEL5 they are still needed.

Does this tool work well on RHEL5+EPEL5?

(In reply to Christopher Meng from comment #3)
> (In reply to Stewart Smith from comment #2)
> > > 4. Please remove these obsoleted lines for ~EPEL5:
> > > 
> > > BuildRoot: %{_tmppath}/%{name}-%{version}-root
> > > 
> > > [ "%{buildroot}" != '/' ] && rm -rf %{buildroot}
> > > 
> > > %defattr(-,root,root)
> > > 
> > > %clean
> > 
> > Done. Would this affect building for EPEL for CentOS/RHEL5?
> 
> Yes, it will affect.
> 
> From Fedora 10/RHEL6 we don't need these but on RHEL5 they are still needed.
> 
> Does this tool work well on RHEL5+EPEL5?

Yes, we do build our own RPMs for RHEL5 and have customers using it on RHEL5 quite successfully (with build-dependencies from EPEL)

Hmm...Ok so you can keep them in EL5 branch when you access the SCM git repo.

I hope you can keep your spec in modern style on rawhide branch, and keep these old things on EL5 branch only.

BTW, I saw:

Requires: perl(DBD::mysql)

Have you tried:

Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))?

Again:

1. Packager: Percona Development Team <mysql-dev>

Fedora doesnt allow this. Please remove.


2. Provides: xtrabackup >= 2.0.0
Obsoletes: xtrabackup < 2.0.0

I think we don't need this.

3. BuildRequires: libaio-devel, libgcrypt-devel, automake, cmake >= 2.6.3, patch, gcc, gcc-c++, libtool, bison, ncurses-devel, openssl-devel, procps

Well some of them can be dropped:

automake, patch, gcc, gcc-c++, libtool

4. I can't see any reference of RPM %{optflags} in build section. Please make sure that optflags are inserted well.

If you have not already seen, 2.1.6 was released (https://bugzilla.novell.com/show_bug.cgi?id=852224) and fixes a possible security issue:

""
A fixed initialization vector (constant string) was used while encrypting
the data. This opened the encrypted stream/data to plaintext attacks among
others. Bug fixed #1185343.
""

(In reply to Murray McAllister from comment #7)
> If you have not already seen, 2.1.6 was released
> (https://bugzilla.novell.com/show_bug.cgi?id=852224) and fixes a possible
> security issue:
> 
> ""
> A fixed initialization vector (constant string) was used while encrypting
> the data. This opened the encrypted stream/data to plaintext attacks among
> others. Bug fixed #1185343.
> ""

I'm aware, I'll update shortly.

2.1.7 was released and it also has security fixes, as noted by a SUSE bug report:

https://bugzilla.novell.com/show_bug.cgi?id=860488

*** This bug has been marked as a duplicate of bug 918431 ***