Spec file: https://github.com/AquaL1te/neofetch/blob/master/neofetch.spec SRPM: https://github.com/AquaL1te/neofetch/blob/master/neofetch-2.0.2-1.fc25.src.rpm Description of package: Neofetch displays information about your system next to an image, your OS logo, or any ascii file of your choice. The main purpose of Neofetch is to be used in screenshots to show other users what OS/distro you're running, what theme/icons you're using and more. This is my first package.
SRPM URL points to HTML document. You should include your FAS account name. Spec file looks quite good. The only real problem I can see from it is that the package should own the whole %{_datadir}/%{name}. Also "rm -rf %{buildroot}" are not needed and should be removed.
> %build > > %install Also be aware of: https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps > install -m755 config/config %{buildroot}%{_datadir}/%{name}/config There is no other configuration method in /etc or so? Just this executable shell script? > install -m644 %{name}.* %{buildroot}%{_mandir}/man1 > %{_mandir}/man1/%{name}.1.gz More correct would be %{_mandir}/man1/%{name}.1* which would cover uncompressed manual pages as well as different compression, because the gzip compression of manual pages is not explicitly done inside this spec file but performed by rpmbuild on-the-fly depending on default rpmbuild configuration.
Hello, I am one of the contributors of neofetch, and the maintainer for the third-party package for neofetch in copr. My only concern will be in >%install Since neofetch does have a Makefile, won't it be better to use %make_install instead of copying the files manually? Also, for > %{_datadir}/%{name}/ascii/distro/* > %{_datadir}/%{name}/config We can simplify it to %{_datadir}/%{name}/*
I think I added all the (In reply to Mikolaj Izdebski from comment #1) > SRPM URL points to HTML document. > You should include your FAS account name. > > Spec file looks quite good. The only real problem I can see from it is that > the package should own the whole %{_datadir}/%{name}. Also "rm -rf > %{buildroot}" are not needed and should be removed. FAS account is 'aqual1te', my OpenPGP key is still syncing across the SKS servers, but can be found on this pool of servers already: https://sks-keyservers.net/pks/lookup?op=vindex&search=0x0E45C98AB51428E6&fingerprint=on&exact=on If there is no result, hit F5, the pool of servers is rotating the requests in round robin. Most of the servers in the pool have my key already. Please let me know if I corrected the problems now for the package (links below). (In reply to Michael Schwendt from comment #2) > > %build > > > > %install > > Also be aware of: > https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps > > > > install -m755 config/config %{buildroot}%{_datadir}/%{name}/config > > There is no other configuration method in /etc or so? Just this executable > shell script? > > > > install -m644 %{name}.* %{buildroot}%{_mandir}/man1 > > > %{_mandir}/man1/%{name}.1.gz > > More correct would be > > %{_mandir}/man1/%{name}.1* > > which would cover uncompressed manual pages as well as different > compression, because the gzip compression of manual pages is not explicitly > done inside this spec file but performed by rpmbuild on-the-fly depending on > default rpmbuild configuration. There is no system-wide configuration in e.g. /etc. When the script is ran for the first time it copies a config to ~/.config/neofetch. I added your suggestions, thanks! (In reply to M. Herdiansyah from comment #3) > Hello, I am one of the contributors of neofetch, and the maintainer for the > third-party package for neofetch in copr. > > My only concern will be in > > >%install > > Since neofetch does have a Makefile, won't it be better to use > > %make_install > > instead of copying the files manually? > > Also, for > > > %{_datadir}/%{name}/ascii/distro/* > > %{_datadir}/%{name}/config > > We can simplify it to > > %{_datadir}/%{name}/* Because of the suggestions from comment #2 I couldn't use the Makefile. This is because of the timestamps that need to be preserved. But still a very useful suggestions just like your other suggestions. I added them. Spec file: https://github.com/AquaL1te/neofetch/blob/master/neofetch.spec SRPM: https://github.com/AquaL1te/neofetch/raw/master/neofetch-2.0.2-1.fc25.src.rpm
> We can simplify it to > > %{_datadir}/%{name}/* No, that's not enough, because it would not include the directory %{_datadir}/{%name} itself. See comment 1. > Because of the suggestions from comment #2 I couldn't use the Makefile. Not true: %make_install INSTALL_PROG="install -p" I would always prefer a working Makefile, if included, and modify it, if necessary, compared with the extra burden that would be imposed on you by having to install everything yourself in the %install section and keeping that section accurate. You could even ship the addition of "-p" upstream. > %clean > rm -rf %{buildroot} https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections > %{_mandir}/man1/%{name}.1.* One dot to much. It would not included an uncompressed manual page. This one would: %{_mandir}/man1/%{name}.1*
(In reply to Michael Schwendt from comment #5) > > Because of the suggestions from comment #2 I couldn't use the Makefile. > > Not true: > > %make_install INSTALL_PROG="install -p" > > I would always prefer a working Makefile, if included, and modify it, if > necessary, compared with the extra burden that would be imposed on you by > having to install everything yourself in the %install section and keeping > that section accurate. You could even ship the addition of "-p" upstream. In the next release of neofetch, we will change our commands to a POSIX-compliant one, see https://github.com/dylanaraps/neofetch/blob/master/Makefile This would eliminate the need for install. Once we release 2.1.0, you can use %make_install just fine.
Also, we have _a lot_ of optional dependencies, see: https://github.com/dylanaraps/neofetch/wiki/Dependencies You may want to add some of it (mainly for Xorg handling) to your spec.
Hiya, thanks for packaging Neofetch. I've got a few suggestions: **Required Dependencies** Neofetch only requires `bash` to function. The only other required dependencies are `awk`, `grep` and `uname`. While these **are** also required programs they're guaranteed to be installed already so I don't bother listing them. You can confirm this by unsetting `$PATH` and re-declaring it with only the 4 programs above. \[1\] Every other dependency is optional and neofetch will just not display information for that function if a program is missing. What I propose for your package is making `bash` the only required dependency. Right now this spec file makes neofetch have an indirect dependency on the X server. I'm packaging neofetch for Arch Linux this way and then listing all other dependencies as 'Optional'. \[2\] **Neofetch 3.0** Neofetch 3.0 was released yesterday which introduced the POSIX Makefile so it *should* work without modification on your side now. \[1\]https://gist.githubusercontent.com/dylanaraps/9343178c7792db3c92414a8a6076285e/raw/245d8a2c1c7ee4558264a65188f0831220b1dd49/path.sh \[2\] https://aur.archlinux.org/packages/neofetch/ Thanks! :)
(In reply to Dylan Araps from comment #8) > Every other dependency is optional and neofetch will just not display > information for that function if a program is missing. What I propose for > your package is making `bash` the only required dependency. Right now this > spec file makes neofetch have an indirect dependency on the X server. > > Neofetch 3.0 was released yesterday which introduced the POSIX Makefile so > it *should* work without modification on your side now. Hi Dylan, Thank you for the suggestions! I indeed list a lot of dependencies because I'm discussing these things in IRC to figure out what's the best way to do this. I changed it to 'Suggests' in the spec file for everything except bash. RPM and the DNF depsolver will totally ignore this, in a future release these weak dependencies will be listed as optional. That way people can use that (in the future when DNF supports it) as a reference if not all functionality of the command switches are available. I will discuss this change as well on IRC and I'll create a test branch in git for my experimental suggestions so they won't show up in this ticket :-) I bumped the spec and SRPM to version 3.0 in the master branch and also did a scratch build on Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=17399343 Later this week I expect to have some final draft for this, I still have to read some packaging docs as well. Stay tuned :-)
I've tried a few times to get some feedback about the 'suggests' listed in the spec file on IRC (as mentioned in comment #9). But I didn't get replies on IRC. So I'll try here again to check if it's sane to do that. The spec file: https://github.com/AquaL1te/neofetch/blob/master/neofetch.spec Koji scratch build + SRPMS: https://github.com/AquaL1te/neofetch/blob/master/neofetch.spec Let me know if this is sufficient, I'm eager to see this in the repositories. Thanks!
(In reply to Kees de Jong from comment #10) > I've tried a few times to get some feedback about the 'suggests' listed in > the spec file on IRC (as mentioned in comment #9). But I didn't get replies > on IRC. So I'll try here again to check if it's sane to do that. > > The spec file: https://github.com/AquaL1te/neofetch/blob/master/neofetch.spec > Koji scratch build + SRPMS: > https://github.com/AquaL1te/neofetch/blob/master/neofetch.spec > > Let me know if this is sufficient, I'm eager to see this in the > repositories. Thanks! Sorry, a copy/paste screw up, here is the Koji scratch build link mentioned in comment #10: https://koji.fedoraproject.org/koji/taskinfo?taskID=17399344
This looks like a fine package to me :-) The only thing that came to mind is that there are too many empty lines.
Informal review (new packager, bare with me): I also like this spec file, but I have to agree with Loic, the blank lines are making it uglier. One other suggestion: Currently you have this: URL: https://github.com/dylanaraps/%{name}/tree/%{version} Source0: https://github.com/dylanaraps/%{name}/archive/%{version}.tar.gz I'm unsure if URL should be version independent. If it should be you could fix this up a bit like so: URL: https://github.com/dylanaraps/%{name} Source0: %{url}/archive/%{version}.tar.gz Just a suggestion, great work otherwise. :)
(In reply to Nemanja Milosevic from comment #13) > Informal review (new packager, bare with me): > > I also like this spec file, but I have to agree with Loic, the blank lines > are making it uglier. > > One other suggestion: > > Currently you have this: > > URL: https://github.com/dylanaraps/%{name}/tree/%{version} > Source0: > https://github.com/dylanaraps/%{name}/archive/%{version}.tar.gz > > I'm unsure if URL should be version independent. If it should be you could > fix this up a bit like so: > > URL: https://github.com/dylanaraps/%{name} > Source0: %{url}/archive/%{version}.tar.gz > > Just a suggestion, great work otherwise. :) Thanks for your review! I will change the URL, the version is indeed not really relevant. (In reply to Loic Dachary from comment #12) > This looks like a fine package to me :-) The only thing that came to mind is > that there are too many empty lines. Yes I agree, I kept the blank lines as is from the `rpmdev-newspec` template. I thought it was some sort of standard layout. I will change this as well!
Did a few package reviews myself: #1421245 and #1419032 (I will do more soon) I think this package is ready to be sponsored :-)
Just as a final note, my FAS account is: keesdejong This is where the spec file can be found: https://github.com/AquaL1te/neofetch This is a Koji scratch build of the latest commit: https://koji.fedoraproject.org/koji/taskinfo?taskID=18028522
Looks cleaner now! Good job. :) About the: Source0: https://github.com/dylanaraps/%{name}/archive/%{version}.tar.gz In the future you can also use something like this: Source0: https://github.com/dylanaraps/%{name}/archive/%{version}.tar.gz#/%{name}-%{version}.tar.gz It works the same, but when you use spectool to download sources you don't end up with '3.0.tar.gz', you get 'neofetch-3.0.tar.gz' which is better when you have a lot of packages in your tree. Cheers!
Bumped to version 3.0.1. Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=19122579 Source: https://github.com/AquaL1te/neofetch/blob/master/neofetch.spec FAS username: keesdejong
I quickly glanced over the spec and didn't see any obvious problems, but could you please provide a link to the latest SRPM as well, and also point at a plaintext version of the spec (i.e. https://raw.githubusercontent.com/AquaL1te/neofetch/master/neofetch.spec)? This will make it easier to run the automated fedora-review tool over this package.
(In reply to Ben Rosser from comment #19) > I quickly glanced over the spec and didn't see any obvious problems, but > could you please provide a link to the latest SRPM as well, and also point > at a plaintext version of the spec (i.e. > https://raw.githubusercontent.com/AquaL1te/neofetch/master/neofetch.spec)? > > This will make it easier to run the automated fedora-review tool over this > package. Thank you. Bumped the version of Neofetch to 3.2.0 (latest as of now). Spec: https://raw.githubusercontent.com/AquaL1te/neofetch/master/neofetch.spec Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=20262006 SRPM: https://kojipkgs.fedoraproject.org//work/tasks/2006/20262006/neofetch-3.2.0-1.fc26.src.rpm FAS username: keesdejong Looking for a sponsor.
Just ran the fedora-review tool myself and fixed these minor errors already: neofetch.src: W: name-repeated-in-summary C Neofetch neofetch.src: W: spelling-error %description -l en_US ascii -> ASCII neofetch.src: W: spelling-error %description -l en_US distro -> bistro, district Spec: https://raw.githubusercontent.com/AquaL1te/neofetch/master/neofetch.spec Spec diff: https://github.com/AquaL1te/neofetch/commit/9eeb72eaaae1be8824f2d694b93e49fdc5beafbf Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=20262958 SRPM: https://kojipkgs.fedoraproject.org//work/tasks/2959/20262959/neofetch-3.2.0-1.fc26.src.rpm FAS username: keesdejong Looking for a sponsor.
In general the package looks very good! I have one blocking issue and a couple of comments/pointers. I am not a sponsor, but I can review the package, at which point you can file a ticket at https://pagure.io/packager-sponsors and ask for a sponsor. A sponsor will almost certainly ask you to do some reviews of other packages though before actually sponsoring you. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues ====== - As per https://fedoraproject.org/wiki/Packaging:Guidelines#Configuration_files, you must mark any configuration files with %config or %config(noreplace). I see that there's an /etc/neofetch/config installed by the package; this should be marked as configuration. (Probably %config(noreplace)). Comments (non-blocking) ======================= - Your justification for using Recommends is fine but outdated; modern dnf _does_ install Recommends these days (but allows their removal without removing the main package, and also this behavior can be configured). - It certainly doesn't hurt, but you probably don't strictly need to condition on bash >= 3. Even RHEL5 has a bash at least this new. :) - The "wrong-script-interpreter" rpmlint messages are complaining because of the use of #!/usr/bin/env bash rather than just #!/bin/bash. Someting like this in %prep will fix this: sed 's,/usr/bin/env bash,%{_bindir}/bash,g' -i neofetch sed 's,/usr/bin/env bash,%{_bindir}/bash,g' -i config/config - Does the config file need a shebang? I assume it doesn't, since it appears neofetch simply sources the config to load it (https://github.com/dylanaraps/neofetch/blob/master/neofetch#L3604). If you remove it in %prep you can eliminate the other rpmlint error too. ===== 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. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated". 131 files have unknown license. Detailed output of licensecheck in /home/bjr/Programming/fedora/reviews/1411984-neofetch/licensecheck.txt [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: 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. [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. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 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 %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [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]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [x]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [x]: Package functions as described. [x]: Latest version is packaged. [x]: 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. [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]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [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: There are rpmlint messages (see attachment). [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: neofetch-3.2.0-1.fc27.noarch.rpm neofetch-3.2.0-1.fc27.src.rpm neofetch.noarch: E: wrong-script-interpreter /usr/bin/neofetch /usr/bin/env bash neofetch.noarch: W: non-conffile-in-etc /etc/neofetch/config neofetch.noarch: E: wrong-script-interpreter /etc/neofetch/config /usr/bin/env bash neofetch.noarch: E: non-executable-script /etc/neofetch/config 644 /usr/bin/env bash 2 packages and 0 specfiles checked; 3 errors, 1 warnings. Rpmlint (installed packages) ---------------------------- sh: /usr/bin/python: No such file or directory neofetch.noarch: W: non-conffile-in-etc /etc/neofetch/config neofetch.noarch: E: wrong-script-interpreter /etc/neofetch/config /usr/bin/env bash neofetch.noarch: E: non-executable-script /etc/neofetch/config 644 /usr/bin/env bash neofetch.noarch: E: wrong-script-interpreter /usr/bin/neofetch /usr/bin/env bash 1 packages and 0 specfiles checked; 3 errors, 1 warnings. Requires -------- neofetch (rpmlib, GLIBC filtered): /usr/bin/env bash Provides -------- neofetch: neofetch Source checksums ---------------- https://github.com/dylanaraps/neofetch/archive/3.2.0.tar.gz#/neofetch-3.2.0.tar.gz : CHECKSUM(SHA256) this package : 6aecd51c165a36692b4f6481b3071ab936aafc3fccffabbbfda140567f16431d CHECKSUM(SHA256) upstream package : 6aecd51c165a36692b4f6481b3071ab936aafc3fccffabbbfda140567f16431d Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02 Command line :/usr/bin/fedora-review -b 1411984 -m fedora-rawhide-x86_64 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
@ Kees de Jong I'd like this package to move forward, are you still interested in packaging it?
(In reply to Robert-André Mauchin from comment #23) > @ Kees de Jong > > I'd like this package to move forward, are you still interested in packaging > it? Hi Robert-Andre, I still am, I got sponsored and the package is approved. But for some reason I can't get access to my git repo, which is needed to commit the spec and push the build. $ fedrepo-req neofetch -t 1411984 Error: The Bugzilla bug's review was approved over 60 days ago ^ The above was after 2 weeks the pagure ticket was approved. So I then thought, let's just continue to the next step. $ kinit keesdejong Password for keesdejong: $ fedpkg clone neofetch Cloning into 'neofetch'... FATAL: R any rpms/neofetch keesdejong DENIED by fallthru (or you mis-spelled the reponame) fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. Could not execute clone: Command '['git', 'clone', 'ssh://keesdejong.org/rpms/neofetch', '--origin', 'origin']' returned non-zero exit status 128 I asked a few times on IRC on what to do, but no one replied. So it slowly went down my priorities. I'll contact my sponsor this weekend to find out what I'm missing. Or maybe you know more about this? In any case, expect an update in about a week.
The package has not yet passed review, comment #22 includes: "- As per https://fedoraproject.org/wiki/Packaging:Guidelines#Configuration_files, you must mark any configuration files with %config or %config(noreplace). I see that there's an /etc/neofetch/config installed by the package; this should be marked as configuration. (Probably %config(noreplace))." Fix that first, and the reviewer will likely approve it then
Mock build: https://koji.fedoraproject.org/koji/taskinfo?taskID=22339006 Spec file: https://raw.githubusercontent.com/AquaL1te/neofetch/master/neofetch.spec SRPM: https://kojipkgs.fedoraproject.org//work/tasks/9007/22339007/neofetch-3.3.0-1.fc28.src.rpm Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== 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: "MIT/X11 (BSD like)", "Unknown or generated". 144 files have unknown license. Detailed output of licensecheck in /home/k.dejong/git/neofetch/review-neofetch/licensecheck.txt [ ]: Package contains no bundled libraries without FPC exception. [ ]: Changelog in prescribed format. [ ]: Sources contain only permissible code or content. [ ]: 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 20480 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: 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 requires other packages for directories it uses. [x]: Package must own all directories that it creates. [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]: %config files are marked noreplace or the reason is justified. [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]: No %config files under /usr. [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]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [ ]: 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. [ ]: 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. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [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: There are rpmlint messages (see attachment). [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: neofetch-3.3.0-1.fc26.noarch.rpm neofetch-3.3.0-1.fc26.src.rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- sh: /usr/bin/python: No such file or directory neofetch.noarch: W: invalid-url URL: https://github.com/dylanaraps/neofetch <urlopen error [Errno -2] Name or service not known> 1 packages and 0 specfiles checked; 0 errors, 1 warnings. Requires -------- neofetch (rpmlib, GLIBC filtered): /usr/bin/bash bash config(neofetch) Provides -------- neofetch: config(neofetch) neofetch Source checksums ---------------- https://github.com/dylanaraps/neofetch/archive/3.3.0.tar.gz#/neofetch-3.3.0.tar.gz : CHECKSUM(SHA256) this package : 4808e76bd81da3602cb5be7e01dfed8223b1109e2792755dd0d54126014ee696 CHECKSUM(SHA256) upstream package : 4808e76bd81da3602cb5be7e01dfed8223b1109e2792755dd0d54126014ee696 Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02 Command line :/usr/bin/fedora-review -n neofetch Buildroot used: fedora-26-x86_64 Active plugins: Generic, Shell-api Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
- I don't think you should be using Recommends for optional dependencies. Just use normal Requires to give all fonctionalities to the user. Following https://github.com/dylanaraps/neofetch/wiki/Dependencies, we need: Requires: w3m-img Requires: ImageMagick Requires: feh Requires: scrot Requires: curl Requires: coreutils Requires: xwininfo Requires: xprop Requires: xrandr Requires: bind-utils Requires: pciutils The gawk dependencies is only for iOS. xdotool is not necessary, the function is already covered by xwininfo + xprop or xwininfo + xdpyinfo provided by xorg-x11-utils + Use a simplified Source0: Source0: https://github.com/dylanaraps/%{name}/archive/%{version}/%{name}-%{version}.tar.gz
(In reply to Robert-André Mauchin from comment #27) > - I don't think you should be using Recommends for optional dependencies. > Just use normal Requires to give all fonctionalities to the user. > > Following https://github.com/dylanaraps/neofetch/wiki/Dependencies, we need: > > Requires: w3m-img > Requires: ImageMagick > Requires: feh > Requires: scrot > Requires: curl > Requires: coreutils > Requires: xwininfo > Requires: xprop > Requires: xrandr > Requires: bind-utils > Requires: pciutils > > The gawk dependencies is only for iOS. xdotool is not necessary, the > function is already covered by xwininfo + xprop or xwininfo + xdpyinfo > provided by xorg-x11-utils > > + Use a simplified Source0: > > Source0: > https://github.com/dylanaraps/%{name}/archive/%{version}/%{name}-%{version}. > tar.gz Those are listed as optional dependencies. The reason I choose weak dependencies is because this is package will mostly will be used in a terminal. Some other functionality need the optional dependencies. So if you're running a bare install i.e. a server, then you won't be in favor (I guess) to install all these extra stuff, you won't need on a system without a GUI. An example on a fresh install of Fedora Server: # dnf install /tmp/neofetch-3.3.0-1.fc26.noarch.rpm Last metadata expiration check: 0:09:19 ago on wo 18 okt 2017 16:40:43 CEST. Dependencies resolved. ====================================================================================================================================================================================== Package Arch Version Repository Size ====================================================================================================================================================================================== Installing: neofetch noarch 3.3.0-1.fc26 @commandline 95 k Installing dependencies: ImageMagick-libs x86_64 6.9.9.15-1.fc26 updates 2.2 M OpenEXR-libs x86_64 2.2.0-6.fc26 fedora 628 k compat-openssl10 x86_64 1:1.0.2j-9.fc26 updates 1.1 M fftw-libs-double x86_64 3.3.5-4.fc26 fedora 980 k gdk-pixbuf2-xlib x86_64 2.36.9-1.fc26 updates 50 k ghostscript-core x86_64 9.20-10.fc26 fedora 4.5 M ghostscript-fonts noarch 5.50-36.fc26 fedora 328 k gpm-libs x86_64 1.20.7-10.fc26 fedora 36 k graphite2 x86_64 1.3.10-1.fc26 fedora 117 k harfbuzz x86_64 1.4.4-1.fc26 fedora 253 k ilmbase x86_64 2.2.0-8.fc26 fedora 104 k jbigkit-libs x86_64 2.1-6.fc26 fedora 51 k lcms2 x86_64 2.8-3.fc26 fedora 158 k libICE x86_64 1.0.9-9.fc26 fedora 70 k libSM x86_64 1.2.2-5.fc26 fedora 42 k libXcomposite x86_64 0.4.4-9.fc26 fedora 26 k libXcursor x86_64 1.1.14-8.fc26 fedora 33 k libXfont x86_64 1.5.2-2.fc26 fedora 154 k libXft x86_64 2.3.2-5.fc26 fedora 63 k libXi x86_64 1.7.9-2.fc26 fedora 44 k libXinerama x86_64 1.1.3-7.fc26 fedora 17 k libXmu x86_64 1.1.2-5.fc26 fedora 74 k libXrandr x86_64 1.5.1-2.fc26 fedora 30 k libXt x86_64 1.1.5-4.fc26 fedora 179 k libXtst x86_64 1.2.3-2.fc26 fedora 24 k libXv x86_64 1.0.11-2.fc26 fedora 21 k libXxf86dga x86_64 1.1.4-7.fc26 fedora 23 k libXxf86misc x86_64 1.0.3-12.fc26 fedora 23 k libdatrie x86_64 0.2.9-4.fc26 fedora 30 k libdmx x86_64 1.1.3-7.fc26 fedora 19 k libfontenc x86_64 1.1.3-4.fc26 fedora 34 k libjpeg-turbo x86_64 1.5.1-0.fc26 fedora 153 k libmcpp x86_64 2.7.2-17.fc26 fedora 78 k librsvg2 x86_64 2.40.18-1.fc26 updates 134 k libthai x86_64 0.1.25-2.fc26 fedora 198 k libtiff x86_64 4.0.8-1.fc26 fedora 179 k libwebp x86_64 0.6.0-2.fc26 fedora 259 k libwmf-lite x86_64 0.2.8.4-53.fc26 updates 72 k libxdo x86_64 1:3.20150503.1-3.fc26 fedora 39 k mcpp x86_64 2.7.2-17.fc26 fedora 29 k openjpeg2 x86_64 2.2.0-3.fc26 updates 140 k pango x86_64 1.40.12-1.fc26 updates 288 k perl x86_64 4:5.24.3-395.fc26 updates 6.1 M perl-Carp noarch 1.40-366.fc26 fedora 28 k perl-Errno x86_64 1.25-395.fc26 updates 69 k perl-Exporter noarch 5.72-367.fc26 fedora 32 k perl-File-Path noarch 2.12-367.fc26 fedora 34 k perl-IO x86_64 1.36-395.fc26 updates 134 k perl-NKF x86_64 1:2.1.4-4.fc26 fedora 135 k perl-PathTools x86_64 3.63-367.fc26 fedora 87 k perl-Scalar-List-Utils x86_64 3:1.48-1.fc26 updates 65 k perl-Socket x86_64 4:2.024-2.fc26 fedora 55 k perl-Text-Tabs+Wrap noarch 2013.0523-366.fc26 fedora 22 k perl-Unicode-Normalize x86_64 1.25-366.fc26 fedora 79 k perl-constant noarch 1.33-368.fc26 fedora 23 k perl-libs x86_64 4:5.24.3-395.fc26 updates 1.5 M perl-macros x86_64 4:5.24.3-395.fc26 updates 65 k perl-parent noarch 1:0.236-2.fc26 fedora 18 k perl-threads x86_64 1:2.16-1.fc26 fedora 58 k perl-threads-shared x86_64 1.57-1.fc26 fedora 45 k poppler-data noarch 0.4.7-7.fc26 fedora 2.2 M urw-fonts noarch 3:2.4-23.fc26 fedora 3.0 M w3m x86_64 0.5.3-31.git20170102.fc26 fedora 1.0 M xorg-x11-font-utils x86_64 1:7.5-33.fc26 fedora 81 k Installing weak dependencies: ImageMagick x86_64 6.9.9.15-1.fc26 updates 181 k w3m-img x86_64 0.5.3-31.git20170102.fc26 fedora 37 k xdotool x86_64 1:3.20150503.1-3.fc26 fedora 52 k xorg-x11-server-utils x86_64 7.7-21.fc26 fedora 184 k xorg-x11-utils x86_64 7.5-23.fc26 updates 118 k Transaction Summary ====================================================================================================================================================================================== Install 70 Packages Total size: 28 M Total download size: 28 M Installed size: 87 M Is this ok [y/N]: And now the same package, but then without the weak dependencies: # dnf --setopt=install_weak_deps=False --best install /tmp/neofetch-3.3.0-1.fc26.noarch.rpm Last metadata expiration check: 0:14:00 ago on wo 18 okt 2017 16:40:43 CEST. Dependencies resolved. ====================================================================================================================================================================================== Package Arch Version Repository Size ====================================================================================================================================================================================== Installing: neofetch noarch 3.3.0-1.fc26 @commandline 95 k Transaction Summary ====================================================================================================================================================================================== Install 1 Package Total size: 95 k Installed size: 255 k Is this ok [y/N]: I will contact the developer to verify this, but from what I've seen by using this package, those optional dependencies are mostly used for 'power users'. Which won't apply to a server without a desktop environment.
I apologize, I'd forgotten I'd taken this review! I think the use of weak dependencies is perfectly reasonable here to separate libraries not required in a headless environment. I'll just double-check that everything is good and then hopefully approve the package.
Alright, package looks fine and is APPROVED. Again, really sorry for the delay.
(In reply to Ben Rosser from comment #30) > Alright, package looks fine and is APPROVED. > > Again, really sorry for the delay. No worries, I am super busy anyway, thank you! I will close the 'bug' once the package is submitted to the build and accepted.
(fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/neofetch
Build successful: https://koji.fedoraproject.org/koji/taskinfo?taskID=22707330
neofetch-3.3.0-1.fc27 has been submitted as an update to Fedora 27. https://bodhi.fedoraproject.org/updates/FEDORA-2017-b9c158f42a
neofetch-3.3.0-1.fc27 has been pushed to the Fedora 27 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-2017-b9c158f42a
neofetch-3.3.0-1.fc27 has been pushed to the Fedora 27 stable repository. If problems still persist, please make note of it in this bug report.