[PATCH] pkg_add -m "OS/ARCH VER"

To: tech-pkg%NetBSD.org@localhost
Subject: [PATCH] pkg_add -m "OS/ARCH VER"
From: Taylor R Campbell <campbell+netbsd-tech-pkg%mumble.net@localhost>
Date: Sat, 13 Jan 2024 20:20:36 +0000

Currently the `pkg_add -m "ARCH"' option specifies a machine
architecture ARCH, like aarch64.

We use this when cross-building packages so pkgsrc can run the native
pkg_add to install cross-built packages into the destdir, overriding
the output of `uname -m' as the architecture pkg_add checks for.

The attached patch series:

1. Extends the syntax to `pkg_add -m "OS/ARCH VER"', that is,
   "${OPSYS}/${MACHINE_ARCH} ${OS_VERSION}" syntax. overriding
   pkg_add's additional checks of the OS and OS version.

   => The old syntax is still accepted, and as far as I know, no
      existing values of MACHINE_ARCH have `/' in them.  So this
      shouldn't affect any existing use cases.

2. Uses the new syntax when installing cross-built packages.

   => This does not affect native builds, because the existing logic
      to pass `-m' is already gated on USE_CROSS_COMPILE, and this
      patch series just changes logic under USE_CROSS_COMPILE.

3. Bumps the pkg_install version requitement when USE_CROSS_COMPILE is
   set.

   => This does not affect native builds, because we only bump the
      requirement when USE_CROSS_COMPILE is set.

This is a step toward enabling cross-OS builds, not just
cross-architecture builds.

Comments?


Side note: Currently the patch sets the pkg_install version to
20230629, which was the date I drafted it.  I plan to update this
before I actually commit.

From 0b5e0bca24399f67c536576ae9d6e10ec0c83f16 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Thu, 11 Jan 2024 23:42:19 +0000
Subject: [PATCH 1/2] pkg_install-20230629: Extend `-m' syntax to allow OPSYS
 too.

- If there's no slash `/', take it all as ${MACHINE_ARCH}.

- If there is a slash, then split it by `/' and ` ' into:

        ${OPSYS}/${MACHINE_ARCH} ${OPSYS_VERSION}

  For example:

        NetBSD/aarch64 10.0

All the variables are restricted to lie in a safe set [a-zA-Z0-9._-],
so the notation can be extended later.

No change to existing syntax (no MACHINE_ARCH has `/' in it, or
anything outside the safe set), and `-m' is generally only used with
cross builds anyway, so this shouldn't break existing cross builds
and should have no impact on native builds.
---
 pkgtools/pkg_install/files/add/Makefile.in   |  2 +-
 pkgtools/pkg_install/files/add/add.h         |  4 +
 pkgtools/pkg_install/files/add/main.c        |  5 +-
 pkgtools/pkg_install/files/add/parse_cross.c | 97 ++++++++++++++++++++
 pkgtools/pkg_install/files/add/perform.c     | 35 ++++---
 pkgtools/pkg_install/files/lib/version.h     |  2 +-
 6 files changed, 129 insertions(+), 16 deletions(-)
 create mode 100644 pkgtools/pkg_install/files/add/parse_cross.c

diff --git a/pkgtools/pkg_install/files/add/Makefile.in b/pkgtools/pkg_install/files/add/Makefile.in
index 1bd11c421778..614dbaf5fdf6 100644
--- a/pkgtools/pkg_install/files/add/Makefile.in
+++ b/pkgtools/pkg_install/files/add/Makefile.in
@@ -37,7 +37,7 @@ INSTALL=	@INSTALL@
 
 PROG=		pkg_add
 
-OBJS=	main.o perform.o
+OBJS=	main.o parse_cross.o perform.o
 
 all: $(PROG)
 
diff --git a/pkgtools/pkg_install/files/add/add.h b/pkgtools/pkg_install/files/add/add.h
index a55f40bb580f..e483f20e2c99 100644
--- a/pkgtools/pkg_install/files/add/add.h
+++ b/pkgtools/pkg_install/files/add/add.h
@@ -27,6 +27,8 @@
 
 extern char *Destdir;
 extern char *OverrideMachine;
+extern char *OverrideOpsys;
+extern char *OverrideOSVersion;
 extern char *Prefix;
 extern char *View;
 extern char *Viewbase;
@@ -42,6 +44,8 @@ extern int ReplaceSame;
 extern Boolean ForceDepends;
 extern Boolean ForceDepending;
 
+void	parse_cross(const char *, char **, char **, char **);
+
 int     make_hierarchy(char *);
 void    apply_perms(char *, char **, int);
 
diff --git a/pkgtools/pkg_install/files/add/main.c b/pkgtools/pkg_install/files/add/main.c
index bec606923320..3c95260f3150 100644
--- a/pkgtools/pkg_install/files/add/main.c
+++ b/pkgtools/pkg_install/files/add/main.c
@@ -40,6 +40,8 @@ static char Options[] = "AC:DIK:P:RVfhm:np:t:Uuv";
 
 char   *Destdir = NULL;
 char   *OverrideMachine = NULL;
+char   *OverrideOpsys = NULL;
+char   *OverrideOSVersion = NULL;
 char   *Prefix = NULL;
 Boolean NoInstall = FALSE;
 Boolean NoRecord = FALSE;
@@ -110,7 +112,8 @@ main(int argc, char **argv)
 			break;
 
 		case 'm':
-			OverrideMachine = optarg;
+			parse_cross(optarg, &OverrideMachine, &OverrideOpsys,
+			    &OverrideOSVersion);
 			break;
 
 		case 'n':
diff --git a/pkgtools/pkg_install/files/add/parse_cross.c b/pkgtools/pkg_install/files/add/parse_cross.c
new file mode 100644
index 000000000000..1026ade8b4ba
--- /dev/null
+++ b/pkgtools/pkg_install/files/add/parse_cross.c
@@ -0,0 +1,97 @@
+/*	$NetBSD$	*/
+
+#if HAVE_CONFIG_H
+#include "config.h"
+#endif
+#include <nbcompat.h>
+#if HAVE_SYS_CDEFS_H
+#include <sys/cdefs.h>
+#endif
+__RCSID("$NetBSD$");
+
+#include "lib.h"
+#include "add.h"
+
+/*
+ * ${OPSYS}/${MACHINE_ARCH} ${OS_VERSION}
+ *
+ * or just
+ *
+ * ${MACHINE_ARCH}
+ */
+void
+parse_cross(const char *text, char **machine_arch, char **opsys,
+    char **os_version)
+{
+	static const char safeset[] = /* XXX */
+	    "abcdefghijklmnopqrstuvwxyz"
+	    "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+	    "0123456789"
+	    "-._";
+	char *copy = xstrdup(text);
+	char *p = copy, *q, *r;
+
+	/*
+	 * If there's no /, treat it as a single MACHINE_ARCH.
+	 */
+	if (*(q = strchr(p, '/')) == '\0') {
+		*machine_arch = copy;
+		*opsys = NULL;
+		*os_version = NULL;
+	} else {
+		/*
+		 * NUL-terminate at the slash so p := text[0..slash)
+		 * is the OPSYS.
+		 */
+		*q++ = '\0';
+
+		/*
+		 * If there's no SPC, fail.
+		 */
+		if (*(r = strchr(q, ' ')) == '\0') {
+			goto fail;
+		}
+
+		/*
+		 * NUL-terminate at the space so
+		 *
+		 *	q := text(slash..space)
+		 *
+		 * is the MACHINE_ARCH.
+		 */
+		*r++ = '\0';
+
+		/*
+		 * The rest is already NUL-terminated, so
+		 *
+		 *	r := text(space..NUL)
+		 *
+		 * is the OS_VERSION.
+		 */
+		*opsys = p;
+		*machine_arch = q;
+		*os_version = r;
+	}
+
+	/*
+	 * Verify that MACHINE_ARCH, and, if specified, OPSYS and
+	 * OS_VERSION lie within the safe set, so we can reserve large
+	 * amounts of the space of inputs for additional syntax.
+	 * Ideally we would impose more constraints here with a
+	 * regular expression to restrict the space even more, but
+	 * this'll do for now.
+	 */
+	if ((*machine_arch)[strspn(*machine_arch, safeset)] != '\0') {
+		goto fail;
+	}
+	if (*opsys != NULL && (*opsys)[strspn(*opsys, safeset)] != '\0') {
+		goto fail;
+	}
+	if (*os_version != NULL &&
+	    (*os_version)[strspn(*os_version, safeset)] != '\0') {
+		goto fail;
+	}
+	return;
+
+fail:	errx(1, "Invalid -m argument: ${OPSYS}/${MACHINE_ARCH} ${OS_VERSION}");
+}
diff --git a/pkgtools/pkg_install/files/add/perform.c b/pkgtools/pkg_install/files/add/perform.c
index 65e20c554266..fb95ded6acc6 100644
--- a/pkgtools/pkg_install/files/add/perform.c
+++ b/pkgtools/pkg_install/files/add/perform.c
@@ -893,19 +893,28 @@ check_platform(struct pkg_task *pkg)
 {
 	struct utsname host_uname;
 	const char *effective_arch;
+	const char *effective_opsys;
+	const char *effective_os_version;
 	int fatal;
 
-	if (uname(&host_uname) < 0) {
-		if (Force) {
-			warnx("uname() failed, continuing.");
-			return 0;
-		} else {
-			warnx("uname() failed, aborting.");
-			return -1;
+        if (OverrideOpsys != NULL && OverrideOSVersion != NULL) {
+		effective_opsys = OverrideOpsys;
+		effective_os_version = OverrideOSVersion;
+	} else {
+		if (uname(&host_uname) < 0) {
+			if (Force) {
+				warnx("uname() failed, continuing.");
+				return 0;
+			} else {
+				warnx("uname() failed, aborting.");
+				return -1;
+			}
 		}
-	}
 
-	normalise_platform(&host_uname);
+		normalise_platform(&host_uname);
+		effective_opsys = OPSYS_NAME;
+		effective_os_version = host_uname.release;
+	}
 
 	if (OverrideMachine != NULL)
 		effective_arch = OverrideMachine;
@@ -913,14 +922,14 @@ check_platform(struct pkg_task *pkg)
 		effective_arch = PKGSRC_MACHINE_ARCH;
 
 	/* If either the OS or arch are different, bomb */
-	if (strcmp(OPSYS_NAME, pkg->buildinfo[BI_OPSYS]) ||
+	if (strcmp(effective_opsys, pkg->buildinfo[BI_OPSYS]) ||
 	    strcmp(effective_arch, pkg->buildinfo[BI_MACHINE_ARCH]) != 0)
 		fatal = 1;
 	else
 		fatal = 0;
 
 	if (fatal ||
-	    compatible_platform(OPSYS_NAME, host_uname.release,
+	    compatible_platform(effective_opsys, effective_os_version,
 				pkg->buildinfo[BI_OS_VERSION]) != 1) {
 		warnx("Warning: package `%s' was built for a platform:",
 		    pkg->pkgname);
@@ -928,9 +937,9 @@ check_platform(struct pkg_task *pkg)
 		    pkg->buildinfo[BI_OPSYS],
 		    pkg->buildinfo[BI_MACHINE_ARCH],
 		    pkg->buildinfo[BI_OS_VERSION],
-		    OPSYS_NAME,
+		    effective_opsys,
 		    effective_arch,
-		    host_uname.release);
+		    effective_os_version);
 		if (!Force && fatal)
 			return -1;
 	}
diff --git a/pkgtools/pkg_install/files/lib/version.h b/pkgtools/pkg_install/files/lib/version.h
index 9bd3ab0a0def..52ce96b5cc61 100644
--- a/pkgtools/pkg_install/files/lib/version.h
+++ b/pkgtools/pkg_install/files/lib/version.h
@@ -27,6 +27,6 @@
 #ifndef _INST_LIB_VERSION_H_
 #define _INST_LIB_VERSION_H_
 
-#define PKGTOOLS_VERSION 20211115
+#define PKGTOOLS_VERSION 20230629
 
 #endif /* _INST_LIB_VERSION_H_ */

From 4c2c21affc3b9294fa3e440dc1df84059ba36311 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Thu, 11 Jan 2024 23:49:59 +0000
Subject: [PATCH 2/2] mk/pkgformat/pkg: Use extended `-m "OS/ARCH VER"' syntax.

Requires pkg_install>=20230629.

No change to native builds, not even the pkg_install requirement,
because this is conditional on cross builds.
---
 mk/pkgformat/pkg/package.mk        | 2 +-
 mk/pkgformat/pkg/pkgformat-vars.mk | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/mk/pkgformat/pkg/package.mk b/mk/pkgformat/pkg/package.mk
index c6be0267326a..924a82dbc499 100644
--- a/mk/pkgformat/pkg/package.mk
+++ b/mk/pkgformat/pkg/package.mk
@@ -150,7 +150,7 @@ su-real-package-install:
 	@${PHASE_MSG} "Installing binary package of "${PKGNAME:Q}
 .if !empty(USE_CROSS_COMPILE:M[yY][eE][sS])
 	@${MKDIR} ${_CROSS_DESTDIR}${PREFIX}
-	${SETENV} ${PKGTOOLS_ENV} ${PKG_ADD} -m ${MACHINE_ARCH} -I -p ${_CROSS_DESTDIR}${PREFIX} ${STAGE_PKGFILE}
+	${SETENV} ${PKGTOOLS_ENV} ${PKG_ADD} -m ${OPSYS:Q}/${MACHINE_ARCH:Q}\ ${OS_VERSION:Q} -I -p ${_CROSS_DESTDIR}${PREFIX} ${STAGE_PKGFILE}
 	@${ECHO} "Fixing recorded cwd..."
 	@${SED} -e 's|@cwd ${_CROSS_DESTDIR}|@cwd |' ${_PKG_DBDIR}/${PKGNAME:Q}/+CONTENTS > ${_PKG_DBDIR}/${PKGNAME:Q}/+CONTENTS.tmp
 	@${MV} ${_PKG_DBDIR}/${PKGNAME:Q}/+CONTENTS.tmp ${_PKG_DBDIR}/${PKGNAME:Q}/+CONTENTS
diff --git a/mk/pkgformat/pkg/pkgformat-vars.mk b/mk/pkgformat/pkg/pkgformat-vars.mk
index 19a3020c1722..226c13e4e25d 100644
--- a/mk/pkgformat/pkg/pkgformat-vars.mk
+++ b/mk/pkgformat/pkg/pkgformat-vars.mk
@@ -40,7 +40,9 @@ NATIVE_PKG_INFO_CMD?=		${NATIVE_PKG_TOOLS_BIN}/pkg_info
 NATIVE_LINKFARM_CMD?=		${NATIVE_PKG_TOOLS_BIN}/linkfarm
 
 # Latest versions of tools required for correct pkgsrc operation.
-.if !empty(USE_PKG_ADMIN_DIGEST:M[Yy][Ee][Ss])
+.if ${USE_CROSS_COMPILE:tl} == "yes"
+PKGTOOLS_REQD=		20230629
+.elif !empty(USE_PKG_ADMIN_DIGEST:M[Yy][Ee][Ss])
 PKGTOOLS_REQD=		20191008
 .else
 PKGTOOLS_REQD=		20100914

Prev by Date: llvm rebuild gets confused
Next by Date: Re: llvm rebuild gets confused
Previous by Thread: llvm rebuild gets confused
Next by Thread: [WIP PATCH] Cross-OS package builds
Indexes:

Home | Main Index | Thread Index | Old Index