[Bug 1085752] Review Request: check-create-certificate - A script that checks for the existance of an SSL certificate

[bugzilla at redhat.com]

https://bugzilla.redhat.com/show_bug.cgi?id=1085752



--- Comment #5 from Mohamed El Morabity <pikachu.2014 at gmail.com> ---
Here are some comments.


The %defattr macro is obsolete for a long time (see
https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions). You can
drop it in %files. You can also drop the %attr(...) macro on
%{_sbindir}/%{name}, the right permissions on this files are already defined at
package build automatically here.


You don't need to add coreutils to the BuildRequires: this package is already
part of the minimal build environment (see
https://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2).


You can also drop perl as Requires: The auto Provides system will detected Perl
requirements at build time. You can see it in the build logs:
[...]
Processing files: check-create-certificate-0.5-4.fc20.noarch
[...]
Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <=
4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
Requires: /usr/bin/perl perl(File::Basename) perl(Getopt::Long) perl(strict)
[...]
As you can see, /usr/bin/perl, as well as the
File::Basename/Getopt::Long/strict Perl libraries, are detected and added as
automatic Provides to your package.


The check-create-certificate script calls the c_rehash command, which is
provided by the openssl-perl package. As a result, you can replace openssl by
openssl-perl in Requires (openssl-perl already requires openssl).


The install command can create on the fly the destination folder if it doesn't
exist (see -D option). You can replace these lines:
   mkdir -p %{buildroot}%{_prefix}/sbin
   install -m 755 script/%{name} %{buildroot}%{_sbindir}/
by this simpler one:
   install -Dpm 755 script/%{name} %{buildroot}%{_sbindir}/%{name}
(notice the -p option to preserver the timestamp).


You don't need to manually deploy in %install the COPYING file (or any other
documentation file not installed by default). Just use the %doc macro in
%files: it will automatically deploy all its arguments to %{_docdir}/%{name}
(or %{_docdir}/%{name}-%{version} in Fedora < 20):
%files
%doc COPYING
%{_sbindir}/%{name}


The upstream versionning is quite weird. The upstream source packaging relies
on a .spec file which contains the current version for the project (0.5 now).
The project history shows instead that the version 0.5 was released 4 years ago
(see
https://github.com/jdsn/check-create-certificate/commit/160fc42e77d44ef39ba84dd6226f61184332a255).
Moreover, the way to retrieve the sources you gave in comments will always
bring you the latest snapshot available, it doesn't correspond to a defined
snapshot.
I suggest you:
- to rely instead on a Git commit to version your package; you can consider
here the latest snapshot available today
([d0971baf5d13e06aaa600581efe3adba6631e06a]) which brings some good
improvements
- as a result, to use a postrelease numbering schema for the release tag (see
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Post-Release_packages).
You can have a look at this .spec file to inspire you:
   http://cbb.fedorapeople.org/packages/dex.spec

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component