Bug 1194798 - Review Request: GeoIP-GeoLite-data - Free GeoLite IP geolocation country database
Summary: Review Request: GeoIP-GeoLite-data - Free GeoLite IP geolocation country data...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-02-20 18:46 UTC by Paul Howarth
Modified: 2015-05-14 08:12 UTC (History)
3 users (show)

Fixed In Version: GeoIP-GeoLite-data-2015.04-1.fc22
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-05-14 08:12:38 UTC
philipp: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Paul Howarth 2015-02-20 18:46:14 UTC
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.02-1.fc23.src.rpm

Description:
The GeoLite databases are free IP geolocation databases. This package contains
a database that maps IPv4 addresses to countries.

This product includes GeoLite data created by MaxMind, available from
http://www.maxmind.com/

Fedora Account System Username: pghmcfc

This is an unbundling from the GeoIP package of the separately-distributed GeoLite legacy database data from MaxMind.

The package has legacy spec file elements necessary for EPEL-5 support.

Comment 1 Philip Prindeville 2015-02-20 21:02:46 UTC
So we just rebuild it and bump the Version field every month?

Comment 2 Philip Prindeville 2015-02-20 21:03:42 UTC
(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?

Comment 3 Paul Howarth 2015-02-21 09:49:52 UTC
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.

Comment 4 Paul Howarth 2015-02-27 15:49:02 UTC
Philip, do you have any objection to this approach?

Would you consider reviewing the package?

Comment 5 Philip Prindeville 2015-03-03 20:57:50 UTC
(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.

Comment 6 Philip Prindeville 2015-03-04 18:51:16 UTC
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?

Comment 7 Paul Howarth 2015-03-05 08:10:55 UTC
(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.

Comment 8 Paul Howarth 2015-03-17 12:39:23 UTC
Ping?

Comment 9 Paul Howarth 2015-03-28 15:25:06 UTC
Ping?

Comment 10 Philip Prindeville 2015-03-28 23:18:43 UTC
(In reply to Paul Howarth from comment #9)
> Ping?

Pong.  Running fedora-review now.

Comment 11 Philip Prindeville 2015-03-31 06:22:37 UTC
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.

Comment 12 Philip Prindeville 2015-03-31 06:23:44 UTC
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.

Comment 13 Paul Howarth 2015-03-31 18:14:42 UTC
(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

Comment 14 Philip Prindeville 2015-03-31 18:39:09 UTC
(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

Comment 15 Paul Howarth 2015-04-01 11:09:55 UTC
(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

Comment 16 Paul Howarth 2015-04-11 19:24:59 UTC
Ping?

Comment 17 Philip Prindeville 2015-04-11 22:36:35 UTC
(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!

Comment 18 Paul Howarth 2015-04-12 14:44:59 UTC
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

Comment 19 Philip Prindeville 2015-04-13 04:21:00 UTC
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

Comment 20 Philip Prindeville 2015-04-13 04:35:29 UTC
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.

Comment 21 Paul Howarth 2015-04-13 06:59:28 UTC
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.

Comment 22 Gwyn Ciesla 2015-04-13 19:23:43 UTC
Git done (by process-git-requests).

Comment 23 Fedora Update System 2015-04-14 07:57:59 UTC
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

Comment 24 Fedora Update System 2015-04-14 07:59:20 UTC
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

Comment 25 Fedora Update System 2015-04-14 08:00:39 UTC
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

Comment 26 Fedora Update System 2015-04-14 08:00:56 UTC
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

Comment 27 Fedora Update System 2015-04-14 08:03:18 UTC
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

Comment 28 Paul Howarth 2015-04-14 14:50:24 UTC
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.

Comment 29 Fedora Update System 2015-04-21 19:15:11 UTC
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.

Comment 30 Fedora Update System 2015-04-26 12:48:42 UTC
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.

Comment 31 Fedora Update System 2015-04-26 12:58:48 UTC
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.

Comment 32 nucleo 2015-04-26 15:20:01 UTC
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

Comment 33 Paul Howarth 2015-04-28 09:38:03 UTC
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.

Comment 34 Fedora Update System 2015-05-14 06:28:06 UTC
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.

Comment 35 Fedora Update System 2015-05-14 06:28:47 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.