Spec URL: http://sundaram.fedorapeople.org/packages/transmission-remote-cli.spec SRPM URL: http://sundaram.fedorapeople.org/packages/transmission-remote-cli-0.5.5-1.20100303git.fc12.src.rpm Description: A simple command line interface using ncurses for the Transmission BitTorrent client. It is an alternative to transmissioncli and can be used to control local and remote instances of the Transmission daemon.
A few notes: * Line 30 is not necessary. You're installing /usr/bin/ there, I guess that directory should already exist on any system. ;) To solve the issue you may wanted to solve with that line I suggest to write the line following to that like this: install -Dpm 755 %{SOURCE0} %{buildroot}/%{_bindir}/transmission-remote-cli (the -D is the important switch here, see 'man install') * The full URL to the README.md file is missing, when trying to build the package, that file can not be found * You should preserve timestamps within %install section (-p for install) * You don't need the copy commands in %prep section, since you do any copying with the install commands in %install section * Suggestion how to install the README.md in %install section: install -pm 755 %{SOURCE1} %{buildroot}README.md * This CLI obviously is not developed by the transmission team itself, so the URL http://www.transmissionbt.com/ may not be the correct one. Since I couldn't find a serious looking web page of fagga or this CLI client, I'd suggest to use the "project page" (http://github.com/fagga/transmission-remote-cli) instead.
Hi Dominic, Thanks for the suggestions. The updated spec file may be found at: http://sundaram.fedorapeople.org/packages/transmission-remote-cli.spec And the updated SRPM is at: http://sundaram.fedorapeople.org/packages/transmission-remote-cli-0.5.5-2.20100303git.fc12.src.rpm
$ rpmlint transmission-remote-cli.spec transmission-remote-cli.spec: W: no-buildroot-tag 0 packages and 1 specfiles checked; 0 errors, 1 warnings. The no-buildroot-tag warning can be ignored since you actually use it to install the files. $ rpmlint transmission-remote-cli.spec transmission-remote-cli.spec: W: no-cleaning-of-buildroot %install transmission-remote-cli.spec: W: no-buildroot-tag The first warning can and should be solved by adding 'rm -rf %{buildroot}' at the beginning of the %install section. For second warning, see above. $ rpmlint transmission-remote-cli-0.5.5-2.20100303git.fc12.noarch.rpm transmission-remote-cli.noarch: W: spelling-error %description -l en_US ncurses -> nurses, curses, n curses 1 packages and 0 specfiles checked; 0 errors, 1 warnings. Guess ncurses actually is spelled correctly. ;) Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines [x] Specfile name matches %{name}.spec [x] Package seems to meet Packaging Guidelines [x] Package successfully compiles and builds into binary RPMs on at least one supported architecture. Tested on: Fedora 12/x86_64 [x] Rpmlint output: source RPM: see above binary RPM: see above [x] Package is not relocatable. [x] License in specfile matches actual License and meets Licensing Guidelines License: GPLv3+ [-] License file is included in %doc. [x] Specfile is legible and written in AE [x] Sourcefile in the Package is the same as provided in the mentioned Source SHA1SUM of Source: 52873f09c773101e3ef982d5406205ae878b3c33 [x] Package compiles successfully [x] All build dependencies are listed in BuildRequires [-] Specfile handles locales properly [-] ldconfig called in %post and %postun if required [-] Package owns directorys it creates [-] Package requires other packages for directories it uses. [x] Package does not list a file more than once in the %files listing [x] %files section includes %defattr and permissions are set properly [x] %clean section is there and contains rm -rf %{buildroot} [x] Macros are consistently used [x] Package contains code, or permissable content. [-] Large documentation files are in a -doc subpackage [x] Program runs properly without files listed in %doc [-] Header files are in a -devel package [-] Static libraries are in a -static package [-] Package requires pkgconfig if .pc files are present [-] .so-files are put into a -devel subpackage [-] Subpackages include fully versioned dependency for the base package [-] Any libtool archives (*.la) are removed [-] contains desktop file (%{name}.desktop) if it is a GUI application [x] Package does not own files or directories owned by other packages. [!] %{buildroot} is removed at beginning of %install [-] Filenames are encoded in UTF-8 === SUGGESTED ITEMS === [!] Package contains latest upstream version [x] Package does not include license text files separate from upstream. [-] non-English translations for description and summary [!] Package builds in mock Tested on: F12/x86_64; see my note concerning README.md below [x] Package should compile and build into binary RPMs on all supported architectures. tested build with koji [x] Program runs [-] Scriptlets must be sane, if used. [-] pkgconfig (*.pc) files are placed in a -devel package [-] require package providing a file instead of the file itself no files outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin are required Issues to point out: * You should write down in your ChangeLog what you actually changed in the package or rather in the specfile * The installation of the README.md is not okay yet. The file actually would get installed in /, which obviuously is not the right place for documentation files. Seems I forgot the important thing in my suggestion before, sorry for that. (It really was a 'quick note' it seems). This one will be even better: install -Dpm 644 %{SOURCE1} %{buildroot}/%{_docdir}/%{name}-%{version}/README.md Note you will also have to change the line in %files section accordingly: %doc %{_docdir}/%{name}-%{version}/README.md * While checking the sha1sum I noted there were some changes on the code just today, maybe you want to update the file and the release tag accordingly then? * The release number should always begin with 0. For example a full version string should be something like 0.5.5-0.20100303git. See [1] for the guidelines about that. The principle you are following at preset is the one for post-releases, but there weren't any releases before. Basically, the guideline which concerns to pre-release packages should be applied here. But, I guess there never will be a release after this "pre-release", since the sources just can be obtained from the git repository. I won't be that strict and would approve your package also when you're staying at increasing the release number. Please fix at least the first two issues and I will approve your package then. [1] http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Snapshot_packages
> [!] %{buildroot} is removed at beginning of %install According to http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag, cleaning up of buildroot isn't required from Fedora 10 onwards. > [!] Package contains latest upstream version > * While checking the sha1sum I noted there were some changes on the code just > today, maybe you want to update the file and the release tag accordingly > then? Corrected. > [!] Package builds in mock > Tested on: F12/x86_64; see my note concerning README.md below > * The installation of the README.md is not okay yet. The file actually would > get installed in /, which obviuously is not the right place for documentation > files. --snip-- Corrected > * You should write down in your ChangeLog what you actually changed in the > package or rather in the specfile Corrected > * The release number should always begin with 0. --snip-- Corrected. The updated spec file may be found at: http://sundaram.fedorapeople.org/packages/transmission-remote-cli.spec And the updated SRPM is at: http://sundaram.fedorapeople.org/packages/transmission-remote-cli-0.5.5-0.3.20100310git.fc12.src.rpm
Dominic Hopf, Please complete this review. I would like to see this as part of the Fedora 13 release and we need some time for testing. We are nearing the beta release.
(In reply to comment #4) > > [!] %{buildroot} is removed at beginning of %install > According to http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag, > cleaning up of buildroot isn't required from Fedora 10 onwards. Satya, the mentioned guideline just allows you to omit the BuildRoot-tag at the beginning of the file. This is not the same as the %{buildroot} macro or the $RPM_BUILD_ROOT variable. rpmlint still claims, that the 'rm -rf %{buildroot}' command at the beginning of the %install section is missing. It wouldn't if it really was obsolete. Please add it before requesting CVS access. :) > > [!] Package contains latest upstream version > > * While checking the sha1sum I noted there were some changes on the code just > > today, maybe you want to update the file and the release tag accordingly > > then? > Corrected. > > > [!] Package builds in mock > > Tested on: F12/x86_64; see my note concerning README.md below > > * The installation of the README.md is not okay yet. The file actually would > > get installed in /, which obviuously is not the right place for documentation > > files. > --snip-- > Corrected > > > * You should write down in your ChangeLog what you actually changed in the > > package or rather in the specfile > Corrected > > > * The release number should always begin with 0. > --snip-- > Corrected. > > > The updated spec file may be found at: > http://sundaram.fedorapeople.org/packages/transmission-remote-cli.spec > > And the updated SRPM is at: > http://sundaram.fedorapeople.org/packages/transmission-remote-cli-0.5.5-0.3.20100310git.fc12.src.rpm Thanks very much for correcting the mentioned isssues and for your patience. I'm sorry I was a bit stressed the last weeks and had lot of things to do at work, which caused my delayed answer to this issue. Except the mentioned issue with the %install section, your package looks good now and is APPROVED. Remember to add the 'rm -rf %{buildroot}' to the %install section before requesting the CVS access.
Actually, rpmlint needs to be updated and a bug report filed against it already. Cleaning up the buuldroot is completely unnecessary and is done automatically by newer versions of RPM a clearly indicated in the guidelines. To quote, "The provided buildroot will automatically be cleaned before commands in %install are called." The rpmlint warning only applies to EPEL and not to Fedora anymore.
Although the BuildRoot tag and cleaning the buildroot are no longer stricktly necessary, what is wrong with having these two lines in the spec and making it work on more platforms? If you really want to save a few letters, I suggest to replace install -Dpm 644 %{SOURCE1} %{buildroot}%{_docdir}/%{name}-%{version}/README.md and %doc %{_docdir}/%{name}-%{version}/README.md with a simple %doc %{SOURCE1}
The RPM packages in Fedora are only meant for Fedora. I don't see the point in cluttering the spec file but that is left to the packager. I am only opposing any claims that is *required* anymore. In fact, even a %clean is not necessary anymore in Fedora 13. The rpmlint warnings can be ignored.
You are right, it's up to the packager, but here are some more things I'd like to point out: The versioning of the package is IMO wrong. The release 0.1git... indicates that it is a pre-release and a final version is still to come, but upstream does no releases and uses git only for hosting. Thus the package can simply be called 0.5.5-3 On the other hand the spec lacks a comment how to regenerate the exact source that was used. Upstream is already at 0.56 now, so there should be a comment how to get 0.5.5. See https://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control VCS keys would be nice too, although they are not yet ratified by FESCo. See http://fedoraproject.org/wiki/User:Walters/Packaging_VCS_key_proposal Last but not least the requires for transmission-deamon should IMHO be versioned since the program only supports >= 1.80 but we have older versions in the 'everything' repos.
> Satya, the mentioned guideline just allows you to omit the BuildRoot-tag at the > beginning of the file. This is not the same as the %{buildroot} macro or the > $RPM_BUILD_ROOT variable. rpmlint still claims, that the 'rm -rf %{buildroot}' > command at the beginning of the %install section is missing. It wouldn't if it > really was obsolete. Please add it before requesting CVS access. :) > Corrected > > Thanks very much for correcting the mentioned isssues and for your patience. > I'm sorry I was a bit stressed the last weeks and had lot of things to do at > work, which caused my delayed answer to this issue. Except the mentioned issue > with the %install section, your package looks good now and is APPROVED. > Remember to add the 'rm -rf %{buildroot}' to the %install section before > requesting the CVS access. No problem :) Thanks a lot for reviewing!
Thank you for the feedback :) (In reply to comment #10) > You are right, it's up to the packager, but here are some more things I'd like > to point out: > > The versioning of the package is IMO wrong. The release 0.1git... indicates > that it is a pre-release and a final version is still to come, but upstream > does no releases and uses git only for hosting. Thus the package can simply be > called 0.5.5-3 > Corrected. > On the other hand the spec lacks a comment how to regenerate the exact source > that was used. Upstream is already at 0.56 now, so there should be a comment > how to get 0.5.5. See > https://fedoraproject.org/wiki/Packaging:SourceURL#Using_Revision_Control > Corrected > VCS keys would be nice too, although they are not yet ratified by FESCo. See > http://fedoraproject.org/wiki/User:Walters/Packaging_VCS_key_proposal > Added > Last but not least the requires for transmission-deamon should IMHO be > versioned since the program only supports >= 1.80 but we have older versions in > the 'everything' repos. Corrected. The updated spec file can be found at http://sundaram.fedorapeople.org/packages/transmission-remote-cli.spec The new SRPM is at http://sundaram.fedorapeople.org/packages/transmission-remote-cli-0.5.5-4.src.rpm
New Package CVS Request ======================= Package Name: transmission-remote-cli Short Description: A console client for the Transmission BitTorrent client Owners: satyak sundaram Branches: F-13 InitialCC:
CVS done (by process-cvs-requests.py).
Is it intentional that there are no files (except the ones created while doing the branch) for this in devel branch in CVS?
Fixing an issue with my ssh keys. Will build for rawhide as soon as the problem is resolved.
Added version 6.5 (the latest upstream to devel branch.