Spec URL: https://gbcox.fedorapeople.org/specs/oggify.spec SRPM URL: https://repos.fedorapeople.org/repos/gbcox/forefront/fedora-21/SRPMS/oggify-2.0.7-1.fc21.src.rpm Description: Oggify provides the tools needed to convert an audio library from one format to another. Originally designed to handle the author's need of FLAC to Ogg Vorbis and MP3 as output formats. Requires flac, vorbis-tools and lame for full operation. Supports other formats through a plugin system. Fedora Account System Username: gbcox
Just a note of clarification... although this application will support conversion to the mp3 format, it does not include any forbidden items and functions fine without lame; in much the same manner that other applications (amarok, qmmp, etc.) will work with mp3, but don't include it or require it.
Greetings, This is un-official review of the package. The source mentioned in the spec file cannot be downloaded. 1. Source: Source0: https://github.com/%{owner}/%{project}/archive/%{project}-%{version}.tar.gz wget https://github.com/spr/Oggify/archive/Oggify-2.0.7.tar.gz --2014-12-18 02:34:05-- https://github.com/spr/Oggify/archive/Oggify-2.0.7.tar.gz Resolving github.com (github.com)... 192.30.252.128 Connecting to github.com (github.com)|192.30.252.128|:443... connected. HTTP request sent, awaiting response... 302 Found Location: https://codeload.github.com/spr/Oggify/tar.gz/Oggify-2.0.7 [following] --2014-12-18 02:34:07-- https://codeload.github.com/spr/Oggify/tar.gz/Oggify-2.0.7 Resolving codeload.github.com (codeload.github.com)... 192.30.252.145 Connecting to codeload.github.com (codeload.github.com)|192.30.252.145|:443... connected. HTTP request sent, awaiting response... 404 Not Found 2. License. Please refer to https://fedoraproject.org/wiki/Licensing:Main for the License to be mentioned in spec file. It's mentioned as GNU GPLv2 or later, it should be GPLv2+ 3. Also run rpmlint on the srpm and correct all the errors.
Hello, Thank you very much for taking the time to review the package. I've made the suggested changes and all the errors are now corrected. The way github handles versions and submodules is a bit quirky but I found a workaround.
Could you follow the method suggested in below link when giving source urls from github https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Github
I saw that, but unfortunately that method won't work. There is an issue with github that submodules are not included in the archives - so if I were to do that, there would be an error mismatch between the local source archive which builds the package and url version. The only way to get around it is to use the version control method: https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Using_Revision_Control I also did a google search and found the emacs-rinari package which is currently in the Fedora Production repository uses a git repo with submodules. I took the same approach. The one change I made was to use the git --branch option which pulls the exact version of code, ensuring a match. Thanks again for taking the time to review my package.
Thanks for the comments, Could you update the spec file and srpm and provide the links here, So that it could be reviewed by others.
Oops, forgot to modify that... here it is: Spec URL: https://gbcox.fedorapeople.org/specs/oggify.spec SRPM URL: https://repos.fedorapeople.org/repos/gbcox/forefront/fedora-21/SRPMS/oggify-2.0.7-3.fc21.src.rpm
Please check the below review: Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - Package is not relocatable. Note: Package has a "Prefix:" tag See: http://fedoraproject.org/wiki/Packaging/Guidelines#RelocatablePackages ===== MUST items ===== Generic: [ ]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [ ]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "GPL (v2 or later) (with incorrect FSF address)", "Unknown or generated". 4 files have unknown license. Detailed output of licensecheck in /home/orion/review/abc/review-oggify/licensecheck.txt [ ]: Package requires other packages for directories it uses. Note: No known owner of /usr/lib/python2.7/site-packages/oggify/plugins [ ]: Package must own all directories that it creates. Note: Directories without known owners: /usr/lib/python2.7/site- packages/oggify/plugins [ ]: Package contains no bundled libraries without FPC exception. [ ]: Changelog in prescribed format. [ ]: Sources contain only permissible code or content. [ ]: Each %files section contains %defattr if rpm < 4.4 Note: %defattr present but not needed [ ]: 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). [ ]: Package is named according to the Package Naming Guidelines. [ ]: Package does not generate any conflict. [ ]: Package obeys FHS, except libexecdir and /usr/target. [ ]: If the package is a rename of another package, proper Obsoletes and Provides are present. [ ]: Requires correct, justified where necessary. [ ]: Spec file is legible and written in American English. [ ]: Package contains systemd file(s) if in need. [ ]: Package is not known to require an ExcludeArch tag. [ ]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 30720 bytes in 2 files. [ ]: 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]: 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]: Package does not own files or directories owned by other packages. [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]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [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]: 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]: Packages must not store files under /srv, /opt or /usr/local Python: [ ]: Python eggs must not download any dependencies during the build process. [ ]: A package which is used by another package via an egg interface should provide egg info. [ ]: Package meets the Packaging Guidelines::Python [x]: Package contains BR: python2-devel or python3-devel [x]: Binary eggs must be removed in %prep ===== SHOULD items ===== Generic: [!]: Packager, Vendor, PreReq, Copyright tags should not be in spec file Note: Found : Packager: gbcox Found : Vendor: Scott Paul Robertson <spr> See: http://fedoraproject.org/wiki/Packaging:Guidelines#Tags [!]: Buildroot is not present Note: Invalid buildroot found: %{_tmppath}/%{project}-%{version}-%{release}-buildroot 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 [ ]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [ ]: Final provides and requires are sane (see attachments). [ ]: Package functions as described. [ ]: 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. [ ]: Packages should try to preserve timestamps of original installed files. [ ]: Spec use %global instead of %define unless justified. Note: %define requiring justification: %define name oggify, %define owner spr, %define project Oggify, %define version 2.0.7, %define packager gbcox, %define release 3, %define build_req python2-devel, python- mutagen, %define program_req flac, vorbis-tools [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]: SourceX is a working URL. ===== 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: oggify-2.0.7-3.fc20.noarch.rpm oggify-2.0.7-3.fc20.src.rpm oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/mp3_cbr.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/mp3.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/mp3_abr.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/utils.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/__init__.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/flac.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/ogg.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/tag_wrapper/__init__.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/tag_wrapper/mp3.py oggify.noarch: E: incorrect-fsf-address /usr/bin/oggify oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/tag_wrapper/m4a.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/__init__.py oggify.noarch: W: install-file-in-docs /usr/share/doc/oggify/INSTALL 2 packages and 0 specfiles checked; 12 errors, 1 warnings. Rpmlint (installed packages) ---------------------------- ^[]0;<mock-chroot>^G<mock-chroot>[root@pkiserver1 /]# rpmlint oggify oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/mp3_cbr.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/mp3.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/mp3_abr.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/utils.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/__init__.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/flac.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/ogg.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/tag_wrapper/__init__.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/tag_wrapper/mp3.py oggify.noarch: E: incorrect-fsf-address /usr/bin/oggify oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/tag_wrapper/m4a.py oggify.noarch: E: incorrect-fsf-address /usr/lib/python2.7/site-packages/oggify/plugins/__init__.py oggify.noarch: W: install-file-in-docs /usr/share/doc/oggify/INSTALL 1 packages and 0 specfiles checked; 12 errors, 1 warnings. ^[]0;<mock-chroot>^G<mock-chroot>[root@pkiserver1 /]# echo 'rpmlint-done:' Requires -------- oggify (rpmlib, GLIBC filtered): /usr/bin/python2 flac python(abi) vorbis-tools Provides -------- oggify: oggify Source checksums ---------------- http://gbcox.fedorapeople.org/oggify/Oggify-2.0.7.tar.gz : CHECKSUM(SHA256) this package : 706aa09fd7bfbde4947283fca5142585f2f39f12aa40dad4777459d287d8268d CHECKSUM(SHA256) upstream package : 706aa09fd7bfbde4947283fca5142585f2f39f12aa40dad4777459d287d8268d
Thanks so much. I'll go ahead and fix the "should" items anyway. I didn't see where you set the review flag... I need that before I can proceed to the next step.
I've posted the changes to take care of the "should" items. Spec URL: https://gbcox.fedorapeople.org/specs/oggify.spec SRPM URL: https://repos.fedorapeople.org/repos/gbcox/forefront/fedora-21/SRPMS/oggify-2.0.7-3.fc21.src.rpm Thanks again!
I wanted to send a message to the developer about the incorrect-fsf-address error, but I don't see an issue with it. I looked at: http://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address downloaded the link and compared it to what was in the git-repository, the address is exactly the same, the only thing was a few spacing differences. If rpmlint is just doing a diff, and throwing out a message if there is any kind of mismatch, they should use another message. If it is really actually checking the address, then there is an issue with it. Am I missing something here? If not, I'm going to file a bug against rpmlint. Thanks again for all your help. It has been a very educational experience. Here are the results of the diff: $ diff gpl-2.0.txt git_repositories/Oggify/COPYING 1,2c1,2 < GNU GENERAL PUBLIC LICENSE < Version 2, June 1991 --- > GNU GENERAL PUBLIC LICENSE > Version 2, June 1991 9c9 < Preamble --- > Preamble 59c59 < GNU GENERAL PUBLIC LICENSE --- > GNU GENERAL PUBLIC LICENSE 258c258 < NO WARRANTY --- > NO WARRANTY 280c280 < END OF TERMS AND CONDITIONS --- > END OF TERMS AND CONDITIONS 282c282 < How to Apply These Terms to Your New Programs --- > How to Apply These Terms to Your New Programs
Nevermind on the incorrect-fsf-address error. My bad... while upstream had the correct entry in the COPYING file, the wrong address was embedded in some source files, which was causing the error.
I will take this review. (In reply to Gerald Cox from comment #10) > Spec URL: https://gbcox.fedorapeople.org/specs/oggify.spec > SRPM URL: > https://repos.fedorapeople.org/repos/gbcox/forefront/fedora-21/SRPMS/oggify- > 2.0.7-3.fc21.src.rpm I'm getting an HTTP 404 on that SRPM. Can you post the latest set of URLs for fedora-review's sake, please?
(In reply to Jerry James from comment #13) > I will take this review. > > (In reply to Gerald Cox from comment #10) > > Spec URL: https://gbcox.fedorapeople.org/specs/oggify.spec > > SRPM URL: > > https://repos.fedorapeople.org/repos/gbcox/forefront/fedora-21/SRPMS/oggify- > > 2.0.7-3.fc21.src.rpm > > I'm getting an HTTP 404 on that SRPM. Can you post the latest set of URLs > for fedora-review's sake, please? Sorry, I had moved it to a copr directory a few weeks ago and neglected to update: https://gbcox.fedorapeople.org/copr/oggify-2.0.7-3.fc22.src.rpm
Spec URL: https://gbcox.fedorapeople.org/specs/oggify.spec SRPM URL: https://gbcox.fedorapeople.org/copr/oggify-2.0.7-3.fc22.src.rpm
[MUSTFIX] Package name does not comply to Fedora package naming conventions. Please rename this package. It's Fedora convention to name packages after the upstream tarball's name, which as I understand your spec is "Oggify": ... %global project Oggify .. Source0: http://gbcox.fedorapeople.org/%{name}/%{project}-%{version}.tar.gz ... If you want to, you can add "oggify*" through appropriate explicit "Provides", instead.
(In reply to Ralf Corsepius from comment #16) > [MUSTFIX] > It's Fedora convention to name packages after the upstream tarball's name, > which as I understand your spec is "Oggify": Can you please provide a link which gives that requirement? I couldn't find it.
(In reply to Ralf Corsepius from comment #16) > [MUSTFIX] > Package name does not comply to Fedora package naming conventions. Please > rename this package. > > It's Fedora convention to name packages after the upstream tarball's name, > which as I understand your spec is "Oggify": > ... > %global project Oggify > .. > Source0: http://gbcox.fedorapeople.org/%{name}/%{project}-%{version}.tar.gz > ... > > If you want to, you can add "oggify*" through appropriate explicit > "Provides", instead. This is not consistent with the package naming guidelines, which express a preference for lower-case names: https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Case_Sensitivity. A case could be made for requiring the name to be "python-oggify" or "python-Oggify", since the package ships a python egg. However, since the package also contains a binary in %{_bindir}, the packaging guidelines give enough latitude for the maintainer to choose any of these names, at the maintainer's discretion: - oggify - Oggify - python-oggify - python-Oggify It's really up to you, Gerald, although if upstream has expressed a preference with regards to capitalization, that would carry a lot of weight.
Issues, in no particular order: - I am very uncomfortable with the way the source is handled. I see the comments above about this issue, but I still don't understand why there is a problem with the official upstream URL: https://github.com/spr/Oggify/archive/v2.0.7.tar.gz What is the problem with using that? - Add "%dir %{python2_sitelib}/%{name}/plugins" to %files - The plugins invoke %{_bindir}/nice, so "Requires: coreutils" is technically needed, although possibly a bit silly. - Neither the %clean script nor the %defattr in %files is necessary. Please remove both (unless you plan to build for EPEL5, in which case you will have to make a few other changes to the spec file, anyway). - Consider adding "%doc README.md" to %files. I think it contains some useful information. - I notice that python-mutagen is a BuildRequires, but not a Requires. Is that intentional? - There is no %check script. Is there a simple test you could perform just to verify basic functionality is working? Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated ===== 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. [x]: License field in the package spec file matches the actual license. [x]: Package requires other packages for directories it uses. [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/lib/python2.7/site- packages/oggify/plugins [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [!]: Each %files section contains %defattr if rpm < 4.4 Note: %defattr present but not needed [-]: 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. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. Possible exception: should python-mutagen be a Requires? [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. [!]: Package complies to the Packaging Guidelines See note above on the source tarball. [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: No rpmlint messages. [x]: 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 %license. [x]: Package does not own files or directories owned by other packages. [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]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [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 does not use a name that already exists. [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 Python: [x]: Python eggs must not download any dependencies during the build process. [x]: A package which is used by another package via an egg interface should provide egg info. [x]: Package meets the Packaging Guidelines::Python [x]: Package contains BR: python2-devel or python3-devel [x]: Binary eggs must be removed in %prep ===== SHOULD items ===== Generic: [!]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) Note: %clean present but not required [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [-]: Final provides and requires are sane (see attachments). [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: 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]: Buildroot is not present [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [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: No rpmlint messages. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: oggify-2.0.7-3.fc23.noarch.rpm oggify-2.0.7-3.fc23.src.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Requires -------- oggify (rpmlib, GLIBC filtered): /usr/bin/python2 flac python(abi) vorbis-tools Provides -------- oggify: oggify Source checksums ---------------- http://gbcox.fedorapeople.org/oggify/Oggify-2.0.7.tar.gz : CHECKSUM(SHA256) this package : d288594f77157d4e1134d9aadf9e63c41b32658d05c61e310a50761f4ce0cc4d CHECKSUM(SHA256) upstream package : d288594f77157d4e1134d9aadf9e63c41b32658d05c61e310a50761f4ce0cc4d Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20 Command line :/usr/bin/fedora-review -b 1175023 -m fedora-rawhide-x86_64 Buildroot used: fedora-rawhide-x86_64 Active plugins: Python, Generic, Shell-api Disabled plugins: Java, C/C++, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
(In reply to Jerry James from comment #19) > Issues, in no particular order: > > - I am very uncomfortable with the way the source is handled. I see the > comments above about this issue, but I still don't understand why there is > a > problem with the official upstream URL: > > https://github.com/spr/Oggify/archive/v2.0.7.tar.gz > > What is the problem with using that? Jerry, I'm open to suggestions. The problem is that the way github packages the archives. It doesn't populate the submodules (in this case tag_wrapper). You have to manually: submodule init and update to get it included, then re-create the archive. It's a known issue, people have been complaining, but it still isn't fixed. I searched and searched and the only way I could find to do it was to copy an approach used by another Fedora package (comment #5). I don't particularly like it either, it's kind of a pain. You'd think that there would be a more elegant way to handle, but I haven't been able to find it. > > - Add "%dir %{python2_sitelib}/%{name}/plugins" to %files OK, no problem. > > - The plugins invoke %{_bindir}/nice, so "Requires: coreutils" is technically > needed, although possibly a bit silly. Yes, encountered that when I build for COPR. > > - Neither the %clean script nor the %defattr in %files is necessary. Please > remove both (unless you plan to build for EPEL5, in which case you will > have > to make a few other changes to the spec file, anyway). Thanks, I will remove. > > - Consider adding "%doc README.md" to %files. I think it contains some > useful > information. Will do. > > - I notice that python-mutagen is a BuildRequires, but not a Requires. Is > that intentional? On a previous review, I was told if something was listed in BuildRequires, that the Requires are automatically provided, and to not double-list. Is that not correct? > > - There is no %check script. Is there a simple test you could perform just > to > verify basic functionality is working? > > I'll look into that and provide if I can find something. Thanks for your input. I'll make the changes listed above.
Jerry, FYI... I submitted a question to the development and packaging mailing list asking for alternative solutions for the source packaging. I searched again most of the day yesterday and couldn't find any other solution other than the one I used (and as I pointed out has been used by other Fedora packages).
Jerry, one more comment: If you read: https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Using_Revision_Control you'll see that this is a documented method... it might not be as elegant as one would want, but it is there for situations where the tarball doesn't accurately reflect upstream's development. In the situation of Git, the tarball doesn't include the project submodule.
Spec URL: https://gbcox.fedorapeople.org/specs/oggify.spec SRPM URL: https://gbcox.fedorapeople.org/copr/oggify-2.0.7-4.fc22.src.rpm OK Jerry, Here are the changes. Upstream doesn't have a check... so that not included. It's a relatively simple program anyway. I've been running for 5 years. Thanks again for your help!
(In reply to Jerry James from comment #18) > (In reply to Ralf Corsepius from comment #16) > > [MUSTFIX] > > Package name does not comply to Fedora package naming conventions. Please > > rename this package. > > > > It's Fedora convention to name packages after the upstream tarball's name, > > which as I understand your spec is "Oggify": > > ... > > %global project Oggify > > .. > > Source0: http://gbcox.fedorapeople.org/%{name}/%{project}-%{version}.tar.gz > > ... > > > > If you want to, you can add "oggify*" through appropriate explicit > > "Provides", instead. > > This is not consistent with the package naming guidelines, which express a > preference for lower-case names: > https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Case_Sensitivity. > > A case could be made for requiring the name to be "python-oggify" or > "python-Oggify", since the package ships a python egg. However, since the > package also contains a binary in %{_bindir}, the packaging guidelines give > enough latitude for the maintainer to choose any of these names, at the > maintainer's discretion: > - oggify > - Oggify > - python-oggify > - python-Oggify > > It's really up to you, Gerald, although if upstream has expressed a > preference with regards to capitalization, that would carry a lot of weight. I do not agree with this interpretation and contine to insist on you to rename the packages. Feel free to file a ticket with FPC to have the situation clarified.
(In reply to Ralf Corsepius from comment #24) > (In reply to Jerry James from comment #18) > > (In reply to Ralf Corsepius from comment #16) > > It's really up to you, Gerald, although if upstream has expressed a > > preference with regards to capitalization, that would carry a lot of weight. That's a good idea Jerry, I'll just ask Scott if he has an issue with the name and defer to him. It's his package. I just used lowercase since the guidelines suggested a preference to lowercase. > I do not agree with this interpretation and contine to insist on you to > rename the packages. > > Feel free to file a ticket with FPC to have the situation clarified. Ralf, it would be helpful if you could provide some documentation which supports your interpretation. There is nothing I found which says that package name must match the tarball. Without that, there is nothing that needs to be clarified.
I went ahead and opened: https://fedorahosted.org/fpc/ticket/541 As I mentioned in the ticket, I have searched extensively for documentation supporting Ralf's contention, but have not been able to find any. If Ralf is correct, the guidelines must be changed, because they are incorrect or misleading at best. I don't see how under any reading they support Ralf's contention.
Sorry for the delay. Real Life has been a pain the last few days. (In reply to Gerald Cox from comment #20) > Jerry, I'm open to suggestions. The problem is that the way github packages > the archives. It doesn't populate the submodules (in this case > tag_wrapper). You have to manually: submodule init and update to get it > included, then > re-create the archive. It's a known issue, people have been complaining, but > it still isn't fixed. I searched and searched and the only way I could find > to do it was to copy an approach used by another Fedora package (comment > #5). I don't particularly like it either, it's kind of a pain. You'd think > that > there would be a more elegant way to handle, but I haven't been able to find > it. Okay, I understand the issue with the tarball now, and your solution to the problem is reasonable. > On a previous review, I was told if something was listed in BuildRequires, > that the Requires are automatically provided, and to not double-list. Is > that not correct? That is correct in some circumstances, but not in this one. If you are building against a C or C++ library named foo, then BuildRequires: foo-devel is all you need, because the dependency generator can look at the list of libraries that your ELF objects are linked against and extract the necessary Requires. The perl dependency generator is able to do something similar in most circumstances (although it needs a helping hand once in awhile). Unfortunately, in the python world, this just doesn't happen. You have to list all of the Requires manually. Take a look: $ rpm -q --requires -p oggify-2.0.7-4.fc23.noarch.rpm/usr/bin/python2 flac python(abi) = 2.7 rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PartialHardlinkSets) <= 4.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rpmlib(PayloadIsXz) <= 5.2-1 vorbis-tools There is no mention of python-mutagen. The rule is to examine the generated Requires after building and make sure everything your package needs is listed there. If it is, then great: the dependency generator figured it out for you. If not, then you have to list it manually in the spec file.
(In reply to Gerald Cox from comment #26) > I went ahead and opened: https://fedorahosted.org/fpc/ticket/541 > > As I mentioned in the ticket, I have searched extensively for documentation > supporting Ralf's contention, but have not been able to find any. > > If Ralf is correct, the guidelines must be changed, because they are > incorrect or misleading at best. I don't see how under any reading they > support Ralf's contention. Agreed. Ralf, it would be *very* helpful if you could point to something in the Guidelines that supports your contention, especially in light of the conclusion here: https://fedorahosted.org/fpc/ticket/336
(In reply to Jerry James from comment #27) > > $ rpm -q --requires -p oggify-2.0.7-4.fc23.noarch.rpm/usr/bin/python2 > flac > python(abi) = 2.7 > rpmlib(CompressedFileNames) <= 3.0.4-1 > rpmlib(FileDigests) <= 4.6.0-1 > rpmlib(PartialHardlinkSets) <= 4.0.4-1 > rpmlib(PayloadFilesHavePrefix) <= 4.0-1 > rpmlib(PayloadIsXz) <= 5.2-1 > vorbis-tools > > There is no mention of python-mutagen. The rule is to examine the generated > Requires after building and make sure everything your package needs is > listed there. If it is, then great: the dependency generator figured it out > for you. If not, then you have to list it manually in the spec file. Yup, understood. I've added. Thanks! I've been thinking about an alternative to cloning and manually issuing the git commands; then rebuilding and posting the new archive. The workaround I've come up with pulls the two archives (Oggify and tag_wrapper), changes their permissions (rpmlint had an issue with the git defaults), then in %prep merge the two archives. This approach required me to add %commitx, %shortcommitx and %checkout. It will be in the next review candidate. Thanks for your help in all this!
Spec URL: https://gbcox.fedorapeople.org/specs/oggify.spec SRPM URL: https://gbcox.fedorapeople.org/copr/oggify-2.0.7-5.20150615git4412e37.fc22.src.rpm OK Jerry, hopefully this will do it. Please take a close look at my new approach. This works fine for this package, but it's a extremely simple implementation of git submodules. If everything checks out I am going to prepare some documentation for both methods and submit them up to FPC for consideration. I know Git submodules isn't a widely used feature (thankfully... LOL), but when someone trips across it, would be nice to something that directly addresses it; rather than having to build a logical inference. Thanks again!
(In reply to Gerald Cox from comment #30) > Please take a close look at my new approach. This works fine for this > package, but it's a extremely simple implementation of git submodules. If > everything checks out I am going to prepare some documentation for both > methods and submit them up to FPC for consideration. I know Git submodules > isn't a widely used feature (thankfully... LOL), but when someone trips > across it, would be nice to something that directly addresses it; rather > than having to build a logical inference. Wow, that makes my eyes hurt. :-) I'm fine with either that approach or the previous one. As far as I'm concerned, this package is ready to go. I guess we just need to wait for a response from the FPC on the package name.
(In reply to Jerry James from comment #31) > Wow, that makes my eyes hurt. :-) I'm fine with either that approach or the > previous one. > > As far as I'm concerned, this package is ready to go. I guess we just need > to wait for a response from the FPC on the package name. LOL... yeah, the commit tags are a bit overwhelming, but there is no getting around that. Scott had fixed all the FSF address errors that were reported in comment #8 but didn't tag a new release, thus the %checkout stuff was required. I'll stick with this method. I like the fact I don't have to worry about cloning and maintaining the repository, generating a new archive and uploading it. Yeah, we can wait until the FPC meeting. It's on Thursday, at 9 PT. Thanks again for all your help. I really appreciate it!
Jerry, Attended the FPC meeting today, you are cleared to approved the package, the package name will remain lowercase. Thanks again for your help!
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#General_Naming "When naming a package you can take some cues from the name of the upstream tarball, project name from which this software came, and what has been used for this package by other distributions/packagers in the past. Do not just blindly follow those examples, however, as package names should strive to be consistent within Fedora more than consistent between distros. You should generally use lowercase and turn underscores into dashes unless there's a compelling reason to follow a different upstream convention." Until today's very unpleasant FPC meeting this was meant to read as a suggestion to use the tarball name, which should only be diverged when there are compelling reasons to diverge from this rule. E.g. name clashes with other packages or historic reasons. Also take into account that we are talking about the rpm-names. Dnf and yum are case insensitive in some aspects of package name handling, but rpm itself (which is the only thing that matters here) is case-sensive. This is different from Debian/Ubuntu which has always had a "lowercase only" naming convention and whose tools (to my knowledge) are completely lowercase only.
The FPC has ruled, and I do not see any other issues with this package. It is APPROVED.
New Package SCM Request ======================= Package Name: oggify Short Description: Audio conversion tool for music library conversion Upstream URL: http://scottr.org/oggify/ Owners: gbcox Branches: f22 f23 InitialCC:
(In reply to Ralf Corsepius from comment #34) > https://fedoraproject.org/wiki/Packaging:NamingGuidelines#General_Naming > > "When naming a package you can take some cues from the name of the upstream > tarball, project name from which this software came, and what has been used > for this package by other distributions/packagers in the past. Do not just > blindly follow those examples, however, as package names should strive to be > consistent within Fedora more than consistent between distros. You should > generally use lowercase and turn underscores into dashes unless there's a > compelling reason to follow a different upstream convention." > > Until today's very unpleasant FPC meeting this was meant to read as a > suggestion to use the tarball name, which should only be diverged when there > are compelling reasons to diverge from this rule. E.g. name clashes with > other packages or historic reasons. > I don't understand where you're getting that interpretation. It simply doesn't say that. In any event, if I agreed with that reading (which I don't) you state "suggestion to use" - how is that a blocker as you infer in Comment #16? > Also take into account that we are talking about the rpm-names. Dnf and yum > are case insensitive in some aspects of package name handling, but rpm > itself (which is the only thing that matters here) is case-sensive. Which aspects are case insensitive? I just did a test on package vim-X11. dnf remove vim-x11 doesn't work dnf install vim-x11 doesn't work Those are the commands that folks are going to be using most of the time; and that is the problem. This to me illustrates why the guideline encouraging the use of lowercase is a value add to Fedora. You don't have to research how something is spelled, you know names should be in lowercase, and underscores are hyphens. As I mentioned in ticket #541, it's much easier to install my-favorite-package than My_faVorITe-package. What is the value add to Fedora to keep mixed case names? I don't get it. Yes, I suppose it's a nod to upstream, but most of the time, they could really care less. The majority of folks understand there is value to a uniform, consistent naming policy. > > This is different from Debian/Ubuntu which has always had a "lowercase only" > naming convention and whose tools (to my knowledge) are completely lowercase > only. I completely understand why they choose that path. Again, what is the value add for Fedora to do otherwise?
You also mention in the ticket that "capitalization matters" and give the example that coke is different than Coke. Yes, Linux respects case. However, in regards to package names, this isn't going to case a conflict because of the "Conflicting Package Names" guideline: "Package names which differ only in case are still considered to be conflicting." So the fact that a mixed case project name in Fedora is being converted to lowercase isn't going to cause a future conflict. How else is this an issue?
Git done (by process-git-requests).
oggify-2.0.7-5.20150615git4412e37.fc22 has been submitted as an update for Fedora 22. https://admin.fedoraproject.org/updates/oggify-2.0.7-5.20150615git4412e37.fc22
Rawhide, F22 Builds complete. Submitted to Bodhi for F22.
oggify-2.0.7-5.20150615git4412e37.fc22 has been pushed to the Fedora 22 stable repository.