[Bug 1074147] Review Request: apt-cacher-ng - Caching proxy for package files from Debian

[bugzilla at redhat.com]

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



--- Comment #9 from Kenjiro Nakayama <knakayam at redhat.com> ---
(In reply to Christopher Meng from comment #8)

Hi, thank you for your great review.

I updated. But some pending things, please check #3 and #7

Updated Spec URL: http://diy-kenjiro.rhcloud.com/rpms/apt-cacher-ng.spec
Updated SRPM URL:
http://diy-kenjiro.rhcloud.com/rpms/apt-cacher-ng-0.7.25-3.fc20.src.rpm
koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6632491

~~~~~~~~~~~~~~~

> 1. [!]: %build honors applicable compiler flags or justifies otherwise.

Updated like following.

i)   -> Added BR: openssl-devel
ii)  -> Added BR: boost-devel.
iii) -> mkdir build && cd build && %cmake ..
        make %{?_smp_mflags}

> 2. Quoted from INSTALL again:
>  
> " - for apt clients, there is a config snipped in contrib/10-apt-cacher-ng.conf
>    which might be installed into /etc/apt/apt.conf.d/."
>  
> Have you tested it if it works as expected on Fedora?

Yes, I have already been using apt-cacher-ng.
This quote is for client configuration.
As all fedora users use apt-cacher-ng as only cache server, we don't need
client configuration on Fedora.

> 3. This package contains a folder named systemd/, it includes sysconfig and systemd unit, you should use the one shipped in the tarball instead of using a patch. And you've forgotten the sysconfig file.

I updated to include *.service file instead of using a patch.

> it includes sysconfig and systemd unit

Hmm... do you mean the sysconfig is /etc/systemd/system/*.service ?
If so, I think it is not necessary to include package, it should be copied by
user.
If it means *.target, *.target is not necessary for apt-cacher-ng.
Sorry if I misunderstand, and I could not find appropriate packaging guideline.
I checked man 5 systemd.unit.

> 4. [!]: Package contains no bundled libraries without FPC exception.
>  
>    apt-cacher-ng-0.7.25/include/sha1.h
>    apt-cacher-ng-0.7.25/source/sha1.cc
>  
> copylib I think, but not granted exception in:
>  
> https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
>  
> like md5:
>  
> https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#cite_note-0
>  
> So I think you need to file an fpc ticket.

OK, I will file an fpc ticket soon.

> 5. zlib/libpng
>
> Maybe this is a C++ port of Peter Deutsch's version, you need to add
>  
> Provides: bundled(md5-deutsch-c++)

Updated.

> 6. apt-cacher-ng.i686: E: description-line-too-long C A caching proxy. Specialized for package files from Linux distributors, primarily for Debian (and Debian based) distributions.

Updated to less than 79 chars.

> 7. apt-cacher-ng.i686: E: non-standard-executable-perm /etc/cron.daily/apt-cacher-ng 0750L
> apt-cacher-ng.i686: E: executable-marked-as-config-file /etc/cron.daily/apt-cacher-ng
>  
> chmod -x, I think a configuration file doesn't need that.

Pending.
I thought a file which is placed in /etc/cron.daily/ need to be executable. Is
chmod -x really necessary?

> 8. apt-cacher-ng.i686: W: dangling-relative-symlink /etc/apt-cacher-ng/backends_ubuntu.default ../../var/lib/apt-cacher-ng/backends_ubuntu.default
> apt-cacher-ng.i686: W: dangling-relative-symlink /etc/apt-cacher-ng/backends_debian.default ../../var/lib/apt-cacher-ng/backends_debian.default
>  
> You can ignore this, but please ensure they are correct ;)

OK, I see.

> 9. apt-cacher-ng.i686: E: non-executable-script /usr/libexec/apt-cacher-ng/expire-caller.pl 0644L /usr/bin/perl
>  
> Not 755?

Updated, yes it should be 0755.
(updated)-> install -pm 0755 build/in.acng expire-caller.pl urlencode-fixer.pl
distkill.pl $RPM_BUILD_ROOT%{_libexecdir}/apt-cacher-ng/

> 10. 
> - Package uses either %{buildroot} or $RPM_BUILD_ROOT
>   Note: Using both %{buildroot} and $RPM_BUILD_ROOT
>   See: http://fedoraproject.org/wiki/Packaging/Guidelines#macros

Updated, use $RPM_BUILD_ROOT :D

-- 
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