Bug 533738 - Review Request: bwa - Burrows-Wheeler Alignment Tool
Review Request: bwa - Burrows-Wheeler Alignment Tool
Status: CLOSED DUPLICATE of bug 597315
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-08 15:32 EST by Albert Bogdanowicz
Modified: 2013-10-19 10:42 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-08-24 04:10:20 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Albert Bogdanowicz 2009-11-08 15:32:27 EST
Spec URL: http://students.mimuw.edu.pl/~ab277290/Fedora/bwa.spec
SRPM URL: http://students.mimuw.edu.pl/~ab277290/Fedora/bwa-0.5.4-1.fc12.src.rpm
Description: 
I just packaged bwa (I think correctly). It's my first package, so I appreciate any suggestions what could be improved.
Burrows-Wheeler Aligner (BWA) is an efficient program that aligns relatively short nucleotide sequences against a long reference sequence such as the human genome.
Comment 1 Scott Collier 2009-11-08 15:57:01 EST
Hi Albert,

I'm not a sponsor, but I do have some suggestions:

1. Package did not build for me by default:

http://boodle.fedorapeople.org/RPMS/bwa_install_error.out

2. Source0: should point to URL, please see:

http://fedoraproject.org/wiki/Packaging/SourceURL

3. You can add this to your make:
CFLAGS="$RPM_OPT_FLAGS"

Then it will compile.

4. For the %install:

You can use install instead of cp, for example:
install -Dp -m0755 bwa %{buildroot}/%{_bindir}/

and change the permissions to whatever you'd like.

After these changes, rpmlint is clean:

$ rpmlint bwa.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

$ rpmlint bwa.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
Comment 2 Albert Bogdanowicz 2009-11-12 17:43:36 EST
I think it should work now.
Comment 3 Scott Collier 2009-11-12 20:25:44 EST
(In reply to comment #2)
> I think it should work now.  

I still had errors building.

http://boodle.fedorapeople.org/RPMS/bwa2_errors

Try changing your make line to:

make %{?_smp_mflags} \
        CFLAGS="$RPM_OPT_FLAGS"

Then it compiles.  I'm happy to test again once you've made the change.

Everything else looks OK to me.  Perhaps someone with more experience would like to give it a once over?
Comment 4 Albert Bogdanowicz 2009-11-13 05:15:47 EST
Thanks for help. Fixed that, hope it's fine now.
Comment 5 Scott Collier 2009-11-13 08:02:32 EST
(In reply to comment #4)
> Thanks for help. Fixed that, hope it's fine now.  

works for me now.  One more thing, in your spec file you have % files listed twice:

%files
%defattr(-,root,root,-)
%doc COPYING NEWS ChangeLog
%{_bindir}/*

%files devel
%defattr(-,root,root,-)
%{_includedir}/*


You can get rid of one and compress it down to:

%files
%defattr(-,root,root,-)
%doc COPYING NEWS ChangeLog
%{_bindir}/*
%{_includedir}/*

rpmlint has one more trivial warning:

$ rpmlint bwa.spec 
bwa.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 20, tab: line 1)
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

Go ahead and fix that, then I think you are good to go.
Comment 6 Scott Collier 2009-11-13 09:14:19 EST
(In reply to comment #5)
> (In reply to comment #4)
> > Thanks for help. Fixed that, hope it's fine now.  
> 
> works for me now.  One more thing, in your spec file you have % files listed
> twice:
> 
> %files
> %defattr(-,root,root,-)
> %doc COPYING NEWS ChangeLog
> %{_bindir}/*
> 
> %files devel
> %defattr(-,root,root,-)
> %{_includedir}/*
> 
> 
> You can get rid of one and compress it down to:
> 
> %files
> %defattr(-,root,root,-)
> %doc COPYING NEWS ChangeLog
> %{_bindir}/*
> %{_includedir}/*
> 

Sorry, missed the %files devel tag, nevermind the comment above.  %files devel is fine.  That just leaves the rpmlint warning.

> rpmlint has one more trivial warning:
> 
> $ rpmlint bwa.spec 
> bwa.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 20, tab: line 1)
> 0 packages and 1 specfiles checked; 0 errors, 1 warnings.
> 
> Go ahead and fix that, then I think you are good to go.
Comment 7 Albert Bogdanowicz 2009-11-13 09:44:12 EST
Fixed, no tabs now. I only get a warning about no documentation for the devel package.
Comment 8 Scott Collier 2009-11-13 10:20:22 EST
(In reply to comment #7)
> Fixed, no tabs now. I only get a warning about no documentation for the devel
> package.  

Great.  Thanks Albert.

At this point you just need an "offical" review and approval.  It looks good to me.  Is there anyone with some time who can review please?
Comment 9 Adam Huffman 2010-01-14 01:10:53 EST
I will take a look, though I'm not a sponsor, just a packager.
Comment 10 Adam Huffman 2010-06-21 19:48:40 EDT
Albert,

Oops - I was in a rush to package this at work and submitted a review request, having forgotten about this existing request.  It's now been accepted:

https://bugzilla.redhat.com/show_bug.cgi?id=597315

Sorry about that.

Hope it's useful to you, at least.
Comment 11 Martin Gieseking 2010-08-24 04:10:20 EDT
Since Adam's package has been accepted, I'm closing this review request. 
Albert, if you're still interested in joining the Fedora packager group, please feel free to submit another package.

*** This bug has been marked as a duplicate of bug 597315 ***

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