From owner-svn-src-head@freebsd.org Mon Oct 26 06:12:10 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 1CD8E43B369 for ; Mon, 26 Oct 2020 06:12:10 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qk1-x72b.google.com (mail-qk1-x72b.google.com [IPv6:2607:f8b0:4864:20::72b]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CKPcn0MlDz3WyV for ; Mon, 26 Oct 2020 06:12:08 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qk1-x72b.google.com with SMTP id s14so7399019qkg.11 for ; Sun, 25 Oct 2020 23:12:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=6ho2WvA8ra3ZkXL05mfwNKb+Xvlu5qTCWf07nTr1Csc=; b=M2aePl4XYfzbwXmVr6+gEQDaRVlRoniWTMvF5fO2Uu5C4pslTmP2cGqIv8DMUIr11X el7MNeGqSQ1PBV0pNN1iiQOHfpZz5CrWSchoiFcb+Une6XHiF6tO3RgHf2kJX9jETpYh 8chHtxXbyDPGErIh2/aO7jkirE6nsDFWhGKpA+X61ZibEKrLXWZaZwzCGWDLME4V8+eR osz+YyezVhA2k4W/rB8amztUp0n2UrFJ6ADhUeDMb71eVf840596X6/TMOn5o8E6WKlJ afhuE05JX46jYsLqeStIGn5pzv5tPAJVofJKVx4LL3tuMRUWMn6YEzMf4fiHO9Ij4XAr k19g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=6ho2WvA8ra3ZkXL05mfwNKb+Xvlu5qTCWf07nTr1Csc=; b=mbSFDkxZl1kUBUbFN1hXE3kKsTgNO3Y07SyJJp8yOHOKkxSW011AcXTBNkBY2Lo4ks xeAwL2rLYffmXMuvdr8kHmbNet+iqHgRIjQx7+Hc76jw8hbuIGY6ZEUAt+aE5FUZwRCB cdukXKRqVCs5NNqZ3KCTSScr6OcUbFM1bxeERfZEIHqqXirBI8FuhAohPq53lyJLF9I7 8BVevw9Wrph0c8TLTJmfp3pEskk/RTrVULKWUsNuLYOdegZqXnL+WAYQ0Bar60ZlvbfB RqegIj6rm460fFctZaBLUTdsQ7xIYICWnZyqnP8IRBXMfRZy7U88ImLjg4iP493O5EjW mrwA== X-Gm-Message-State: AOAM532n7hAOKFFdehKAStKJhIoatLQ33QATbyt1Fz4I6Rem190cShIy SrrRpKEEVIcYop6+MOkVj0jS4T6PrhYtQy1JS/+tpg== X-Google-Smtp-Source: ABdhPJyNQP2kkSQd0S+gVZkSkWLQu/EVFk6MsgIqXVHGIBhZCfLp5K41ewVEMsP73YvmyRi410gcTgAYhI/375QF++U= X-Received: by 2002:a37:63c1:: with SMTP id x184mr13396539qkb.195.1603692727551; Sun, 25 Oct 2020 23:12:07 -0700 (PDT) MIME-Version: 1.0 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> In-Reply-To: <20201026060038.GA78455@ravenloft.kiev.ua> From: Warner Losh Date: Mon, 26 Oct 2020 00:11:56 -0600 Message-ID: Subject: Re: svn commit: r366962 - in head: include usr.bin/calendar To: Alex Kozlov Cc: Stefan Esser , src-committers , svn-src-all , svn-src-head X-Rspamd-Queue-Id: 4CKPcn0MlDz3WyV X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=bsdimp-com.20150623.gappssmtp.com header.s=20150623 header.b=M2aePl4X; dmarc=none; spf=none (mx1.freebsd.org: domain of wlosh@bsdimp.com has no SPF policy when checking 2607:f8b0:4864:20::72b) smtp.mailfrom=wlosh@bsdimp.com X-Spamd-Result: default: False [-3.38 / 15.00]; ARC_NA(0.00)[]; MAILMAN_DEST(0.00)[svn-src-head]; R_DKIM_ALLOW(-0.20)[bsdimp-com.20150623.gappssmtp.com:s=20150623]; NEURAL_HAM_MEDIUM(-0.92)[-0.919]; FROM_HAS_DN(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; PREVIOUSLY_DELIVERED(0.00)[svn-src-head@freebsd.org]; DMARC_NA(0.00)[bsdimp.com]; RCPT_COUNT_FIVE(0.00)[5]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[bsdimp-com.20150623.gappssmtp.com:+]; NEURAL_HAM_SHORT(-1.46)[-1.461]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::72b:from]; R_SPF_NA(0.00)[no SPF record]; FORGED_SENDER(0.30)[imp@bsdimp.com,wlosh@bsdimp.com]; MIME_TRACE(0.00)[0:+,1:+,2:~]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; RCVD_TLS_ALL(0.00)[]; FROM_NEQ_ENVFROM(0.00)[imp@bsdimp.com,wlosh@bsdimp.com]; RCVD_COUNT_TWO(0.00)[2] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.33 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 06:12:10 -0000 On Mon, Oct 26, 2020 at 12:01 AM Alex Kozlov 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? Warner