Bug 1194798

So we just rebuild it and bump the Version field every month?

(In reply to Philip Prindeville from comment #1)
> So we just rebuild it and bump the Version field every month?

Or are we still suggesting everyone use the geoipupdate-cron* subpackages?

I was thinking to update this roughly quarterly, and then anyone that wanted updates more often could use the cron scripts.

If you wanted to co-maintain and push updates monthly though I'd be fine with that too. There would still be a case for the cron scripts though, due to the time the updates had to stay in testing.

Philip, do you have any objection to this approach?

Would you consider reviewing the package?

(In reply to Paul Howarth from comment #4)
> Philip, do you have any objection to this approach?
> 
> Would you consider reviewing the package?

I'll try to do this this week.  If I've not done it by Saturday, give me a prod.

Spec file looks okay.  Have you run rpmlint on both files?

Also, what's the script to run over the package to review it and do the automated checks?

(In reply to Philip Prindeville from comment #6)
> Spec file looks okay.  Have you run rpmlint on both files?

$ rpmlint GeoIP-GeoLite-data GeoIP-GeoLite-data-extra
GeoIP-GeoLite-data.noarch: W: spelling-error Summary(en_US) geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data.noarch: W: spelling-error %description -l en_US geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data.noarch: W: non-etc-or-var-file-marked-as-conffile /usr/share/GeoIP/GeoIP.dat
GeoIP-GeoLite-data.noarch: W: no-documentation
GeoIP-GeoLite-data-extra.noarch: W: spelling-error Summary(en_US) geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data-extra.noarch: W: spelling-error %description -l en_US geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data-extra.noarch: W: no-documentation
2 packages and 0 specfiles checked; 0 errors, 7 warnings.

Nothing to worry about there I think; all present in the old GeoIP package.

> Also, what's the script to run over the package to review it and do the
> automated checks?

It's fedora-review (in the same-named package). I've never used it myself so that's all I can say about it.

Ping?

Ping?

(In reply to Paul Howarth from comment #9)
> Ping?

Pong.  Running fedora-review now.

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- No %config files under /usr.
  Note: %config(noreplace) /usr/share/GeoIP/GeoIP.dat
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#Configuration_files
- Sources used to build the package match the upstream source, as provided in
  the spec URL.
  Note: Upstream MD5sum check error, diff is in /home/philipp/fedora/GeoIP-
  GeoLite-data/review-GeoIP-GeoLite-data/diff.txt
  See: http://fedoraproject.org/wiki/Packaging/SourceURL


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[-]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[x]: License field in the package spec file matches the actual license.
     Note: There is no build directory. Running licensecheck on vanilla
     upstream sources. No licenses found. Please check the source files for
     licenses manually.
[-]: License file installed when any subpackage combination is installed.
[-]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/share/GeoIP(geoipupdate-
     cron6, GeoIP-data)
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[!]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
[-]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[-]: Package uses nothing in %doc for runtime.
[-]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: %config files are marked noreplace or the reason is justified.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: Buildroot is not present
     Note: Invalid buildroot found:
     %{_tmppath}/%{name}-%{version}-%{release}-root-%(id -nu)
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: %clean present but not required
[x]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[-]: Package functions as described.
[x]: Latest version is packaged.
[-]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[-]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: GeoIP-GeoLite-data-2015.02-1.fc23.noarch.rpm
          GeoIP-GeoLite-data-extra-2015.02-1.fc23.noarch.rpm
          GeoIP-GeoLite-data-2015.02-1.fc23.src.rpm
GeoIP-GeoLite-data.noarch: W: spelling-error Summary(en_US) geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data.noarch: W: spelling-error %description -l en_US geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data.noarch: W: non-etc-or-var-file-marked-as-conffile /usr/share/GeoIP/GeoIP.dat
GeoIP-GeoLite-data.noarch: W: no-documentation
GeoIP-GeoLite-data-extra.noarch: W: spelling-error Summary(en_US) geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data-extra.noarch: W: spelling-error %description -l en_US geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data-extra.noarch: W: no-documentation
GeoIP-GeoLite-data.src: W: spelling-error Summary(en_US) geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data.src: W: spelling-error %description -l en_US geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data.src: W: file-size-mismatch GeoIPASNumv6.dat.gz = 2378270, http://download.maxmind.com/download/geoip/database/asnum/GeoIPASNumv6.dat.gz = 2395433
GeoIP-GeoLite-data.src: W: file-size-mismatch GeoIPASNum.dat.gz = 2086766, http://download.maxmind.com/download/geoip/database/asnum/GeoIPASNum.dat.gz = 2103287
GeoIP-GeoLite-data.src: W: file-size-mismatch GeoLiteCityv6.dat.gz = 12209168, http://geolite.maxmind.com/download/geoip/database/GeoLiteCityv6-beta/GeoLiteCityv6.dat.gz = 12523932
GeoIP-GeoLite-data.src: W: file-size-mismatch GeoLiteCity.dat.gz = 11949432, http://geolite.maxmind.com/download/geoip/database/GeoLiteCity.dat.gz = 12263571
GeoIP-GeoLite-data.src: W: file-size-mismatch GeoIPv6.dat.gz = 690323, http://geolite.maxmind.com/download/geoip/database/GeoIPv6.dat.gz = 712061
GeoIP-GeoLite-data.src: W: file-size-mismatch GeoIP.dat.gz = 436680, http://geolite.maxmind.com/download/geoip/database/GeoLiteCountry/GeoIP.dat.gz = 455464
3 packages and 0 specfiles checked; 0 errors, 15 warnings.

I'll need to re-run the review tool when you've updated the .src.rpm file and I've downloaded it. Need to make sure that the rpmlint messages regarding sizes goes away.

(In reply to Philip Prindeville from comment #11)
> Issues:
> =======
> - No %config files under /usr.
>   Note: %config(noreplace) /usr/share/GeoIP/GeoIP.dat
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#Configuration_files

This file is a symlink to GeoLiteCountry.dat, the default free database.
Upstream also provides commercial versions of the databases, which users may wish to install to /usr/share/GeoIP/GeoIP.dat so that the library uses that instead of the default free database. Marking this file as %config(noreplace) means that rpm package updates won't blow away the user's paid-for database file. This approach has been present in the existing GeoIP package for a long time now, and is being carried forward to this package.

> - Sources used to build the package match the upstream source, as provided in
>   the spec URL.
>   Note: Upstream MD5sum check error, diff is in /home/philipp/fedora/GeoIP-
>   GeoLite-data/review-GeoIP-GeoLite-data/diff.txt
>   See: http://fedoraproject.org/wiki/Packaging/SourceURL

Upstream releases new versions of the database files at least once a month. They changed between when I prepared the packages for review and when the review was done, hence the size/checksum differences.

> Why does the %files section treat GeoIP.dat differently from
> GeoLiteCountry.dat ?

GeoLiteCountry.dat and the other database files from upstream are expected to be rpm-maintained, or updated by the cron scripts. The GeoIP.dat symlink is never touched after being installed in case the user wants to use a different default database, as explained above.

> Also, the .spec files says that the license is CC-BY-SA but I can’t
> find explicit licensing on the databases anywhere.

See the license statement at the upstream URL:
http://dev.maxmind.com/geoip/legacy/geolite/

"The GeoLite databases are distributed under the Creative Commons Attribution-ShareAlike 3.0 Unported License"

> I’d also wrap the comment lines at less than 80 characters.

OK, done.

> Why does the %install section need "rm -rf %{buildroot}”?

The following spec elements are needed for EL-5 support:
 * BuildRoot: and Group: tags
 * Cleaning of %{buildroot} in %install and %clean

Package updated:

Spec URL: http://subversion.city-fan.org/repos/cfo-repo/GeoIP-GeoLite-data/trunk/GeoIP-GeoLite-data.spec
SRPM URL: http://www.city-fan.org/~paul/extras/GeoIP-GeoLite-data/GeoIP-GeoLite-data-2015.03-2.fc23.src.rpm

(In reply to Paul Howarth from comment #13)
> (In reply to Philip Prindeville from comment #11)
> > Issues:
> > =======
> > - No %config files under /usr.
> >   Note: %config(noreplace) /usr/share/GeoIP/GeoIP.dat
> >   See: http://fedoraproject.org/wiki/Packaging/Guidelines#Configuration_files
> 
> This file is a symlink to GeoLiteCountry.dat, the default free database.
> Upstream also provides commercial versions of the databases, which users may
> wish to install to /usr/share/GeoIP/GeoIP.dat so that the library uses that
> instead of the default free database. Marking this file as
> %config(noreplace) means that rpm package updates won't blow away the user's
> paid-for database file. This approach has been present in the existing GeoIP
> package for a long time now, and is being carried forward to this package.

I get all of that, I was just wondering if we could use %verify(...) instead of %config(noreplace) so that we have fewer rpmlint warnings.

> > - Sources used to build the package match the upstream source, as provided in
> >   the spec URL.
> >   Note: Upstream MD5sum check error, diff is in /home/philipp/fedora/GeoIP-
> >   GeoLite-data/review-GeoIP-GeoLite-data/diff.txt
> >   See: http://fedoraproject.org/wiki/Packaging/SourceURL
> 
> Upstream releases new versions of the database files at least once a month.
> They changed between when I prepared the packages for review and when the
> review was done, hence the size/checksum differences.

Yeah, sorry about taking too long.  Hence my offer to rerun the review tool as soon as you update the .src.rpm.

> > Why does the %files section treat GeoIP.dat differently from
> > GeoLiteCountry.dat ?
> 
> GeoLiteCountry.dat and the other database files from upstream are expected
> to be rpm-maintained, or updated by the cron scripts. The GeoIP.dat symlink
> is never touched after being installed in case the user wants to use a
> different default database, as explained above.

Right, right, I get that.  It's just that it's not a config file, so I hate abusing that directive. And if tools like etckeeper pay attention to files marked %config, I don't want the file being checked into SCM, either.

> > Also, the .spec files says that the license is CC-BY-SA but I can’t
> > find explicit licensing on the databases anywhere.
> 
> See the license statement at the upstream URL:
> http://dev.maxmind.com/geoip/legacy/geolite/
> 
> "The GeoLite databases are distributed under the Creative Commons
> Attribution-ShareAlike 3.0 Unported License"

Can you wget that file and bundle it as a license file?

> > I’d also wrap the comment lines at less than 80 characters.
> 
> OK, done.

Thanks.

> > Why does the %install section need "rm -rf %{buildroot}”?
> 
> The following spec elements are needed for EL-5 support:
>  * BuildRoot: and Group: tags
>  * Cleaning of %{buildroot} in %install and %clean

Okay, right.  Thought so.  Can you add a comment to that effect?

> Package updated:
> 
> Spec URL:
> http://subversion.city-fan.org/repos/cfo-repo/GeoIP-GeoLite-data/trunk/GeoIP-
> GeoLite-data.spec
> SRPM URL:
> http://www.city-fan.org/~paul/extras/GeoIP-GeoLite-data/GeoIP-GeoLite-data-
> 2015.03-2.fc23.src.rpm

(In reply to Philip Prindeville from comment #14)
> (In reply to Paul Howarth from comment #13)
> > (In reply to Philip Prindeville from comment #11)
> > > Issues:
> > > =======
> > > - No %config files under /usr.
> > >   Note: %config(noreplace) /usr/share/GeoIP/GeoIP.dat
> > >   See: http://fedoraproject.org/wiki/Packaging/Guidelines#Configuration_files
> > 
> > This file is a symlink to GeoLiteCountry.dat, the default free database.
> > Upstream also provides commercial versions of the databases, which users may
> > wish to install to /usr/share/GeoIP/GeoIP.dat so that the library uses that
> > instead of the default free database. Marking this file as
> > %config(noreplace) means that rpm package updates won't blow away the user's
> > paid-for database file. This approach has been present in the existing GeoIP
> > package for a long time now, and is being carried forward to this package.
> 
> I get all of that, I was just wondering if we could use %verify(...) instead
> of %config(noreplace) so that we have fewer rpmlint warnings.

No, the %verify and %config(noreplace) are doing two very different things:

By using %verify, we tell rpm that we may change files underneath it and not to worry about it. It has no effect on whether rpm will itself overwrite those files on updates.

By using %config(noreplace), rpm notices if we change a file underneath it and will then not overwrite it on updates, creating a .rpmnew file with the updated content instead.

So for the free database files we're providing from upstream, %verify is the right approach as we want people to be able to update the databases using the cron scripts, not be worried by "rpm --verify" output, and get new versions of the files when we push updates.

However, for the GeoIP.dat symlink, we don't want to overwrite it if the end user has replaced it with their own database. Using %config(noreplace) is the way we have traditionally done this in the GeoIP package. There is another way though: instead of shipping GeoIP.dat as part of the package, create the symlink in %posttrans if it did not already exist. It can't be done in %post as it would break updates, where the old GeoIP.dat was still present during %post but deleted before %posttrans. An added complication during updates is that rpm will rename a modified GeoIP.dat to GeoIP.dat.rpmsave when the file is no longer packaged, so we have to rename it back again if necessary. This is the approach I've now taken, with the result that we get rid of the rpmlint warning about %config files outside /etc and replace it with one about running the "dangerous command" "mv" in %posttrans:
GeoIP-GeoLite-data.noarch: W: dangerous-command-in-%posttrans mv

I think this is the right thing to do though, as GeoIP.dat really isn't a config file.

> > > Why does the %files section treat GeoIP.dat differently from
> > > GeoLiteCountry.dat ?
> > 
> > GeoLiteCountry.dat and the other database files from upstream are expected
> > to be rpm-maintained, or updated by the cron scripts. The GeoIP.dat symlink
> > is never touched after being installed in case the user wants to use a
> > different default database, as explained above.
> 
> Right, right, I get that.  It's just that it's not a config file, so I hate
> abusing that directive. And if tools like etckeeper pay attention to files
> marked %config, I don't want the file being checked into SCM, either.

This is addressed in the new approach.

> > > Also, the .spec files says that the license is CC-BY-SA but I can’t
> > > find explicit licensing on the databases anywhere.
> > 
> > See the license statement at the upstream URL:
> > http://dev.maxmind.com/geoip/legacy/geolite/
> > 
> > "The GeoLite databases are distributed under the Creative Commons
> > Attribution-ShareAlike 3.0 Unported License"
> 
> Can you wget that file and bundle it as a license file?

The guidelines sort of discourage this:

https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#License_Text

  "It is important to reiterate that in situations where the indicated
   license does not imply a requirement that the license be distributed
   along with the source/binaries, Fedora packagers are NOT required to
   manually include the full license text when it is absent from the
   source code. but are still encouraged to point out this issue to upstream
   and encourage them to remedy it."

http://fedoraproject.org/wiki/Packaging:ReviewGuidelines

  "MUST: If (and only if) the source package includes the text of the
  license(s) in its own file, then that file, containing the text of the
  license(s) for the package must be included in %license."

Given that upstream is distributing raw database files rather than tarballs, it's not practical for them to distribute a separate license file. I've added a comment in the spec pointing to the URL where upstream declares the license.

> > > Why does the %install section need "rm -rf %{buildroot}”?
> > 
> > The following spec elements are needed for EL-5 support:
> >  * BuildRoot: and Group: tags
> >  * Cleaning of %{buildroot} in %install and %clean
> 
> Okay, right.  Thought so.  Can you add a comment to that effect?

Done.

Package updated:

Spec URL:
http://subversion.city-fan.org/repos/cfo-repo/GeoIP-GeoLite-data/trunk/GeoIP-GeoLite-data.spec
SRPM URL:
http://www.city-fan.org/~paul/extras/GeoIP-GeoLite-data/GeoIP-GeoLite-data-2015.03-3.fc23.src.rpm

Ping?

(In reply to Paul Howarth from comment #16)
> Ping?

Just reran the review tool, and the sizes changed again.  Looks like you ran this on 1 April, and upstream updated again on 5 April.  Real moving target!

Updated to April databases.

Also added %preun script to remove the GeoIP.dat symlink if the package is uninstalled and GeoIP.dat is still a symlink pointing to the Country database.
This will allow rpm to cleanly remove /usr/share/GeoIP, which it wouldn't be able to do if we left the symlink there.

Spec URL:
http://subversion.city-fan.org/repos/cfo-repo/GeoIP-GeoLite-data/trunk/GeoIP-GeoLite-data.spec
SRPM URL:
http://www.city-fan.org/~paul/extras/GeoIP-GeoLite-data/GeoIP-GeoLite-data-2015.04-1.fc23.src.rpm

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[-]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[x]: License field in the package spec file matches the actual license.
     Note: There is no build directory. Running licensecheck on vanilla
     upstream sources. No licenses found. Please check the source files for
     licenses manually.
[-]: License file installed when any subpackage combination is installed.
[x]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/share/GeoIP(geoipupdate-
     cron6, GeoIP-data)
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf %{buildroot} present but not required
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[x]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[!]: Buildroot is not present
     Note: Invalid buildroot found:
     %{_tmppath}/%{name}-%{version}-%{release}-root-%(id -nu)
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
[!]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: %clean present but not required
[x]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Scriptlets must be sane, if used.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Dist tag is present (not strictly required in GL).
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Fully versioned dependency in subpackages if applicable.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: GeoIP-GeoLite-data-2015.04-1.fc23.noarch.rpm
          GeoIP-GeoLite-data-extra-2015.04-1.fc23.noarch.rpm
          GeoIP-GeoLite-data-2015.04-1.fc23.src.rpm
GeoIP-GeoLite-data.noarch: W: spelling-error Summary(en_US) geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data.noarch: W: spelling-error %description -l en_US geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data.noarch: W: no-documentation
GeoIP-GeoLite-data.noarch: W: dangerous-command-in-%preun rm
GeoIP-GeoLite-data.noarch: W: dangerous-command-in-%posttrans mv
GeoIP-GeoLite-data-extra.noarch: W: spelling-error Summary(en_US) geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data-extra.noarch: W: spelling-error %description -l en_US geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data-extra.noarch: W: no-documentation
GeoIP-GeoLite-data.src: W: spelling-error Summary(en_US) geolocation -> echolocation, collocation, allocation
GeoIP-GeoLite-data.src: W: spelling-error %description -l en_US geolocation -> echolocation, collocation, allocation
3 packages and 0 specfiles checked; 0 errors, 10 warnings.




Rpmlint (installed packages)
----------------------------
Cannot parse rpmlint output:


Requires
--------
GeoIP-GeoLite-data-extra (rpmlib, GLIBC filtered):
    GeoIP-GeoLite-data

GeoIP-GeoLite-data (rpmlib, GLIBC filtered):
    /bin/sh



Provides
--------
GeoIP-GeoLite-data-extra:
    GeoIP-GeoLite-data-extra

GeoIP-GeoLite-data:
    GeoIP-GeoLite-data
    GeoIP-data
    geoip-geolite



Source checksums
----------------
http://geolite.maxmind.com/download/geoip/database/GeoLiteCity.dat.gz :
  CHECKSUM(SHA256) this package     : aa005e191fff2b1e31974b79feec739ddaa35461cadc04733fff35d695498651
  CHECKSUM(SHA256) upstream package : aa005e191fff2b1e31974b79feec739ddaa35461cadc04733fff35d695498651
http://geolite.maxmind.com/download/geoip/database/GeoLiteCityv6-beta/GeoLiteCityv6.dat.gz :
  CHECKSUM(SHA256) this package     : fbad6e68012b4b1b170606325d68afa3053955c8cdf29b53a88d0dc2d18b1acd
  CHECKSUM(SHA256) upstream package : fbad6e68012b4b1b170606325d68afa3053955c8cdf29b53a88d0dc2d18b1acd
http://geolite.maxmind.com/download/geoip/database/GeoLiteCountry/GeoIP.dat.gz :
  CHECKSUM(SHA256) this package     : 33104eb35340c97227ea93f7ba1c84c38742a4f8adea23a3201350e9bd918795
  CHECKSUM(SHA256) upstream package : 33104eb35340c97227ea93f7ba1c84c38742a4f8adea23a3201350e9bd918795
http://geolite.maxmind.com/download/geoip/database/GeoIPv6.dat.gz :
  CHECKSUM(SHA256) this package     : 71245e34e80e45257183eaab5392fbb2bd84c8e1757d9649bc22e39faff4e64f
  CHECKSUM(SHA256) upstream package : 71245e34e80e45257183eaab5392fbb2bd84c8e1757d9649bc22e39faff4e64f
http://download.maxmind.com/download/geoip/database/asnum/GeoIPASNum.dat.gz :
  CHECKSUM(SHA256) this package     : 0be44352fd85e9d6efd448ac21bb8fb705c54246955691f18383e8e19b5a2d22
  CHECKSUM(SHA256) upstream package : 0be44352fd85e9d6efd448ac21bb8fb705c54246955691f18383e8e19b5a2d22
http://download.maxmind.com/download/geoip/database/asnum/GeoIPASNumv6.dat.gz :
  CHECKSUM(SHA256) this package     : f21703899723562b8a206c6de506a156831655345ce7d7a7180e8a41a20db3e5
  CHECKSUM(SHA256) upstream package : f21703899723562b8a206c6de506a156831655345ce7d7a7180e8a41a20db3e5


Generated by fedora-review 0.5.2 (63c24cb) last change: 2014-07-14
Command line :/usr/bin/fedora-review -o --no-cleanup-after -r fedora-rawhide-x86_64 --yum -b 1194798
Buildroot used: fedora-21-x86_64
Active plugins: Generic, Shell-api
Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG

One suggestion I'd make is to clearly identify the EPEL5 related stuff with:

%if %{?el5}

to bracket the %prep and %clean sections that are EL5-specific.

New Package SCM Request
=======================
Package Name: GeoIP-GeoLite-data
Short Description: Free GeoLite IP geolocation country database
Upstream URL: http://dev.maxmind.com/geoip/legacy/geolite/
Owners: pghmcfc philipp
Branches: f20 f21 f22 el5 el6 epel7

Thanks for the review Philip.

Git done (by process-git-requests).

GeoIP-GeoLite-data-2015.04-1.el5,geoipupdate-2.2.1-2.el5,GeoIP-1.6.5-1.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/GeoIP-GeoLite-data-2015.04-1.el5,geoipupdate-2.2.1-2.el5,GeoIP-1.6.5-1.el5

GeoIP-GeoLite-data-2015.04-1.el6,geoipupdate-2.2.1-2.el6,GeoIP-1.6.5-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/GeoIP-GeoLite-data-2015.04-1.el6,geoipupdate-2.2.1-2.el6,GeoIP-1.6.5-1.el6

GeoIP-GeoLite-data-2015.04-1.fc21,geoipupdate-2.2.1-2.fc21,GeoIP-1.6.5-1.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/GeoIP-GeoLite-data-2015.04-1.fc21,geoipupdate-2.2.1-2.fc21,GeoIP-1.6.5-1.fc21

GeoIP-GeoLite-data-2015.04-1.fc22,geoipupdate-2.2.1-2.fc22,GeoIP-1.6.5-1.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/GeoIP-GeoLite-data-2015.04-1.fc22,geoipupdate-2.2.1-2.fc22,GeoIP-1.6.5-1.fc22

GeoIP-GeoLite-data-2015.04-1.fc20,geoipupdate-2.2.1-2.fc20,GeoIP-1.6.5-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/GeoIP-GeoLite-data-2015.04-1.fc20,geoipupdate-2.2.1-2.fc20,GeoIP-1.6.5-1.fc20

EL-7 ships with GeoIP 1.5.0, and bundles the GeoLite databases. This package would therefore conflict with the EL-7 package if we built it for EPEL-7.

So unless something very unusual happens, I think the epel7 branch of this package will have to be retired.

GeoIP-GeoLite-data-2015.04-1.fc22, geoipupdate-2.2.1-2.fc22, GeoIP-1.6.5-1.fc22 has been pushed to the Fedora 22 stable repository.  If problems still persist, please make note of it in this bug report.

GeoIP-GeoLite-data-2015.04-1.fc20, geoipupdate-2.2.1-2.fc20, GeoIP-1.6.5-1.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.

GeoIP-GeoLite-data-2015.04-1.fc21, geoipupdate-2.2.1-2.fc21, GeoIP-1.6.5-1.fc21 has been pushed to the Fedora 21 stable repository.  If problems still persist, please make note of it in this bug report.

I think that should be symlinks
GeoIPCity.dat -> GeoLiteCity.dat
GeoIPCityv6.dat -> GeoLiteCityv6.dat

Without this symlinks geoiplookup -v don't show "GeoIP City Edition" and "GeoIP City Edition V6"

GeoIP Country Edition: GEO-106FREE 20150407 Build 1 Copyright (c) 2015 MaxMind Inc All Rights Reserved                                                       
GeoIP ASNum Edition: GEO-117 20150405 Build 1 Copyright (c) 2015 MaxMind Inc All Rights Reserved                                                             
GeoIP Country V6 Edition: GEO-106FREE 20150407 Build 1 Copyright (c) 2015 MaxMind Inc All Rights Reserved                                                    
GeoIP ASNum V6 Edition: GEO-117 20150405 Build 1 Copyright (c) 2015 MaxMind Inc All Rights Reserved                                                          

With symlinks

GeoIP Country Edition: GEO-106FREE 20150407 Build 1 Copyright (c) 2015 MaxMind Inc All Rights Reserved                                                       
GeoIP City Edition, Rev 1: GEO-533LITE 20150407 Build 1 Copyright (c) 2015 MaxMind Inc All Rights Reserved                                                   
GeoIP ASNum Edition: GEO-117 20150405 Build 1 Copyright (c) 2015 MaxMind Inc All Rights Reserved                                                             
GeoIP Country V6 Edition: GEO-106FREE 20150407 Build 1 Copyright (c) 2015 MaxMind Inc All Rights Reserved                                                    
GeoIP ASNum V6 Edition: GEO-117 20150405 Build 1 Copyright (c) 2015 MaxMind Inc All Rights Reserved                                                          
GeoIP City Edition V6, Rev 1: GEO-536LITE 20150407 Build 1 Copyright (c) 2015 MaxMind Inc All Rights Reserved

I've added the symlinks in Rawhide and updated the EPEL packages in testing. Other Fedora releases will get the symlinks when they're updated with the May databases in a couple of weeks.

Thanks for mentioning this, I didn't know about the -v option.

geoipupdate-2.2.1-2.el6, GeoIP-1.6.5-1.el6, GeoIP-GeoLite-data-2015.04-2.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.

geoipupdate-2.2.1-2.el5, GeoIP-GeoLite-data-2015.04-2.el5, GeoIP-1.6.5-2.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.