DaehwanKimLab / tophat

Spliced read mapper for RNA-Seq
http://ccb.jhu.edu/software/tophat
Boost Software License 1.0
91 stars 46 forks source link

Problem building w/ automake@1.16.1, with patch #53

Closed hartzell closed 6 years ago

hartzell commented 6 years ago

The Spack package manager has a recipe for Tophat. It has been working. Recently Spack's default automake was upgraded from 1.15.1 to 1.16.1 and Tophat's build broke.

A spack user reported the problem, his issue has additional details.

I have a patch that changes a few things in src/Makefile.am, the patched tree builds with both automake@1.15.1 and automake@1.16.1. Details and etc... are included in my Spack PR.

I'd appreciate any feedback. If the changes seem reasonable I can PR them here too/instead.

Thanks

gpertea commented 6 years ago

Seems to not affect the actual software functionality in any way -- and admittedly that Makefile.am was always a pain and a mess to begin with. If Daehwan has no objections I'd welcome any cleanup/simplification of the build system that still allows the creation/installation of the same binaries and programs, as long as it's not adding any other dependencies and still works on older Linux distributions (e.g. RHEL 6). The patch seems very reasonable -- it would indeed be easier for me to check the changes if you PR them here, so please do so. As you can tell we largely abandoned this legacy software (using Hisat2 instead, hope Spack provides that too) and I see there are a few other pending PRs here that I could integrate with this occasion -- hopefully there are no conflicts between those and your patch.

infphilo commented 6 years ago

@hartzell, thank you for your suggestion. We'd be happy to incorporate it into TopHat.

hartzell commented 6 years ago

I've submitted a PR. I've used the patch within the Spack system but haven't tried to wire up the auto* and etc... bit outside of Spack. I figure you have that in place already and can test it better than I can.

hartzell commented 6 years ago

@gpertea -- Yep, Spack has a package for Hisat2.

Looking at it I'm not sure why she added the top level directory (prefix) onto PATH in addition to the normal prefix/bin, but no one has complained, so....

Here's the PR that merged it in: https://github.com/spack/spack/pull/5488