Re: htslib: Use pkgsrc install macros instead of mkdir

[Jason Bacon <bacon4000%gmail.com@localhost>]

Thanks for taking the time to examine this so carefully.  I've taken 
notes on all these comments and updated my checklist for reviewing a 
package.  Responses embedded below.

On 05/23/17 09:54, Thomas Klausner wrote:
> On Fri, May 19, 2017 at 10:54:35AM -0500, Jason Bacon wrote:
> > I had to add one more patch to htslib to solve this problem:
> >
> > https://sourceforge.net/p/samtools/mailman/message/31883152/
> >
> > They're using cpp pasting to simulate polymorphic functions. NetBSD was
> > expanding uintXX_t to __unintXX_t in one macro, but not others.  This patch
> > brings them all into agreement.
> Please add a comment to the patch.
Done.
> > With that, samtools should be ready for review now as well.
> files/pkg-message.in looks like a leftover from the FreeBSD port;
> also, it does not seem to apply to pkgsrc, so it can be removed.
Done.
> I suggest making perl a runtime dependency:
>
> USE_TOOLS+=... perl:run
Done.
> About the patches:
>
> "=" assignments in Makefiles can be overridden by
> passing the appropriate values on the make command line with
> MAKE_FLAGS, e.g.
>
> MAKE_FLAGS+=	CC=${CC}
>
> then you don't need the parts that change '=' to '?='.
This I was aware of, but the Makefile requires many other patches anyway 
and I prefer to fix everything in one place.
> -ALL_LIBS     = -lz -ldl $(LIBS)
> +ALL_LIBS     = -lz $(LIBS)
>
> libdl is probably still needed on Linux and some other operating
> systems. Look at mk/dlopen.buildlink3.mk.
I can verify that it's not required for CentOS or NetBSD.  Would you 
suggest leaving it in anyway?
> +m4_include([ax_with_htslib.m4])
> +m4_include([ax_with_curses.m4])
>
> If these are not needed for finding the appropriate libraries, you can
> skip that parts. It is something upstream should fix, but in pkgsrc
> we'll make sure the dependencies are installed anyway.
Configure fails without these patches.  1.4 has been released, but I 
want to commit 1.3 due to API compatibility issues I mentioned before.  
I don't intend to send any 1.3 patches upstream since development has 
ceased on that branch.  I will be working on a 1.4.x package as well and 
intend to send all those patches upstream.
> TODO seems to be fixed (see above).
Removed.

Thanks again for the feedback.

   JB

--
Earth is a beta site.