Spec URL: ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf.spec SRPM URL: ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf-1.1-1.fc18.src.rpm Description: pypolicyd-spf is a Postfix policy engine for Sender Policy Framework (SPF) checking. It is implemented in pure Python and uses the python-spf (pyspf) module. This SPF policy server implementation provides flexible options for different receiver policies and sender whitelisting to enable it to support a very wide range of requirements. Fedora Account System Username: bojan
*** Bug 606003 has been marked as a duplicate of this bug. ***
Bojan, are you planning to take this to release then? If there is something I can do to help, I will.
Bojan, one question, shouldn't this require postfix or does sendmail use the same policy protocol? Also, it may be good to change Source0: https://launchpad.net/pypolicyd-spf/1.1/1.1/+download/pypolicyd-spf-1.1.tar.gz to Source0: https://launchpad.net/pypolicyd-spf/1.1/%{version}/+download/pypolicyd-spf-%{version}.tar.gz If you wanted to go the extra length, change the first 1.1 to a version code the strips any micro version (1.1.2 is the current, but the 1.1 above is still valid). At least, this seems to be the way many packages are done (including python-pyspf, although it doesn't do the micro bit as it doesn't seem to have that).
No, sendmail uses milters, while pypolicyd uses postfix's own policy addons, documented at http://www.postfix.org/SMTPD_POLICY_README.html. So, no, requiring sendmail or generic MTA won't work here.
New files: ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf.spec ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf-1.1.2-1.fc18.src.rpm
(In reply to comment #2) > Bojan, are you planning to take this to release then? If there is something > I can do to help, I will. Yes, that was the idea. To be honest, I have no idea how I missed bug #606003, because I would never have opened this one if I did. Anyhow, feel free to do a package review.
I've done a quick review: Look into your special and found some problem: 1. Your Source0 use %{version} already so why not replace the %{name} like this: https://launchpad.net/%{name}/1.1/%{version}/+download/%{name}-%{version}.tar.gz 2. No need this in %install section: rm -rf $RPM_BUILD_ROOT
Traceback (most recent call last): File "/usr/libexec/postfix/policyd-spf", line 684, in <module> instance_dict, configData, peruser) File "/usr/libexec/postfix/policyd-spf", line 462, in _spfcheck header += str(authres.AuthenticationResultsHeader(authserv_id = configData.get('Authserv_Id'), NameError: global name 'authres' is not defined I am getting this when trying to test the module.
It appears that the AR header needs the following: https://pypi.python.org/pypi/authres/0.402 I think this would be useful.
It is NOT required, you just have to use the SPF headers.
Other than the AR header problem, which isn't strictly necessary for things to work, this is working great on my end. I couldn't do the spec file any better myself! I hope someone will sponsor this.
(In reply to comment #7) > I've done a quick review: > > Look into your special and found some problem: > > 1. Your Source0 use %{version} already so why not replace the %{name} like > this: > > https://launchpad.net/%{name}/1.1/%{version}/+download/%{name}-%{version}. > tar.gz Done in: ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf-1.1.2-2.fc18.src.rpm ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf.spec > 2. No need this in %install section: > > rm -rf $RPM_BUILD_ROOT This is the result of rpmdev-newspec --type python. Also, it is mentioned here: https://fedoraproject.org/wiki/Packaging:Python I don't know whether it's truly required, but it does not hurt, so I'll leave it in.
(In reply to comment #8) > Traceback (most recent call last): > File "/usr/libexec/postfix/policyd-spf", line 684, in <module> > instance_dict, configData, peruser) > File "/usr/libexec/postfix/policyd-spf", line 462, in _spfcheck > header += str(authres.AuthenticationResultsHeader(authserv_id = > configData.get('Authserv_Id'), > NameError: global name 'authres' is not defined > > I am getting this when trying to test the module. I don't think this is packaged for Fedora yet. But by all means, feel free to package it and then we can make this package require authres. I am guessing you have a non-default config when this happens, right?
Correct. Header_Type = AR is what causes it. = SPF is fine. I will look into packaging it. Have you been able to find a sponsor for the package?
(In reply to comment #14) > I will look into packaging it. Have you been able to find a sponsor for the > package? Don't need a sponsor. Already have several packages that I maintain. Just need someone to review.
I am afraid I cannot do the review as I am not part of the packagers group. I would be more than happy to do it if I were.
Changelog is NOT in the proper format as you have two messages about bumping to the new version, one is in the wrong position. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed Issues: ======= - Package contains BR: python2-devel or python3-devel See: http://fedoraproject.org/wiki/Packaging:Python#BuildRequires ===== 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]: Package contains no bundled libraries without FPC exception. [!]: 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 requires other packages for directories it uses. [x]: Package uses nothing in %doc for runtime. [x]: Package is not known to require ExcludeArch. [x]: Package complies to the Packaging Guidelines [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Apache (v2.0)", "Unknown or generated". 1 files have unknown license. Detailed output of licensecheck in /home/trever/review-pypolicyd- spf/licensecheck.txt [x]: Package consistently uses macro is (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]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [-]: Large documentation must go in a -doc subpackage. Note: Documentation size is 40960 bytes in 5 files. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [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]: Each %files section contains %defattr if rpm < 4.4 [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Fully versioned dependency in subpackages, if present. [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [x]: Package 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 do not use a name that already exist [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 [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). Python: [x]: Python eggs must not download any dependencies during the build process. [-]: A package which is used by another package via an egg interface should provide egg info. [x]: Package meets the Packaging Guidelines::Python [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). [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x]: Package should compile and build into binary rpms on all supported architectures. [!]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: Dist tag is present. [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: SourceX tarball generation or download is documented. [x]: SourceX is a working URL. [x]: Spec use %global instead of %define. ===== EXTRA items ===== Generic: [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [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: pypolicyd-spf-1.1.2-2.fc18.noarch.rpm pypolicyd-spf.noarch: W: spelling-error %description -l en_US pyspf -> Pepys pypolicyd-spf.noarch: W: spelling-error %description -l en_US whitelisting -> white listing, white-listing, whitewashing 1 packages and 0 specfiles checked; 0 errors, 2 warnings. pyspf is correct, however, white-listing may be the proper spelling sought here. Rpmlint (installed packages) ---------------------------- # rpmlint pypolicyd-spf pypolicyd-spf.noarch: W: spelling-error %description -l en_US pyspf -> Pepys pypolicyd-spf.noarch: W: spelling-error %description -l en_US whitelisting -> white listing, white-listing, whitewashing 1 packages and 0 specfiles checked; 0 errors, 2 warnings. # echo 'rpmlint-done:' Requires -------- pypolicyd-spf (rpmlib, GLIBC filtered): /usr/bin/python config(pypolicyd-spf) postfix python(abi) python-pyspf Provides -------- pypolicyd-spf: config(pypolicyd-spf) pypolicyd-spf Source checksums ---------------- https://launchpad.net/pypolicyd-spf/1.1/1.1.2/+download/pypolicyd-spf-1.1.2.tar.gz : CHECKSUM(SHA256) this package : f8999a9febc5ccb4a66f9961e4ec2d8f81b407bdc3ac1c84563cd5085429cdea CHECKSUM(SHA256) upstream package : f8999a9febc5ccb4a66f9961e4ec2d8f81b407bdc3ac1c84563cd5085429cdea Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29 Buildroot used: fedora-18-x86_64 Command line :/usr/bin/fedora-review -u https://bugzilla.redhat.com/show_bug.cgi?id=921797
I believe that this will work with python 2 or 3, so please change the BuildRequires to reflect that.
(In reply to comment #18) > I believe that this will work with python 2 or 3, so please change the > BuildRequires to reflect that. Even EL5 has python above 2. Do we really need to specify the version?
(In reply to comment #19) > (In reply to comment #18) > > I believe that this will work with python 2 or 3, so please change the > > BuildRequires to reflect that. > > Even EL5 has python above 2. Do we really need to specify the version? What I mean here is, given that this software will work with either, leaving it as default will just pick up whatever is the current default python (i.e. right now, version 2). When this becomes the next version (i.e. 3), the package gets rebuilt and files go into different directories.
(In reply to comment #17) > [!]: Changelog in prescribed format. Sorry - bad cut and paste. Fixed in the next version: ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf-1.1.2-3.fc18.src.rpm ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf.spec > [!]: %check is present and all tests pass. Hmm, not sure what do do about this one. Are there any tests that we can run while building? > pypolicyd-spf.noarch: W: spelling-error %description -l en_US whitelisting > -> white listing, white-listing, whitewashing > 1 packages and 0 specfiles checked; 0 errors, 2 warnings. > > pyspf is correct, however, white-listing may be the proper spelling sought > here. Given this is just a warning, I think we should just keep the cut and paste from the original software.
As as I said, I am not in the PackagingGroup, so my review was completely unofficial. Secondly, I cannot change the rules. As for %check, I haven't seen it before, so I do not know what it should or should not do. I do note that it isn't required.
Packages aren't sponsored, people are.
Well, there is a new version out. As for the check, the following is from "man policyd-spf" TESTING THE POLICY DAEMON Testing the policy daemon To test the policy daemon by hand, execute: policyd-spf Each query is a bunch of attributes. Order does not matter, and the daemon uses only a few of all the attributes shown below: request=smtpd_access_policy protocol_state=RCPT protocol_name=SMTP helo_name=some.domain.tld queue_id=8045F2AB23 instance=12345.6789 sender=foo recipient=bar client_address=1.2.3.4 client_name=another.domain.tld [empty line] The policy daemon will answer in the same style, with an attribute list followed by a empty line: action=dunno [empty line] Perhaps if Fedora servers are SPF enabled, modify the above to fake a message from the Fedora mail servers, but with an IP address that does NOT match the SPF records. Then do the same with one that does. I wish you could get someone from the packagers group to review this. This is very much in use by me.
ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf.spec ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf-1.2-1.fc19.src.rpm
Will you support EL5?
I've built the latest python-pyspf and python-pydns for EL6, F18 and F19 and submitted updates: https://admin.fedoraproject.org/updates/python-pyspf-2.0.8-1.el6 https://admin.fedoraproject.org/updates/python-pydns-2.3.6-1.el6 https://admin.fedoraproject.org/updates/python-pyspf-2.0.8-1.fc18 https://admin.fedoraproject.org/updates/python-pydns-2.3.6-1.fc18 https://admin.fedoraproject.org/updates/python-pyspf-2.0.8-1.fc19 https://admin.fedoraproject.org/updates/python-pydns-2.3.6-1.fc19
Trever: the build systems intentionally have no external network access, so you cannot rely on anything like DNS resolution during the build process. %check is not required to be present in any package.
I'm a packager, so I can review this formally. Mostly I endorse what Trever has done so far in his unofficial review. Additional notes: 1. Is the dependency on postfix really necessary? The script can be called on its own, and it's at least conceivable that something other than postfix could call it to do something useful. I'm not sure the postfix dep achieves anything sane. 2. The definition of python_sitelib is unnecessary and unwanted unless you intend to build for EL-5. If you do intend to build for EL-5, you should at least conditionalize it. https://fedoraproject.org/wiki/Packaging:Python#Macros has a snippet you can copy/paste. 3. I think the guidelines are encouraging explicit specification of python2-devel or python3-devel, rather than python-devel. 4. You include the comment line "# Remove CFLAGS=... for noarch packages (unneeded)" in the %build section, but then include the CFLAGS= definition? This is a noarch package. It seems like you should drop that comment line, and drop the "CFLAGS="$RPM_OPT_FLAGS"" from the next line. 5. I'm not sure if it's valid to mark an *entire directory* as %config(noreplace) . I don't think I've seen that before. I think: %dir %{_sysconfdir}/python-policyd-spf %config(noreplace) %{_sysconfdir}/python-policyd-spf/policyd-spf.conf would be more conventional. I'm not actually sure what the effects of marking a directory as noreplace would be, and Google isn't immediately helpful. Aside from those notes, the package looks good to me. If you can resolve the above I'll mark it as approved.
(In reply to Adam Williamson from comment #29) > I'm a packager, so I can review this formally. Mostly I endorse what Trever > has done so far in his unofficial review. Additional notes: > > 1. Is the dependency on postfix really necessary? The script can be called > on its own, and it's at least conceivable that something other than postfix > could call it to do something useful. I'm not sure the postfix dep achieves > anything sane. The /usr/libexec/postfix path is owned by postfix. So, to place the policyd-spf there, we do need to have it, I guess. > 2. The definition of python_sitelib is unnecessary and unwanted unless you > intend to build for EL-5. If you do intend to build for EL-5, you should at > least conditionalize it. > https://fedoraproject.org/wiki/Packaging:Python#Macros has a snippet you can > copy/paste. I am not personally interested in it, but I think someone here asked, so we may as well do it. I can do that. > 3. I think the guidelines are encouraging explicit specification of > python2-devel or python3-devel, rather than python-devel. OK, I guess we'll go with python2-devel then, given that this is more backwards compatible, right? > 4. You include the comment line "# Remove CFLAGS=... for noarch packages > (unneeded)" in the %build section, but then include the CFLAGS= definition? > This is a noarch package. It seems like you should drop that comment line, > and drop the "CFLAGS="$RPM_OPT_FLAGS"" from the next line. He, he... rpmdev-newspec artefact. Will kill. > 5. I'm not sure if it's valid to mark an *entire directory* as > %config(noreplace) . I don't think I've seen that before. I think: > > %dir %{_sysconfdir}/python-policyd-spf > %config(noreplace) %{_sysconfdir}/python-policyd-spf/policyd-spf.conf > > would be more conventional. I'm not actually sure what the effects of > marking a directory as noreplace would be, and Google isn't immediately > helpful. Yeah, we can do that. > Aside from those notes, the package looks good to me. If you can resolve the > above I'll mark it as approved. Thank you for reviewing.
Agreed on the postfix dep, if it's for the directory, that makes sense. And yeah, I think BuildRequires: python2-devel is the appropriate choice.
ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf.spec ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf-1.2-2.fc19.src.rpm
You forgot: %dir %{_sysconfdir}/python-policyd-spf so the package now doesn't own the config directory. Other than that, looks good!
python-ipaddr, which is required by python-pyspf, is not built for EL6. I have filed a bug requesting a build: https://bugzilla.redhat.com/show_bug.cgi?id=994741 if one is not forthcoming in a week, I'll do it myself (as per EPEL policies). Someone poke me if I seem to have forgotten.
(In reply to Adam Williamson from comment #33) > You forgot: > > %dir %{_sysconfdir}/python-policyd-spf > > so the package now doesn't own the config directory. Other than that, looks > good! OOPS! Sorry. Fixed in -3: ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf.spec ftp://ftp.rexursive.com/pub/pypolicyd-spf/pypolicyd-spf-1.2-3.fc19.src.rpm
OK! Approved.
New Package SCM Request ======================= Package Name: pypolicyd-spf Short Description: SPF Policy Server for Postfix (Python implementation) Owners: bojan Branches: f18 f19 el6 el5 InitialCC:
(In reply to Adam Williamson from comment #28) > Trever: the build systems intentionally have no external network access, so > you cannot rely on anything like DNS resolution during the build process. > %check is not required to be present in any package. Thank you for letting me know. I will try to remember both of these facts. Thank you to all who have got this in!
> The /usr/libexec/postfix path is owned by postfix. > So, to place the policyd-spf there, we do need to have it, I guess. No. Nowadays, the packaging guidelines are more lax, and you are permitted to include the directory in your package, provided that you keep the same permissions. https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_by_a_package_which_is_not_required_for_your_package_to_function
Git done (by process-git-requests).
(In reply to Christopher Meng from comment #26) > Will you support EL5? I have to revise that - the software requires Python 2.6 or better. RHEL5 ships with 2.4. So, that will not work.
So, drop all EL5 stuffs before SCM. Next time please be careful.
(In reply to Christopher Meng from comment #42) > So, drop all EL5 stuffs before SCM. > > Next time please be careful. Yeah, well - life is not perfect. Mistakes are made etc. Sorry about that. Anyway, git branch -d, followed by push?
Hmm, actually - there is python26 in EPEL... Maybe there is a way to do this.
(In reply to Bojan Smojver from comment #43) > (In reply to Christopher Meng from comment #42) > > So, drop all EL5 stuffs before SCM. > > > > Next time please be careful. > > Yeah, well - life is not perfect. Mistakes are made etc. Sorry about that. > > Anyway, git branch -d, followed by push? Just leave it there or retire the branch. Not sure if 2.6 can work well on EL5, good luck.
(In reply to Christopher Meng from comment #45) > Just leave it there or retire the branch. > > Not sure if 2.6 can work well on EL5, good luck. It would require a lot more stuff to be ported over to python26 in EL5, which is probably more work than I'd like to do on this here. So, let's retire the branch. Then I will remove all the artefacts of it from the spec.
Package Change Request ====================== Package Name: pypolicyd-spf New Branches: f18 f19 el6 Owners: bojan
Branch retirement is handled in pkgdb, not via SCM requests.
(In reply to Jon Ciesla from comment #48) > Branch retirement is handled in pkgdb, not via SCM requests. OK, done now.
I'm not a reviewer, but I've been building this package for a while (https://messinet.com/rpms/browser/python-pypolicyd-spf) and what you've done looks good. Also, not to derail your progress, but as of version 1.2, this module also supports Python3. If python-pyspf could get updated for Python3, the dep on python-ipaddr could be removed and the whole chain could be done in Python3. It might be good to add in the Python3 subpackaging build bits to your spec.
(In reply to Anthony Messina from comment #50) > Also, not to derail your progress, but as of version 1.2, this module also > supports Python3. EL6 has python 2, so I picked that to be more compatible.
Works fine in f19. But filenames in "man policyd-spf" do not correspond with "rpm -ql pypolicyd-spf": /usr/libexec/postfix/policyd-spf /etc/python-policyd-spf /etc/python-policyd-spf/policyd-spf.conf
(In reply to ArcFi from comment #52) > Works fine in f19. > > But filenames in "man policyd-spf" do not correspond with "rpm -ql > pypolicyd-spf": > /usr/libexec/postfix/policyd-spf > /etc/python-policyd-spf > /etc/python-policyd-spf/policyd-spf.conf The man page clearly states that exact paths vary depending on packaging and distribution.
(In reply to Bojan Smojver from comment #53) > The man page clearly states that exact paths vary depending on packaging and > distribution. Sorry, my mistake. You're right.
The package is now live in the distro, so there's no reason this bug needs to be open any more so far as I can see.
Package Change Request ====================== Package Name: pypolicyd-spf New Branches: epel7 Owners: bojan
Sorry - I'm an idiot. This branch (epel7) already exists.
(In reply to Trever Adams from comment #14) > Correct. Header_Type = AR is what causes it. = SPF is fine. > > I will look into packaging it. Have you been able to find a sponsor for the > package? Hello, what is the progress of python-authres packaging?
George, I haven't looked into it. I am afraid it got lost in the noise of my work.
Until bug 1230373 and bug 1232595 get fixed, I will not be looking at this.
George Notaras, regarding Comment 58, I am beginning to look into this now. If you are interested in trying it out, let me know.