Spec URL: http://www.city-fan.org/~paul/extras/perl-Crypt-IDEA/perl-Crypt-IDEA.spec SRPM URL: http://www.city-fan.org/~paul/extras/perl-Crypt-IDEA/perl-Crypt-IDEA-1.08-1.fc11.src.rpm This perl extension is an implementation of the IDEA block cipher algorithm. The module implements the Crypt::BlockCipher interface. The IDEA algorithm is patented in Europe and the United States by Ascom-Tech AG. The package is rpmlint-clean. This is my first RPM Fusion package; I am a sponsor in Fedora and have already been sponsored for RPM Fusion cvs access by Thorsten Leemhuis.
The package seems in shape. There are a suggestion (!), a few issues that need to be corrected (*) and a question (?) ! If you use a delimiter other than / on sed, it will make things easier to read. * Please explain all non-trivial things you do in a SPEC file (especially the sed and rm commands) as comments * Please preserve the timestamps of non-compiled files. * The file "changes" should go to %doc ? Packages must not own files or directories already owned by other packages. %{perl_vendorarch}/Crypt/ and %{perl_vendorarch}/auto/Crypt/ are owned by crypto-utils. But there is an exception rule for perl packages and I believe this package falls into that category. Am I right?
(In reply to comment #1) > The package seems in shape. There are a suggestion (!), a few issues that need > to be corrected (*) and a question (?) > ! If you use a delimiter other than / on sed, it will make things easier to > read. Wrong. / is mandatory on sed "deletes". > * Please explain all non-trivial things you do in a SPEC file (especially the > sed and rm commands) as comments These are common Fedora perl packaging standards, you should be familiar with. > * Please preserve the timestamps of non-compiled files. Superfluous - It's an urban legend that this would fix anything. > * The file "changes" should go to %doc Correct ... SHOULD. > ? Packages must not own files or directories already owned by other packages. > %{perl_vendorarch}/Crypt/ and %{perl_vendorarch}/auto/Crypt/ are owned by > crypto-utils. But there is an exception rule for perl packages and I believe > this package falls into that category. Am I right? Almost. Your final conclusion is right (these dirs must be owned), but your explanation is not quite right.
(In reply to comment #2) > (In reply to comment #1) > > The package seems in shape. There are a suggestion (!), a few issues that need > > to be corrected (*) and a question (?) > > > ! If you use a delimiter other than / on sed, it will make things easier to > > read. > Wrong. / is mandatory on sed "deletes". > Wrong. You can use other delimiters with sed "delete". These can probably be found in some basic bash scripting documentation which you should be familiar with. > > * Please explain all non-trivial things you do in a SPEC file (especially the > > sed and rm commands) as comments > These are common Fedora perl packaging standards, you should be familiar with. > There is nothing in the Perl guidelines [1] about removal of .bs and .packlist files. Also no specific perl guideline about removing shebangs [1]. > > * Please preserve the timestamps of non-compiled files. > Superfluous - It's an urban legend that this would fix anything. > It is a very common practice in Fedora and rpmfusion and is told by many reviewers. You'd see this before if you weren't that picky for picking out bugzilla threads to troll and took your time to do some actual review(s). > > * The file "changes" should go to %doc > Correct ... SHOULD. > That's what the guidelines say. > > ? Packages must not own files or directories already owned by other packages. > > %{perl_vendorarch}/Crypt/ and %{perl_vendorarch}/auto/Crypt/ are owned by > > crypto-utils. But there is an exception rule for perl packages and I believe > > this package falls into that category. Am I right? > Almost. Your final conclusion is right (these dirs must be owned), but your > explanation is not quite right. > You might be correct but I don't think I am eager to hear your explanation. I am kindly asking you to not write in the bugs that I write. I don't like your attitude and I think that I can be more productive without your presence. While I can't force you to do this, I can only re-iterate this request. [1] http://fedoraproject.org/wiki/Packaging/Perl
> > * Please preserve the timestamps of non-compiled files. > Superfluous - It's an urban legend that this would fix anything. It's a MUST in the Fedora guidelines.
c.f. https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps "consider" => it is not a MUST. Actually, this rule is the reflection of some people's stylishness.
No, it reflects older versions of RPM (which are probably still in use at least in older RHEL releases) which treated files with different timestamps as different, causing multilib conflicts.
If this was the case, 1000's of packages would not install - This is not the case.
Older RHEL releases don't have multilib anyways, which makes this not a problem (I guess).
Listen, "install -p" doesn't guarantee timestamps on any generated file. In case you are not aware about, * all compiled programs/libraries are generated files. * many package copy around file when building (They appear as having been generated) ... All "install -p" does in these case, is these files to carry different timestamps than without.
The original rationale for recommending "install -p" and "cp -p" when installing files manually (inside the spec file e.g.) has been in preserving timestamps for _prebuilt_ files in tarballs. Such as various forms of documentation files. It is considered helpful by many package users, because they can judge about the age of documentation files simply by checking timestamps. Particularly helpful with but not limited to larger pdf/ps files and html trees. No need to revisit such files after package updates, if the documentation is still unchanged since 2001, for example, and other files are several months old, too. User would "cd /usr/share/doc/..." and quickly notice that only a README file has changed for this update. A few corner-cases have been found where install -p helped, I think related to .rpmnew creation of config files just because the mtime changed (and not just the checksum). Perhaps some have been only temporary. Some reviewers have expanded the recommendation to use "install -p ..." to also run "make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p", which I think is somewhat over the top even if covers a few more prebuilt files. Some tarballs mix cp/install and mkdir/install, so one would need to switch to "cp -p" for the full show. So, conclusively: Historically it has only been pedantic eye-candy (albeit considered helpful by our users). If nowadays there is knowledge that it actually fixes anything else, please document that.
> Some reviewers have expanded the recommendation to use "install -p ..." to also > run "make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p", which I think > is somewhat over the top even if covers a few more prebuilt files. Ugh, why don't the autotools (and other build systems - I haven't checked what CMake does, for example) use -p by default? IMHO that's a bug in those build systems. cp and install's default of not preserving timestamps is just historical, it is stupid to use them without -p.
CMake appears to get this right, stuff installed with INSTALL(FILES ...) in CMake gets installed with the timestamp in the tarball.
(In reply to comment #12) > CMake appears to get this right, Right/wrong is a matter of perspective. Technically, preserving timestamps is not necessary in the overwhelming majority of cases. Actually, it is easy to construct cases where preserving timestamps on installs are harmful.
(In reply to comment #1) > The package seems in shape. There are a suggestion (!), a few issues that need > to be corrected (*) and a question (?) > > ! If you use a delimiter other than / on sed, it will make things easier to > read. Done. > * Please explain all non-trivial things you do in a SPEC file (especially the > sed and rm commands) as comments I added a comment about the sed command. The other commands are all basically idioms found in all perl modules (see the perl spec template from the rpmdevtools package) relating to packaging perl modules sensibly in rpm format without having them all conflict with each other - the removed files aren't needed in rpm-based distributions. We don't want to add identical commentary to hundreds (thousands?) of perl module packages,do we? > * Please preserve the timestamps of non-compiled files. This is already done with the exception of IDEA.pm, and the Crypt::IDEA manpage, which is derived from IDEA.pm. The IDEA.pm is edited in %prep and I don't think it's appropriate to fiddle the timestamp of the edited file to be the same as the original one. > * The file "changes" should go to %doc Done. > ? Packages must not own files or directories already owned by other packages. > %{perl_vendorarch}/Crypt/ and %{perl_vendorarch}/auto/Crypt/ are owned by > crypto-utils. But there is an exception rule for perl packages and I believe > this package falls into that category. Am I right? The guideline about directory ownership basically states that a package should own all directories that it uses that are not already owned by by any of its dependency packages. It is very common amongst perl modules for unrelated (i.e. no dependency relationship) packages to own the same directories because of the directory hierarchy used for perl module install paths, e.g. all architecture-specific perl-Crypt-* packages will own {perl_vendorarch}/Crypt/ unless they depend on another perl-Crypt-* package. crypto-utils is in fact such a perl package but it doesn't follow the usual perl package naming format. Since perl-Crypt-IDEA does not have a dependency on any other package owning these directories, it's correct that it owns them itself. Updated package: Spec URL: http://www.city-fan.org/~paul/extras/perl-Crypt-IDEA/perl-Crypt-IDEA.spec SRPM URL: http://www.city-fan.org/~paul/extras/perl-Crypt-IDEA/perl-Crypt-IDEA-1.08-2.fc11.src.rpm
Thank you for the nice explanations. --------------------------------------------------------------------- This package (perl-Crypt-IDEA) is APPROVED by oget for rpmfusion-free ---------------------------------------------------------------------
Package CVS request ====================== Package Name: perl-Crypt-IDEA Short Description: Perl interface to IDEA block cipher Owners: pghmcfc Branches: F-9 F-10 EL-5 InitialCC: ---------------------- License tag: free
(In reply to comment #16) > Package CVS request > ====================== > Package Name: perl-Crypt-IDEA Done
All branches built successfully. Thanks everyone.
Package Change Request ====================== Package Name: perl-Crypt-IDEA New Branches: EL-6