From owner-svn-src-head@freebsd.org Mon Oct 26 07:51:31 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 0783C43D5B7; Mon, 26 Oct 2020 07:51:31 +0000 (UTC) (envelope-from bapt@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CKRqQ6CQLz3bwb; Mon, 26 Oct 2020 07:51:30 +0000 (UTC) (envelope-from bapt@FreeBSD.org) Received: from ivaldir.etoilebsd.net (etoilebsd.net [178.32.217.76]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) (Authenticated sender: bapt) by smtp.freebsd.org (Postfix) with ESMTPSA id 8CA2C20530; Mon, 26 Oct 2020 07:51:30 +0000 (UTC) (envelope-from bapt@FreeBSD.org) Received: by ivaldir.etoilebsd.net (Postfix, from userid 1001) id B6C6933BA0; Mon, 26 Oct 2020 08:50:57 +0100 (CET) Date: Mon, 26 Oct 2020 08:50:57 +0100 From: Baptiste Daroussin To: Warner Losh Cc: Alex Kozlov , Stefan Esser , src-committers , svn-src-all , svn-src-head Subject: Re: svn commit: r366962 - in head: include usr.bin/calendar Message-ID: <20201026075057.rbiwxbinzpkhh2tp@ivaldir.net> 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> <20201026060038.GA78455@ravenloft.kiev.ua> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="te5rghvxt5kons5s" Content-Disposition: inline In-Reply-To: X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Oct 2020 07:51:31 -0000 --te5rghvxt5kons5s Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Oct 26, 2020 at 12:11:56AM -0600, Warner Losh wrote: > On Mon, Oct 26, 2020 at 12:01 AM Alex Kozlov wrote: >=20 > > 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 =3D /usr/local. Pl= ease > > 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 o= ther > > > > > PREFIX, as of now). > > > > > > > > > I understand what you are asking for, but in such a case I'd rath= er > > > > > think you want to rebuild FreeBSD with _PATH_LOCALBASE modified in > > > > > paths.h. > > > > The PREFIX !=3D LOCALBASE and both !=3D /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/port= ing-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 val= ue > > > 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/loc= al > > > 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 usef= ul, > > > > > since it would allow to search an additional user selected direct= ory > > > > > (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. > > >=20 > 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, doi= ng > that has some really ugly, nasty dependencies). >=20 > 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... >=20 > So I'm a bit puzzled what makes this the wrong approach? >=20 I think what Alex revents to is the following: Some utilities in base base either have a configurable way to look for thin= gs 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) Bapt --te5rghvxt5kons5s Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEgOTj3suS2urGXVU3Y4mL3PG3PloFAl+Wf9oACgkQY4mL3PG3 PlpTORAArCx5TliZ1kjVu9A90Dsb2nj/MncYEgeFlji/dcBycAHy4lBFBnoFWGno Mn4TA4ZQTNTvyEyko0slrsE12T7h/pp+9026Grf9A3yLy39TixKPhBU3FEQbcVkx 8zDpo+iC0LxMnQqHNDOu8oLrFhCRzttr5kHV51wK/n/RCVY/jtLynfoVNMXrOeFe rZmKS/sg1wT+rVtgTU3jEz+N2xCepma0ffKLv8j7iCMJh+fpMqgBdpBSIgQB9MpO Z5qWkCsIAS4BGyZCg7O20WJW/bea9M8bB2uBweo4n8sH1RmQdv/YKttImfAVdnsX k7UwsNkaP0dY62vYUTmOulCr2/Ij+TI7Hy2NmgRYi8MsSemPytCcqlzQGz1e6rCz bJ+LetObwvFuC0nRlNg2Xy1luZIiGGhF7h9GjoU25PRHurS5zyS98CLjk+NLI0gV txvT5zeP97CnP3m7ViPkggUknedzP9ErdywADVuMBz4YjvM+Ck3g8KPpI4YqaGnr tXZW35iu+w9KMgOqQgASTXMOS1SM7OkTL7n+5QrZHR89kF8O9+DzjqY6FV1lAluV g5L4VgD8OoBmJEFlE/VPYniefqBz63rsF47MXXO+oDgLuuNPGTe5ipciYP96NtHd miVhrcteWi/oI3sOqCUXUJVGgIcuOB4pGq5Tk8vtqxbM8KdCN5g= =taSX -----END PGP SIGNATURE----- --te5rghvxt5kons5s--