Date: Mon, 26 Oct 2020 07:00:38 +0100 From: Alex Kozlov <ak@FreeBSD.org> To: Stefan Esser <se@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r366962 - in head: include usr.bin/calendar Message-ID: <20201026060038.GA78455@ravenloft.kiev.ua> In-Reply-To: <0140ae63-3044-9946-4047-c64331be0b50@freebsd.org> References: <202010230922.09N9MNZu040921@repo.freebsd.org> <20201024074840.GA26119@ravenloft.kiev.ua> <38d15142-1cb1-eb1f-215e-cee165743d99@freebsd.org> <20201025055633.GA52119@ravenloft.kiev.ua> <0140ae63-3044-9946-4047-c64331be0b50@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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. -- Alex
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20201026060038.GA78455>