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:


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