* On 2024-10-04 at 14:21 BST, Adam wrote:
I think this should be correct:
.if ${BUILTIN_LIB_FOUND.iconv:U:tl} == yes
# Hack: allow building on Darwin without Command Line Tools.
. if ${USE_BUILTIN.iconv:U:tl} == yes && ${OPSYS} == "Darwin" && !exists(/usr/include/iconv.h)
CONFIGURE_ARGS+= --with-iconv=shared,${OSX_SDK_PATH:Q}/${BUILDLINK_PREFIX.iconv}
. else
CONFIGURE_ARGS+= --with-iconv=shared,${BUILDLINK_PREFIX.iconv}
. endif
.else
CONFIGURE_ARGS+= --with-iconv
.endif
I think that from a syntax perspective it might now be ok, but I still have a concern from a logic perspective.
Why does the behaviour change based on whether there is a builtin library, regardless of whether I am using it or not? If I'm using libiconv from pkgsrc then the configure arguments should be identical, regardless of whether there is a native version installed or not. At best it's confusing, at worst it might lead to different behaviour.
It should be totally irrelevant what the state of a native implementation is if I am using the pkgsrc implementation.
I also have a concern that by passing a path directly to OSX_SDK_PATH you'll end up with references to it in the final package, rather than letting it be found by the usual methods (COMPILER_INCLUDE_DIRS etc), especially after I appear to have broken check-shlibs-macho.awk for the best part of a year :/
If you can confirm this works with the up-to-date checks then yes it's certainly an improvement over what currently exists as that is a syntax error stopping bulk builds from running right now, but it would be good if you or someone else could fix this correctly, or at least add very specific comments explaining why we are doing something highly unusual that may lead to undefined behaviour.
Thanks, -- Jonathan Perkin - mnx.io - pkgsrc.smartos.org Open Source Complete Cloud www.tritondatacenter.com