Bug 472683 (jpcap)
| Summary: | Review Request: jpcap - Packet capturing library for Java | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Patrick Dignan <dignan.patrick> |
| Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> |
| Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | dignan.patrick, fedora-package-review, mtasaka, notting |
| Target Milestone: | --- | Flags: | mtasaka:
fedora-review+
kevin: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | 0.7-6.fc9 | Doc Type: | Bug Fix |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2009-01-26 15:24:28 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Patrick Dignan
2008-11-23 12:14:53 UTC
I'd like to note that this library was written by Keita Fujii (see the URL in the spec file), and I repackaged it to conform with Fedora's standards. Well: - First of all, your srpm doesn't build on F-11: http://koji.fedoraproject.org/koji/taskinfo?taskID=1031148 - Please explain why you want to define %jpcap, %jpcap_version - For license tag: - No version of LGPL is specified in the codes - However some files are licensed under BSD with advertising (and BSD) So the license tag should be "LGPLv2+ and BSD with advertising" - Source must be given by full URL: https://fedoraproject.org/wiki/Packaging/SourceURL - Please remove Vendor item. This is defined automatically when rebuilding your srpm on Fedora buildsystem. - For BuildRequires: * Please explicitly write "BuildRequires: jpackage-utils" to honor Java packaging guidelines (even if java-devel implicitly pulls in jpackage-utils dependency) (same for Requires) * "BuildRequires: libpcap" should be removed as libpcap-devel always requires it. - Please explain why you set "Autoreq: 0". - Remove all pre-compiled binaries (like foo.jar) at %prep stage to make it sure that all binaries are compiled from the source. - We now recommend %defattr(-,root,root,-) - The directory %{_libdir}/%{name} is not owned by any packages (ref: https://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership https://fedoraproject.org/wiki/Packaging/UnownedDirectories ) Ok, updated the package. - I tested it in mock for fedora-devel-i386, and it works fine now, I hadn't set the location for jni.h properly for the libjpcap.so to build. - I had those set because I saw it in an example, but it seems quite useless in retrospect, since I can just use %name and %version - Fixed the licensing issue - Changed to full upstream URL - Removed the Vendor item - Added jpackage-utils to both BuildRequires and Requires - Removed libpcap as a BuildRequire - I used "Autoreq: 0" because upstream had done so in their RPM spec file (which was nowhere near current Fedora spec) - Added an rm -f statement to the %prep which removes the jarfiles and .so pre-built in the upstream tarball - Changed to %defattr(-,root,root,-) - Now uses install -D which should fix the permissions I uploaded new versions here: Spec File: http://users.wpi.edu/~dignan/jpcap.spec Source RPM: http://users.wpi.edu/~dignan/jpcap-0.7-4.fc10.src.rpm The originals are now moved to http://users.wpi.edu/~dignan/archive/0.7-3/jpcap.spec and http://users.wpi.edu/~dignan/archive/0.7-3/jpcap-0.7-3.fc10.src.rpm Thank you for the review, I look forward to your forthcoming comments. For 0.7-4:
* SourceURL
- I recommend to use %{name} and %{version}, especially
%{version}. With this you probably won't have to change
the source URL when version is updated:
https://fedoraproject.org/wiki/Packaging/SourceURL#Using_.25.7Bversion.7D
* Tarball
- By the way the tarball I could download from the Source URL
differs from what in your srpm:
--------------------------------------------------------------
716370 2007-06-12 03:27 downloaded/jpcap-0.7.tar.gz
715154 2009-01-06 15:51 jpcap-0.7-4.fc10.src/jpcap-0.7.tar.gz
--------------------------------------------------------------
(If there is CRLF line terminators difference, please don't
fix this in advance but fix CRLF terminators at %prep)
* Requires
- "Requires: libpcap" is not needed.
rpmbuild detects libraries related dependencies automatically
(in this case libpcap.so.0.9 on F-11) and adds the dependencies
to binary rpms.
* JAVA_HOME
- There is %java_home macro and you can use it
- By the way you may want to use %ant macro instead
(please try $ rpm --eval %ant)
* Build failure/optflags
- Your srpm fails to build on x86_64:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1038602
This is because C codes are not compiled with -fPIC.
Also Fedora specific compilation flags are not correctly
honored:
https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
For this package
---------------------------------------------------------------
make %{?_smp_mflags} CC="gcc %{optflags} -fPIC"
---------------------------------------------------------------
seems to work.
* Don't strip binaries
- Please don't strip binaries. Debug information is required
to create debuginfo rpm correctly (i.e. rpmbuild properly
handles this: also please make sure that binaries are compiled
with -g -- %{optflags} contains this)
* Directory ownership issue
- The directory %{_libdir}/%{name} itself is not yet owned
by this package.
* rpmlint - Also fix these: ------------------------------------------------------------- W: wrong-file-end-of-line-encoding /usr/share/javadoc/jpcap/javadoc/package-list W: wrong-file-end-of-line-encoding /usr/share/javadoc/jpcap/javadoc/stylesheet.css ------------------------------------------------------------- Alright! Fixed all of those problems, and tested using koji on F10 and F11 (I didn't know I had the permissions to do so!) Uploaded new source rpm and new SPEC file to: http://users.wpi.edu/~dignan/jpcap-0.7-5.fc10.src.rpm and http://users.wpi.edu/~dignan/jpcap.spec Moved the old versions to an archive (lost the jpcap spec file to 0.7-3 somehow), and symlinked the source rpm files jpcap-0.7-3.fc10.src.rpm can be found: http://users.wpi.edu/~dignan/jpcap-0.7-3.fc10.src.rpm or http://users.wpi.edu/~dignan/archive/jpcap/0.7-3/jpcap-0.7-3.fc10.src.rpm jpcap-0.7-4.fc10.src.rpm can be found: http://users.wpi.edu/~dignan/jpcap-0.7-4.fc10.src.rpm or http://users.wpi.edu/~dignan/archive/jpcap/0.7-4/jpcap-0.7-4.fc10.src.rpm jpcap-0.7-4 spec file can be found: http://users.wpi.edu/~dignan/archive/jpcap/0.7-4/jpcap.spec Okay, now: ------------------------------------------------------------- NOTE: Before being sponsored: This package will be accepted with another few (or no) work. But before I accept this package, someone (I am a candidate) must sponsor you. Once you are sponsored, you have the right to review other submitters' review requests and approve the packages formally. For this reason, the person who want to be sponsored (like you) are required to "show that you have an understanding of the process and of the packaging guidelines" as is described on : http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored Usually there are two ways to show this. A. submit other review requests with enough quality. B. Do a "pre-review" of other person's review request (at the time you are not sponsored, you cannot do a formal review) When you have submitted a new review request or have pre-reviewed other person's review request, please write the bug number on this bug report so that I can check your comments or review request. Fedora package collection review requests which are waiting for someone to review can be checked on: http://fedoraproject.org/PackageReviewStatus/NEW.html (NOTE: please don't choose "Merge Review") Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------ I Did an informal review here: https://bugzilla.redhat.com/show_bug.cgi?id=478412 I am now looking into packaging the program Jailkit, which is on the wishlist, and seems to be a pretty useful tool. Jailkit is packaged and up for review here: https://bugzilla.redhat.com/show_bug.cgi?id=479546 Okay, - For your pre-review for bug 478412, about compiler flags: * %configure macro sets CFLAGS environment value (you can check what %configure does by "$ rpm --eval %configure"). Usually when using configure/Makefile generated by recent autotools this $CFLAGS value is correctly honored. However when using some old configure/Makefile, which are originally written by the upstream developer sometimes these files does not correctly catch $CFLAGS. In such cases you may have to use "CC='gcc %{optflags}'", for example. - For jailkit, packaging itself seems almost good (I wrote some comments) ---------------------------------------------------------- This package (jpcap) is APPROVED by mtasaka ---------------------------------------------------------- Please follow the procedure written on: http://fedoraproject.org/wiki/PackageMaintainers/Join from "Install the Client Tools (Koji) ". Now I am sponsoring you. If you want to import this package into Fedora 8/9, you also have to look at http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT (after once you rebuilt this package on koji Fedora rebuilding system). If you have questions, please ask me. Removing NEEDSPONSOR. New Package CVS Request ======================= Package Name: jpcap Short Description: Packet capturing library for Java Owners: dignan Branches: F-9 F-10 InitialCC: dignan mtasaka Thank you for the sponsorship and review Mr. Tasaka. I understand now when the %{optflags} tag is needed, have made the appropriate changes to Jailkit, and have taken the appropriate next steps for this package.
Thank you very much.
cvs done. Please rebuild this package on koji and for F-10/9 submit requests to push the rebuilt packages into repositories. Sorry, experiencing some hardware difficulties with my main computer (which runs Fedora), so I've had some downtime. I should hopefully be back up tomorrow, as I hadn't had time to repair it over the week and reinstall. First on my TODO list is getting this built and pushed. Built in Koji for F9, F10, and devel. Submitted bodhi update requests for F-9 and F-10. Thanks for all the help! Okay, now closing, thank you. jpcap-0.7-6.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. jpcap-0.7-6.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report. |