Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 21 Apr 2023 06:14:19 +0100
From:      Jessica Clarke <jrtc27@freebsd.org>
To:        "Simon J. Gerraty" <sjg@FreeBSD.org>
Cc:        "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org>
Subject:   Re: git: 0c3627f44d49 - main - bsdinstall avoid subdir depending on parent
Message-ID:  <09DDC25F-63F8-440A-A674-31F190C087B4@freebsd.org>
In-Reply-To: <202304210501.33L51PBT011707@gitrepo.freebsd.org>
References:  <202304210501.33L51PBT011707@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 21 Apr 2023, at 06:01, Simon J. Gerraty <sjg@FreeBSD.org> wrote:
>=20
> The branch main has been updated by sjg:
>=20
> URL: =
https://cgit.FreeBSD.org/src/commit/?id=3D0c3627f44d49b460d5b9156145dec9d4=
a91beb2c
>=20
> commit 0c3627f44d49b460d5b9156145dec9d4a91beb2c
> Author:     Simon J. Gerraty <sjg@FreeBSD.org>
> AuthorDate: 2023-04-21 05:00:40 +0000
> Commit:     Simon J. Gerraty <sjg@FreeBSD.org>
> CommitDate: 2023-04-21 05:00:40 +0000
>=20
>    bsdinstall avoid subdir depending on parent
>=20
>    When not doing tree walks, it is bad for sub-dirs to depend on
>    parents.  Move the generation of opt_osname.h to distextract
>    and have others that need that depend on it.
>=20
>    In usr.sbin/bsdinstall use SUBDIR_DEPEND_ so tree walking still =
works.
>=20
>    Reviewed by:    obrien
>    Differential Revision:  https://reviews.freebsd.org/D39742
> ---
> usr.sbin/bsdinstall/Makefile             |  9 ++-------
> usr.sbin/bsdinstall/distextract/Makefile | 11 ++++++++++-
> usr.sbin/bsdinstall/distfetch/Makefile   |  2 +-
> usr.sbin/bsdinstall/partedit/Makefile    |  2 +-
> 4 files changed, 14 insertions(+), 10 deletions(-)
>=20
> diff --git a/usr.sbin/bsdinstall/Makefile =
b/usr.sbin/bsdinstall/Makefile
> index e71cae726536..aaa006694222 100644
> --- a/usr.sbin/bsdinstall/Makefile
> +++ b/usr.sbin/bsdinstall/Makefile
> @@ -3,19 +3,14 @@
> OSNAME?=3D	FreeBSD
> SUBDIR=3D	distextract distfetch partedit runconsoles scripts
> SUBDIR_PARALLEL=3D
> +SUBDIR_DEPEND_distfetch =3D distextract
> +SUBDIR_DEPEND_partedit =3D distextract
> SCRIPTS=3D bsdinstall
> MAN=3D bsdinstall.8
> PACKAGE=3D	bsdinstall
> -GENHDRS=3D	opt_osname.h
> -SRCS+=3D		${GENHDRS}
> -CLEANFILES+=3D	${GENHDRS}
>=20
> SCRIPTS+=3D	startbsdinstall
> SCRIPTSDIR_startbsdinstall=3D	${LIBEXECDIR}/bsdinstall
>=20
> -opt_osname.h: .PHONY
> -	if ! grep -q "^#define OSNAME \"${OSNAME}\"$"" ${.TARGET}; then =
\
> -		echo "#define OSNAME \"${OSNAME}\"" > ${.TARGET}; \
> -	fi
>=20
> .include <bsd.prog.mk>
> diff --git a/usr.sbin/bsdinstall/distextract/Makefile =
b/usr.sbin/bsdinstall/distextract/Makefile
> index 6ae9bb65e8fb..0292c01e78f4 100644
> --- a/usr.sbin/bsdinstall/distextract/Makefile
> +++ b/usr.sbin/bsdinstall/distextract/Makefile
> @@ -2,9 +2,18 @@
>=20
> BINDIR=3D ${LIBEXECDIR}/bsdinstall
> PROG=3D	distextract
> -CFLAGS+=3D -I${SRCTOP}/contrib/bsddialog/lib -I${.OBJDIR}/..
> +CFLAGS+=3D -I${SRCTOP}/contrib/bsddialog/lib -I.
> LIBADD=3D	archive bsddialog m
> +SRCS=3D	distextract.c
>=20
> MAN=3D
> +GENHDRS=3D	opt_osname.h
> +SRCS+=3D		${GENHDRS}
> +CLEANFILES+=3D	${GENHDRS}
> +
> +opt_osname.h: .PHONY
> +	if ! grep -q "^#define OSNAME \"${OSNAME}\"$"" ${.TARGET}; then =
\
> +		echo "#define OSNAME \"${OSNAME}\"" > ${.TARGET}; \
> +	fi
>=20
> .include <bsd.prog.mk>
> diff --git a/usr.sbin/bsdinstall/distfetch/Makefile =
b/usr.sbin/bsdinstall/distfetch/Makefile
> index 0104df0e3aec..1555719dd15d 100644
> --- a/usr.sbin/bsdinstall/distfetch/Makefile
> +++ b/usr.sbin/bsdinstall/distfetch/Makefile
> @@ -2,7 +2,7 @@
>=20
> BINDIR=3D ${LIBEXECDIR}/bsdinstall
> PROG=3D	distfetch
> -CFLAGS+=3D	-I${SRCTOP}/contrib/bsddialog/lib -I${.OBJDIR}/..
> +CFLAGS+=3D	-I${SRCTOP}/contrib/bsddialog/lib =
-I${.OBJDIR}/../distextract
> LIBADD=3D	fetch bsddialog
>=20
> MAN=3D
> diff --git a/usr.sbin/bsdinstall/partedit/Makefile =
b/usr.sbin/bsdinstall/partedit/Makefile
> index 96c4ddb53961..df17028eab2a 100644
> --- a/usr.sbin/bsdinstall/partedit/Makefile
> +++ b/usr.sbin/bsdinstall/partedit/Makefile
> @@ -5,7 +5,7 @@ PROG=3D	partedit
> LINKS=3D ${BINDIR}/partedit ${BINDIR}/autopart \
>        ${BINDIR}/partedit ${BINDIR}/scriptedpart
> SYMLINKS=3D ../libexec/bsdinstall/partedit /usr/sbin/sade
> -CFLAGS+=3D	-I${SRCTOP}/contrib/bsddialog/lib -I${.OBJDIR}/..
> +CFLAGS+=3D	-I${SRCTOP}/contrib/bsddialog/lib =
-I${.OBJDIR}/../distextract

Surely this is a sign that this is a worse solution? The header isn=E2=80=99=
t a
part of distextract any more than partedit, so this is entirely
arbitrary. It also blocks the ability to do the subdirectories in
parallel with each other.

I would much rather this reverted; this feels like a regression to me,
with the only justification being that it =E2=80=9Cis bad=E2=80=9D, =
according to your
commit message, but so is this, and I would argue it=E2=80=99s worse.

Or go put it in its own common directory.

Jess

> LIBADD+=3D	geom util bsddialog
>=20
> PARTEDIT_ARCH=3D ${MACHINE}




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?09DDC25F-63F8-440A-A674-31F190C087B4>