Re: HEADS-UP: devel/cmake update - needs bulk tests

2022-05-25 Thread Stuart Henderson
On 2022/05/24 22:26, Theo Buehler wrote:
> > Nobody? New synced diff below.
> 
> How can we move this forward? I think the diff is ready to go in. If no
> bulk builders of !amd64 and !lld arches run tests, I suppose we'll have
> to sort out the fallout the hard way.

I think at this point just commit it. OK




Re: HEADS-UP: devel/cmake update - needs bulk tests

2022-05-24 Thread Theo Buehler
> Nobody? New synced diff below.

How can we move this forward? I think the diff is ready to go in. If no
bulk builders of !amd64 and !lld arches run tests, I suppose we'll have
to sort out the fallout the hard way.



Re: HEADS-UP: devel/cmake update - needs bulk tests

2022-04-27 Thread Theo Buehler
Three more with fixes. audio/mscore will conflict with what you already
have.

audio/chromaprint:

FAILED: tests/all_tests
[...]
ld: error: unable to find library -lgtest

audio/mscore:

FAILED: main/mscore
[...]
ld: error: unable to find library -lfreetype

graphics/digikam:

FAILED: core/app/libdigikamcore.so.4.0
[...]
ld.lld: error: unable to find library -llqr-1
ld.lld: error: unable to find library -lglib-2.0
ld.lld: error: unable to find library -lintl

Index: audio/chromaprint/Makefile
===
RCS file: /cvs/ports/audio/chromaprint/Makefile,v
retrieving revision 1.15
diff -u -p -r1.15 Makefile
--- audio/chromaprint/Makefile  11 Mar 2022 18:20:06 -  1.15
+++ audio/chromaprint/Makefile  27 Apr 2022 09:22:00 -
@@ -30,6 +30,8 @@ LIB_DEPENDS = graphics/ffmpeg
 CONFIGURE_ARGS =   -DBUILD_TOOLS=ON \
-DBUILD_TESTS=ON
 
+MODCMAKE_LDFLAGS +=-L${LOCALBASE}/lib
+
 TEST_TARGET =  check
 
 pre-configure:
Index: audio/mscore/Makefile
===
RCS file: /cvs/ports/audio/mscore/Makefile,v
retrieving revision 1.59
diff -u -p -r1.59 Makefile
--- audio/mscore/Makefile   1 Apr 2022 17:18:15 -   1.59
+++ audio/mscore/Makefile   27 Apr 2022 09:31:58 -
@@ -41,7 +41,6 @@ RUN_DEPENDS = devel/desktop-file-utils \
x11/gtk+3,-guic
 
 CONFIGURE_ARGS =-DCMAKE_INSTALL_PREFIX="${PREFIX}" \
-   -DCMAKE_CXX_FLAGS="${CXXFLAGS} 
-Wno-inconsistent-missing-override" \
-DBUILD_ALSA=OFF \
-DBUILD_JACK=OFF \
-DBUILD_PORTMIDI=OFF \
@@ -52,6 +51,9 @@ CONFIGURE_ARGS =-DCMAKE_INSTALL_PREFIX="
-DBUILD_CRASH_REPORTER=OFF \
-DDOWNLOAD_SOUNDFONT=OFF \
-DBUILD_TELEMETRY_MODULE=OFF
+
+CXXFLAGS +=-Wno-inconsistent-missing-override
+MODCMAKE_LDFLAGS = -L${LOCALBASE}/lib -L${X11BASE}/lib
 
 .if ${MACHINE_ARCH} == "mips64" || ${MACHINE_ARCH} == "mips64el"
 CXXFLAGS +=-mxgot
Index: graphics/digikam/Makefile
===
RCS file: /cvs/ports/graphics/digikam/Makefile,v
retrieving revision 1.81
diff -u -p -r1.81 Makefile
--- graphics/digikam/Makefile   16 Mar 2022 20:31:58 -  1.81
+++ graphics/digikam/Makefile   27 Apr 2022 07:14:46 -
@@ -112,6 +112,8 @@ WRKDIST =   ${WRKDIR}/${PKGNAME}
 CONFIGURE_ARGS +=  -DCMAKE_DISABLE_FIND_PACKAGE_Doxygen=ON \
-DENABLE_QWEBENGINE=ON
 
+MODCMAKE_LDFLAGS +=-L${LOCALBASE}/lib
+
 .if ${FLAVOR:Mmysql}
 BUILD_DEPENDS +=   databases/mariadb
 RUN_DEPENDS += databases/mariadb



Re: HEADS-UP: devel/cmake update - needs bulk tests

2022-04-26 Thread Theo Buehler
First victim was x11/qt6/qtbase

ld: error: undefined symbol: backtrace

It's just a typo:

> diff --git a/x11/qt6/qtbase/Makefile b/x11/qt6/qtbase/Makefile
> index decd207f5d0..15fc859c01f 100644
> --- a/x11/qt6/qtbase/Makefile
> +++ b/x11/qt6/qtbase/Makefile
> @@ -105,7 +105,7 @@ CONFIGURE_ARGS += -DCMAKE_INSTALL_PREFIX=${PREFIX} \
>   -DQT_FEATURE_openssl_linked=ON \
>   -DQT_FEATURE_dtls=OFF
>  
> -LDFLAGS +=   -lexecinfo
> +MOCMAKE_LDFLAGS +=   -lexecinfo
 ^
missing D in MODCMAKE_LDFLAGS



Re: HEADS-UP: devel/cmake update - needs bulk tests

2022-04-24 Thread Rafael Sadowski
On Sun Apr 24, 2022 at 09:41:35PM +0100, Stuart Henderson wrote:
> On 2022/04/24 17:27, Rafael Sadowski wrote:
> > On Sun Apr 24, 2022 at 12:46:35PM +0100, Stuart Henderson wrote:
> 
> > New reworked diff:
> 
> I will try and get an i386 test bulk started in the next few days
> with whatever diff is current for this at the time.
> 

Thanks but wait a bit, a better one is coming soon.



Re: HEADS-UP: devel/cmake update - needs bulk tests

2022-04-24 Thread Stuart Henderson
On 2022/04/24 17:27, Rafael Sadowski wrote:
> On Sun Apr 24, 2022 at 12:46:35PM +0100, Stuart Henderson wrote:
> > On 2022/04/24 12:33, Rafael Sadowski wrote:
> > > - CFLAGS, CXXFLAGS, LDFLAGS
> > > cmake.port.mk now respects C/CXX/LD flags. With is CMAKE_*_FLAGS_* is 
> > > obsolete.
> > 
> > I am 100% happy with doing this for CFLAGS/CXXFLAGS. Those are supported
> > by other ports modules (autoconf, etc) and already partly supported by
> > cmake module?.
> 
> +1
> 
> > 
> > LDFLAGS not so much. Nothing else in ports infrastructure supports
> > this. It is used in some ports but that's as an individual thing
> > within those ports. It is not something that the user can set in the
> > same way they can override CFLAGS/CXXFLAGS during a build.
> > 
> > (Not saying it would be a bad thing to support it more widely, but having
> > it honoured automatically in cmake but not in say meson and autoconf
> > seems like asking for trouble).
> 
> 
> I understand!
> How else would we pass it? By CONFIGURE_ENV = LDFLAGS="-L${LOCALBASE}/lib"?
> x11/gnome sets it by MODGNOME_LDFLAGS and then in CONFIGURE_ENV.

Something like MODCMAKE_LDFLAGS would make a lot more sense to me.
If cmake.port.mk doesn't have a default value for this (unlike gnome.port.mk
which automatically sets -L) then I would only set LDFLAGS iff MODCMAKE_LDFLAGS
is set (i.e. avoid setting LDFLAGS= just leave it unset in that case).

> I would prefer CONFIGURE_ENV = LDFLAGS="-L${LOCALBASE}/lib" in cmake. No
> hard opinions here.

I'm in two minds about this :)

> > Also this gets rid of the distinction betweed CMAKE_SHARED_LINKER_FLAGS,
> > CMAKE_MODULE_LINKER_FLAGS, CMAKE_EXE_LINKER_FLAGS - is that a good idea?
> 
> If you do that, you have already lost as a porter. Only upstream can do this.
> IMHO that was always an anti-pattern in the ports tree.

Ah I was missing that the added -L flags were due to changes in cmake
3.23 - I was trying to make sense of why you needed to add them and one
possible reason I thought was that maybe setting LDFLAGS is causing
cmake to override existing config in the other *_LINKER_FLAGS variables.

> New reworked diff:

I will try and get an i386 test bulk started in the next few days
with whatever diff is current for this at the time.



Re: HEADS-UP: devel/cmake update - needs bulk tests

2022-04-24 Thread Stuart Henderson
On 2022/04/24 12:33, Rafael Sadowski wrote:
> - CFLAGS, CXXFLAGS, LDFLAGS
> cmake.port.mk now respects C/CXX/LD flags. With is CMAKE_*_FLAGS_* is 
> obsolete.

I am 100% happy with doing this for CFLAGS/CXXFLAGS. Those are supported
by other ports modules (autoconf, etc) and already partly supported by
cmake module?.

LDFLAGS not so much. Nothing else in ports infrastructure supports
this. It is used in some ports but that's as an individual thing
within those ports. It is not something that the user can set in the
same way they can override CFLAGS/CXXFLAGS during a build.

(Not saying it would be a bad thing to support it more widely, but having
it honoured automatically in cmake but not in say meson and autoconf
seems like asking for trouble).

Also this gets rid of the distinction betweed CMAKE_SHARED_LINKER_FLAGS,
CMAKE_MODULE_LINKER_FLAGS, CMAKE_EXE_LINKER_FLAGS - is that a good idea?

> --- a/cad/graywolf/Makefile
> +++ b/cad/graywolf/Makefile
> @@ -21,7 +21,7 @@ MODULES =   devel/cmake
>  LIB_DEPENDS =devel/gsl
>  
>  # Really really not C99
> -CONFIGURE_ARGS = -DCMAKE_C_FLAGS="${CFLAGS} -Wno-return-type"
> +CFLAGS +=-Wno-return-type"

stray "

> --- a/converters/opencc/Makefile
> +++ b/converters/opencc/Makefile
> @@ -27,8 +27,9 @@ MODULES =   devel/cmake \
>  
>  LIB_DEPENDS =textproc/libmarisa,-main
>  
> -CONFIGURE_ARGS = -DUSE_SYSTEM_MARISA=ON \
> - -DCMAKE_CXX_FLAGS="-I${LOCALBASE}/include 
> -L${LOCALBASE}/lib"
> +CONFIGURE_ARGS = -DUSE_SYSTEM_MARISA=ON
> +
> +CXXFLAGS +=  -I${LOCALBASE}/include -I${X11BASE}/include
>  
>  MODPY_RUNDEP =   No

this loses setting -L

>  Source/cmComputeLinkInformation.cxx.orig
> -+++ Source/cmComputeLinkInformation.cxx

this definitely wants checking with ld.bfd as well as LLD

> diff --git a/emulators/ppsspp/Makefile b/emulators/ppsspp/Makefile
> index ba22f00f61a..9280479005a 100644
> --- a/emulators/ppsspp/Makefile
> +++ b/emulators/ppsspp/Makefile
> @@ -58,14 +58,14 @@ LIB_DEPENDS = archivers/snappy \
>   graphics/png
>  COMPILER =   base-clang ports-gcc
>  
> -CONFIGURE_ARGS = -DCMAKE_CXX_FLAGS="-I${X11BASE}/include" \
> - -DCMAKE_CXX_FLAGS="${CXXFLAGS}" \
> - -DUSE_SYSTEM_FFMPEG=ON \
> +CONFIGURE_ARGS = -DUSE_SYSTEM_FFMPEG=ON \
>   -DUSE_SYSTEM_LIBZIP=ON \
>   -DUSE_SYSTEM_SNAPPY=ON \
>   -DUSE_DISCORD=OFF \
>   -DUSING_EGL=OFF
>  
> +CXXFLAGS +=  -I${X11BASE}/include

I wonder what the original with two separate -DCMAKE_CXX_FLAGS ended
up actually doing.

> --- a/games/gemrb/Makefile
> +++ b/games/gemrb/Makefile
> @@ -37,12 +37,12 @@ RUN_DEPENDS = devel/desktop-file-utils \
>  CONFIGURE_ARGS +=-DSYSCONF_DIR=${SYSCONFDIR}/gemrb \
>   -DEXAMPLE_CONF_DIR=${PREFIX}/share/examples/gemrb/ \
>   -DMAN_DIR=${PREFIX}/man/man6 \
> - -DCMAKE_MODULE_LINKER_FLAGS=-lc \
> - -DCMAKE_CXX_FLAGS="${CXXFLAGS}" \
>   -DDISABLE_WERROR=1 \
>   -DSDL_BACKEND=SDL \
>   -Wno-dev
>  
> +LDFLAGS +=   -lc

ugh to the "-lc"

> index c0302100fcd..0b260d888be 100644
> --- a/games/gzdoom/Makefile
> +++ b/games/gzdoom/Makefile
> @@ -32,7 +32,8 @@ MODULES =   devel/cmake
>  CONFIGURE_ARGS +=-DNO_GTK=ON
>  CONFIGURE_ARGS +=-DDYN_OPENAL=NO
>  CONFIGURE_ARGS +=-DHAVE_STDINT_H=1
> -CONFIGURE_ARGS +=-DCMAKE_CXX_FLAGS="${CXXFLAGS} -I. 
> -I${LOCALBASE}/include"
> +
> +CXXFLAGS +=  -I${LOCALBASE}/include
>  
>  LIB_DEPENDS =audio/openal \
>   audio/zmusic \

loses "-I."

> diff --git a/games/irrlamb/Makefile b/games/irrlamb/Makefile
> index 24a7de825dc..837f5834946 100644
> --- a/games/irrlamb/Makefile
> +++ b/games/irrlamb/Makefile
> @@ -26,6 +26,8 @@ RUN_DEPENDS =   devel/desktop-file-utils
>  
>  WRKDIST =${WRKDIR}/irrlamb-$Vr613
>  
> +LDFLAGS +=   -L${X11BASE}/lib -L${LOCALBASE}/lib
> +
>  NO_TEST =Yes
>  
>  .if ${MACHINE_ARCH} == "amd64" || ${MACHINE_ARCH} == "i386"

these weren't needed before, why are they now? same/similar in most
of the others in games/ (some gain both X11BASE and LOCALBASE, some
gain one or the other, openmw changes X11BASE to LOCALBASE)
and in geo/lib2geom, graphics/gdcm, actually so many others that
I stopped keeping notes.

Seems this change is causing it to override existing paths set inside
the ports CMakeFiles themselves?

> diff --git a/graphics/ksnip/Makefile b/graphics/ksnip/Makefile
> index 8b4c5fe1cf6..4fe09d135bf 100644
> --- a/graphics/ksnip/Makefile
> +++ b/graphics/ksnip/Makefile
> @@ -26,7 +26,7 @@ LIB_DEPENDS =   graphics/kcolorpicker \
>   graphics/kimageannotator>=0.4.1 \
>   x11/qt5/qtx11extras
>  
>