Sunday, December 31, 2023

Re: lang/rust: roadmap for using SYSTEM_VERSION

On Sat, Dec 30, 2023 at 07:32:18PM +0100, Sebastien Marie wrote:
> Hi,
>
> We need a way to bump all ports built using lang/rust when the port is
> updated (compiler change or rust stdlib changes).
>
> Currently, lang/go is using SYSTEM_VERSION-go variable for that, so I
> intent to copy the mecanism for rustc.
>
> I can't simply use devel/cargo module for that: some (usually complexes)
> ports aren't using it but are using rustc and rust stdlib. So I would
> like to add a lang/rust module for this purpose, and move the bits
> specific to rustc from devel/cargo to this new lang/rust module (mostly
> the WANTLIB definition for code using rust stdlib).
>
> We have 77 ports with lang/rust inside BUILD_DEPENDS. So modifying all
> of them is doable but require a bit of preparation.
>
> I intent to do the following:
>
> 1. add the lang/rust module and add the arch-defines.mk bits for
> SYSTEM_VERSION-rust
>
> nothing is using it at this stage. no changes.
>
>
> 2. modify all the ports (one by one) using lang/rust as compiler to use
> the module lang/rust.
>
> as soon MODULES += lang/rust is added, the port will be automatically
> bumped (due to SYSTEM_VERSION-rust), which is fine (I modified the rust
> stdlib on 30.12.2023, and all ports would need a bump anyway).
>
> I will do the conversion from using MODCARGO_WANTLIB to MODRUST_WANTLIB
> in the port at the same time.
>
> During the step, both "new way" and "old way" will coexist and shouldn't
> conflict. A port will be either "new way" or "old way".
>
>
> 3. modify the devel/cargo module to remove unused bits
> (MODCARGO_WANTLIB), and add some checks for ensuring using devel/cargo
> implies using also lang/rust.
>
> It is mostly a cleaning step, and to ensure a hard fail if the "old way"
> is used (missing changes, and new code added using "old way").
>
>
> Diffs for 1 and 3 are below. Step 2 could be done on the fly with
> MAINTAINER in Cc for the more complex cases.
>
> Any comments or OK ?

The outlined procedure makes sense to me and I like the approach, but I
am a bit worried that it makes it harder for people to write Rust ports
using the devel/cargo module, which is already tricky.

Porters will now need to be able to grasp the separation of concerns to
understand the distinction between the devel/cargo and lang/rust
modules. They must now deal with two sets of variables: MODCARGO_* and
MODRUST_*. The latter only has two user-visible bits for now, so maybe
it's not that bad.

A small nit below. At the end is a small diff for port-modules which
you can tweak and commit once we decide to go ahead with your plan.


> --
> Sebastien Marie
>
>
> Step 1 diffs:
>
> diff /home/semarie/repos/openbsd/ports/mystuff/lang/rust
> commit - 14500989167797cabee408b40583056fe24a9f23
> path + /home/semarie/repos/openbsd/ports/mystuff/lang/rust
> blob - /dev/null
> file + rust.port.mk (mode 640)
> --- /dev/null
> +++ rust.port.mk
> @@ -0,0 +1,30 @@
> +# increment after rust compiler update to trigger updates of
> +# all compiled rust packages (see arch-defines.mk)
> +_MODRUST_SYSTEM_VERSION = 1
> +
> +CATEGORIES += lang/rust
> +
> +# WANTLIB for Rust compiled code
> +# it should be kept in sync with lang/rust code
> +# - c/pthread : all syscalls
> +# - c++abi / libgcc.a : unwind
> +MODRUST_WANTLIB += c pthread
> +
> +.if "${MACHINE_ARCH}" != "sparc64"
> +MODRUST_WANTLIB += c++abi
> +.else
> +# libgcc.a is static
> +MODRUST_WANTLIB +=
> +.endif
> +
> +CHECK_LIB_DEPENDS_ARGS += -S MODRUST_WANTLIB="${MODRUST_WANTLIB}"
> +CHECK_LIB_DEPENDS_ARGS += -F c++abi
> +
> +MODRUST_BUILDDEP ?= Yes
> +.if ${MODRUST_BUILDDEP:L} == "yes"
> +BUILD_DEPENDS += lang/rust
> +.endif
> +
> +# Location of rustc/rustdoc binaries
> +MODRUST_RUSTC_BIN = ${LOCALBASE}/bin/rustc
> +MODRUST_RUSTDOC_BIN = ${LOCALBASE}/bin/rustdoc
>
>
> diff /home/semarie/repos/openbsd/ports
> commit - a1995e6a715404d542f5d69eadb9a9bac7bbca61
> path + /home/semarie/repos/openbsd/ports
> blob - dacb59716e3724cd0aad0110c42d6b2c1f672bfb
> file + infrastructure/mk/arch-defines.mk
> --- infrastructure/mk/arch-defines.mk
> +++ infrastructure/mk/arch-defines.mk
> @@ -105,6 +105,12 @@ _SYSTEM_VERSION-clang = 2
> _SYSTEM_VERSION-go = ${_MODGO_SYSTEM_VERSION}
> .endif
>
> +# defined in rust.port.mk; added to version for all rust arches so that
> +# rust-compiled packages can be updated easily for a new rust compiler
> +.if defined(MODULES) && ${MODULES:Mlang/rust}
> +_SYSTEM_VERSION-rust = ${_MODRUST_SYSTEM_VERSION}
> +.endif
> +
> # @version = ${_SYSTEM_VERSION} + ${_SYSTEM_VERSION-${MACHINE_ARCH}}
> _PKG_ARGS_VERSION += -V ${_SYSTEM_VERSION} -V ${_SYSTEM_VERSION-${MACHINE_ARCH}}
> .if ${ARCH} != ${MACHINE_ARCH}
>
>
>
> Step 3 diff:
> diff /home/semarie/repos/openbsd/ports
> commit - a1995e6a715404d542f5d69eadb9a9bac7bbca61
> path + /home/semarie/repos/openbsd/ports
> blob - 4c5723bf509e5aaaf1541b76acd3b48119bb5b7c
> file + devel/cargo/cargo.port.mk
> --- devel/cargo/cargo.port.mk
> +++ devel/cargo/cargo.port.mk
> @@ -1,4 +1,8 @@
> -CATEGORIES += lang/rust
> +# we can't just add lang/rust to MODULES
> +# it makes _SYSTEM_VERSION-rust (in arch-defines.mk) not properly defined
> +.if defined(MODULES) && ! ${MODULES:Mlang/rust}
> +ERRORS += "devel/cargo module needs also lang/rust in MODULES"

this should be 'also needs'

ERRORS += "devel/cargo requires lang/rust to be set in MODULES"

> +.endif
>
> # List of static dependencies. The format is cratename-version.
> # MODCARGO_CRATES will be downloaded from SITES_CRATESIO.
> @@ -23,22 +27,6 @@ MODCARGO_VENDOR_DIR ?= ${WRKSRC}/modcargo-crates
> # Default path for cargo manifest.
> MODCARGO_CARGOTOML ?= ${WRKSRC}/Cargo.toml
>
> -# WANTLIB for Rust compiled code
> -# it should be kept in sync with lang/rust code
> -# - c/pthread : all syscalls
> -# - c++abi / libgcc.a : unwind
> -MODCARGO_WANTLIB = c pthread
> -
> -.if "${MARCHINE_ARCH}" != "sparc64"
> -MODCARGO_WANTLIB += c++abi
> -.else
> -# libgcc.a is static
> -MODCARGO_WANTLIB +=
> -.endif
> -
> -CHECK_LIB_DEPENDS_ARGS += -S MODCARGO_WANTLIB="${MODCARGO_WANTLIB}"
> -CHECK_LIB_DEPENDS_ARGS += -F c++abi
> -
> # Define SITES_CRATESIO for crates.io
> SITES.cargo = https://crates.io/api/v1/crates/
>
> @@ -274,16 +262,9 @@ MODCARGO_configure += ;
> .endif
>
> # Build dependencies.
> -MODCARGO_BUILD_DEPENDS = lang/rust
> -
> # devel/cargo-generate-vendor is mandatory for hooks.
> BUILD_DEPENDS += devel/cargo-generate-vendor
>
> -MODCARGO_BUILDDEP ?= Yes
> -.if ${MODCARGO_BUILDDEP:L} == "yes"
> -BUILD_DEPENDS += ${MODCARGO_BUILD_DEPENDS}
> -.endif
> -
> # Location of cargo binary (default to devel/cargo binary)
> MODCARGO_CARGO_BIN ?= ${LOCALBASE}/bin/cargo
>
> @@ -305,8 +286,8 @@ MODCARGO_ENV += \
> CARGO_BUILD_JOBS=${MAKE_JOBS} \
> CARGO_TARGET_DIR=${MODCARGO_TARGET_DIR} \
> RUST_BACKTRACE=full \
> - RUSTC=${LOCALBASE}/bin/rustc \
> - RUSTDOC=${LOCALBASE}/bin/rustdoc \
> + RUSTC=${MODRUST_RUSTC_BIN} \
> + RUSTDOC=${MODRUST_RUSTDOC_BIN} \
> RUSTFLAGS="${MODCARGO_RUSTFLAGS}"
>
> # Helper to shorten cargo calls.
>

Index: port-modules.5
===================================================================
RCS file: /cvs/src/share/man/man5/port-modules.5,v
diff -u -p -r1.266 port-modules.5
--- port-modules.5 14 Sep 2023 03:53:26 -0000 1.266
+++ port-modules.5 31 Dec 2023 08:33:48 -0000
@@ -978,6 +978,20 @@ See
.It lang/ruby
See
.Xr ruby-module 5 .
+.It lang/rust
+Ports using Rust must use this module so a rebuild can be triggered via
+.Ev SYSTEM_VERSION-rust
+on updates of the lang/rust port or changes to the Rust standard library.
+Sets
+.Ev MODRUST_WANTLIB
+as appropriate for the architecture so it can be added to
+.Ev WANTLIB .
+It adds lang/rust to the
+.Ev BUILD_DEPENDS
+unless
+.Ev MODRUST_BUILDDEP
+is set to anything but
+.Dq yes .
.It lang/tcl
Sets
.Ev MODTCL_VERSION ,

No comments:

Post a Comment