Bug 251065 - [new port] biology/pooler: optimise primer-set combinations
Summary: [new port] biology/pooler: optimise primer-set combinations
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Jason W. Bacon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-11-12 08:38 UTC by Silas S. Brown
Modified: 2020-11-15 18:51 UTC (History)
1 user (show)

See Also:


Attachments
new port (3.62 KB, patch)
2020-11-12 08:38 UTC, Silas S. Brown
no flags Details | Diff
pooler.diff take 2 (3.11 KB, patch)
2020-11-13 00:28 UTC, Silas S. Brown
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Silas S. Brown 2020-11-12 08:38:05 UTC
Created attachment 219586 [details]
new port

PrimerPooler: optimise primer-set combinations
Comment 1 Jason W. Bacon freebsd_committer 2020-11-12 21:57:11 UTC
Comment on attachment 219586 [details]
new port

Thanks for your interest in contributing to FreeBSD ports!

You're off to a good start, but the Makefile is going to need some work.  No worries, I'll guide you along.

Please have a look at the Porter's handbook, in particular the variable order section:

https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/porting-order.html

You can run "portlint -A" to identify most style issues and potential problems.

I'll help identify and fix any issues that remain.
Comment 2 Silas S. Brown 2020-11-13 00:28:34 UTC
Created attachment 219616 [details]
pooler.diff take 2
Comment 3 Silas S. Brown 2020-11-13 00:29:03 UTC
Thanks Jason.  I've attached a replacement diff.
Comment 4 Jason W. Bacon freebsd_committer 2020-11-13 14:56:14 UTC
Comment on attachment 219616 [details]
pooler.diff take 2

Much better.  A few more things:

You can learn a lot by looking at other ports, though they don't all adhere to standards, so take what you see with a grain of salt.

portlint is still flagging problems with pkg-descr.  We can drop text that doesn't really help the reader understand what the port is for, such as "This is a program I wrote" and details about utilizing CPU features.

There should be one or more tabs following '=' in each variable assignment. I generally line up the rvalues in each block with as few tabs as possible.

The canonical way to handle Github tags with prefixes like 'v' is using DISTVERSIONPREFIX.

MAKE_FLAGS should be moved down.  Hint: Variable block order generally follows the port stages, so fetch-related stuff at the top, then dependencies, build, install.

There is a LICENSE file in the dist, so that should be cited in the port Makefile.

The build should respect CC, CFLAGS, etc.from the environment and make arguments  Since you appear to be the author, maybe you can update the dist Makefile.

Use WRKSRC_SUBDIR if the build should not start in the root of the dist.  FreeBSD ports has a variable for just about every common situation, to avoid the need for ad hoc code.

If you set DEVELOPER=yes in /etc/make.conf, you'll get additional quality checks that portlint can't do, like:

====> Running Q/A tests (stage-qa)
Warning: 'bin/pooler' is not stripped consider trying INSTALL_TARGET=install-strip or using ${STRIP_CMD}

I incorporated these changes into my work-in-progress collection, including a Makefile patch:

https://github.com/outpaddling/freebsd-ports-wip/tree/master/pooler

Also check out

https://github.com/outpaddling/freebsd-ports-wip/tree/master/port-dev

for some tools to speed up port development like port-check for testing changes, port-grep for searching Makefiles, plists, etc., port-list for finding ports via patterns...
Comment 5 Jason W. Bacon freebsd_committer 2020-11-13 15:55:06 UTC
Also gmake is not actually needed.

Lastly, the name "pooler" is OK here since that's the name of the sole binary installed, but we might consider naming the port primerpooler to match upstream and be a little more descriptive. FreeBSD ports does not have a safe way to change port/package names, so we want to choose one we can stay with long-term, to avoid problems with pkg upgrade.

I'm about ready to commit this, unless you want to update the dist Makefile and tag a new release to make the patch unnecessary and/or rename it.
Comment 6 Silas S. Brown 2020-11-14 00:43:22 UTC
Thanks Jason.  The only thing that worries me about that patch is it means -fopenmp will not be set if CFLAGS is set, and that means we don't get the parallelisation.  So I wonder if doing this upstream is any better (tagged as v1.74) : https://github.com/ssb22/PrimerPooler/commit/798b0b2aee97b783570a49bf0f58696848e52f24
Comment 7 Jason W. Bacon freebsd_committer 2020-11-14 01:25:19 UTC
(In reply to Silas S. Brown from comment #6)

Oops, I missed that one buried in the 1-liner test prog.  ;-)

What I normally do in situations like this is use a ?= to set flags that the user's env should be able to override, followed by a += with everything critical to the app.

CC ?=           $(CC0) $(CC1)
CFLAGS ?=       ${UnixFlags}
CFLAGS +=       -fopenmp

Then you don't need to sprinkle a second variable into all your make commands.  Seem effect at your 1.74 release but a little more concise.  Up to you...

With this we also need USES=compiler:openmp in the port Makefile.  This currently adds a gcc dependency, but I'm hoping that will change.  I think it's mainly still there for 2nd-tier platform support (e.g. powerpc).
Comment 8 Jason W. Bacon freebsd_committer 2020-11-14 01:38:00 UTC
One minor issue with the 1.74 Makefile:

Your openmp auto-detection is the only essential part of the Makefile that uses gmake syntax.  Not a big deal, but it's best to minimize dependencies, especially when building from source.  If we can avoid gmake, it will save the package builds some time at the very least.

On that note, do you really expect anyone to build pooler using a compiler with no openmp support?  I would think openmp support is a reasonable assumption here and we can just add it to CFLAGS unconditionally.
Comment 9 Jason W. Bacon freebsd_committer 2020-11-14 14:32:52 UTC
Also noticed that the install target was rerunning the pooler build command after it already succeeded.  This was because the pooler build command was under a phony "unix:" target instead of the real "pooler:" target.  I added a patch to the wip port to reorganize the targets.  I included a hack for the mac-portable target as well, though there's probably a cleaner way to handle that case.
Comment 10 Silas S. Brown 2020-11-15 07:56:45 UTC
Thanks, unfortunately I do have to keep detecting -fopenmp support from the Makefile because MacOS is rather inconsistent (10.7's gcc has it, 10.9's does not) and we do want Mac users to still be able to compile it even if they don't have OpenMP (although they'd be much better off using the binary if that's the case).  But specifying it via CFLAGS in FreeBSD is a good idea, that way the detection code doesn't have to run.

But I don't really need the upstream "mac-portable" target (it's only for a "make publish" rule I use when releasing a new version, and I can do the same thing by calling "make pooler" with a CFLAGS argument), so I might as well fix that upstream so the patch file isn't needed.

Tagged as v1.75: https://github.com/ssb22/PrimerPooler/commit/d7bce14b555147043d220ec0a2f0e68117ebbaeb
Comment 11 commit-hook freebsd_committer 2020-11-15 18:50:20 UTC
A commit references this bug:

Author: jwb
Date: Sun Nov 15 18:50:19 UTC 2020
New revision: 555418
URL: https://svnweb.freebsd.org/changeset/ports/555418

Log:
  biology/pooler: Optimise DNA sequencing primer-set combinations

  Optimise combinations of primers and minimise the formation of dimers in
  multiplexed PCR.

  Primer Pooler can:

  * Check through each proposed pool for combinations that are likely to form
    dimers

  * Automatically move prospective amplicons between proposed pools to reduce
    dimer formation

  * Automatically search the genome sequence to find which amplicons overlap, and
    place their corresponding primers in separate pools

  * Optionally keep pool sizes within a specified range

  * Handle thousands of primers without being slow (useful for high-throughput
    sequencing applications)

  * Do all of the above with degenerate primers too.

  WWW: http://ssb22.user.srcf.net/pooler/

  PR:             ports/251065
  Submitted by:   Silas S. Brown <silas-freebsd@flatline.org.uk>

Changes:
  head/biology/Makefile
  head/biology/pooler/
  head/biology/pooler/Makefile
  head/biology/pooler/distinfo
  head/biology/pooler/pkg-descr
Comment 12 Jason W. Bacon freebsd_committer 2020-11-15 18:51:11 UTC
Thanks for the new port!