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

Mon Oct 26 08:08:35 UTC 2020

On Mon, Oct 26, 2020 at 02:05:28AM -0600, Scott Long wrote:
> 
> > On Oct 26, 2020, at 1:50 AM, Baptiste Daroussin <bapt at FreeBSD.org> wrote:
> > 
> > On Mon, Oct 26, 2020 at 12:11:56AM -0600, Warner Losh wrote:
> >> On Mon, Oct 26, 2020 at 12:01 AM Alex Kozlov <ak at freebsd.org> wrote:
> >> 
> >>> On Sun, Oct 25, 2020 at 11:37:34AM +0100, Stefan Esser wrote:
> >>>> Am 25.10.20 um 06:56 schrieb Alex Kozlov:
> >>>>> On Sat, Oct 24, 2020 at 04:37:45PM +0200, Stefan Esser wrote:
> >>>>>> Am 24.10.20 um 09:48 schrieb Alex Kozlov:
> >>>> [...]
> >>>>>>> You are hardcoding assumption that LOCALBASE = /usr/local. Please
> >>> make it
> >>>>>>> overridable with LOCALBASE environment variable.
> >>>>>> This was a trivial change to get us going with calendars provided by
> >>>>>> a port (which has not been committed, yet - therefore there are no
> >>>>>> port-provided calendars, neither under /usr/local nor under any other
> >>>>>> PREFIX, as of now).
> >>>>> 
> >>>>>> I understand what you are asking for, but in such a case I'd rather
> >>>>>> think you want to rebuild FreeBSD with _PATH_LOCALBASE modified in
> >>>>>> paths.h.
> >>>>> The PREFIX != LOCALBASE and both != /usr/local configurations
> >>>>> are supported in the ports tree and the base for a long time, please
> >>> see
> >>>>> 
> >>> https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/porting-prefix.html
> >>>> 
> >>>> Yes, and I do not need to look that up in the handbook, having been
> >>>> a ports committer for 2 decades by now.
> >>>> 
> >>>>> If after this commit you need to rebuild base to use non-default
> >>> LOCALBASE/PREFIX
> >>>>> it is pretty big regression and POLA.
> >>>> 
> >>>> How is that any different than before?
> >>>> 
> >>>> What I did is make the PATH easier to change when you rebuild base.
> >>>> 
> >>>> There are numerous programs in base that contain the literal string
> >>>> /usr/local - and what I did was implement a mechanism that allows
> >>>> to replace this literal reference with a simple change in paths.h.
> >>>> 
> >>>> If you do not modify paths.h for a different LOCALBASE, then you'll
> >>>> get a wrong _PATH_DEFPATH compiled into your binaries, for example.
> >>>> 
> >>>>>> And I have made this a single instance that needs to be changed.
> >>>>>> Before my change there were 2 instances of /usr/local hard-coded
> >>>>>> in _PATH_DEFPATH - now you have to only change the definition of
> >>>>>> _PATH_LOCALBASE to adjust all 3 locations that use it.
> >>>>> I think you made situation worse, there were two stray hardcoded
> >>>>> string and now there is official LOCALBASE define which likely will be
> >>>>> used by other people in the future.
> >>>> 
> >>>> I'd hope so to get rid of many of the 1713 literal uses of /usr/local
> >>>> in our source tree.
> >>>> 
> >>>>>> If you can show me precedence of a LOCALBASE environment variable
> >>>>>> being used in the way you suggest, I'd be willing to make calendar
> >>>>>> use it.
> >>>>> Just an analogy from LOCALBASE make variable, perhaps CALENDAR_HOME
> >>>>> is a better name.
> >>>> 
> >>>> Yes, I already suggested CALENDAR_HOME, but as an environment variable
> >>>> to check, if you want to be able to path an additional directory (or
> >>>> search path) to the calendar program at run-time. But why introduce
> >>>> a CALENDAR_HOME macro in the sources, if the port supplied calendar
> >>>> files are known to be found at LOCALBASE/share/calendar (for some value
> >>>> of LOCALBASE).
> >>>> 
> >>>> I want to make more programs that currently hard-code /usr/local use
> >>>> _PATH_LOCALBASE instead. This C macro can then be default to /usr/local
> >>>> but can be overridden by passing LOCALBASE to the compiler (from the
> >>>> build infrastructure) when paths.h is included.
> >>>> 
> >>>> Instead of referring to _PATH_LOCALBASE these files could directly use
> >>>> LOCALBASE, but since other paths are defined as _PATH_xxx in paths.h I
> >>>> think it is best to follow this precedent.
> >>>> 
> >>>>>> But then I think a CALENDAR_HOME variable would be even more useful,
> >>>>>> since it would allow to search an additional user selected directory
> >>>>>> (and not just share/calendar within what you provide as LOCALBASE).
> >>>> 
> >>>> My change did not add any dependency on LOCALBASE to any previously
> >>>> existing functionality. It added support for calendar files provided
> >>>> by a port (a feature that did not exist before) at a location that is
> >>>> correct for the big majority of users (who do not modify LOCALBASE).
> >>>> 
> >>>> As I said: I'm going to make it easier to build the base system with
> >>>> a different LOCALBASE, but not by run-time checking an environment
> >>>> variable that specifies LOCALBASE in each affected program.
> >>> It seems that you intend to follow through no matter what. So, just for
> >>> the record, I think that hardcoding LOCALBASE and requiring base rebuild
> >>> to change it is a very wrong approach.
> >>> 
> >> 
> >> So, first off, it's already hard coded. Stefan's changes change the hard
> >> coding from 'impossible to change' to 'changeable with a recompile' which
> >> is an improvement. It might even wind up as a build variable (or not, doing
> >> that has some really ugly, nasty dependencies).
> >> 
> >> But even in ports-land, it's a compile time constant. Quite a large number
> >> of ports will allow you to change it at compile / build time, but not
> >> after. You have to rebuild if you want to change PREFIX...
> >> 
> >> So I'm a bit puzzled what makes this the wrong approach?
> >> 
> > 
> > I think what Alex revents to is the following:
> > 
> > Some utilities in base base either have a configurable way to look for things in
> > localbase (via configuration entries for instances):
> > - syslog
> > - periodic
> > - rc
> > - man
> > Some have hardcoded LOCALBASE but only after looking first at the LOCALBASE env
> > var:
> > - usr.sbin/pkg
> > - mailwrapper
> > 
> > which means with a prebuilt base I can still rebuild all my packages with a
> > different localbase and it will work with only a few configurations changes.
> > which imho is a good target.
> > 
> > The list of tools which hardcodes /usr/local
> > - calendar
> > - fortune
> > - cron
> > - bsnmp
> > - nvmecontrol
> > - cpucontrol (at least can be workaround via -d option)
> > 
> > 
> 
> It would be pretty trivial to add a new libc function, something like getlocaldir.2,
> that took care of searching the environment and the invoking a fallback to the
> compile-time default from path.h.  I’ll see if I can come up with something for
> review before I fall asleep.

Exactly what I was thinking about ;)

could also be a simple static inline function somewhere (path.h?) if we don't
want to "pollute" libc.

I am fine with both.

Best regards,
Bapt
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20201026/301abe25/attachment.sig>