Bug 1117209 - Review Request: sscep - Simple SCEP client with modifications for engine support & more
Summary: Review Request: sscep - Simple SCEP client with modifications for engine sup...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-07-08 09:34 UTC by Thorsten Scherf
Modified: 2014-10-28 11:02 UTC (History)
4 users (show)

Fixed In Version: sscep-0.5.0-6.20140820git5669006.el6
Clone Of:
Environment:
Last Closed: 2014-10-28 06:43:40 UTC
Type: ---
Embargoed:
rdieter: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Thorsten Scherf 2014-07-08 09:34:20 UTC
Spec URL: https://people.redhat.com/tscherf/fedora/rpms/sscep.spec
SRPM URL: https://people.redhat.com/tscherf/fedora/rpms/sscep-0.5.0-1.fc20.src.rpm
Description: SCEP is a PKI communication protocol which leverages existing technology by using PKCS#7 and PKCS#10. Currently, SSCEP implements all of the SCEP operations using SCEP query messages.
Fedora Account System Username: tscherf

Comment 1 Christopher Meng 2014-07-08 09:44:05 UTC
Would you like to do a review swap?

Comment 2 Thorsten Scherf 2014-07-08 11:36:12 UTC
What kind of review swap?

Comment 3 Christopher Meng 2014-07-08 11:47:57 UTC
bug 957715.

Comment 4 Thorsten Scherf 2014-07-08 11:57:32 UTC
set it NEEDINFO for Volker Fröhlich

Comment 6 Rex Dieter 2014-07-10 13:28:01 UTC
fyi,
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Changelogs

"Every time you make changes, that is, whenever you increment the E-V-R of a package, add a changelog entry. This is important not only to have an idea about the history of a package, but also to enable users, fellow packages, and QA people to easily spot the changes that you make."

I see no changelog entry for what changed between 0.5.0-1 and 0.5.0-2

Comment 7 Rex Dieter 2014-07-10 13:30:10 UTC
and blocking  FE-NEEDSPONSOR, per
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Create_Your_Review_Request

(I don't see you in the packagers group yet, feel free to correct me if I'm wrong)

Comment 8 Thorsten Scherf 2014-07-10 13:34:37 UTC
(In reply to Rex Dieter from comment #6)
> fyi,
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#Changelogs
> 
> "Every time you make changes, that is, whenever you increment the E-V-R of a
> package, add a changelog entry. This is important not only to have an idea
> about the history of a package, but also to enable users, fellow packages,
> and QA people to easily spot the changes that you make."
> 
> I see no changelog entry for what changed between 0.5.0-1 and 0.5.0-2

Ack. I didn't add a changelog entry since it's the initial package and it was only a minor change. I think that's ok in this case.

Comment 9 Thorsten Scherf 2014-07-10 13:38:49 UTC
(In reply to Rex Dieter from comment #7)
> and blocking  FE-NEEDSPONSOR, per
> https://fedoraproject.org/wiki/
> Join_the_package_collection_maintainers#Create_Your_Review_Request
> 
> (I don't see you in the packagers group yet, feel free to correct me if I'm
> wrong)

This is not my first package. I already owned prelude-* and the Democracy packages. I gave away ownership after some time. Don't know if the packages are still around.  Anyway, setting FE-NEEDSPONSOR is fine for me.

Comment 10 Rex Dieter 2014-07-10 13:50:05 UTC
Re: comment #8

The guideline about changelogs are fairly unambiguous here. "*Every* time you make changes... add a changelog entry" (emphasis mine).

Comment 11 Thorsten Scherf 2014-07-10 14:18:33 UTC
(In reply to Rex Dieter from comment #10)
> Re: comment #8
> 
> The guideline about changelogs are fairly unambiguous here. "*Every* time
> you make changes... add a changelog entry" (emphasis mine).

SPEC file has been changed and uploaded along with an updated SRPM. Last links are still valid.

Comment 12 Parag AN(पराग) 2014-08-19 15:46:20 UTC
Thorsten,
  If you are in need of sponsorship then have you reviewed any other packages un-officially? Sponsorer need to know how many packages you have un-officially reviewed and the quality of those reviews like you have suggested correctly for the issues you found in those reviews. Also, I have not seen every time you made a change you posted new links of SPEC and SRPM. Please follow the packaging guidelines.

Comment 13 Parag AN(पराग) 2014-08-19 15:47:49 UTC
Ah you did update the package once. I missed that comment.

Comment 14 Rex Dieter 2014-08-19 15:54:59 UTC
I'll take another closer look shortly...

Comment 15 Rex Dieter 2014-08-19 16:50:56 UTC
Initially,

1. SourceURL MUST be fixed

Source: %{name}.tar.gz

MUST be a full URL, or instructions provided on how to reproducibly create the source tarball, see also:
https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Github

Comment 16 Rex Dieter 2014-08-19 16:53:52 UTC
and trying an initial build, I see

...
gcc -Wall -O    -c -o sscep.o sscep.c
...

2.  $RPM_OPT_FLAGS MUST be used (or rationale given why not),
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Compiler_flags

Comment 17 Rex Dieter 2014-08-19 17:03:28 UTC
3.  SHOULD drop unused
BuildRequires: automake

I don't see this actually needed or used anywhere.


If you're going to be installing stuff by hand...
4. SHOULD use 'install -p' flag to preserve timestamps


Licensing: NOT ok
5. COPYRIGHT file mentions a bunch of different licenses, none of which is GPL

the source files themselves only say "See the file COPYRIGHT for licensing information."  ugh, without clarification, we may have to conclude *all* mentioned licenses apply, including: BSD, OpenSSL, and 2 others not mentioned in 
https://fedoraproject.org/wiki/Licensing
SSLeay and OpenOSP, may need to ask fedora legal mailing list for help here,
https://admin.fedoraproject.org/mailman/listinfo/legal

(Do you want to, or would you rather I do it?)

Comment 18 Rex Dieter 2014-08-19 17:18:29 UTC
the OpenSDP variant looks close to "BSD with attribution" variant.  so, maybe we can go with that for now, and use:

5.  License: (BSD with attribution) and OpenSSL

Comment 19 Thorsten Scherf 2014-08-20 11:46:10 UTC
>1. SourceURL MUST be fixed
>
>Source: %{name}.tar.gz

Has been fixed.

>2.  $RPM_OPT_FLAGS MUST be used (or rationale given why not),

Has been fixed.

>3.  SHOULD drop unused
>BuildRequires: automake

Requirement removed.

>4. SHOULD use 'install -p' flag to preserve timestamps

Has been fixed.

>5. COPYRIGHT file mentions a bunch of different licenses, none of which is GPL
>
>the source files themselves only say "See the file COPYRIGHT for licensing >information."  ugh, without clarification, we may have to conclude *all* >mentioned licenses apply, including: BSD, OpenSSL, and 2 others not mentioned >in 
>https://fedoraproject.org/wiki/Licensing
>SSLeay and OpenOSP, may need to ask fedora legal mailing list for help here,
>https://admin.fedoraproject.org/mailman/listinfo/legal
>
>(Do you want to, or would you rather I do it?)

Go ahead. Thanks.

>the OpenSDP variant looks close to "BSD with attribution" variant.  so, maybe >we can go with that for now, and use:
>
>5.  License: (BSD with attribution) and OpenSSL

License has been replaced.


Find the new package and spec here:

http://people.redhat.com/tscherf/fedora/rpms/sscep-0.5.0-3.fc20.src.rpm
http://people.redhat.com/tscherf/fedora/rpms/sscep.spec

Comment 20 Rex Dieter 2014-08-20 13:17:58 UTC
SourceURL seems not correct yet.

spectool -g *.spec
Getting https://github.com/certnanny/sscep/b914f33fdb6479c9d87d4cab6edce0b0e06a370c/sscep-b914f33fdb6479c9d87d4cab6edce0b0e06a370c.tar.gz to ./sscep-b914f33fdb6479c9d87d4cab6edce0b0e06a370c.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
curl: (22) The requested URL returned error: 404 Not Found

Comment 21 Rex Dieter 2014-08-20 13:20:24 UTC
One other suggestion:

6.  SHOULD handle docs with %doc macro instead of
%install
...
install -pm 644 COPYRIGHT HISTORY TODO README draft-nourse-scep-06.txt draft-nourse-scep-11.txt %{buildroot}%{_defaultdocdir}/%{name}

and
%files
%doc %{_defaultdocdir}/%{name}/

do this instead:
%files
%doc COPYRIGHT HISTORY TODO README draft-nourse-scep-06.txt draft-nourse-scep-11.txt

(feel free to include multiple %doc lines if you prefer better readability)

Comment 22 Thorsten Scherf 2014-08-20 13:37:30 UTC
> SourceURL seems not correct yet.

SourceURL fixed.

6.  SHOULD handle docs with %doc macro instead of

doc tag added and install removed.

http://people.redhat.com/tscherf/fedora/rpms/sscep-0.5.0-4.fc20.src.rpm
http://people.redhat.com/tscherf/fedora/rpms/sscep.spec

Comment 23 Rex Dieter 2014-08-20 13:43:15 UTC
Sources: NOT ok.  checksums do not match.

copy inside your src.rpm:

$ md5sum *.gz
c732104a77335b6a32ff4ce0c236cc8f  v0.5.0.tar.gz

$ rm *.gz
$ spectool -g *.spec
Getting https://github.com/certnanny/sscep/archive/v0.5.0.tar.gz to ./v0.5.0.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  129k  100  129k    0     0   115k      0  0:00:01  0:00:01 --:--:-- 3086k
$ md5sum *.gz
ecb381f180ce09b8d4f550a6cc9d2006  v0.5.0.tar.gz


Please verify.

Comment 24 Thorsten Scherf 2014-08-20 15:33:23 UTC
Added github archive as SourceURL:

http://people.redhat.com/tscherf/fedora/rpms/sscep-0.5.0-5.fc20.src.rpm

Comment 25 Rex Dieter 2014-08-20 15:41:47 UTC
Sources: OK  

md5sum verified:
44a60e9867949de06b7add230cb946ce  sscep-56690068c795a1c8a4616c98814cf3244ad6cb91.tar.gz

But now we need to adjust Version/Release to account for this being a snapshot build, see:
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages

where you'll likely want to use the %{short_commit} macro
referenced on:
https://fedoraproject.org/wiki/Packaging:SourceURL#Github

and use something like:

Release: 6.20140820git%{short_commit}%{?dist}

Comment 26 Thorsten Scherf 2014-08-20 15:49:12 UTC
snapshot based release tag added.

http://people.redhat.com/tscherf/fedora/rpms/sscep-0.5.0-6.20140820git5669006.fc20.src.rpm

Comment 27 Rex Dieter 2014-08-20 16:22:08 UTC
$ rpmlint *.src */*.rpm
(none): E: no installed packages by name *.src
sscep.x86_64: W: no-manual-page-for-binary sscep
sscep.x86_64: W: no-manual-page-for-binary mkrequest
2 packages and 0 specfiles checked; 0 errors, 2 warnings.

Looks good now, thanks.


APPROVED (and sponsored).

Comment 28 Thorsten Scherf 2014-08-20 20:11:09 UTC
New Package SCM Request
=======================
Package Name: sscep
Short Description: Simple SCEP client with modifications for engine support & more
Upstream URL: https://github.com/certnanny/sscep
Owners: Thorsten Scherf
Branches: f20 f21 el6 epel7
InitialCC:

Comment 29 Gwyn Ciesla 2014-08-21 11:56:04 UTC
Please use your FAS account name, not your real name.

Comment 30 Thorsten Scherf 2014-08-21 12:11:33 UTC
New Package SCM Request
=======================
Package Name: sscep
Short Description: Simple SCEP client with modifications for engine support & more
Upstream URL: https://github.com/certnanny/sscep
Owners: tscherf
Branches: f20 f21 el6 epel7
InitialCC:

Comment 31 Gwyn Ciesla 2014-08-21 13:13:26 UTC
Git done (by process-git-requests).

Comment 32 Fedora Update System 2014-09-02 11:58:38 UTC
sscep-0.5.0-6.20140820git5669006.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/sscep-0.5.0-6.20140820git5669006.fc20

Comment 33 Fedora Update System 2014-09-02 11:59:28 UTC
sscep-0.5.0-6.20140820git5669006.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/sscep-0.5.0-6.20140820git5669006.el6

Comment 34 Fedora Update System 2014-09-03 15:38:16 UTC
sscep-0.5.0-6.20140820git5669006.el6 has been pushed to the Fedora EPEL 6 testing repository.

Comment 35 Fedora Update System 2014-10-28 06:43:40 UTC
sscep-0.5.0-6.20140820git5669006.fc20 has been pushed to the Fedora 20 stable repository.

Comment 36 Fedora Update System 2014-10-28 11:02:37 UTC
sscep-0.5.0-6.20140820git5669006.el6 has been pushed to the Fedora EPEL 6 stable repository.


Note You need to log in before you can comment on or make changes to this bug.