Bug 1054933 (esteidcerts)

I updated spec file. Moved TEST certs to a separate devel package. Spec and srpm urls are same, but just in case here is updated koji task:  http://koji.fedoraproject.org/koji/taskinfo?taskID=6423902

I'd be happy to help review this...

Some initial comments:

rpmlint is clean:
$ rpmlint *.rpm */*.rpm
3 packages and 0 specfiles checked; 0 errors, 0 warnings


sources verified:
92b79074e4f6db85453f561f9d9ab35f  esteidcerts-3.8.0.9128.tar.gz


1.  SHOULD remove deprecated .spec items
Group: tags, %defattr

these are deprecated and unused, see also:
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Group_tag
and
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_Permissions
respectively


2. -devel subpkg name SHOULD be named something else
This does seem really a Development package in the
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Devel_Packages
sense to me.  I'd suggest naming it something else, like -test


3.  MUST fix directory ownership
Nothing owns
%{_datadir}/esteid
I'd suggest adding to base pkg:
%dir %{_datadir}/esteid

and -devel (or whatever it ends up called), misses anything owning parent dirs:
%{_datadir}/esteid
%{_datadir}/esteid/certs
I'd suggest adding a dependency on the base pkg:
Requires: %{name} = %{version}-%{release}
or add here:
%dir %{_datadir}/esteid
%dir %{_datadir}/esteid/certs

See also:
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_and_Directory_Ownership


4.  licensing cannot be verified.  the source archive doesn't seem to mention licensing anywhere.  where do you get
License: LGPLv2
from?

Spec URL: http://mihkel.fedorapeople.org/SPECS/esteidcerts.spec
SRPM URL: http://mihkel.fedorapeople.org/SRPMS/esteidcerts-3.8.0.9128-1.fc20.src.rpm
Koji task: http://koji.fedoraproject.org/koji/taskinfo?taskID=6540301

Hi.

Thanks for you time and review. I hope I got everything sorted out the way you suggested.


(In reply to Rex Dieter from comment #3)
 
> 1.  SHOULD remove deprecated .spec items
> Group: tags, %defattr

I remove Group lines and %defattr lines

I'll do the same for other spec files also, but not today.

 
> 2. -devel subpkg name SHOULD be named something else
> This does seem really a Development package in the
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/
> Guidelines#Devel_Packages
> sense to me.  I'd suggest naming it something else, like -test

-test it is

> 3.  MUST fix directory ownership
> Nothing owns
> %{_datadir}/esteid
> I'd suggest adding to base pkg:
> %dir %{_datadir}/esteid
> 
> and -devel (or whatever it ends up called), misses anything owning parent
> dirs:
> %{_datadir}/esteid
> %{_datadir}/esteid/certs
> I'd suggest adding a dependency on the base pkg:
> Requires: %{name} = %{version}-%{release}
> or add here:
> %dir %{_datadir}/esteid
> %dir %{_datadir}/esteid/certs
> 

I added 
%dir %{_datadir}/esteid
to %files section and a
Requires:       %{name} = %{version}-%{release}
line

> 
> 4.  licensing cannot be verified.  the source archive doesn't seem to
> mention licensing anywhere.  where do you get
> License: LGPLv2
> from?

My mistake. According to esteidcerts.spec file in source tarball, these certs are public domain. Although they should include a LICENSE file in tarball. I'll mention that to upstream when (if) I'll finish packaging these packages and then they probably include that file in next release.

Item 3 was fixed incorrectly, you put
Requires:       %{name} = %{version}-%{release}
in the main pkg (it requires itself? :) ), when it should be in the %package test section

5. SHOULD remove from %install:
rm -rf %{buildroot}

This is another deprecation-like item, that rpm handles automatically these days.

Spec file: http://mihkel.fedorapeople.org/SPECS/esteidcerts.spec

(In reply to Rex Dieter from comment #5)
> Item 3 was fixed incorrectly, you put
> Requires:       %{name} = %{version}-%{release}
> in the main pkg (it requires itself? :) ), when it should be in the %package
> test section
> 

It should be in right place now.


> 5. SHOULD remove from %install:
> rm -rf %{buildroot}
> 
> This is another deprecation-like item, that rpm handles automatically these
> days.

I removed that line.

Spec file: http://mihkel.fedorapeople.org/SPECS/esteidcerts.spec

Just a little update. I took gnome-music.spec as a example of what other lines are deprecated and removed 
mkdir -p %{buildroot}
as well.

Also I removed Group: tags, %defattr and rm -rf %{buildroot} (from install section) from other spec files as well.
http://mihkel.fedorapeople.org/SPECS/

There are in total of 7 spec files (I've made only 3 bugzilla reports so far) plus I'd like to create a metapackage or a software group which installs all required packages for Estonian national id card.

Hi Rex.

Its been little over a month. Are you still interested in reviewing my packages?

Meanwhile I've set up a copr build:
http://copr.fedoraproject.org/coprs/mihkel/esteid/
and just for fun an epel build too:
http://copr.fedoraproject.org/coprs/mihkel/esteid-epel/

I'm still not sure weather I should target EL 6 and 7, because AFAIR in el7 I needed to build additional packages that were not currently available in epel repo and in el6 I needed to use newer compiler and probably some other hacks too.

Anyway it would be nice to hear what are your plans regarding my packages.

Thanks for the reminder, I'll pick things up today.

Technically, things look good.

My only last concern is licensing.  In particular, I don't see anything in the tarball to support that these items are in the Public domain.

I tried looking around a bit on http://www.id.ee/ and https://installer.id.ee , but my not knowing the language means I didn't get very far.

Though looking over 
https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing

helps:

"Being in the public domain is not a license; rather, it means the material is not copyrighted and no license is needed."

which I think is satisfied in this case.


APPROVED

OK, I just sponsored you in FAS too, welcome to Fedora.

You can now continue on the process at this step:
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner

Please let me know if you have any other questions or concerns.  (here, email, or irc, #fedora-devel are all good ways to get me or others to help)

Thanks. I guess the next thing to do is to request a repo for esteidcerts.

New Package SCM Request
=======================
Package Name: esteidcerts
Short Description: Estonian ID-card certificates
Owners: mihkel
Branches: f19 f20
InitialCC:

Git done (by process-git-requests).

Okay. I've added this package to SCM system and I think this one is ready. Should I close this bug?

Also. Rex are you interested in reviewing my other packages as well (6 packages to go plus one metapackage)?
Next one would be esteidpkcs11loader:
https://bugzilla.redhat.com/show_bug.cgi?id=1054938

Yes, this can be closed or let bodhi autoclose it when/if you submit package updates for f19/f20

I could possibly do more reviews as time allows, but not sure when that will happen.

esteidcerts-3.8.0.9128-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/esteidcerts-3.8.0.9128-1.fc19

esteidcerts-3.8.0.9128-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/esteidcerts-3.8.0.9128-1.fc20

esteidcerts-3.8.0.9128-1.fc19 has been pushed to the Fedora 19 testing repository.

esteidcerts-3.8.0.9128-1.fc19 has been pushed to the Fedora 19 stable repository.

esteidcerts-3.8.0.9128-1.fc20 has been pushed to the Fedora 20 stable repository.