svn commit: r366962 - in head: include usr.bin/calendar

[Warner Losh]

On Sun, Oct 25, 2020, 9:04 AM Kyle Evans <kevans at freebsd.org> wrote:

> On Sun, Oct 25, 2020 at 5:21 AM Stefan Esser <se at freebsd.org> wrote:
> >
> > Am 25.10.20 um 04:46 schrieb Rodney W. Grimes:
> > >> On Sat, Oct 24, 2020 at 8:51 PM Rodney W. Grimes <
> freebsd at gndrsh.dnsmgr.net>
> > >> wrote:
> > >>>> +#define      _PATH_LOCALBASE "/usr/local"
> > >>>> +
> > >>>
> > >>> Something feels very wrong about this becoming a  defined path in
> base,
> > >>> it is further dependence on /usr/local which in the early days we
> spent
> > >>> a great deal of time removing.
> > >>>
> > >>> I believe the whole ports system allows this to be something other
> > >>> than /usr/local.  Package should also allow it to be some other
> place.
> > >>>
> > >>
> > >> This removes a couple of instances of /usr/local being hardcoded and
> > >> replaces with a define, so net it's better.
> > >
> > > No, its net worse as it now creates a define that is highly likely
> > > to propogate adding additional dependencies on this value.
> > >
> > >>
> > >> It could be even better, but this is slightly better than it was
> before.
> > >
> > > I disagree, as it is now easier for additional contamination of
> > > the base system.
> >
> > There already are places that hard-code /usr/local, and I do agree
> > that this is architecturally bad, if you want to keep the base system
> > and ports as independent from each other as possible.
> >
> > But I do disagree that this was worse than before, and I'd even consider
> > replacing other verbatim occurrences of /usr/local with _PATH_LOCALBASE
> > in our sources (but not introduce new references to LOCALBASE in base).
> >
> > This would simplify a grep for such source files, for example, and also
> > to build base for systems with modified LOCALBASE.
> >
> > The following C header files in base (ignoring contrib) contain the
> > string /usr/local:
> >
> > ...
> > sys/contrib/openzfs/include/sys/lua/luaconf.h (FreeBSD specific?)
> > ...
>
> I see that you've excluded sys/contrib from the initial review, but I
> would not bother with luaconf.h in particular. These definitions just
> come from a stock Lua 5.2 and are not used in a ZFS context, they've
> ripped out loadlib and anything else that could try.
>

As an aside: I looked at using this Lua for the boot loader, but quickly
found that it was no good for that purpose due to changes like that. And it
was too far hacked from 5.2 to try to update easily...

Warner

Thanks,
>
> Kyle Evans
>