Spec URL: https://www.math.uh.edu/~tibbs/review/perl-Encode-IMAPUTF7/perl-Encode-IMAPUTF7.spec SRPM URL: https://www.math.uh.edu/~tibbs/review/perl-Encode-IMAPUTF7/perl-Encode-IMAPUTF7-1.05-1.fc24.src.rpm Description: This module is able to encode and decode IMAP mailbox names using the UTF-7 modification specified in RFC2060 section 5.1.3. Fedora Account System Username: tibbs Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=16340823 Fedora-review output (note that any strange-permissions errors are due to rpmlint not being able to cope with my umask): 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. [ ]: 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. [ ]: License field in the package spec file matches the actual license. Note: There is no build directory. Running licensecheck on vanilla upstream sources. No licenses found. Please check the source files for licenses manually. [ ]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [ ]: Package does not own files or directories owned by other packages. Note: Dirs in package are owned also by: /usr/share/perl5/vendor_perl/Encode(perl-Encode-Locale) [ ]: 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. ἐπιθυμία:..perl-Encode-IMAPUTF7/review-perl-Encode-IMAPUTF7> cat review.txt This is a review *template*. Besides handling the [ ]-marked tests you are also supposed to fix the template before pasting into bugzilla: - Add issues you find to the list of issues on top. If there isn't such a list, create one. - Add your own remarks to the template checks. - Add new lines marked [!] or [?] when you discover new things not listed by fedora-review. - Change or remove any text in the template which is plain wrong. In this case you could also file a bug against fedora-review - Remove the "[ ] Manual check required", you will not have any such lines in what you paste. - Remove attachments which you deem not really useful (the rpmlint ones are mandatory, though) - Remove this text 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. [ ]: 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. [ ]: License field in the package spec file matches the actual license. Note: There is no build directory. Running licensecheck on vanilla upstream sources. No licenses found. Please check the source files for licenses manually. [ ]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [ ]: Package does not own files or directories owned by other packages. Note: Dirs in package are owned also by: /usr/share/perl5/vendor_perl/Encode(perl-Encode-Locale) [ ]: 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 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]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [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 Perl: [ ]: Package contains the mandatory BuildRequires and Requires:. [ ]: CPAN urls should be non-versioned. ===== 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. [ ]: 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: No rpmlint messages. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: perl-Encode-IMAPUTF7-1.05-1.fc24.noarch.rpm perl-Encode-IMAPUTF7-1.05-1.fc24.src.rpm perl-Encode-IMAPUTF7.src: W: strange-permission Encode-IMAPUTF7-1.05.tar.gz 660 perl-Encode-IMAPUTF7.src: W: strange-permission perl-Encode-IMAPUTF7.spec 660 2 packages and 0 specfiles checked; 0 errors, 2 warnings. Rpmlint (installed packages) ---------------------------- sh: /usr/bin/python: No such file or directory 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Requires -------- perl-Encode-IMAPUTF7 (rpmlib, GLIBC filtered): perl(:MODULE_COMPAT_5.22.2) perl(Encode) perl(Encode::Encoding) perl(MIME::Base64) perl(base) perl(strict) Provides -------- perl-Encode-IMAPUTF7: perl(Encode::IMAPUTF7) perl-Encode-IMAPUTF7 Source checksums ---------------- http://www.cpan.org/authors/id/P/PM/PMAKHOLM/Encode-IMAPUTF7-1.05.tar.gz : CHECKSUM(SHA256) this package : 470305ddc37483cfe8d3c16d13770a28011f600bf557acb8c3e07739997c37e1 CHECKSUM(SHA256) upstream package : 470305ddc37483cfe8d3c16d13770a28011f600bf557acb8c3e07739997c37e1 Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02 Command line :/usr/bin/fedora-review -v -n perl-Encode-IMAPUTF7 Buildroot used: fedora-24-x86_64 Active plugins: Generic, Shell-api, Perl Disabled plugins: Java, C/C++, Python, fonts, SugarActivity, Ocaml, Haskell, R, PHP Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6
Source file is ok Summary is ok License is ok Description is ok URL and Source0 are ok All tests passed $ rpm -qp --requires perl-Encode-IMAPUTF7-1.05-1.fc26.noarch.rpm | sort | uniq -c 1 perl(:MODULE_COMPAT_5.24.0) 1 rpmlib(CompressedFileNames) <= 3.0.4-1 1 rpmlib(FileDigests) <= 4.6.0-1 1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 1 rpmlib(PayloadIsXz) <= 5.2-1 FIX: Most of binary requires are missing, because perl-generators was not installed. $ rpm -qp --provides perl-Encode-IMAPUTF7-1.05-1.fc26.noarch.rpm | sort | uniq -c 1 perl-Encode-IMAPUTF7 = 1.05-1.fc26 FIX: Provides 'erl(Encode::IMAPUTF7) = 1.05 is not listed due to missing perl-generators $ rpmlint ./perl-Encode-IMAPUTF7* perl-Encode-IMAPUTF7.src: W: strange-permission Encode-IMAPUTF7-1.05.tar.gz 660 perl-Encode-IMAPUTF7.src: W: strange-permission perl-Encode-IMAPUTF7.spec 660 2 packages and 1 specfiles checked; 0 errors, 2 warnings. Rpmlint is ok BuildRequires: FIX: Please add following build-requires: coreutils - because 'tr', 'mv', 'touch',... are used in spec file findutils - 'find' is used in spec file perl - it is used in spec file perl-generators - is necessary for generating of package provides and requires. The package is not part of default buildroot Encode-IMAPUTF7-1.05/Makefile.PL perl(base) - lib/Encode/IMAPUTF7.pm:7 perl(Encode::Encoding) - lib/Encode/IMAPUTF7.pm:7 perl(File::Basename) - t/0-test.t:12 perl(File::Spec) - t/0-test.t:13 perl(MIME::Base64) - lib/Encode/IMAPUTF7.pm:10 perl(strict) - lib/Encode/IMAPUTF7.pm:5 perl(Test::More) - t/0-test.t:12 perl(warnings) - lib/Encode/IMAPUTF7.pm:7 TODO: Please replace PERL_INSTALL_ROOT with more common DESTDIR. If you want to add the package to EPEL, please ignore these two TODO TODO: The easier way to remove .packlist is used NO_PACKLIST option, which is part of perl(ExtUtils::MakeMaker) >= 6.76. It can be used in all Fedoras. The command is %{__perl} Makefile.PL INSTALLDIRS=vendor NO_PACKLIST=1 TODO: Remove the deleting empty directories in %install section. This is default behavior for Fedoras. Please correct all 'FIX' issues and consider fixing 'TODO' items and provide new spec file. The package is not approved.
Dang it, I swear one version of the package I made had perl-generators in there. Unfortunately I started with cpanspec, and it really needs to be fixed. I fixed a lot of its output but it would have been simpler to start from scratch, I think. In fact, I realize it's sufficiently useless that I went ahead and remove mention of it from the Perl guidelines entirely. Anyway, cpanspec seems to be poor enough now that I just removed reference to it from the Perl guidelines. I didn't even know about the NO_PACKLIST, and the deletion of empty directories by default. That's great. I sure wish cpanspec knew about it, too. Is %_fixperms still required? And is it still required to call "make pure_install"? If "make install" worked we could just recommend %make_install. Here are new files: Updated spec: https://www.math.uh.edu/~tibbs/review/perl-Encode-IMAPUTF7/perl-Encode-IMAPUTF7.spec Updated SRPM: https://www.math.uh.edu/~tibbs/review/perl-Encode-IMAPUTF7/perl-Encode-IMAPUTF7-1.05-2.fc25.src.rpm (f25 this time so that perl-generators isn't there by default) But, a note, from the person who wrote the packaging guidelines relating to build dependencies: I did not add the dependencies on coreutils and findutils. If you're asking others to add them, I would kindly ask that you please stop doing so as it only causes confusion among the packagers. The guidelines guarantee that RPM can run basic shell scripts and actually build packages. You get mv and find and tar and sed and patch. (I did add a dep on make, which elicits a complaint from fedora-review. Sometimes you just can't win.) The intent was never to give anyone the impression that those were required. I went ahead and altered the Perl guidelines to use wording closer to that of the main guidelines to try and minimize any confusion. But if you have suggestions for any other clarifications I might add (short of just including a complete list, which is what we want to avoid) then please let me know.
> FIX: Most of binary requires are missing, because perl-generators was not > installed. $ rpm -qp --requires perl-Encode-IMAPUTF7-1.05-2.fc26.noarch.rpm | sort | uniq -c 1 perl(:MODULE_COMPAT_5.24.0) 1 perl(Encode) 1 perl(Encode::Encoding) 1 perl(MIME::Base64) 1 perl(base) 1 perl(strict) 1 rpmlib(CompressedFileNames) <= 3.0.4-1 1 rpmlib(FileDigests) <= 4.6.0-1 1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 1 rpmlib(PayloadIsXz) <= 5.2-1 Binary requires are ok. > FIX: Provides 'erl(Encode::IMAPUTF7) = 1.05 is not listed due to missing > perl-generators $ rpm -qp --provides perl-Encode-IMAPUTF7-1.05-2.fc26.noarch.rpm | sort | uniq -c 1 perl(Encode::IMAPUTF7) = 1.05 1 perl-Encode-IMAPUTF7 = 1.05-2.fc26 Binary provides are ok. > BuildRequires: > FIX: Please add following build-requires: > coreutils - because 'tr', 'mv', 'touch',... are used in spec file It is not necessary to listed. Ok > findutils - 'find' is used in spec file 'find' was removed from spec. Ok > perl - it is used in spec file > perl-generators - is necessary for generating of package provides and > requires. The package is not part of default > buildroot > Encode-IMAPUTF7-1.05/Makefile.PL > perl(base) - lib/Encode/IMAPUTF7.pm:7 > perl(Encode::Encoding) - lib/Encode/IMAPUTF7.pm:7 > perl(File::Basename) - t/0-test.t:12 > perl(File::Spec) - t/0-test.t:13 > perl(MIME::Base64) - lib/Encode/IMAPUTF7.pm:10 > perl(strict) - lib/Encode/IMAPUTF7.pm:5 > perl(Test::More) - t/0-test.t:12 > perl(warnings) - lib/Encode/IMAPUTF7.pm:7 -BuildRequires: perl(Encode) -BuildRequires: perl(ExtUtils::MakeMaker) -BuildRequires: perl(Test::NoWarnings) + +BuildRequires: make perl perl-generators +BuildRequires: perl(base) perl(Encode) perl(Encode::Encoding) +BuildRequires: perl(ExtUtils::MakeMaker) perl(File::Basename) perl(File::Spec) +BuildRequires: perl(MIME::Base64) perl(strict) perl(Test::More) +BuildRequires: perl(Test::NoWarnings) perl(warnings) Ok. > TODO: Please replace PERL_INSTALL_ROOT with more common DESTDIR. -make pure_install PERL_INSTALL_ROOT=%buildroot +make pure_install DESTDIR=%buildroot Ok. > TODO: The easier way to remove .packlist is used NO_PACKLIST option, > which is part of perl(ExtUtils::MakeMaker) >= 6.76. It can be > used in all Fedoras. The command is > %{__perl} Makefile.PL INSTALLDIRS=vendor NO_PACKLIST=1 -%{__perl} Makefile.PL INSTALLDIRS=vendor +%{__perl} Makefile.PL INSTALLDIRS=vendor NO_PACKLIST=1 Ok. Please add version restriction for perl(ExtUtils::MakeMaker) to require version 6.76 or higher. > TODO: Remove the deleting empty directories in %install section. This is > default behavior for Fedoras. -%global remove_packlist find %buildroot -type f -name .packlist -exec rm -f {} \\; ; find %buildroot -depth -type d -exec rmdir {} 2>/dev/null \\; ... -%remove_packlist Ok. Difference between 'make install' and 'make pure_install is described here. https://fedoraproject.org/wiki/Perl/Tips#ExtUtils::MakeMaker I know that cpanspec is outdated. There are lots of patches, but the upstream does not accept them yet. The package looks good. Approved.
(In reply to Jason Tibbitts from comment #2) > The guidelines guarantee that > RPM can run basic shell scripts and actually build packages. Yes. > You get mv and > find and tar and sed and patch. Oh, really? Where can I get to know what involves "basic shell scripts" commands? The undefined set is what confuses everybody.
(In reply to Jason Tibbitts from comment #2) > I went ahead and altered the Perl guidelines to use wording closer to that > of the main guidelines to try and minimize any confusion. I recommend not to clutter Perl or any other language-specific guidelines with these main Fedora guidelines statements.
(In reply to Petr Pisar from comment #5) > I recommend not to clutter Perl or any other language-specific guidelines > with these main Fedora guidelines statements. Me, too, but... the guideline already had a misstatement of the current guideline when it should have had just a link. Perl folks wanted that statement in there. I just made sure it wasn't incorrect.
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/perl-Encode-IMAPUTF7
Imported and built in rawhide, and added to Anitya. Thanks for the review and pointers to the Perl tips; it's been a rather long time since I did any Perl work. I'm trying to package enough of the stack to run Cassandane, the Cyrus IMAPd test suite: https://github.com/cyrusimap/cassandane Net up will be perl-Mail-IMAPTalk. This is all part of a larger effort to trying to get our Cyrus IMAPd package updated.
I will help you. Please check a package is not submitted for a review before packing it so we do not stomp on our feet.
I submitted all the dependencies for a review.
Hmm, I didn't see any open reviews for any of the deps and the IMAPTalk one was just submitted so I don't think any work has been duplicated besides the IMAPTalk package which I was about to submit but now don't have to. Still need to package Mail::JMAPTalk, Net::DAVTalk, Net::CalDAVTalk, Net::CardDAVTalk (which aren't on CPAN and haven't yet had tagged releases)and double check the rest of the dependencies listed at https://github.com/cyrusimap/cassandane/blob/master/doc/README.deps
They are on the CPAN and the reviews were submitted Mail::JMAPTalk bug #1394151 Net::DAVTalk bug #1394242 Net::CalDAVTalk bug #1394252 Net::CardDAVTalk bug #1394262 You have to use Bugzilla to search them. Fedora's wiki hides them if they have opened prerequisites.
Oh, cool, thanks. The cassandane docs mention that they aren't on cpan so I didn't bother to search CPAN.