Re: git: 0c3627f44d49 - main - bsdinstall avoid subdir depending on parent
Date: Fri, 21 Apr 2023 05:14:19 UTC
On 21 Apr 2023, at 06:01, Simon J. Gerraty <sjg@FreeBSD.org> wrote: > > The branch main has been updated by sjg: > > URL: https://cgit.FreeBSD.org/src/commit/?id=0c3627f44d49b460d5b9156145dec9d4a91beb2c > > 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 > > bsdinstall avoid subdir depending on parent > > 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. > > In usr.sbin/bsdinstall use SUBDIR_DEPEND_ so tree walking still works. > > 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(-) > > 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?= FreeBSD > SUBDIR= distextract distfetch partedit runconsoles scripts > SUBDIR_PARALLEL= > +SUBDIR_DEPEND_distfetch = distextract > +SUBDIR_DEPEND_partedit = distextract > SCRIPTS= bsdinstall > MAN= bsdinstall.8 > PACKAGE= bsdinstall > -GENHDRS= opt_osname.h > -SRCS+= ${GENHDRS} > -CLEANFILES+= ${GENHDRS} > > SCRIPTS+= startbsdinstall > SCRIPTSDIR_startbsdinstall= ${LIBEXECDIR}/bsdinstall > > -opt_osname.h: .PHONY > - if ! grep -q "^#define OSNAME \"${OSNAME}\"$"" ${.TARGET}; then \ > - echo "#define OSNAME \"${OSNAME}\"" > ${.TARGET}; \ > - fi > > .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 @@ > > BINDIR= ${LIBEXECDIR}/bsdinstall > PROG= distextract > -CFLAGS+= -I${SRCTOP}/contrib/bsddialog/lib -I${.OBJDIR}/.. > +CFLAGS+= -I${SRCTOP}/contrib/bsddialog/lib -I. > LIBADD= archive bsddialog m > +SRCS= distextract.c > > MAN= > +GENHDRS= opt_osname.h > +SRCS+= ${GENHDRS} > +CLEANFILES+= ${GENHDRS} > + > +opt_osname.h: .PHONY > + if ! grep -q "^#define OSNAME \"${OSNAME}\"$"" ${.TARGET}; then \ > + echo "#define OSNAME \"${OSNAME}\"" > ${.TARGET}; \ > + fi > > .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 @@ > > BINDIR= ${LIBEXECDIR}/bsdinstall > PROG= distfetch > -CFLAGS+= -I${SRCTOP}/contrib/bsddialog/lib -I${.OBJDIR}/.. > +CFLAGS+= -I${SRCTOP}/contrib/bsddialog/lib -I${.OBJDIR}/../distextract > LIBADD= fetch bsddialog > > MAN= > 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= partedit > LINKS= ${BINDIR}/partedit ${BINDIR}/autopart \ > ${BINDIR}/partedit ${BINDIR}/scriptedpart > SYMLINKS= ../libexec/bsdinstall/partedit /usr/sbin/sade > -CFLAGS+= -I${SRCTOP}/contrib/bsddialog/lib -I${.OBJDIR}/.. > +CFLAGS+= -I${SRCTOP}/contrib/bsddialog/lib -I${.OBJDIR}/../distextract Surely this is a sign that this is a worse solution? The header isn’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 “is bad”, according to your commit message, but so is this, and I would argue it’s worse. Or go put it in its own common directory. Jess > LIBADD+= geom util bsddialog > > PARTEDIT_ARCH= ${MACHINE}