Bug 457038 - Review Request: primer3 - PCR primer design tool
Summary: Review Request: primer3 - PCR primer design tool
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nicolas Chauvet (kwizart)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-07-29 12:10 UTC by Pierre-YvesChibon
Modified: 2008-10-30 12:55 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-10-30 12:51:27 UTC
Type: ---
kwizart: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Pierre-YvesChibon 2008-07-29 12:10:07 UTC
Spec URL: http://pingou.fedorapeople.org/RPMs/primer3.spec
SRPM URL: http://pingou.fedorapeople.org/RPMs/primer3-1.1.4-1.fc9.src.rpm
Description: 
Primer3 is a widely used program for designing PCR primers (PCR = 
"Polymerase Chain Reaction"). PCR is an essential and ubiquitous 
tool in genetics and molecular biology. Primer3 can also design 
hybridization probes and sequencing primers.

PCR is used for many different goals. Consequently, primer3 has 
many different input parameters that you control and that tell 
primer3 exactly what characteristics make good primers for your goals.

Comment 1 Pierre-YvesChibon 2008-07-29 12:10:42 UTC
build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=745087

Comment 2 Nicolas Chauvet (kwizart) 2008-07-29 15:07:30 UTC
- starting review -

1/ It would be fine to keep timestamps while converting README.txt to UTF8
2/ This package doesn't use our RPM_OPT_FLAGS
gcc -c -g -Wall -D__USE_FIXED_PROTOTYPES__ -O2 -DDPAL_MAX_ALIGN=36
-DMAX_PRIMER_LENGTH=36 primer3_main.c
3/ It seems possible to build a Shared library :
OLIGOTM_DYN_LIB = liboligotm.so.1.2.0
 You can note that this library will probably need to be installed manually as
oligotm.h will probably need the releated liboligotm.a (or .so) to be bundled in
prism3-devel 
Do you know any dependent package to test primer3-devel ?
I guess not all headers will be needed...
4/ Make check is usally done in a separate job control (before %clean)
%check
pushd src
%{?_with_tests:make check}
popd
This will probably need to be conditionalized as some tests take time.
(tested succeded on F-8 x86_64 local rpmbuild - so it's good)
5/ Why BuildRequires:  perl ?
This is in the default buildRequirement, but I wonder why you have added it ?


Comment 3 Pierre-YvesChibon 2008-07-30 13:15:38 UTC
(In reply to comment #2)
> 1/ It would be fine to keep timestamps while converting README.txt to UTF8
I am using now touch -r COPYING.txt README.txt after the conversion to UTF8.
Is it fine ?

> 2/ This package doesn't use our RPM_OPT_FLAGS
> gcc -c -g -Wall -D__USE_FIXED_PROTOTYPES__ -O2 -DDPAL_MAX_ALIGN=36
> -DMAX_PRIMER_LENGTH=36 primer3_main.c
Could you give me some insigth on how I can change this ? 

> 3/ It seems possible to build a Shared library :
> OLIGOTM_DYN_LIB = liboligotm.so.1.2.0
>  You can note that this library will probably need to be installed manually as
> oligotm.h will probably need the releated liboligotm.a (or .so) to be bundled in
> prism3-devel 
> Do you know any dependent package to test primer3-devel ?
Actually, I do not know any.

> I guess not all headers will be needed...
I would say so to, but since I am not sure I prefered to put them all rather
than miss one.

> 4/ Make check is usally done in a separate job control (before %clean)
> %check
> pushd src
> %{?_with_tests:make check}
> popd
added

> This will probably need to be conditionalized as some tests take time.
> (tested succeded on F-8 x86_64 local rpmbuild - so it's good)

> 5/ Why BuildRequires:  perl ?
> This is in the default buildRequirement, but I wonder why you have added it ?
It's the only dependency that I found on the website. I can removed it if it's
there by default



Comment 4 Pierre-YvesChibon 2008-08-06 14:16:05 UTC
There are the new files
http://pingou.fedorapeople.org/RPMs/primer3.spec
http://pingou.fedorapeople.org/RPMs/primer3-1.1.4-2.fc9.src.rpm

%changelog
* Wed Aug 06 2008 pingou <pingoufc4@yahoo.fr> 1.1.4-2
- Keep the timestamp in the README.txt
- Change the CFLAG for the compilation
- Remove BR perl

Comment 5 Nicolas Chauvet (kwizart) 2008-08-07 07:15:08 UTC
- README.txt convertion can be simplified. 
- you need to have export INIT_CFLAGS="%{optflags}" in the %build section
- make test remains in the %build section.
- non-standard perms for binaries, should be 755

It still seems strange if you don't install the .a or .so when you install some .h for the -devel package. Is it possible or relevant to build a shared onject library instead ?

Comment 6 Pierre-YvesChibon 2008-08-11 11:38:18 UTC
(In reply to comment #5)
> - README.txt convertion can be simplified.
I have always done this in that way. Does it really matter ? I would prefer be consistent over my spec file.
> - you need to have export INIT_CFLAGS="%{optflags}" in the %build section
Done
> - make test remains in the %build section.
Done
> - non-standard perms for binaries, should be 755
Done


> It still seems strange if you don't install the .a or .so when you install some
> .h for the -devel package. Is it possible or relevant to build a shared onject
> library instead ?

I have never done such sub-package could you point me to an example or so doc ?

SPEC
http://pingou.fedorapeople.org/RPMs/primer3.spec
SRPM
http://pingou.fedorapeople.org/RPMs/primer3-1.1.4-3.fc9.src.rpm

Comment 7 Pierre-YvesChibon 2008-08-12 15:19:14 UTC
I corrected the make test to include the conditionnality
--with tests does the tests (make test)
defaults does not

SPEC
http://pingou.fedorapeople.org/RPMs/primer3.spec
SRPM
http://pingou.fedorapeople.org/RPMs/primer3-1.1.4-4.fc9.src.rpm

Comment 8 Nicolas Chauvet (kwizart) 2008-08-12 16:04:27 UTC
export INIT_CFLAGS="%{optflags} -fPIC -DPIC"
make %{?_smp_mflags} liboligotm.so.1.2.0
make %{?_smp_mflags}
make %{?_smp_mflags} primer3_core.dyn

This will build a shared version of the library along with a binary linked with the shared library. Maybe -fPIC isn't needed on i386 for some little library (you need to test), but on x86_64, it is mandatory.

Comment 9 Pierre-YvesChibon 2008-10-06 08:32:00 UTC
Ok I have removed the -devel package and I did not do the shared library because I do not know any project that needs those headers files.

I am keeping this package as simple as possible.

SPEC
http://pingou.fedorapeople.org/RPMs/primer3.spec
SRPM
http://pingou.fedorapeople.org/RPMs/primer3-1.1.4-5.fc9.src.rpm

Comment 10 Nicolas Chauvet (kwizart) 2008-10-06 14:09:43 UTC
--------------------
This package (primer3) is APPROVED by me
--------------------

Comment 11 Pierre-YvesChibon 2008-10-07 12:32:23 UTC
New Package CVS Request
=======================
Package Name: primer3
Short Description: PCR primer design tool
Owners: pingou
Branches: F-8 F-9
InitialCC:

Comment 12 Kevin Fenzi 2008-10-07 17:20:46 UTC
cvs done.

Comment 13 Fedora Update System 2008-10-16 19:50:18 UTC
primer3-1.1.4-5.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/primer3-1.1.4-5.fc9

Comment 14 Fedora Update System 2008-10-16 19:50:22 UTC
primer3-1.1.4-5.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/primer3-1.1.4-5.fc8

Comment 15 Fedora Update System 2008-10-30 12:51:24 UTC
primer3-1.1.4-5.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 16 Fedora Update System 2008-10-30 12:55:35 UTC
primer3-1.1.4-5.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.


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