On Tue, 4 Sep 2001, Ruslan Ermilov wrote:

> On Mon, Sep 03, 2001 at 09:36:46PM +1000, Bruce Evans wrote:
> > Index: Makefile
> > ===================================================================
> > RCS file: /home/ncvs/src/usr.bin/xinstall/Makefile,v
> > retrieving revision 1.15
> > diff -u -2 -r1.15 Makefile
> > --- Makefile        2 Apr 2001 11:54:59 -0000       1.15
> > +++ Makefile        3 Sep 2001 11:18:33 -0000
> > @@ -2,6 +2,9 @@
> >  # $FreeBSD: src/usr.bin/xinstall/Makefile,v 1.15 2001/04/02 11:54:59 ru Exp $
> >
> > +.PATH: ${.CURDIR}/../../lib/libc/gen
> > +
> >  PROG=              xinstall
> >  PROGNAME=  install
> > +SRCS=              strtofflags.c xinstall.c
> >  MAN=               install.1
> >
> I don't like this, as it unconditionally compiles in strtofflags.c, even if
> the host has libc support for it.  This also breaks single-module-checkout
> compilation.  How about this instead?

It shouldn't matter if libc already has strtofflags().  strtofflags is not
in the POSIX namespace and our libc and headers don't have any pollution
;-);-).  (Actually, strtofflags is in the (reserved) ISO namespace and thus
in the (reserved) POSIX namespace, especially since xinstall.c includes
<string.h> and it's already a bug that strtofflags in in <unistd.h>.)
Single-module checkout is already broken in hundreds of other places, so
I don't much mind one more (it's broken almost everywhere that uses .PATH,
in particular in all Makefiles for contrib'ed programs).

> The strtofflags() interface has been added
> to HEAD in unistd.h,v 1.36, on 2000/06/17, and
> to RELENG_4 in unistd.h,v 1.35.2.1, on 2000/07/03.
>
> This change has not been reflected by the __FreeBSD_version bump.

No problem.  That mistake is not used much outside of /sys :-).
(We've still got ifdefs to support RELENG_2_2 using it :-().

> The nearest (by date) version bumps are as follows:
>
> CURRENT, param.h,v 1.72, 500007
> RELENG_4, param.h,v 1.61.2.6: 400021
>
> Index: Makefile
> ===================================================================
> RCS file: /home/ncvs/src/usr.bin/xinstall/Makefile,v
> retrieving revision 1.15
> diff -u -r1.15 Makefile
> --- Makefile  2001/04/02 11:54:59     1.15
> +++ Makefile  2001/09/04 08:11:52
> @@ -3,6 +3,22 @@
>
>  PROG=                xinstall
>  PROGNAME=    install
> +SRCS=                xinstall.c
>  MAN=         install.1
> +
> +# Get __FreeBSD_version
> +.if !defined(OSVERSION)
> +.if exists(/sbin/sysctl)
> +OSVERSION!=  /sbin/sysctl -n kern.osreldate
> +.else
> +OSVERSION!=  /usr/sbin/sysctl -n kern.osreldate
> +.endif
> +.endif
> +
> +.if ${OSVERSION} < 400021 || \
> +    ${OSVERSION} >= 500000 && ${OSVERSION} < 500007
> +.PATH: ${.CURDIR}/../../lib/libc/gen
> +SRCS+=               strtofflags.c
> +.endif
>
>  .include <bsd.prog.mk>

Ugh.  This is even worse than using __FreeBSD_version.  It is broken
for non-FreeBSD hosts.  Hmm, this case is broken in my version too.
Non-FreeBSD hosts might not have have FreeBSD file flags, so they might
not have the definitions of the magic numbers necessary for strtofflags.c
to compile; they might not even have the necessary headers.  It is
unreasonable to expect the target source strtofflags.c to be compilable
in the host environment.  The correct fix is to not support file flags in
the bootstrap version of install.

> I also think that the ${OSVERSION} stuff should be in sys.mk.
>
> > Unfixed bugs: this will have to be fixed better before turning on WARNS.
> > strtofflags() won't be declared in the host includes if the host libraries
> > don't have it.  Similarly in mtree (where I obtained this fix from) and
> > in any other tools that use strtofflags().  All these bugs were missing in
> > the old versions that used ls's version of strtofflags.
> >
> This could be solved similarly from within xinstall.c.  I.e., we could
> prototype strtofflags() and fflagstostr() manually if __FreeBSD_version
> checks fail.

Better to leave it out.

> > Nearby bugs: mtree/Makefile uses !defined(WORLD) to avoid depending on
> > the host having libmd, but someone removed the definition of WORLD from
> > src/Makefile.inc1.
> >
> The correct fix would be:
>
> Index: Makefile
> ===================================================================
> RCS file: /home/ncvs/src/usr.sbin/mtree/Makefile,v
> retrieving revision 1.21
> diff -u -r1.21 Makefile
> --- Makefile  2001/07/20 06:20:02     1.21
> +++ Makefile  2001/09/04 08:22:19
> @@ -8,10 +8,10 @@
>  SRCS=        compare.c crc.c create.c excludes.c misc.c mtree.c spec.c verify.c \
>       strtofflags.c
>
> -.if !defined(WORLD)
> +.include <bsd.prog.mk>
> +
> +.if defined(LIBMD) && exists(${LIBMD})
>  CFLAGS+= -DMD5 -DSHA1 -DRMD160
>  DPADD=       ${LIBMD}
>  LDADD=       -lmd
>  .endif
> -
> -.include <bsd.prog.mk>

No, this only detects the presence of libmd, not of the necessary
functions.  Something like autoconfig would be needed to detect the
presence and non-brokenness of the necessary functions, but it shouldn't
be necessary to go there.

Bruce


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message

Reply via email to