Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: fmirror - Mirror directories from ftp servers
https://bugzilla.redhat.com/show_bug.cgi?id=478362
Summary: Review Request: fmirror - Mirror directories from ftp servers Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: fabian@bernewireless.net QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/fmirror.spec SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/fmirror-0.8.4-1.fc9.src.rpm
Description: fmirror is a C program to mirror a directory tree from a remote ftp server. It allows regexp (regular expression) pattern matching to help target files that are to be included and excluded. It uses a combination of timestamp, file size and file permissions to decide what files to transfer from the ftp server.
Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1024075
rpmlint output: [fab@laptop024 i386]$ rpmlint fmirror* 2 packages and 0 specfiles checked; 0 errors, 0 warnings.
[fab@laptop024 SRPMS]$ rpmlint fmirror-0.8.4-1.fc9.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=478362
Christoph Wickert fedora@christoph-wickert.de changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |fedora@christoph-wickert.de AssignedTo|nobody@fedoraproject.org |fedora@christoph-wickert.de Flag| |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=478362
--- Comment #1 from Christoph Wickert fedora@christoph-wickert.de 2009-01-03 16:35:57 EDT --- REVIEW FOR deecdd74d33f7ed0cb2cd358c27663c0 fmirror-0.8.4-1.fc9.src.rpm
OK - MUST: $ rpmlint /var/lib/mock/fedora-rawhide-i386/result/fmirror-* 3 packages and 0 specfiles checked; 0 errors, 0 warnings. OK - MUST: The package is named according to the Package Naming Guidelines. OK - MUST: The spec file matches the base package %{name}, in the format %{name}.spec. FIX - MUST: The package must meet the Packaging Guidelines, see below for details
OK - MUST: The package is licensed with a Fedora approved license and meets the Licensing Guidelines (GPLv2+) OK - MUST: The License field in the package spec file matches the actual license (GPLv2+) OK - MUST: The source package includes the text of the license in its own file, and that file is included in %doc. OK - MUST: The spec file is written in American English. OK - MUST: The spec file for the package is legible. OK - MUST: The sources used to build the package match the upstream source, as provided in the spec URL: md5sum 78652ce5bb50e6c120c9ca0988cb9dca OK - MUST: The package successfully compiles and builds into binary rpms on at least one primary architecture: tested on i386 N/A - MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. OK - MUST: All build dependencies are listed in BuildRequires: None, because all build deps are listed in the exceptions section of the Packaging Guidelines. N/A - MUST: The spec file MUST handle locales properly. N/A - MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun. N/A - MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the rationalization for relocation of that specific package. OK - MUST: The package owns all directories that it creates: None except in docdir FIX - MUST: The package does not contain duplicate files in the %files listing, but the files listing needs work, see below
OK - MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line. OK - MUST: The package has a %clean section, which contains rm -rf %{buildroot}. OK - MUST: The package consistently uses macros, as described in the macros section of Packaging Guidelines. OK - MUST: The package contains code. N/A - MUST: Large documentation files should go in a -doc subpackage. OK - MUST: Files included something as %doc do not affect the runtime of the application. N/A - MUST: Header files must be in a -devel package. N/A - MUST: Static libraries must be in a -static package. N/A - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig' (for directory ownership and usability). N/A - MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. N/A - MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release} N/A - MUST: Packages must NOT contain any .la libtool archives. N/A - MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. OK - MUST: The packages does not own files or directories already owned by other packages. OK - MUST: At the beginning of %install, the package runs rm -rf %{buildroot}. OK - MUST: All filenames in rpm packages are be valid UTF-8.
SHOULD Items: N/A - SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available. OK - SHOULD: The package builds in mock. OK - SHOULD: The package compiles and builds into binary rpms on all supported architectures: tested in koji OK - SHOULD: The package functions as described N/A - SHOULD: If scriptlets are used, those scriptlets must be sane. N/A - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. N/A - SHOULD: The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. N/A - SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself. Please see File Dependencies in the Guidelines for further information.
Issues: I'm not happy with the URL tag, but there seems to be no upstream and it's better than nothing.
The timestamp of Source0 does not match, see https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps
Files section: I'm not happy with the two folders in doc. Several Options: 1. leave the files in /usr/share/fmirror, but mark the confif files as %config and the README as %doc. 2. if you mark something as %config, it should usually should be in /etc. If it's in there it should better be %config(noreplace) 3. install the config /usr/share/doc/fmirror-%{version} and rename configs/README. Mark them as %doc, not as %config 4. install the config folder as a subfolder of /usr/share/doc/fmirror-%{version} 5. Patch the Makefile for one of the previous options
Option 2:
%configure --datadir=%{_sysconfdir} make %{?_smp_mflags}
%install rm -rf %{buildroot} make install DESTDIR=%{buildroot} INSTALL="install -p" chmod -x %{buildroot}%{_mandir}/man1/fmirror.1 mv configs/README configs/README.configs rm -rf %config %{buildroot}%{_sysconfdir}/%{name}/README
...
%files %defattr(-,root,root,-) %doc ChangeLog COPYING README %config(noreplace) %{_sysconfdir}/%{name}/
Option 3:
%install rm -rf %{buildroot} make install DESTDIR=%{buildroot} INSTALL="install -p" chmod -x %{buildroot}%{_mandir}/man1/fmirror.1 # these files are packaged in doc rm -rf %{buildroot}%{_datadir}/%{name} mv configs/README configs/README.configs
...
%files %defattr(-,root,root,-) %doc ChangeLog COPYING README %doc configs/*.conf %doc configs/README.configs
Option 4:
%install rm -rf %{buildroot} make install DESTDIR=%{buildroot} INSTALL="install -p" chmod -x %{buildroot}%{_mandir}/man1/fmirror.1 # these files are packaged in doc rm -rf %{buildroot}%{_datadir}/%{name} rm -rf configs/CVS
...
%files %defattr(-,root,root,-) %doc ChangeLog COPYING README %docdir configs/ %doc configs/
Ideas: - Include a fedora.conf file - Debian seems to have a couple of interesting patches, see http://ftp.de.debian.org/debian/pool/main/f/fmirror/fmirror_0.8.4-13.diff.gz
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=478362
--- Comment #2 from Fabian Affolter fabian@bernewireless.net 2009-01-12 08:03:35 EDT --- Thanks Christoph for your review and your help. I will incorporate the changes as soon as possible.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=478362
Christoph Wickert cwickert@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(fabian@bernewirel | |ess.net)
--- Comment #3 from Christoph Wickert cwickert@fedoraproject.org 2009-11-22 22:47:26 EDT --- Please let me know if you are still interested in maintaining this package.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=478362
Christoph Wickert cwickert@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Blocks| |201449(FE-DEADREVIEW) Resolution| |NOTABUG Flag|fedora-review?, |fedora-review- |needinfo?(fabian@bernewirel | |ess.net) |
--- Comment #4 from Christoph Wickert cwickert@fedoraproject.org 2010-04-02 18:04:35 EDT --- This bug was in NEEDINFO state for months now. I will close it now as per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=478362
Christoph Wickert cwickert@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review- |
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla