https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Bug ID: 1269609 Summary: Review Request: ari-backup - A wrapper around rdiff-backup Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: rbarlow@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://raw.githubusercontent.com/rbarlow/ari-backup/fedora-package/distribu... SRPM URL: media.electronsweatshop.com/ari-backup-1.0.10-1.fc22.src.rpm Description: A helpful wrapper around rdiff-backup that allows backup jobs to be simple Python modules. Fedora Account System Username: rbarlow
This is my first package, and I need a sponsor. I am one of the contributors to this package upstream. I work full time on the Pulp Project as well, and am an experienced Python programmer.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Randy Barlow rbarlow@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841 (FE-NEEDSPONSOR)
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hobbes1069@gmail.com
--- Comment #1 from Richard Shaw hobbes1069@gmail.com --- I'm tempted to take this review but I'm going to be busy until sometime next week. We'll see if anyone else jumps in.
Until then, here's some quick feedback on your spec file:
What distro & versions are you wanting to support? Unless you're going to go back to RHEL 5 I think a lot of stuff can be removed but I would have to double check the guidelines to be sure for every case.
1. All currently supported versions of Fedora and RHEL 6+ define python_sitelib / python_sitearch: (EPEL 6 machine) $ rpm -E %python_sitelib /usr/lib/python2.6/site-packages
2. Group: tags are optional
3. BuildRoot can be removed
4. rm -rf %{buildroot} in %install is only needed for RHEL < 6
5. %clean is done automatically (only needs to be specified in certain multi-source situations)
6. %defattr(-,root,root,-) is automatic, you only need to specify it for your special cases.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #2 from Richard Shaw hobbes1069@gmail.com --- Spec URL: https://raw.githubusercontent.com/rbarlow/ari-backup/fedora-package/distribu... SRPM URL: http://media.electronsweatshop.com/ari-backup-1.0.10-1.fc22.src.rpm
Fixing the SRPM url for fedora-review
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #3 from Richard Shaw hobbes1069@gmail.com --- Ok, I'm not going to post the full fedora-review output since there's some things that need fixing, but here's some excerpts.
Issues: ======= - Permissions on files are set properly. Note: See rpmlint output See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
I noticed these were intentional but it may help to describe in comments why the special permissions are necessary (more detail in rpmlint output)
- Package contains BR: python2-devel or python3-devel
I'm questioning this one, assuming this package is both python 2 and 3 compatible, and you're using %{_python}, then you would want the build requirement to be whichever python is the default for that system. If it's not python3 compatible we'll need to specify python2 everywhere.
- Package does not contain duplicates in %files. Note: warning: File listed twice: /etc/ari-backup/ari-backup.conf.yaml See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles
- 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. Note: License file LICENSE.txt is marked as %doc instead of %license See: http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
If you want to support EPEL 6 the simplest solution is something like this: %if 0%{?rhel} < 7 || 0%{?fedora} < 21 %doc LICENSE.txt %else %license LICENSE.txt %endif
Rpmlint ------- Checking: ari-backup-1.0.10-1.fc24.noarch.rpm ari-backup-1.0.10-1.fc24.src.rpm ari-backup.noarch: E: description-line-too-long C A helpful wrapper around rdiff-backup that allows backup jobs to be simple Python modules. ari-backup.noarch: E: no-changelogname-tag ari-backup.noarch: W: conffile-without-noreplace-flag /etc/cron.daily/ari-backup ari-backup.noarch: E: non-readable /etc/ari-backup/jobs.d/ari-backup-remote-lvm-demo 600 ari-backup.noarch: E: non-executable-script /etc/ari-backup/jobs.d/ari-backup-remote-lvm-demo 600 /usr/bin/env ari-backup.noarch: E: non-readable /usr/share/doc/ari-backup/LICENSE.txt 700 ari-backup.noarch: E: non-standard-executable-perm /usr/share/doc/ari-backup/LICENSE.txt 700 ari-backup.noarch: W: spurious-executable-perm /usr/share/doc/ari-backup/LICENSE.txt ari-backup.noarch: E: wrong-script-end-of-line-encoding /usr/share/doc/ari-backup/LICENSE.txt ari-backup.noarch: E: non-readable /etc/ari-backup/jobs.d/ari-backup-local-lvm-demo 600 ari-backup.noarch: E: non-executable-script /etc/ari-backup/jobs.d/ari-backup-local-lvm-demo 600 /usr/bin/env ari-backup.noarch: E: non-readable /etc/ari-backup/jobs.d/ari-backup-remote-demo 600 ari-backup.noarch: E: non-executable-script /etc/ari-backup/jobs.d/ari-backup-remote-demo 600 /usr/bin/env ari-backup.noarch: E: non-readable /etc/cron.daily/ari-backup 700 ari-backup.noarch: E: non-standard-executable-perm /etc/cron.daily/ari-backup 700 ari-backup.noarch: E: executable-marked-as-config-file /etc/cron.daily/ari-backup ari-backup.noarch: E: non-standard-dir-perm /etc/ari-backup 700 ari-backup.noarch: E: non-standard-dir-perm /var/lib/ari-backup 700 ari-backup.noarch: E: non-standard-dir-perm /etc/ari-backup/jobs.d 700 ari-backup.noarch: E: non-readable /etc/ari-backup/jobs.d/ari-backup-local-demo 600 ari-backup.noarch: E: non-executable-script /etc/ari-backup/jobs.d/ari-backup-local-demo 600 /usr/bin/env ari-backup.noarch: E: non-standard-dir-perm /usr/share/doc/ari-backup 700 ari-backup.noarch: E: non-readable /usr/share/doc/ari-backup/README.md 700 ari-backup.noarch: E: non-standard-executable-perm /usr/share/doc/ari-backup/README.md 700 ari-backup.noarch: W: spurious-executable-perm /usr/share/doc/ari-backup/README.md ari-backup.noarch: E: non-readable /etc/ari-backup/ari-backup.conf.yaml 600 ari-backup.src: E: description-line-too-long C A helpful wrapper around rdiff-backup that allows backup jobs to be simple Python modules. ari-backup.src: E: no-changelogname-tag
Hopefully we can clear these up or justify them (in the case of the permissions).
If it's important for the executables to only be readable by root, I wonder if they should be in /usr/sbin instead?
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #4 from Randy Barlow rbarlow@redhat.com --- Hi Richard, thank you for the helpful notes! I've updated the spec file (at the same URL) and the SRPM as well. I've replied to a few notes from your first comment below:
(In reply to Richard Shaw from comment #1)
What distro & versions are you wanting to support?
For now I only plan to support Fedora 22 and 23. I may expand support to RHEL 7 later, but I may also wait for RHEL 8 for simplicity. I hope to work with upstream soon to convert this project to Python 3 so supporting only Fedora to begin with would make that easier I think.
- All currently supported versions of Fedora and RHEL 6+ define
python_sitelib / python_sitearch
I removed this.
- Group: tags are optional
I removed this.
- BuildRoot can be removed
Done.
- rm -rf %{buildroot} in %install is only needed for RHEL < 6
I removed this.
- %clean is done automatically (only needs to be specified in certain
multi-source situations)
I removed it.
- %defattr(-,root,root,-) is automatic, you only need to specify it for
your special cases.
I removed this too.
Thanks! I'll reply to your second commend separately as I didn't completely follow everything.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #5 from Randy Barlow rbarlow@redhat.com --- (In reply to Richard Shaw from comment #3)
- Permissions on files are set properly. Note: See rpmlint output
I didn't realize that rpmlint was meant to be run against the packages and not the spec file, so I apologize for not catching some of this myself. rpmlint said there were no errors against the spec file when I tried that before, but I now see these same errors against the srpm and rpm. Good to know!
I noticed these were intentional but it may help to describe in comments why the special permissions are necessary (more detail in rpmlint output)
I adjusted the permissions a little bit, but rpmlint is still upset at me. Since this is backup software it manages sensitive information about other computers, and that is why I felt the need to protect much of the data. I added comments to the spec file to explain why I chose the permissions I did. What do you think, given those comments?
- Package contains BR: python2-devel or python3-devel
I'm questioning this one, assuming this package is both python 2 and 3 compatible, and you're using %{_python}, then you would want the build requirement to be whichever python is the default for that system. If it's not python3 compatible we'll need to specify python2 everywhere.
I think I'm missing something here. I don't see the string "devel" in my spec file. Am I doing something that is bringing this requirement in automatically? This package is only Python 2 currently - do I need to do something different than I currently am doing to make that clear?
- Package does not contain duplicates in %files. Note: warning: File listed twice: /etc/ari-backup/ari-backup.conf.yaml See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles
This one ended up being confusing to figure out, but I think I've got it now. It ended up being a mistake to have a path and a path's parent path listed. I reduced that section down to two lines and the complaints have disappeared.
- 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.
Fixed.
If it's important for the executables to only be readable by root, I wonder if they should be in /usr/sbin instead?
This package only includes one executable - the cron job. The cron job looks at /etc/ari-backup/jobs.d/ for executable files and runs any of them that it finds (this is the upstream design).
I will post the remaining rpmlint output I still see with inline comments explaining what I think about it in my next comment.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #6 from Randy Barlow rbarlow@redhat.com --- Here is the current rpmlint
$ rpmlint -i rpmbuild/RPMS/noarch/ari-backup-1.0.10-1.fc22.noarch.rpm rpmbuild/SRPMS/ari-backup-1.0.10-1.fc22.src.rpm ari-backup.noarch: W: spelling-error Summary(en_US) rdiff -> riff, diff, r diff The value of this tag appears to be misspelled. Please double-check.
I don't think this is a problem. There is another warning about the same thing that I've omitted.
ari-backup.noarch: W: conffile-without-noreplace-flag /etc/cron.daily/ari-backup A configuration file is stored in your package without the noreplace flag. A way to resolve this is to put the following in your SPEC file: %config(noreplace) /etc/your_config_file_here
I didn't put the noreplace on here because it's not really a configuration file, it's a daily cron job. If there are updates to the cron job, I would expect that a user would want the updated cron job. Do you think I should make it config(noreplace) as suggested by the lint?
ari-backup.noarch: E: non-readable /etc/ari-backup/jobs.d/ari-backup-remote-lvm-demo 600 The file can't be read by everybody. Review if this is expected.
This is a demo backup job that is really meant to help guide the user on how to create backup jobs. A normal backup job probably shouldn't be world readable because it may contain sensitive information, so the demo has realistic permissions set as well. Do you think this is reasonable? There are four of these files, so I have trimmed the other three similar notes from this output.
ari-backup.noarch: E: non-executable-script /etc/ari-backup/jobs.d/ari-backup-remote-lvm-demo 600 /usr/bin/env This text file contains a shebang or is located in a path dedicated for executables, but lacks the executable bits and cannot thus be executed. If the file is meant to be an executable script, add the executable bits, otherwise remove the shebang or move the file elsewhere.
This file is not executable because the cron job executes files in this folder that have the execute bit set. Since this is a demo job, we don't really want it to be executable, but we do want it to have the shebang because a real backup job needs a shebang as well. Another option might be to put these examples in /usr/share/doc/ari-backup and maybe drop a README in this job.d that mentions that users can check out examples over there. What do you think? I've also trimmed the other three complaints about the other examples.
ari-backup.noarch: E: non-standard-executable-perm /etc/cron.daily/ari-backup 744 A standard executable should have permission set to 0755. If you get this message, it means that you have a wrong executable permissions in some files included in your package.
I chose 744 because it's very likely that ordinary users should not be able to execute the backup jobs. Of course, if the files in the jobs.d are also not user executable this cron job won't be able to do anything anyway, so perhaps it's not harmful to give it 755 to get this complaint to disappear. What do you think?
ari-backup.noarch: E: non-standard-dir-perm /var/lib/ari-backup 700 A standard directory should have permission set to 0755. If you get this message, it means that you have wrong directory permissions in some dirs included in your package.
I put 0700 on this folder because it contains the backup data from other computers. Since we don't know what kind of data this might be, keeping prying eyes out of this folder is a good idea.
ari-backup.noarch: E: non-standard-dir-perm /etc/ari-backup/jobs.d 700 A standard directory should have permission set to 0755. If you get this message, it means that you have wrong directory permissions in some dirs included in your package.
The backup jobs may contain sensitive information about the hosts that are being backed up. At the very least, they will contain the list of inclusions and exclusions, but they might also contains arbitrary Python code to perform certain actions on the remote hosts that might need to be kept secret.
Let me know what you think, and thanks again for your assistance!
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #7 from Randy Barlow rbarlow@redhat.com --- (In reply to Randy Barlow from comment #6)
ari-backup.noarch: W: conffile-without-noreplace-flag /etc/cron.daily/ari-backup A configuration file is stored in your package without the noreplace flag. A way to resolve this is to put the following in your SPEC file: %config(noreplace) /etc/your_config_file_here
I didn't put the noreplace on here because it's not really a configuration file, it's a daily cron job. If there are updates to the cron job, I would expect that a user would want the updated cron job. Do you think I should make it config(noreplace) as suggested by the lint?
I removed the %config directive and this seems to have gone away.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #8 from Richard Shaw hobbes1069@gmail.com --- Ok, yes it looks like most of the rpmlint issues have been resolved. Also, rpmlint doesn't always get it right so we can choose to ignore it, we just need to understand why it's ok.
Removing %config is probably the right way to go, generally %config is only used for files the admin may need to edit.
Also, I didn't worry about it this time but it's customary to bump the revision of the package when making adjustments to the spec file so it has a clear history. I usually post the new spec and SRPM link (the spec one doesn't change) and then cut-n-paste the relevant changelog entry below the links.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #9 from Richard Shaw hobbes1069@gmail.com --- (In reply to Randy Barlow from comment #5)
- Package contains BR: python2-devel or python3-devel
I'm questioning this one, assuming this package is both python 2 and 3 compatible, and you're using %{_python}, then you would want the build requirement to be whichever python is the default for that system. If it's not python3 compatible we'll need to specify python2 everywhere.
I think I'm missing something here. I don't see the string "devel" in my spec file. Am I doing something that is bringing this requirement in automatically? This package is only Python 2 currently - do I need to do something different than I currently am doing to make that clear?
Ok, python-setuptools is pulling in python2-devel for you. Since it appears that Python 3 is not supported at this time then you must specifically require Python 2 in two different places:
1. In the build requirements BuildRequires: python2-devel
2. When calling python:
%{__python} -> %{__python2}
This is required because at some point Fedora will switch to Python 3 by default and the definition of %{__python} will change.
https://fedoraproject.org/wiki/Packaging:Python
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #10 from Randy Barlow rbarlow@redhat.com --- Hi again Richard! I made the fixes you suggested, and I went ahead and moved those example jobs into the docs folder so that rpmlint would be happier. I think they make sense to be documentation anyway, so IMO it works for everyone. I've raised the release to 2.
Currently, rpmlint only complains about spelling and these two errors which I think are justifiable to ignore:
ari-backup.noarch: E: non-standard-dir-perm /var/lib/ari-backup 700 ari-backup.noarch: E: non-standard-dir-perm /etc/ari-backup/jobs.d 700
Here are the new URLs:
Spec URL: https://raw.githubusercontent.com/rbarlow/ari-backup/fedora-package/distribu... SRPM URL: http://media.electronsweatshop.com/ari-backup-1.0.10-2.fc22.src.rpm
Let me know if there are further improvements I can make, and thanks for taking the time to help!
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #11 from Richard Shaw hobbes1069@gmail.com --- Looks good!
I'm going to be out of pocket for the next few days so what I would recommend if you haven't done so already is do some informal reviews (it's a good idea to let people know it's an informal review so they know you're not doing the real review) on some packages and post the links here.
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group#...
Also, what are you interests? Mostly python packaging? Are there any packages you'd like to co-maintain?
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #12 from Randy Barlow rbarlow@redhat.com --- (In reply to Richard Shaw from comment #11)
I'm going to be out of pocket for the next few days so what I would recommend if you haven't done so already is do some informal reviews (it's a good idea to let people know it's an informal review so they know you're not doing the real review) on some packages and post the links here.
Hi again Richard! Sure thing, I've now done one informal review here on another Python package:
https://bugzilla.redhat.com/show_bug.cgi?id=1268380#c2
I'll try to do a few more reviews in the coming days. If you have a review of my review, that would be helpful (and meta!)
Also, what are you interests? Mostly python packaging? Are there any packages you'd like to co-maintain?
I do work as a full time Python developer so Python is my strength these days. In the past I have also done work in C/C++, Java, and PHP. I would not rate my Java or PHP knowlege to be current enough to be very useful, but I am still pretty good in C/C++ land.
One of my long term goals for wanting to join the Fedora project is getting my full time project included in Fedora:
It is also a Python project, and I suspect that you might cringe if you saw it's current spec file ☺ It's a pretty complicated project with quite a few packages (it's a distributed system) so I didn't want to use it as my first package review. Once I gain some more experience I'd love to get it included in Fedora. We currently host our own repository for Fedora on repos.fedorapeople.org/pulp. I also happen to know that Fedora has some interest in possibly using Pulp to manage some of their systems so it may be beneficial to the project itself.
I also have side interests in network security and scientific computing. My C/C++ experience is from my graduate studies as a computational electromagnetism scientist. I've also done some work in natural language processing with both Python and C++ in the past.
As far as co-maintaining packages, I'd love to help in that way. I don't know of any specific package that both needs a co-maintainer and happens to be a package that I have extensive knowledge of, but I am willing to learn. Do you have a suggestion that might fall into my areas of knowledge?
Thanks again for your guidance!
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #13 from Richard Shaw hobbes1069@gmail.com --- (In reply to Randy Barlow from comment #12)
Hi again Richard! Sure thing, I've now done one informal review here on another Python package:
That's a good start, it wouldn't hurt to find a few more.
Here's a good way to find some, I would skip over the older ones unless you find one you are really interested in:
https://fedoraproject.org/PackageReviewStatus/NEW.html
Also, what are you interests? Mostly python packaging? Are there any packages you'd like to co-maintain?
I do work as a full time Python developer so Python is my strength these days. In the past I have also done work in C/C++, Java, and PHP. I would not rate my Java or PHP knowlege to be current enough to be very useful, but I am still pretty good in C/C++ land.
There is a Fedora Python SIG you may want to join then:
https://fedoraproject.org/wiki/SIGs/Python
We can certainly work towards me becoming your sponsor but if someone in the Python SIG is intereated in becoming your sponsor then that's probably a better fit.
One of my long term goals for wanting to join the Fedora project is getting my full time project included in Fedora:
That looks pretty neat. I wouldn't mind trying it out but it may be a bit overkill for my 2 desktop and 3 laptop home network :)
It is also a Python project, and I suspect that you might cringe if you saw it's current spec file ☺ It's a pretty complicated project with quite a few packages (it's a distributed system) so I didn't want to use it as my first package review. Once I gain some more experience I'd love to get it included in Fedora. We currently host our own repository for Fedora on repos.fedorapeople.org/pulp. I also happen to know that Fedora has some interest in possibly using Pulp to manage some of their systems so it may be beneficial to the project itself.
Yeah, we should use Fedora supplied packages wherever possible. If you want to start working towards getting it into Fedora the best option might be to start with COPR[1] as the only real requirement is that all the content is legal (acceptable license). The full packaging guidelines don't apply.
[1] https://copr.fedoraproject.org/coprs/
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #14 from Randy Barlow rbarlow@redhat.com --- Hi Richard! I haven't forgotten that I need to do some more reviews. I've been traveling for the past week, and will be traveling until mid next week as well. I'll be sure to do a few more when I return home!
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #15 from Randy Barlow rbarlow@redhat.com --- Hi Richard! I did one more review on another Python package here:
https://bugzilla.redhat.com/show_bug.cgi?id=1275517
As for whether you or someone from the Python SIG could sponsor me, I'd appreciate it if you would consider being my sponsor. I feel like we have had a good rapport so far!
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #16 from Randy Barlow rbarlow@redhat.com --- I have reviewed one more package here:
https://bugzilla.redhat.com/show_bug.cgi?id=1268090
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
William Moreno williamjmorenor@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |williamjmorenor@gmail.com Assignee|nobody@fedoraproject.org |williamjmorenor@gmail.com Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
William Moreno williamjmorenor@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC|williamjmorenor@gmail.com | Assignee|williamjmorenor@gmail.com |nobody@fedoraproject.org
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
William Moreno williamjmorenor@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |williamjmorenor@gmail.com Flags|fedora-review? |
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Docs Contact| |hobbes1069@gmail.com Flags| |fedora-review?
--- Comment #17 from Richard Shaw hobbes1069@gmail.com --- Alright, I'm formally taking the review now.
Have you already done a self introduction on the devel list? It's pretty much required to be subscribed as a packager.
I think we've gotten the package in pretty good shape, however I think it would be a good idea to ask on the fedora devel mailing list about the permissions just to make sure there's no unwanted side effects that we can't think of.
Perhaps kill two birds with one stone.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #18 from Randy Barlow rbarlow@redhat.com --- Thank you Richard! I had introduced myself in early October on the devel mailing list:
https://lists.fedoraproject.org/pipermail/devel/2015-October/215426.html
I replied to my introduction (and CC'd you) to ask what others thought about my deviations from ordinary permissions.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #19 from Richard Shaw hobbes1069@gmail.com --- Ok, one more question!
Do you want to support RHEL through EPEL? Since this is designed to run on a server it would seem to be a good fit. EPEL 7 should already work but EPEL 6 doesn't have the %license macro defined, but you can fix that with a one liner:
%files %{!?_licensedir:%global license %doc} %license LICENSE.txt ...
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #20 from Randy Barlow rbarlow@redhat.com --- (In reply to Richard Shaw from comment #19)
Do you want to support RHEL through EPEL?
I had not intended to support EPEL 6, as I would like to work with upstream to port ari-backup to be a Python 3 only application. I'm not certain that we would do that, but I'd like to leave that possibility open for now. If we don't end up doing that I could always add EPEL 6 later, right?
I do intend this to be included in EPEL 7.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #21 from Richard Shaw hobbes1069@gmail.com --- (In reply to Randy Barlow from comment #20)
(In reply to Richard Shaw from comment #19)
Do you want to support RHEL through EPEL?
I do intend this to be included in EPEL 7.
Ok, keep in mind python 3 is only available through Fedora EPEL and the package name is python34.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841 (FE-NEEDSPONSOR) |
--- Comment #22 from Richard Shaw hobbes1069@gmail.com --- The only thing it looks like you need to fix is ownership of /etc/ari-backup. A simple "%dir %{_sysconfdir}/%{name}" before %config should do it.
I have also sponsored you into the packagers group. Use you powers for good!
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
===== 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: "BSD (3 clause)", "Unknown or generated". 22 files have unknown license. Detailed output of licensecheck in /home/build/fedora- review/1269609-ari-backup/licensecheck.txt [x]: Package requires other packages for directories it uses. Note: No known owner of /etc/ari-backup [!]: Package must own all directories that it creates. Note: Directories without known owners: /etc/ari-backup [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [x]: 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. [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 30720 bytes in 5 files. [x]: 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 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
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: [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). [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [-]: 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]: 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]: 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: ari-backup-1.0.10-2.fc24.noarch.rpm ari-backup-1.0.10-2.fc24.src.rpm ari-backup.noarch: W: spelling-error Summary(en_US) rdiff -> riff, diff, r diff ari-backup.noarch: W: spelling-error %description -l en_US rdiff -> riff, diff, r diff ari-backup.noarch: E: non-standard-dir-perm /var/lib/ari-backup 700 ari-backup.noarch: E: non-standard-dir-perm /etc/ari-backup/jobs.d 700 ari-backup.src: W: spelling-error Summary(en_US) rdiff -> riff, diff, r diff ari-backup.src: W: spelling-error %description -l en_US rdiff -> riff, diff, r diff 2 packages and 0 specfiles checked; 2 errors, 4 warnings.
Rpmlint (installed packages) ---------------------------- ari-backup.noarch: E: non-standard-dir-perm /etc/ari-backup/jobs.d 700 ari-backup.noarch: E: non-standard-dir-perm /var/lib/ari-backup 700 1 packages and 0 specfiles checked; 2 errors, 0 warnings.
Requires -------- ari-backup (rpmlib, GLIBC filtered): PyYAML config(ari-backup) crontabs python(abi) python-gflags python-setuptools rdiff-backup
Provides -------- ari-backup: ari-backup config(ari-backup)
Source checksums ---------------- https://github.com/jpwoodbu/ari-backup/archive/1.0.10.tar.gz : CHECKSUM(SHA256) this package : 4626b73455193b3ba67f155eca15cb8e71a9630bb9245d493213d18d2ac81be0 CHECKSUM(SHA256) upstream package : 4626b73455193b3ba67f155eca15cb8e71a9630bb9245d493213d18d2ac81be0
Generated by fedora-review 0.5.3 (bcf15e3) last change: 2015-05-04 Command line :/bin/fedora-review -b 1269609 -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
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #23 from Jon Ciesla limburgher@gmail.com --- Package request has been denied with the reason: fedora-review flag is no `+` but is still `?`
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #24 from Jon Ciesla limburgher@gmail.com --- Package request has been denied with the reason: fedora-review flag is no `+` but is still `?`
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #25 from Jon Ciesla limburgher@gmail.com --- Package request has been denied with the reason: fedora-review flag is no `+` but is still `?`
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #26 from Jon Ciesla limburgher@gmail.com --- Package request has been denied with the reason: fedora-review flag is no `+` but is still `?`
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #27 from Randy Barlow rbarlow@redhat.com --- I have fixed the ownership of /etc/ari-backup. The fixed spec file and source RPMs are here:
SPEC: https://raw.githubusercontent.com/rbarlow/ari-backup/fedora-package/distribu... SRPM: https://storm.electronsweatshop.com/index.php/s/WqRjt6SgyHKDFIH
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #28 from Randy Barlow rbarlow@redhat.com --- Hi again Richard! I assume that it's up to you to set the + flag that Jon mentioned, and that you were waiting on my fix for that folder. Is that right? Are we all set now?
Thanks so much for your guidance as I've been learning the ropes!
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Richard Shaw hobbes1069@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #29 from Richard Shaw hobbes1069@gmail.com --- Yeah, I don't know why his script was picking up this package review since I hadn't approved it yet...
Anyhow, it looks good now!
*** APPROVED ***
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #30 from Jon Ciesla limburgher@gmail.com --- Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/ari-backup
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #31 from Fedora Update System updates@fedoraproject.org --- ari-backup-1.0.10-3.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-24ef0be480
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #32 from Fedora Update System updates@fedoraproject.org --- ari-backup-1.0.10-3.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-120a2062db
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Randy Barlow rbarlow@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |rbarlow@redhat.com
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #33 from Fedora Update System updates@fedoraproject.org --- ari-backup-1.0.10-3.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with $ su -c 'yum --enablerepo=epel-testing update ari-backup' You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-120a2062db
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #34 from Fedora Update System updates@fedoraproject.org --- ari-backup-1.0.10-3.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with $ su -c 'dnf --enablerepo=updates-testing update ari-backup' You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-24ef0be480
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #35 from Fedora Update System updates@fedoraproject.org --- ari-backup-1.0.10-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed| |2015-11-25 16:52:29
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #36 from Fedora Update System updates@fedoraproject.org --- ari-backup-1.0.10-3.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-120a2062db
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |ON_QA Resolution|ERRATA |--- Keywords| |Reopened
--- Comment #37 from Fedora Update System updates@fedoraproject.org --- ari-backup-1.0.10-3.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with $ su -c 'yum --enablerepo=epel-testing update ari-backup' You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-120a2062db
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #38 from Fedora Update System updates@fedoraproject.org --- ari-backup-1.0.10-3.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-120a2062db
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |MODIFIED
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |ON_QA
--- Comment #39 from Fedora Update System updates@fedoraproject.org --- ari-backup-1.0.10-3.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with $ su -c 'yum --enablerepo=epel-testing update ari-backup' You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2015-120a2062db
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
--- Comment #40 from Fedora Update System updates@fedoraproject.org --- ari-backup-1.0.10-3.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=1269609
Randy Barlow rbarlow@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ON_QA |CLOSED Resolution|--- |CURRENTRELEASE Last Closed|2015-11-25 16:52:29 |2016-01-01 01:51:36
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla