Jellyfish is a tool for fast, memory-efficient counting of k-mers in DNA. A k-mer is a substring of length k, and counting the occurrences of all such substrings is a central step in many analyses of DNA sequence. JELLYFISH can count k-mers quickly by using an efficient encoding of a hash table and by exploiting the "compare-and-swap" CPU instruction to increase parallelism. WWW: http://www.genome.umd.edu/jellyfish.html portlint: OK (WARN: Makefile: [43]: use ${VARIABLE}, instead of $(VARIABLE). 0 fatal errors and 1 warning found.) testport: OK (poudriere: 10.1-{amd64,i386}, 9.3-amd64) Attempted to build on 9.3-i386, but it looks like too much work. Nobody is likely to run genomics tools on i386 anyway, so I considered ONLY_FOR_ARCHS=amd64, but the 10.1-i386 fix was simple.
Created attachment 168261 [details] Shar archive
Some suggestions after a quick review: * You can drop the pkg-config sed call and replace it with USES=pathfix and PATHFIX_MAKEFILEIN=Makefile.am. * The version and architecture checks can be simplified: 1. Use a helper to mark the port as broken: BROKEN_FreeBSD_9_i386= multiple code issues on i386 < 10.0-RELEASE 2. Use MACHINE_CPU to check if the CPU supports SSE: .if empty(MACHINE_CPU:sse) CONFIGURE_ARGS+= --without-sse .endif This also allows you to just include bsd.port.mk. * BUILD_DEPENDS+= should be BUILD_DEPENDS= * Should --without-sse work on amd64? I tried passing it here and the build failed.
A commit references this bug: Author: amdmi3 Date: Thu Mar 24 13:53:07 UTC 2016 New revision: 411785 URL: https://svnweb.freebsd.org/changeset/ports/411785 Log: Jellyfish is a tool for fast, memory-efficient counting of k-mers in DNA. A k-mer is a substring of length k, and counting the occurrences of all such substrings is a central step in many analyses of DNA sequence. JELLYFISH can count k-mers quickly by using an efficient encoding of a hash table and by exploiting the "compare-and-swap" CPU instruction to increase parallelism. WWW: http://www.genome.umd.edu/jellyfish.html PR: 207929 Submitted by: bacon4000@gmail.com Changes: head/biology/Makefile head/biology/jellyfish/ head/biology/jellyfish/Makefile head/biology/jellyfish/distinfo head/biology/jellyfish/files/ head/biology/jellyfish/files/patch-include_jellyfish_file__header.hpp head/biology/jellyfish/pkg-descr head/biology/jellyfish/pkg-plist
Dmitry, we were in the middle of code review, it would've been nice if you hadn't committed the original patch.
I'll submit a maintainer update ASAP. Cheers, Jason
(In reply to Raphael Kubo da Costa from comment #4) > Dmitry, we were in the middle of code review, it would've been nice if you > hadn't committed the original patch. The port was not assigned so I take it as mere suggestion, and I don't agree with most of these. > * You can drop the pkg-config sed call and replace it with USES=pathfix and PATHFIX_MAKEFILEIN=Makefile.am. This is a hack. Sed is more straightforward. > * The version and architecture checks can be simplified: > 1. Use a helper to mark the port as broken: > BROKEN_FreeBSD_9_i386= multiple code issues on i386 < 10.0-RELEASE IGNORE is OK if it's not intended to work. But yes, if it's `code issues', BROKEN should be used. > 2. Use MACHINE_CPU to check if the CPU supports SSE: > .if empty(MACHINE_CPU:sse) > CONFIGURE_ARGS+= --without-sse > .endif I don't think you may use MACHINE_CPU in ports. > * BUILD_DEPENDS+= should be BUILD_DEPENDS= Fixed. > I'll submit a maintainer update ASAP. There's no need to hurry, you may submit improvements along with further port updates. BROKEN_FreeBSD_9_i386 is the only one that I'd actually add.
(In reply to Dmitry Marakasov from comment #6) > The port was not assigned so I take it as mere suggestion, and I don't agree with most of these. You could have at least said _something_, including asking Jason if he intended to take any of the suggestions or not. It probably wasn't even your intention, but the way this was handled felt like everything I wrote was simply ignored. > > * You can drop the pkg-config sed call and replace it with USES=pathfix and PATHFIX_MAKEFILEIN=Makefile.am. > > This is a hack. Sed is more straightforward. Ues/pathfix.mk exists precisely to avoid the duplication of similar sed calls in the tree. > > * The version and architecture checks can be simplified: > > 1. Use a helper to mark the port as broken: > > BROKEN_FreeBSD_9_i386= multiple code issues on i386 < 10.0-RELEASE > > IGNORE is OK if it's not intended to work. But yes, if it's `code issues', > BROKEN should be used. > > > 2. Use MACHINE_CPU to check if the CPU supports SSE: > > .if empty(MACHINE_CPU:sse) > > CONFIGURE_ARGS+= --without-sse > > .endif > > I don't think you may use MACHINE_CPU in ports. Yes you can. At the moment there are 69 occurrences in the tree and I am not aware of any policies forbidding its use.
(In reply to Raphael Kubo da Costa from comment #7) > Ues/pathfix.mk exists precisely to avoid the duplication of similar sed > calls in the tree. It exists to fix Makefile.am, not .in, and aliasing .in as .am looks like a hack and using a tool wrong way. Either pathfix should be fixed to not require PATHFIX_MAKEFILEIN, or plain REINPLACE_CMD should be used in ports. > > I don't think you may use MACHINE_CPU in ports. > > Yes you can. At the moment there are 69 occurrences in the tree and I am not > aware of any policies forbidding its use. The fact that something widely used wrong way is not an argument to use it even more. It's share/mk feature not related to ports, and ports should be as little tied to share/mk features as possible (instead it may lead to inconsistent ports behavior on different major FreeBSD branches and/or dragonfly). I also though it will make a package tied to a features of machine it's built on, but it doesn't seem to be the case. I don't see any proof that it's guaranteed though.
Sigh, I can see there's no point in continuing this argument. I guess next time I'll just assign the bug to myself.
(In reply to Raphael Kubo da Costa from comment #9) > Sigh, I can see there's no point in continuing this argument. I guess next > time I'll just assign the bug to myself. That's just strange reaction when we're about to resolve either your or main delusion.
Oh, and I forgot to mention: Jason, I've removed USES=compiler:openmp because it didn't work (USES=compiler only supports single instance with single argument). Also I haven't found any mention of openmp in the package so I'm not sure it's needed at all.
Gentlemen, Disagreements aside, I appreciate all the work you both contribute to the project and the feedback on my ports. I will submit a maintainer update shortly with some of these suggestions. Regards, JB
BROKEN appears to be ignored by my Poudriere installation. Is this normal? If I add BROKEN_FreeBSD_10_amd64, I get the expected message when trying to build outside Poudriere, but even BROKEN=message has no effect in Poudriere. It attempts to build anyway. MACHINE_CPU is not going to work in this case. The problem is not whether the underlying CPU supports SSE, but that the #ifdefs in the source code assume SSE means amd64 and vice versa. Regards, JB
(In reply to Jason Bacon from comment #13) > BROKEN appears to be ignored by my Poudriere installation. Is this normal? Afaik, `testport' always ignores BROKEN. `bulk' does honor it though.