Re: CVS commit: pkgsrc/devel/libusb-compat

[Leonardo Taccari <leot%NetBSD.org@localhost>]

Hello Pierre,

Pierre Pronchery writes:
> [...]
> This is a first patch, still work-in-progress, only tested with
> devel/libftdi so far. It would be greatly helpful if someone could tell
> me if this looks like the right direction.
>
> It is still missing updates to everything using
> devel/libusb/buildlink3.mk (except for devel/libftdi for my tests) and I
> will test this next.
> [...]

Thanks for working on that, some minor comments inline (but just
mostly nitpicks!).

> [...]
> Index: mk/libusb.mk
> ===================================================================
> RCS file: mk/libusb.mk
> diff -N mk/libusb.mk
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ mk/libusb.mk	28 Jan 2018 02:28:12 -0000

Instead of `libusb.mk' IMHO it would be better to rename it to
libusb.buildlink3.mk. This is more consistent with others
*.buildlink3.mk in pkgsrc/mk.

> @@ -0,0 +1,17 @@
> +# $NetBSD$
> +#
> +
> +.include "../mk/bsd.fast.prefs.mk"
> +

It is better to define MK_LIBUSB_BUILDLINK3_MK and then check for it to
avoid multiple inclusions.

> +.if defined(LIBUSB_TYPE)
> +.  if !empty(LIBUSB_TYPE:Mcompat)
> +.    include "../../devel/libusb-compat/buildlink3.mk"
> +.  elif !empty(LIBUSB_TYPE:Mnative)
> +.    include "../../devel/libusb/buildlink3.mk"
> +.  else
> +PKG_FAIL_REASON+="Unknown LIBUSB_TYPE: ${LIBUSB_TYPE:Q}"
> +.  endif
> +.else
> +WARNINGS+="[libusb.mk] LIBUSB_TYPE is not set"
> +.  include "../../devel/libusb/buildlink3.mk"
> +.endif

I think we can first check if LIBUSB_TYPE is defined and initialize it
to none if it isn't. In that way we can avoid the nested if-else.

Apart that I think we can add LIBUSB_TYPE to BUILD_DEFS list and a
comment on the top of libusb.buildlink3.mk should be added as well (and
also clarify in mk/defaults/mk.conf that we are referring to libusb
"version 0", not libusb1!).

A possible libusb.buildlink3.mk with these changes applied (but still
with a big TODO comment on the top) is attached.

# $NetBSD$
#
# TODO: add a comment/intro/rationale to this!
#

MK_LIBUSB_BUILDLINK3_MK:=    ${MK_LIBUSB_BUILDLINK3_MK}+

.include "../../mk/bsd.fast.prefs.mk"

.if !empty(MK_LIBUSB_BUILDLINK3_MK:M+)

.if !defined(LIBUSB_TYPE)
LIBUSB_TYPE=	none
.endif

BUILD_DEFS+=	LIBUSB_TYPE

.if ${LIBUSB_TYPE} == "native"
.  include "../../devel/libusb/buildlink3.mk"
.elif ${LIBUSB_TYPE} == "compat"
.  include "../../devel/libusb-compat/buildlink3.mk"
.else
PKG_FAIL_REASON+=	"[libusb.buildlink3.mk] Invalid value ${LIBUSB_TYPE} for LIBUSB_TYPE."
.endif

.endif	# MK_LIBUSB_BUILDLINK3_MK