On Thu, Jan 30, 2025 at 07:14:11AM +0100, Sebastien Marie wrote:
> Theo Buehler <t...@theobuehler.org> writes:
> 
> > I think sthen is right that a variable for this in cargo.port.mk would
> > make sense to obviate the need to keep these in sync. On rust updates,
> > all rust ports are rebulit anyway, so if rust switches to llvm 19 that
> > would be picked up automatically.
> 
> I didn't precisly followed the problem that a variable in cargo.port.mk
> should solve.

Right.

I think we're dealing with two problems and I'm no longer convinced it's
modcargo's job to solve either of them:

The first problem is one of usability and more related to cargo build
scripts than ports: porters run into the verbose cargo output and fail
to spot the important bit amidst all the other unreadable noise.
I think that is a problem for the rust and cargo developers to solve
and I don't think the cargo module can really help.

The second problem is more directly related to the two spotify ports and
concerns the questions you ask as well:

> is it just to have clang-sys crate to have MAKE_ENV +=
> LIBCLANG_PATH=/usr/local/llvm18/lib + BUILD_DEPENDS ?
>
> and what is important ? llvm from ports (base vs ports version, whatever
> the llvm version is) ? exact llvm version used in rustc ?

For ncspot and spotify-player the reason that clang-sys is built is
bindgen, which is required by aws-lc-sys everywhere except on the
popular platform rectangle {linux, mac, win} x {amd64, arm64}.

Right now the libclang API version required by bindgen is clang_6_0
and it is only used to generate ffi bindings and the, the precise
version of llvm we pull in does not really matter.

However, I think we can improve the situation in the two spotify ports.
We can use the ports default clang version, which is much more likely to
be present already. The ncspot binary produced with the diff below has
nobtcfi set and has text in a load with R E, so I would expect it to
work.

I am not sure the cargo module can easily figure out whether aws-lc-sys
or bindgen are actually used during the build and I'm not convinced it
should set or override these variables. That seems to depend a lot on
the specific port and should probably handled in the port's makefile.

Index: Makefile
===================================================================
RCS file: /cvs/ports/audio/ncspot/Makefile,v
diff -u -p -r1.51 Makefile
--- Makefile    28 Jan 2025 19:15:26 -0000      1.51
+++ Makefile    30 Jan 2025 14:26:03 -0000
@@ -13,6 +13,7 @@ COMMENT =             ncurses Spotify client
 GH_ACCOUNT =           hrkfdn
 GH_PROJECT =           ncspot
 GH_TAGNAME =           v1.2.1
+REVISION =             0
 
 CATEGORIES =           audio
 
@@ -21,13 +22,15 @@ PERMIT_PACKAGE =    Yes
 
 WANTLIB +=             ${MODCARGO_WANTLIB} crypto curses dbus-1 m portaudio ssl
 
-MODULES =              devel/cargo
+MODULES =              devel/cargo \
+                       lang/clang
+
 LIB_DEPENDS=           audio/portaudio-svn \
                        x11/dbus,-main
 
-# same branch as MODCLANG_VERSION used by lang/rust
-BUILD_DEPENDS =                devel/llvm/18
-MAKE_ENV =             LIBCLANG_PATH=/usr/local/llvm18/lib
+# libclang needed for bindgen used from aws-lc-sys
+MAKE_ENV =             LIBCLANG_PATH=/usr/local/llvm${MODCLANG_VERSION}/lib
+MODCLANG_COMPILER_LINKS = no
 
 SEPARATE_BUILD =       Yes
 NO_TEST =              Yes

Reply via email to