From: Klemens Nanni <kn@openbsd.org>
Subject: Re: archivers/py-zstandard: improve tests
To: OpenBSD ports <ports@openbsd.org>, "Kirill A. Korinsky" <kirill@korins.ky>
Date: Sun, 20 Oct 2024 15:32:00 +0000

05.10.2024 13:17, Klemens Nanni пишет:
> 05.10.2024 00:31, Kirill A. Korinsky пишет:
>> ping
>>
>> On Mon, 23 Sep 2024 20:10:02 +0200,
>> Kirill A. Korinsky <kirill@korins.ky> wrote:
>>>
>>> On Mon, 23 Sep 2024 18:08:46 +0200,
>>> Stuart Henderson <stu@spacehopper.org> wrote:
>>>>
>>>> IIRC the build/lib.openbsd-${OSREV}-... format is just for setuptools,
>>>> so it may be better to define a new variable containing the directory
>>>> name and set it as appropriate for each MODPY_PYBUILD backend.
>>>> (They don't need to be done all at once, just error out if someone
>>>> tries to use the new option and the dir isn't set - I can help with
>>>> looking for other ports which could use this later, but too busy right
>>>> now).
>>>>
>>>
>>> Thanks for review and help to track ports which needed.
>>>
>>> Here is much cleaner approach which I really like.
> 
> py-zstandard tests pass, great!
> 
>> Index: lang/python/python.port.mk
>> ===================================================================
>> RCS file: /cvs/ports/lang/python/python.port.mk,v
>> retrieving revision 1.192
>> diff -u -p -r1.192 python.port.mk
>> --- lang/python/python.port.mk	25 Jul 2024 15:42:03 -0000	1.192
>> +++ lang/python/python.port.mk	4 Oct 2024 21:30:49 -0000
>> @@ -336,6 +336,18 @@ MODPY_TEST_TARGET +=	${TEST_TARGET}
>>  .  endif
>>  .endif
>>  
>> +MODPY_TEST_LINK_SO ?=	No
> 
> This should be documented in python-module(5).
> 
>> +
>> +.if ${MODPY_TEST_LINK_SO:L} == "yes" && !empty(MODPY_TEST_LIBDIR)
>> +MODPY_TEST_TARGET := for _dir in ${MODPY_TEST_LIBDIR:S,:, ,g}; do \
>> +	if [ -e $$$${_dir} ]; then \
>> +		cd $$$${_dir} && \
>> +		find . -name '*.so' -type f \
>> +			-exec ln -sf $$$${_dir}/{} ${WRKSRC}/{} \; ;\
>> +	fi; done; ${MODPY_TEST_TARGET}
>> +.endif
> 
> Looks fine make-wise, but I lack python experience to judge the approach itself.
> 
> I think you can do without double-escaping and prepending the target variable
> by using your own and using it fist, this way it looks a clearer;  diff below.
> 
>> +
>> +
>>  # dirty way to do it with no modifications in bsd.port.mk
>>  .if empty(CONFIGURE_STYLE)
>>  .  if !target(do-build)
> 
> Since that is a default-off opt-in, I'm fine committing it and tweaking in-tree.
> 
> Feedback or OKs for either of the two diffs?

None so far,  we'll land this in a bit unless there's objection.

> 
> 
> Index: python.port.mk
> ===================================================================
> RCS file: /cvs/ports/lang/python/python.port.mk,v
> diff -u -p -r1.192 python.port.mk
> --- python.port.mk	25 Jul 2024 15:42:03 -0000	1.192
> +++ python.port.mk	5 Oct 2024 10:16:58 -0000
> @@ -303,6 +303,18 @@ MODPY_COMPILEALL = ${MODPY_BIN} -m compi
>  MODPY_COMPILEALL = ${MODPY_BIN} -m compileall -j ${MAKE_JOBS} -s ${WRKINST} -o 0 -o 1
>  .endif
>  
> +MODPY_TEST_TARGET =
> +MODPY_TEST_LINK_SO ?=	No
> +.if ${MODPY_TEST_LINK_SO:L} == "yes" && !empty(MODPY_TEST_LIBDIR)
> +MODPY_TEST_SO_CMD = for _dir in ${MODPY_TEST_LIBDIR:S,:, ,g}; do \
> +	if [ -e $${_dir} ]; then \
> +		cd $${_dir} && \
> +		find . -name '*.so' -type f \
> +			-exec ln -sf $${_dir}/{} ${WRKSRC}/{} \; ;\
> +	fi; done
> +MODPY_TEST_TARGET +=	${MODPY_TEST_SO_CMD};
> +.endif
> +
>  .if ${MODPY_PYBUILD:L} != no
>  .  if ! ${MODPY_PYBUILD:Msetuptools_scm}
>  _MODPY_PRE_BUILD_STEPS += ; if [ -e ${WRKSRC}/pyproject.toml ]; then \
> @@ -317,7 +329,7 @@ MODPY_BUILD_TARGET = ${_MODPY_PRE_BUILD_
>  MODPY_INSTALL_TARGET = \
>  	${INSTALL_DATA_DIR} ${WRKINST}${MODPY_LIBDIR}; \
>  	${_MODPY_RUNBIN} -m installer -d ${WRKINST} ${WRKSRC}/dist/*.whl
> -MODPY_TEST_TARGET =	${MODPY_TEST_CMD}
> +MODPY_TEST_TARGET +=	${MODPY_TEST_CMD}
>  .  if ${MODPY_PYTEST:L} == "yes"
>  MODPY_TEST_TARGET +=	${MODPY_PYTEST_ARGS}
>  .  endif
> @@ -328,7 +340,7 @@ MODPY_INSTALL_TARGET = \
>  	${MODPY_CMD} ${MODPY_DISTUTILS_BUILD} ${MODPY_DISTUTILS_BUILDARGS} \
>  		${MODPY_DISTUTILS_INSTALL} ${MODPY_DISTUTILS_INSTALLARGS}
>  
> -MODPY_TEST_TARGET =	${MODPY_TEST_CMD}
> +MODPY_TEST_TARGET +=	${MODPY_TEST_CMD}
>  .  if ${MODPY_PYTEST:L} == "yes"
>  MODPY_TEST_TARGET +=	${MODPY_PYTEST_ARGS}
>  .  elif ${MODPY_SETUPUTILS:L} == "yes"
>