Bug 656892 - (ghc-augeas) Review Request: ghc-augeas - Haskell bindings for the augeas library
Review Request: ghc-augeas - Haskell bindings for the augeas library
Status: CLOSED DEFERRED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Jens Petersen
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-24 08:47 EST by JudeNagurney
Modified: 2012-10-30 06:27 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-10-26 01:42:43 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
petersen: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description JudeNagurney 2010-11-24 08:47:26 EST
Spec URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas.spec
SRPM URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas-0.3.4-1.fc13.src.rpm
Description: This is my attempt at packaging the Haskell bindings for the augeas library.  The package is in Hackage at http://hackage.haskell.org/package/augeas.  I used cabal2spec to generate the spec file.
Comment 1 JudeNagurney 2010-11-24 08:49:51 EST
This is my first package, and I am seeking a sponsor.
Comment 2 Jens Petersen 2010-12-01 19:59:46 EST
Can I ask you to update to cabal2spec-0.22.2, please?
(Sorry I only just get round to backporting it to F13 until now.)

Also please add a changelog entry and fill in the license.
We use ghc_pkg_deps for the haskell dependencies rather
than direct Buildrequires.
Comment 3 Jens Petersen 2010-12-01 20:02:50 EST
Also it would be nice to hear what you plan to use augeas for in fedora
in terms of further dependencies, etc.
Comment 4 JudeNagurney 2010-12-11 08:07:40 EST
I have published an updated spec file and SPRM at the following locations:Spec URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas.spec
SRPM URL http://code.haskell.org/augeas/packaging/rpm/ghc-augeas-0.3.4-1.fc14.src.rpm

I've rebuilt the spec file using caabal2spec-0.22.2, filled out the license, updated the changelog, and switched to using ghc_pkg_deps and ghc_c_pkg_deps instead of direct BuildRequires.

(In reply to comment #2)
> Can I ask you to update to cabal2spec-0.22.2, please?
> (Sorry I only just get round to backporting it to F13 until now.)
> 
> Also please add a changelog entry and fill in the license.
> We use ghc_pkg_deps for the haskell dependencies rather
> than direct Buildrequires.
Comment 5 JudeNagurney 2010-12-11 08:18:45 EST
ghc-augeas is a Haskell binding for the augeas library from the augeas-devel package, so it would be used if you wanted to use the augeas library in your Haskell program.  

It's similar to already-existing python-augeas, java-augeas, ocamal-augeas, and ruby-augeas packages, only with Haskell.

I have augeas-devel listed in the ghc_c_pkg_deps,

(In reply to comment #3)
> Also it would be nice to hear what you plan to use augeas for in fedora
> in terms of further dependencies, etc.
Comment 6 Jens Petersen 2011-01-28 06:07:22 EST
As earlier discussed on Fedora haskell-devel list,
dropping libraries for now from the toplevel of Haskell-pkg-reviews,
so we, the Haskell SIG, can focus more on getting Haskell apps into Fedora.

To get you library back under the tracker please submit a bin or binlib
package that depends on this library and make this bug block that package
review.  It is a good idea to submit full stacks of packages and then
to add the toplevel program to the tracker.
Comment 7 Jens Petersen 2011-08-16 03:43:58 EDT
Sorry I completely lost track of this one...

Some time has passed so would be good if you could refresh
the package to the current cabal2spec templates.
Comment 8 JudeNagurney 2011-11-06 17:49:10 EST
I rebuilt the spec file with cabal2spec-0.24.1 and republished an updated spec file and SPRM at the following locations:

Spec URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas.spec
SRPM URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas-0.5.1-1.fc15.src.rpm

Please let me know if there's anything else I should be doing.  Thanks !
Comment 9 Narasimhan 2011-11-07 02:22:05 EST
Hi

Please provide a summary and description relevant for the package instead of using the default ones provided by the template.

In BuildRequires : ghc-HUnit-devel should be ghc-HUnit-prof , if this package will be built for f14.

Also, remove the comment under %build section.

Add a new changelog entry in addition to the default one created by the template. Please provide a summary of the changes you have made (in one or two sentences). Increment Release from 0 to 1.

Otherwise looks fine.

Thanks.
Comment 10 JudeNagurney 2011-11-13 17:51:31 EST
Thanks for the feedback.  I've updated the spec file based on your comments and pushed the spec file and SRPM to the following locations:

Spec URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas.spec
SRPM URL:
http://code.haskell.org/augeas/packaging/rpm/ghc-augeas-0.5.1-1.fc15.src.rpm

Please let me know if there's anything else I should be doing.  Thanks !
Comment 11 Jens Petersen 2011-11-14 05:01:22 EST
Thanks for updating I think I can review this for you as sponsor
if you can clean up a little more:

We usually put the summary and description in common_summary
and common_description.

Please remember to bump the release number and add a new changelog
entry when you update the package.

You should use _bindir and _datadir instead of explicitly writing
/usr/bin and /usr/share.

You can add (doc) files under the %file sections
but you need to be careful since secondary archs
don't have shared libraries (ie base subpackage).


If you are serious about packaging it would also help
if you looked at some other packages that need reviewing
and posted comments on them - feel free to put links here
to any reviews you have looked over.
Comment 12 Jens Petersen 2012-04-16 05:53:44 EDT
Ping?
Comment 13 Jens Petersen 2012-04-18 01:53:44 EDT
Thanks for you comment on bug 747031.

If you have updated this package then great if you
can post the new url and srpm here. :)
Comment 14 JudeNagurney 2012-04-18 01:54:49 EDT
I have a new spec file and source RPM available.  I didn't understand the comment about the secondary archs not having shared libraries, so I may have missed something there.

Spec URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas.spec
SRPM URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas-0.6.1-1.fc16.src.rpm

As for looking at other packages, I've left the following comments, which looked to be similar Haskell library binding projects.  
https://bugzilla.redhat.com/show_bug.cgi?id=648095#c3
https://bugzilla.redhat.com/show_bug.cgi?id=664214#c7
https://bugzilla.redhat.com/show_bug.cgi?id=747031#c5
Comment 15 Jens Petersen 2012-04-18 02:40:22 EDT
Ok, thanks for updating.

Builds in koji rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=4000859

It would be nice to see a changelog entry in the spec file
showing what has changed - though I understand you have used
cabal2spec to help generate the updated spec file,
but it is useful to have this package history.

rpmlint output:

ghc-augeas.src: W: spelling-error %description -l en_US http -> HTTP
ghc-augeas.src: W: spelling-error %description -l en_US api -> pi, ape, apt
ghc-augeas.src: W: spelling-error %description -l en_US html -> HTML, ht ml, ht-ml

ok

ghc-augeas.src:49: W: macro-in-comment %doc
ghc-augeas.src:49: W: macro-in-comment %{name}

BTW seems the spec file in the srpm has a comment not
in the spec file you also published and that comment
is causing the above 2 warnings.

ghc-augeas.src: W: file-size-mismatch augeas-0.6.1.tar.gz = 56246, http://hackage.haskell.org/packages/archive/augeas/0.6.1/augeas-0.6.1.tar.gz = 56263
1 packages and 0 specfiles checked; 0 errors, 6 warnings.

Can you not package the tarball released on Hackage?
Comment 16 Jens Petersen 2012-05-02 04:59:48 EDT
Ok I changed the whiteboard to NotReady - please change back
to Ready after updating if you can.
Comment 17 JudeNagurney 2012-06-09 15:05:11 EDT
It looks like I had some configuration management and darcs tagging issues that I've hopefully resolved now.  I ran rpmlint on the srpm and I'm just getting the spelling-error warnings in the URL now.  There is a new spec file and source RPM available.

Spec URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas.spec
SRPM URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas-0.6.1-2.fc16.src.rpm
Comment 18 Jens Petersen 2012-06-21 06:27:30 EDT
Thanks for the submission.  My only quibble is that currently
the the spec file url and spec file in the srpm have differing
last changelog entries.

Build succeeds in koji f18: http://koji.fedoraproject.org/koji/taskinfo?taskID=4182838

I am offering to sponsor Jude upon successful completion of this review.

Here is my review:


Here is the review:

 +:ok, NA: not applicable, !: needs attention

MUST Items:
[+] MUST: rpmlint output [1]

ghc-augeas.src: W: spelling-error %description -l en_US http -> HTTP
ghc-augeas.src: W: spelling-error %description -l en_US api -> pi, ape, apt
ghc-augeas.src: W: spelling-error %description -l en_US html -> HTML, ht ml, ht-ml
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
ghc-augeas.x86_64: W: spelling-error %description -l en_US http -> HTTP
ghc-augeas.x86_64: W: spelling-error %description -l en_US api -> pi, ape, apt
ghc-augeas.x86_64: W: spelling-error %description -l en_US html -> HTML, ht ml, ht-ml
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
ghc-augeas-devel.x86_64: W: spelling-error %description -l en_US http -> HTTP
ghc-augeas-devel.x86_64: W: spelling-error %description -l en_US api -> pi, ape, apt
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

[+] MUST: package named according to Package Naming Guidelines
[+] MUST: spec file name must match base package %{name} [2]
[+] MUST: meet Packaging Guidelines
[+] MUST: Fedora approved license and Licensing Guidelines
[!] MUST: License field in the package spec file must match actual license. [3]

Since you allow LGPL 3 or later I think the correct License tag is LGPLv3+

[+] MUST: include license files in %doc if available in source [4]
[+] MUST: The spec file must be written in American English [5] and legible. [6]
[+] MUST: source md5sum matches upstream release (from upstream URL)

123cb7d684447c03b645e9439f0b70bb  augeas-0.6.1.tar.gz

[+] MUST: successfully compile and build into binary rpms on a primary arch [7]

http://koji.fedoraproject.org/koji/taskinfo?taskID=4182838

[NA] MUST: if necessary use ExcludeArch for other archs [8]
[+] MUST: All build dependencies must be listed in BuildRequires
[NA] MUST: use %find_lang macro for .po translations [9]
[NA] MUST: packages which store shared library files in the dynamic linker's default paths, must call ldconfig in %post and %postun. [10]
[+] MUST: Packages must NOT bundle copies of system libraries. [11]
[NA] MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review [12]
[+] MUST: A package must own all directories that it creates. [13]
[+] MUST: A package must not contain any duplicate files in the %files listing. [14]
[+] MUST: Permissions on files must be set properly. [15]
[+] MUST: consistently use macros [16]
[+] MUST: The package must contain code, or permissable content. [17]
[NA] MUST: Large documentation files should go in a doc subpackage. [18]
[+] MUST: If a package includes something as %doc, it must not affect the runtime of the application. [18]
[NA] MUST: Static libraries must be in a -static package. [19]
[+] MUST: Development files must be in a -devel package. [20]
[NA] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), then library files that end in .so (without suffix) must go in a -devel package. [19]
[+] MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency [21]
[+] MUST: Packages must NOT contain any .la libtool archives, these should be removed in the spec. [20]
[NA] MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. [22]
[+] MUST: Packages must not own files or directories already owned by other packages. [23]
[+] MUST: All filenames in rpm packages must be valid UTF-8. [24]

SHOULD Items:
[+] SHOULD: The reviewer should test that the package builds in mock. [27]
[+] SHOULD: If scriptlets are used, those scriptlets must be sane. [29]


So only remaining issue is just correcting the License tag.

Since this is your first package can I ask you to update the
package and I will approve the package and sponsor you.
Comment 19 Jens Petersen 2012-06-21 06:29:47 EDT
I would also suggest the following minor tweaks in the .spec
but they do not block the review.


-BuildRequires: ghc-HUnit-prof
-BuildRequires: augeas-devel%{?isa}
+BuildRequires:  ghc-HUnit-prof
+BuildRequires:  augeas-devel%{?isa}
 
 %description
 %{common_description}
@@ -43,8 +43,8 @@
 %install
 %ghc_lib_install
 
-rm -rf $RPM_BUILD_ROOT/%{_bindir}
-rm -rf $RPM_BUILD_ROOT/%{_datadir}/%{pkg_name}-%{version}
+rm -r $RPM_BUILD_ROOT/%{_bindir}
+rm -r $RPM_BUILD_ROOT/%{_datadir}/%{pkg_name}-%{version}
 
 
 # devel subpackage
@@ -57,12 +57,13 @@
 %ghc_devel_post_postun
 
 
-%ghc_files LICENSE AUTHORS ChangeLog README THANKS
+%ghc_files LICENSE
+%doc AUTHORS ChangeLog README THANKS
Comment 20 JudeNagurney 2012-06-26 08:55:34 EDT
I've applied the changes from comments 18 and 19.  There is a new spec file and source RPM available.

Spec URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas.spec
SRPM URL: http://code.haskell.org/augeas/packaging/rpm/ghc-augeas-0.6.1-3.fc16.src.rpm

Please let me know if there's anything else I should be doing.  Thanks !
Comment 21 Jens Petersen 2012-06-26 21:08:47 EDT
Thanks for submitting your package and updating.

Looks good to me now (though your spec files are still different;-).

http://koji.fedoraproject.org/koji/taskinfo?taskID=4199008

Package is APPROVED


You are now here:

http://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Get_Sponsored

so please apply for membership of the Packager (I think it is still called) and I will sponsor you: if you can let me know when you have done so it will be easier for me.  Then you can proceed to make the SCM request for adding this package.
Comment 22 Jens Petersen 2012-07-23 06:26:26 EDT
Ping?
Comment 23 JudeNagurney 2012-07-23 16:27:03 EDT
I must be overlooking something pretty obvious.  How do I go about applying for membership in the Packager group ?
Comment 24 Jens Petersen 2012-07-25 00:44:30 EDT
Sorry my confusion - it seems the process has changed - I need to add you.

What is your fedora account id?
Comment 25 Jens Petersen 2012-09-07 02:38:24 EDT
I can't find your email in FAS.
Comment 26 Jens Petersen 2012-09-13 06:38:36 EDT
I should warn that package reviews expire after 3 months...
Comment 27 JudeNagurney 2012-10-28 17:14:28 EDT
I dropped the ball here.
My Fedora account name is 'pwan'
The email associated with the account is 'fedora@pwan.org'
Comment 28 Jens Petersen 2012-10-29 01:03:07 EDT
I think it is better you use the same email address for your fedora account
and bugzilla.

Anyway if you are still keen to add this package to Fedora
we could do a new review.
Comment 29 JudeNagurney 2012-10-29 09:16:12 EDT
OK, I've updated my FAS account so both emails be 'jude@pwan.org' now.

I'm up for doing a new review.
Comment 30 Jens Petersen 2012-10-30 06:27:06 EDT
Ok great

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