Bug 502130 - Review Request: openocd - Open On-Chip Debugger
Review Request: openocd - Open On-Chip Debugger
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Chitlesh GOORAH
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-21 22:30 EDT by Dean Glazeski
Modified: 2010-09-08 13:55 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-09-03 09:38:52 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
chitlesh: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Dean Glazeski 2009-05-21 22:30:40 EDT
Spec URL: http://files.dinoprojects.com/openocd.spec
SRPM URL: http://files.dinoprojects.com/openocd-0.1.0-1.fc10.src.rpm
Description: 
This is my first package and I am looking for a sponsor.  My user name is dnglaze over in FAS.

The Open On-Chip Debugger (OpenOCD) provides debugging, in-system programming
and boundary-scan testing for embedded devices.  Various different boards,
targets, and interfaces are supported to ease development time.

Install OpenOCD if you are looking for an open source solution for hardware
debugging.
Comment 1 Dean Glazeski 2009-05-21 22:34:15 EDT
Running rpmlint on the final binary RPM does result in errors concerning a statically linked library that is built for targets and not to be run by OpenOCD.  Reference: https://bugzilla.redhat.com/show_bug.cgi?id=502112
Comment 2 Chitlesh GOORAH 2009-05-22 05:42:01 EDT
I will sponsor you. Can you give me your FAS username please ?
Comment 3 Dean Glazeski 2009-05-22 07:47:41 EDT
(In reply to comment #2)
> I will sponsor you. Can you give me your FAS username please ?  
My username is dnglaze in FAS.
Comment 4 Chitlesh GOORAH 2009-06-06 15:44:10 EDT
#1: The contents of /usr/share/openocd/contrib should go to %doc
%doc contrib/

and remove the /usr/share/openocd/contrib at the end of the %install section to avoid duplicates

#2: source url should be complete

#3: Preserve timestamps
add INSTALL="install -p"  to your make install
Comment 5 Dean Glazeski 2009-07-01 20:35:13 EDT
(In reply to comment #4)
> #1: The contents of /usr/share/openocd/contrib should go to %doc
> %doc contrib/
>
> and remove the /usr/share/openocd/contrib at the end of the %install section to
> avoid duplicates

This makes more warnings appear from rpmlint.  For some reason, doing the "fuller path" %doc %{_datadir}/%{name}/contrib/* results in no warnings.
 
> #2: source url should be complete

I think I got this done.  I replaced the URL parameter with the full path to the source tar on OpenOCD's project site.

> #3: Preserve timestamps
> add INSTALL="install -p"  to your make install  

Added this.

New links:
http://files.dinoprojects.com/openocd/openocd-0.1.0-2.fc10.src.rpm
http://files.dinoprojects.com/openocd/openocd.spec
Comment 6 Chitlesh GOORAH 2009-07-05 08:47:51 EDT
(In reply to comment #5)
> I think I got this done.  I replaced the URL parameter with the full path to
> the source tar on OpenOCD's project site.

No actually, source0 should have the full URL for downloading the sources.

URL is the url for the upstream website.
Comment 7 Dean Glazeski 2009-07-18 21:23:11 EDT
(In reply to comment #6)
> No actually, source0 should have the full URL for downloading the sources.
> 
> URL is the url for the upstream website.  


Alright, I've finally made these updates to the spec file.  Here are the new files.  This includes a spec bump.  Thanks.

http://files.dinoprojects.com/openocd/openocd-0.1.0-3.fc11.src.rpm
http://files.dinoprojects.com/openocd/openocd.spec
Comment 8 Dean Glazeski 2009-08-08 12:52:47 EDT
I have now updated the spec file and source RPM to reflect a new version of OpenOCD that was released recently.  Koji builds are showing clean for Fedora 10 and 11.

http://files.dinoprojects.com/openocd/openocd-0.2.0-1.fc11.src.rpm
http://files.dinoprojects.com/openocd/openocd.spec
Comment 9 Chitlesh GOORAH 2009-08-14 16:25:54 EDT
Is there any reason you are shipping static libraries ?
Comment 10 Dean Glazeski 2009-08-14 17:44:43 EDT
(In reply to comment #9)
> Is there any reason you are shipping static libraries ?  

That's my bad.  I didn't see the information on static libraries in the packaging guidelines.  There is an option to build a shared object instead, but when I do that I get the following warnings from rpmlint:

openocd.x86_64: W: shared-lib-calls-exit /usr/lib64/libopenocd.so.0.0.0 exit@GLIBC_2.2.5
openocd.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libopenocd.so

I've removed 2 errors that have already been resolved as documented in the spec file.  I'm not sure what to do with the unversioned shared object.  According to the guidelines, this should be in the devel package.  Should I just remove the symlink?

Here are links to the version of the spec file that builds a shared object instead of a static library.

http://files.dinoprojects.com/openocd/openocd-0.2.0-2.fc11.src.rpm
http://files.dinoprojects.com/openocd/openocd.spec
Comment 11 Chitlesh GOORAH 2009-08-20 09:07:56 EDT
#001 : ScriptletSnippets - Texinfo

Please turn the spec file with respect to the Fedora packaging guidelines as stated on 

http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Texinfo

#002 : %prep instead of %build

Move the following to the %prep section

-------------
cd doc
iconv -f iso8859-1 -t utf-8 openocd.info > openocd.info.conv
mv -f openocd.info.conv openocd.info
-------------

#003 : Improve future commit patches and logs

Use the following style for the %configure :

%configure \
    --disable-werror \
    --disable-static \
    --enable-shared \
    --enable-parport \
    --enable-parport_ppdev \
    --enable-ft2232_libftdi 
    --enable-ep93xx \
    --enable-at91rm9200 \
    --enable-usbprog \
    --enable-presto_libftdi \
    --enable-jlink \
    --enable-vsllink \
    --enable-rlink \
    --enable-dummy \
    --enable-gw16012 \
    --enable-amtjtagaccel \
    --enable-arm-jtag-ew

This is because, when openocd will be approved and everytime you will update it, you will have to commit the srpm to fedora cvs repositories. For each commit, a log message (a patch-like message) will automatically be sent to the fedora-cvs-commits mailing list, hence it would be nice that the updated item can be easily spotted in the logs.

#004: Build requires:

libftdi-devel already requires libusb-devel see

$ rpm -qR libftdi-devel

Remove libusb-devel from the buildrequires
Comment 12 Dean Glazeski 2009-08-21 18:47:20 EDT
(In reply to comment #11)

I made changes #1-4.  I also switched back to a static library, but removed the library from the distribution.  This was suggested by upstream.  Here is the new spec file and SRPM.

http://files.dinoprojects.com/openocd/openocd-0.2.0-3.fc11.src.rpm
http://files.dinoprojects.com/openocd/openocd.spec
Comment 13 Chitlesh GOORAH 2009-08-22 03:25:31 EDT
I sponsored you as a Fedora packager with its relative rights.
Comment 14 Chitlesh GOORAH 2009-08-22 03:47:47 EDT
The package looks good to me except the following warnings are found during the rpmbuild


warning: File listed twice: /usr/share/openocd/contrib/libdcc
warning: File listed twice: /usr/share/openocd/contrib/libdcc/README
warning: File listed twice: /usr/share/openocd/contrib/libdcc/dcc_stdio.c
warning: File listed twice: /usr/share/openocd/contrib/libdcc/dcc_stdio.h
warning: File listed twice: /usr/share/openocd/contrib/libdcc/example.c
warning: File listed twice: /usr/share/openocd/contrib/openocd.udev


To fix it, replace

%{_datadir}/%{name}

by

%dir %{_datadir}/%{name}
%{_datadir}/%{name}/scripts
Comment 15 Chitlesh GOORAH 2009-08-22 03:48:28 EDT
[Pending] MUST: the package does not contain any duplicate files in the %files listing.
[Pending] MUST: the package owns all directories that it creates.

Fix the %files as mentioned above

---------------------------------------------------------------------------

- MUST: The package is named according to the Package Naming Guidelines.
- MUST: The spec file name matches the base package %{name}
- MUST: The package meets the Packaging Guidelines.
- MUST: The package is licensed (GPLv2) with an open-source compatible license and meet other legal requirements as defined in the legal section of Packaging Guidelines.
- MUST: The License field in the package spec file matches the actual license.
- MUST: the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc.
- MUST: The spec file must be written in American English.
- MUST: The spec file for the package is be legible. 
- MUST: The sources used to build the package must matches the upstream source, as provided in the spec URL.
- MUST: The package successfully compiles and builds into binary rpms on at least i586.
- MUST: All build dependencies is listed in BuildRequires.
- MUST: The spec file handles locales properly.: No locales in this package
- MUST: the package is not designed to be relocatable
- MUST: Permissions on files are set properly.
- MUST: The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
- MUST: The package consistently uses macros, as described in the macros section of Packaging Guidelines.
- MUST: The package contains code, or permissible content. This is described in detail in the code vs. content section of Packaging Guidelines.
- MUST: There are no Large documentation files
- MUST: %doc does not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present.
- MUST: There are no Header files or static libraries 
- MUST: The package does not contain library files with a suffix 
- MUST: Package does NOT contain any .la libtool archives
- MUST: Package does not own files or directories already owned by other packages. 

SHOULD Items:

 - SHOULD: The source package does include license text(s) as COPYING
 - SHOULD: mock builds succcessfully in i586.
 - SHOULD: The reviewer tested that the package functions as described. A
package should not segfault instead of running, for example.
 - SHOULD:  Those scriptlets used are sane. 
 - SHOULD: No subpackages present.



Update the package for approval.
Comment 16 Dean Glazeski 2009-08-22 10:04:50 EDT
(In reply to comment #15)

I've updated the spec file to have the fix for the warnings about the files being listed twice.  New links given below.

http://files.dinoprojects.com/openocd/openocd-0.2.0-4.fc11.src.rpm
http://files.dinoprojects.com/openocd/openocd.spec
Comment 17 Chitlesh GOORAH 2009-08-24 06:49:59 EDT
Approved.

Please commit the packaging process as described as from this section:
https://fedoraproject.org/wiki/PackageMaintainers/Join#Add_Package_to_CVS_and_Set_Owner
Comment 18 Dean Glazeski 2009-08-24 22:05:28 EDT
New Package CVS Request
=======================
Package Name: openocd
Short Description: Debugging, in-system programming and boundary-scan testing for embedded devices
Owners: dnglaze
Branches: F-10 F-11
InitialCC:
Comment 19 Kevin Fenzi 2009-08-26 18:26:38 EDT
cvs done.
Comment 20 Chitlesh GOORAH 2009-08-28 06:30:19 EDT
Please proceed for build and push as described in
https://fedoraproject.org/wiki/PackageMaintainers/Join#Check_out_the_module
Comment 21 Chitlesh GOORAH 2009-09-03 01:14:05 EDT
Dean ? Are you there ?
Comment 22 Dean Glazeski 2009-09-03 08:42:06 EDT
(In reply to comment #21)
> Dean ? Are you there ?  
Yeah, I'm still here.  I've got the package uploaded, tagged, and built for devel, F-10, and F-11.  I've been swamped recently so I keep forgetting to submit the new package in Bodhi.  I went ahead and took care of this this morning.  See https://admin.fedoraproject.org/updates/openocd
Comment 23 Chitlesh GOORAH 2009-09-03 09:38:52 EDT
Next add the bug number on bodhi while you are requesting a push. Bodhi will automatically close this package review.

I'm closing it manually.
Comment 24 Dean Glazeski 2009-11-06 13:13:01 EST
Package Change Request
======================
Package Name: openocd
New Branches: F-12
Owners: dnglaze
Comment 25 Dean Glazeski 2009-11-06 13:25:43 EST
My bad.  I didn't use the right command to get the F-12 branch.  Everything is fine, I'm just new at this.
Comment 26 Chitlesh GOORAH 2009-11-06 14:21:42 EST
FYI: https://admin.fedoraproject.org/pkgdb/packages/name/openocd

this can be handy to figure out quickly your branches.
Comment 27 Dean Glazeski 2010-09-07 12:40:06 EDT
Package Change Request
======================
Package Name: openocd
New Branches: el6
Owners:  dnglaze
InitialCC:
Comment 28 Kevin Fenzi 2010-09-08 13:55:17 EDT
Git done (by process-git-requests).

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