Bug 1303411

Summary: Review Request: geteltorito - El Torito boot image extractor
Product: [Fedora] Fedora Reporter: Igor Gnatenko <ignatenko>
Component: Package ReviewAssignee: Petr Pisar <ppisar>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: package-review, ppisar
Target Milestone: ---Flags: ppisar: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-07-05 04:58:54 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

Description Igor Gnatenko 2016-01-31 14:07:06 UTC
Spec URL: https://ignatenkobrain.fedorapeople.org/for-review/geteltorito.spec
SRPM URL: https://ignatenkobrain.fedorapeople.org/for-review/geteltorito-0.6-1.fc24.src.rpm
Description:
call:   geteltorito CD-image > toritoimagefile
example:geteltorito /dev/sr0  > /tmp/bootimage

The perl-script will extract the initial/default boot image from a CD if
existant. It will not extract any of other possibly existing bootimages
that are allowed by the El Torito standard.
The imagedata are written to STDOUT all other information is written to
STDERR (eg type and size of image).
If you want to write the image to a file instead of STDOUT you can
secify the filename wanted on the commandline using option -o <filename>
Fedora Account System Username: ignatenkobrain

Comment 1 Upstream Release Monitoring 2016-02-01 15:03:51 UTC
ppisar's scratch build of geteltorito-0.6-1.fc24.src.rpm for f24 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12771616

Comment 2 Petr Pisar 2016-02-01 15:05:25 UTC
URL and Source0 and Source1 are usable. Ok.
The source files are original:
378a6305edb9397978e60b7908a85dd8c2546f2808cb845552d5e4a8ba9baab3 *geteltorito.pl
a76f3347f8e2221d9b6550078ac57a0a2c4e8b1da6ce1a1a8aacab93be7e8e23 *gpl.txt
Ok.

TODO: Use as Source0 <http://userpages.uni-koblenz.de/~krienke/ftp/noarch/geteltorito/geteltorito> instead of <http://userpages.uni-koblenz.de/~krienke/ftp/noarch/geteltorito/geteltorito.pl>. The files are identical while the former name matches the name you want to install it under.

FIX: I think the license is "GPL+" because the source code states "License: GPL" and the gpl.txt file states "If the Program does not specify a version number of
this License, you may choose any version ever published by the Free Software
Foundation" in section 9.

FIX: Correct Summary spelling. Either remove the relading "A" or change it into "An".
FIX: Correct description. The "existant" and "secify" are misspelled. A comma is missing in "STDOUT all", in "STDOUT you". The full stop period is missing at the end of the description.

FIX: The spec file calls "cp" command. You should build-require "coreutils".

FIX: Make the package noarch. Neither the Perl script, or the GPL text depend on an architecture.

$ rpmlint geteltorito.spec ../SRPMS/geteltorito-0.6-1.fc24.src.rpm ../RPMS/x86_64/geteltorito-0.6-1.fc24.x86_64.rpm
geteltorito.spec: W: no-%build-section
geteltorito.src: W: spelling-error %description -l en_US toritoimagefile -> territoriality
geteltorito.src: W: spelling-error %description -l en_US dev -> deb, derv, div
geteltorito.src: W: spelling-error %description -l en_US tmp -> mp, temp, tamp
geteltorito.src: W: spelling-error %description -l en_US bootimage -> boot image, boot-image, bootlegged
geteltorito.src: W: spelling-error %description -l en_US perl -> Perl, peel, perk
geteltorito.src: W: spelling-error %description -l en_US existant -> existent, exist ant, exist-ant
geteltorito.src: W: spelling-error %description -l en_US bootimages -> boot images, boot-images, sabotages
geteltorito.src: W: spelling-error %description -l en_US imagedata -> image data, image-data, imaginative
geteltorito.src: W: spelling-error %description -l en_US eg -> eh, e, g
geteltorito.src: W: spelling-error %description -l en_US secify -> specify, specif
geteltorito.src: W: spelling-error %description -l en_US filename -> file name, file-name, filament
geteltorito.src: W: spelling-error %description -l en_US commandline -> command line, command-line, commandment
geteltorito.src: W: no-%build-section
geteltorito.x86_64: W: spelling-error %description -l en_US toritoimagefile -> territoriality
geteltorito.x86_64: W: spelling-error %description -l en_US dev -> deb, derv, div
geteltorito.x86_64: W: spelling-error %description -l en_US tmp -> mp, temp, tamp
geteltorito.x86_64: W: spelling-error %description -l en_US bootimage -> boot image, boot-image, bootlegged
geteltorito.x86_64: W: spelling-error %description -l en_US perl -> Perl, peel, perk
geteltorito.x86_64: W: spelling-error %description -l en_US existant -> existent, exist ant, exist-ant
geteltorito.x86_64: W: spelling-error %description -l en_US bootimages -> boot images, boot-images, sabotages
geteltorito.x86_64: W: spelling-error %description -l en_US imagedata -> image data, image-data, imaginative
geteltorito.x86_64: W: spelling-error %description -l en_US eg -> eh, e, g
geteltorito.x86_64: W: spelling-error %description -l en_US secify -> specify, specif
geteltorito.x86_64: W: spelling-error %description -l en_US filename -> file name, file-name, filament
geteltorito.x86_64: W: spelling-error %description -l en_US commandline -> command line, command-line, commandment
geteltorito.x86_64: E: no-binary
geteltorito.x86_64: W: no-documentation
geteltorito.x86_64: W: no-manual-page-for-binary geteltorito
2 packages and 1 specfiles checked; 1 errors, 28 warnings.
FIX: Correct the spelling.

$ rpm -q -lv -p ../RPMS/x86_64/geteltorito-0.6-1.fc24.x86_64.rpm 
-rwxr-xr-x    1 root    root                     6602 Feb  1 15:34 /usr/bin/geteltorito
drwxr-xr-x    2 root    root                        0 Feb  1 15:53 /usr/share/licenses/geteltorito
-rw-rw-r--    1 root    root                    17996 Feb  1 15:34 /usr/share/licenses/geteltorito/gpl.txt
File permissions and layout are Ok.

$ rpm -q --requires -p ../RPMS/x86_64/geteltorito-0.6-1.fc24.x86_64.rpm | sort -f | uniq -c
      1 /usr/bin/perl
      1 perl(Getopt::Std)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
Binary requires are Ok.

$ rpm -q --provides -p ../RPMS/x86_64/geteltorito-0.6-1.fc24.x86_64.rpm | sort -f | uniq -c
      1 geteltorito = 0.6-1.fc24
      1 geteltorito(x86-64) = 0.6-1.fc24
Binary provides Ok.

$ resolvedeps rawhide ../RPMS/x86_64/geteltorito-0.6-1.fc24.x86_64.rpm 
Binary dependencies resolvable. Ok.

Package builds in F24 (http://koji.fedoraproject.org/koji/taskinfo?taskID=12771616). Ok.

Otherwise the package in line with Fedora and Perl packaging guideline.

Please correct all `FIX' items, condider fixing `TODO' items and provide new spec file.
Resolution: Package NOT approved.

Comment 3 Igor Gnatenko 2016-02-01 16:49:18 UTC
License is clear GPLv2+.

Others I will fix.

Comment 5 Petr Pisar 2016-06-28 13:55:33 UTC
Spec file changes:

--- geteltorito.spec.old        2016-01-31 15:06:06.000000000 +0100
+++ geteltorito.spec    2016-06-28 15:35:30.387000000 +0200
@@ -1,24 +1,26 @@
-Name:     geteltorito
-Version:  0.6
-Release:  1%{?dist}
-Summary:  A El Torito boot image extractor
-
-License:  GPLv2
-URL:      http://userpages.uni-koblenz.de/~krienke/ftp/noarch/geteltorito/
-Source0:  %{url}/%{name}.pl
-Source1:  %{url}/gpl.txt
+Name:           geteltorito
+Version:        0.6
+Release:        2%{?dist}
+Summary:        El Torito boot image extractor
+
+License:        GPLv2+
+URL:            http://userpages.uni-koblenz.de/~krienke/ftp/noarch/geteltorito/
+Source0:        %{url}/%{name}
+Source1:        %{url}/gpl.txt
+
+BuildArch:      noarch
 
 %description
 call:   geteltorito CD-image > toritoimagefile
 example:geteltorito /dev/sr0  > /tmp/bootimage
 
 The perl-script will extract the initial/default boot image from a CD if
-existant. It will not extract any of other possibly existing bootimages
+existent. It will not extract any of other possibly existing bootimages
 that are allowed by the El Torito standard.
-The imagedata are written to STDOUT all other information is written to
+The imagedata are written to STDOUT, all other information is written to
 STDERR (eg type and size of image).
 If you want to write the image to a file instead of STDOUT you can
-secify the filename wanted on the commandline using option -o <filename>
+specify the filename wanted on the commandline using option -o <filename>.
 
 %prep
 %autosetup -T -c
@@ -33,5 +35,9 @@
 %{_bindir}/%{name}
 
 %changelog
+* Tue Jun 28 2016 Igor Gnatenko <ignatenko> - 0.6-2
+- Fix spelling
+- Trivial fixes
+
 * Sun Jan 31 2016 Igor Gnatenko <i.gnatenko.brain> - 0.6-1
 - Initial package

> TODO: Use as Source0 <http://userpages.uni-koblenz.de/~krienke/ftp/noarch/geteltorito/geteltorito>
> instead of <http://userpages.uni-koblenz.de/~krienke/ftp/noarch/geteltorito/geteltorito.pl>.
> The files are identical while the former name matches the name you want to install it under.
+Source0:        %{url}/%{name}
+Source1:        %{url}/gpl.txt
Ok.
TODO: The %url already contains trailing slash. Please remove the slash separator in Source0 and Source1.

> FIX: I think the license is "GPL+" because the source code states "License: GPL" and
> the gpl.txt file states "If the Program does not specify a version number of
> this License, you may choose any version ever published by the Free Software
> Foundation" in section 9.
+License:        GPLv2+
Ok. Although I still think "GPL+" is correct.

> FIX: Correct Summary spelling. Either remove the relading "A" or change it into "An".
> FIX: Correct description. The "existant" and "secify" are misspelled. A comma is
> missing in "STDOUT all", in "STDOUT you". The full stop period is missing at the end
> of the description.
 The perl-script will extract the initial/default boot image from a CD if
-existant. It will not extract any of other possibly existing bootimages
+existent. It will not extract any of other possibly existing bootimages
 that are allowed by the El Torito standard.
-The imagedata are written to STDOUT all other information is written to
+The imagedata are written to STDOUT, all other information is written to
 STDERR (eg type and size of image).
 If you want to write the image to a file instead of STDOUT you can
-secify the filename wanted on the commandline using option -o <filename>
+specify the filename wanted on the commandline using option -o <filename>.
Ok.

> FIX: The spec file calls "cp" command. You should build-require "coreutils".
TODO: I really recommend to do it.

> FIX: Make the package noarch. Neither the Perl script, or the GPL text depend on an
> architecture.
+BuildArch:      noarch
Ok.

FIX: Build-require `perl-generators'. This is a dependency required by new Perl packaging guidelines <https://fedoraproject.org/wiki/Packaging:Perl#Build_Dependencies>. Without the dependency, your binary package will miss some run-time dependencies.

$ rpmlint geteltorito.spec ../SRPMS/geteltorito-0.6-2.fc25.src.rpm ../RPMS/noarch/geteltorito-0.6-2.fc25.noarch.rpm
geteltorito.spec: W: no-%build-section
(none): E: error while reading /home/test/rpmbuild/SRPMS/geteltorito-0.6-2.fc25.src.rpm: cannot use a string pattern on a bytes-like object
geteltorito.noarch: W: spelling-error Summary(en_US) El -> E, L, Eli
geteltorito.noarch: W: spelling-error %description -l en_US toritoimagefile -> territoriality
geteltorito.noarch: W: spelling-error %description -l en_US dev -> deb, derv, div
geteltorito.noarch: W: spelling-error %description -l en_US tmp -> mp, temp, tamp
geteltorito.noarch: W: spelling-error %description -l en_US bootimage -> boot image, boot-image, bootlegged
geteltorito.noarch: W: spelling-error %description -l en_US perl -> Perl, peel, perk
geteltorito.noarch: W: spelling-error %description -l en_US bootimages -> boot images, boot-images, sabotages
geteltorito.noarch: W: spelling-error %description -l en_US imagedata -> image data, image-data, imaginative
geteltorito.noarch: W: spelling-error %description -l en_US eg -> eh, e, g
geteltorito.noarch: W: spelling-error %description -l en_US filename -> file name, file-name, filament
geteltorito.noarch: W: spelling-error %description -l en_US commandline -> command line, command-line, commandment
geteltorito.noarch: W: no-documentation
geteltorito.noarch: W: no-manual-page-for-binary geteltorito
1 packages and 1 specfiles checked; 0 errors, 14 warnings.
rpmlint is Ok.

Package builds in F25 (http://koji.fedoraproject.org/koji/taskinfo?taskID=14689062). Ok.

Please correct all `FIX' items, consider fixing `TODO' items and provide a new spec file.
Resolution: Package NOT approved.

Comment 7 Petr Pisar 2016-06-29 05:57:42 UTC
Spec file changes:

--- geteltorito.spec.old        2016-06-28 15:35:30.387000000 +0200
+++ geteltorito.spec    2016-06-28 16:04:52.000000000 +0200
@@ -1,13 +1,16 @@
 Name:           geteltorito
 Version:        0.6
-Release:        2%{?dist}
+Release:        3%{?dist}
 Summary:        El Torito boot image extractor
 
 License:        GPLv2+
-URL:            http://userpages.uni-koblenz.de/~krienke/ftp/noarch/geteltorito/
+URL:            http://userpages.uni-koblenz.de/~krienke/ftp/noarch/geteltorito
 Source0:        %{url}/%{name}
 Source1:        %{url}/gpl.txt
 
+BuildRequires:  coreutils
+BuildRequires:  perl-generators
+
 BuildArch:      noarch
 
 %description
@@ -35,6 +38,10 @@
 %{_bindir}/%{name}
 
 %changelog
+* Tue Jun 28 2016 Igor Gnatenko <ignatenko> - 0.6-3
+- Add few BRs
+- Trivial fixes
+
 * Tue Jun 28 2016 Igor Gnatenko <ignatenko> - 0.6-2
 - Fix spelling
 - Trivial fixes


> TODO: The %url already contains trailing slash. Please remove the slash
> separator in Source0 and Source1.
-URL:            http://userpages.uni-koblenz.de/~krienke/ftp/noarch/geteltorito/
+URL:            http://userpages.uni-koblenz.de/~krienke/ftp/noarch/geteltorito
 Source0:        %{url}/%{name}
 Source1:        %{url}/gpl.txt

This is not the best fix. The URL value should end with a slash because the HTTP document is a directory, not a file. If you try retrieving the URL, you can see redirects leading to a URL with a slash. Moreover the final URL has https schema. Not http.

The slash should be removed from Source0 and Source1 values instead. Like this:

URL:            https://userpages.uni-koblenz.de/~krienke/ftp/noarch/geteltorito/
Source0:        %{url}%{name}
Source1:        %{url}gpl.txt

> > FIX: The spec file calls "cp" command. You should build-require "coreutils".
> TODO: I really recommend to do it.
+BuildRequires:  coreutils
Ok.

> FIX: Build-require `perl-generators'.
+BuildRequires:  perl-generators
Ok.

$ rpmlint geteltorito.spec ../SRPMS/geteltorito-0.6-3.fc25.src.rpm ../RPMS/noarch/geteltorito-0.6-3.fc25.noarch.rpm
geteltorito.spec: W: no-%build-section
(none): E: error while reading /home/test/rpmbuild/SRPMS/geteltorito-0.6-3.fc25.src.rpm: cannot use a string pattern on a bytes-like object
geteltorito.noarch: W: spelling-error Summary(en_US) El -> E, L, Eli
geteltorito.noarch: W: spelling-error %description -l en_US toritoimagefile -> territoriality
geteltorito.noarch: W: spelling-error %description -l en_US dev -> deb, derv, div
geteltorito.noarch: W: spelling-error %description -l en_US tmp -> mp, temp, tamp
geteltorito.noarch: W: spelling-error %description -l en_US bootimage -> boot image, boot-image, bootlegged
geteltorito.noarch: W: spelling-error %description -l en_US perl -> Perl, peel, perk
geteltorito.noarch: W: spelling-error %description -l en_US bootimages -> boot images, boot-images, sabotages
geteltorito.noarch: W: spelling-error %description -l en_US imagedata -> image data, image-data, imaginative
geteltorito.noarch: W: spelling-error %description -l en_US eg -> eh, e, g
geteltorito.noarch: W: spelling-error %description -l en_US filename -> file name, file-name, filament
geteltorito.noarch: W: spelling-error %description -l en_US commandline -> command line, command-line, commandment
geteltorito.noarch: W: no-documentation
geteltorito.noarch: W: no-manual-page-for-binary geteltorito
1 packages and 1 specfiles checked; 0 errors, 14 warnings.
rpmlint is Ok.

Package builds in F25 (http://koji.fedoraproject.org/koji/taskinfo?taskID=14696456). Ok.

Please fix the URL and Source values before building this package.
Resolution: Package APPROVED.

Comment 8 Gwyn Ciesla 2016-06-29 13:00:09 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/geteltorito

Comment 9 Fedora Update System 2016-06-29 13:24:09 UTC
geteltorito-0.6-3.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-93c326e183

Comment 10 Fedora Update System 2016-06-30 22:26:57 UTC
geteltorito-0.6-3.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-93c326e183

Comment 11 Fedora Update System 2016-07-05 04:58:53 UTC
geteltorito-0.6-3.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.