[Bug 1194798] Review Request: GeoIP-GeoLite-data - Free GeoLite IP geolocation country database

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1194798



--- Comment #14 from Philip Prindeville <philipp@xxxxxxxxxxxxxxxxxxxxx> ---
(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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]