On Fri, Oct 24, 2025 at 10:14:05AM +0200, Rafael Sadowski wrote:
> On Fri Oct 24, 2025 at 09:38:47AM +0200, Theo Buehler wrote:
> > On Fri, Oct 24, 2025 at 07:11:51AM +0200, Rafael Sadowski wrote:
> > > Before we commit cmake4, I would like to have cmake.mk reviewed
> > > and committed separately.
> > > 
> > > 1. I would prefer not to have CMAKE_AUTOGEN_PARALLEL at the
> > >    beginning, all by itself, before the actual configure arguments
> > >    are set.
> > 
> > Sure.
> > 
> > > 2. Nothing uses "samurai". If anyone wants to use it, they can set
> > >    USE_NINJA=No and CONFIGURE_ARGS += -DCMAKE_MAKE_PROGRAM=samu
> > >    themselves. I don't want that in the module.
> > > 
> > > 2.1 Once "samurai" is gone, we can write the first if-else
> > >     cleanly. If Ninja otherwise ‘Unix Makefiles’ The second large
> > >     block is the one without else. This also makes it easier to read.
> > 
> > I don't really understand this. I don't see a massive improvement in
> > cleanliness or readability coming from dropping one .elif, one || and
> > two additional lines. I think a single port using samurai will have
> > to deal with more complexity than that.
> > 
> > Then again I don't feel strongly about having to support samu right now
> > and would also be fine with adding this logic back when samu is needed.
> > 
> 
> Thanks for the feedback. Another idea would be to combine the two
> if-else blocks into one:

The reason there are two blocks right now is the difference in bdeps for
"yes" and "samurai". As I said, I can live with dropping samurai support
right now but let's not make it harder to add it back than it needs to be.
I would suggest we move forward with the first diff (fwiw, I'm ok with
that if no one else objects soon).

> 
> 
> Index: cmake.port.mk
> ===================================================================
> RCS file: /cvs/ports/devel/cmake/cmake.port.mk,v
> diff -u -p -r1.86 cmake.port.mk
> --- cmake.port.mk	20 Feb 2025 15:28:27 -0000	1.86
> +++ cmake.port.mk	24 Oct 2025 08:11:39 -0000
> @@ -13,20 +13,10 @@ CONFIGURE_ENV += MODCMAKE_USE_SHARED_LIB
>  MAKE_ENV += MODCMAKE_USE_SHARED_LIBS=yes
>  .endif
>  
> -# Limit the number of moc/uic processes started by cmake_autogen
> -# (default: number of CPUs on the system)
> -CONFIGURE_ARGS += -DCMAKE_AUTOGEN_PARALLEL=${MAKE_JOBS}
> -
>  USE_NINJA ?= Yes
>  
>  .if ${USE_NINJA:L} == "yes"
>  BUILD_DEPENDS += devel/ninja
> -.elif ${USE_NINJA:L} == "samurai"
> -BUILD_DEPENDS += devel/samurai
> -CONFIGURE_ARGS += -DCMAKE_MAKE_PROGRAM=samu
> -.endif
> -
> -.if ${USE_NINJA:L} == "yes" || ${USE_NINJA:L} == "samurai"
>  _MODCMAKE_GEN = Ninja
>  MODCMAKE_BUILD_TARGET = cd ${WRKBUILD} && exec ${SETENV} ${MAKE_ENV} \
>  	cmake --build ${WRKBUILD} ${_MAKE_VERBOSE} -j ${MAKE_JOBS}
> @@ -63,7 +53,7 @@ do-test:
>  	@${MODCMAKE_TEST_TARGET}
>  .endif
>  
> -.else
> +.else # USE_NINJA
>  _MODCMAKE_GEN = "Unix Makefiles"
>  # XXX cmake include parser is bogus
>  DPB_PROPERTIES += nojunk
> @@ -116,6 +106,10 @@ CONFIGURE_ENV +=	MODTK_VERSION=${MODTK_V
>  			MODTK_LIBDIR=${MODTK_LIBDIR} \
>  			MODTK_LIB=${MODTK_LIB}
>  .endif
> +
> +# Limit the number of moc/uic processes started by cmake_autogen
> +# (default: number of CPUs on the system)
> +CONFIGURE_ARGS += -DCMAKE_AUTOGEN_PARALLEL=${MAKE_JOBS}
>  
>  .if ! empty(MODCMAKE_LDFLAGS)
>  # https://cmake.org/cmake/help/latest/envvar/LDFLAGS.html