svn commit: r360596 - in stable: 11/stand/defaults 11/stand/lua 12/stand/defaults 12/stand/lua

Kyle Evans kevans at FreeBSD.org
Sun May 3 03:53:39 UTC 2020


Author: kevans
Date: Sun May  3 03:53:38 2020
New Revision: 360596
URL: https://svnweb.freebsd.org/changeset/base/360596

Log:
  MFC lualoader read-conf support: r360420-r360423, r360425, r360427, r360486,
  \r360505-r360506
  
  r360420:
  lualoader config: don't call loader.getenv() as much
  
  We don't actually need to fetch loader_conf_files as much as we do; we've
  already fetched it once at the beginning, we only really need to fetch it
  again after each file we've processed. If it changes, then we can stash that
  off into our local prefiles.
  
  While here, drop a note about the recursion so that I stop trying to
  change it. It may very well make redundant some of the work we're doing, but
  that's OK.
  
  r360421:
  lualoader: config: start exporting readConfFiles
  
  In the process, change it slightly: readConfFiles will take a string like
  loader_conf_files in addition to the loaded_files table that it normally
  takes. This is to facilitate the addition of a read-conf CLI command, which
  will just pass in the single file to read and an empty table.
  
  r360422:
  lualoader: cli: add read-conf
  
  This is a straightforward match to the command used by many in forthloader;
  it uses the newly-exported config.readConfFiles() to make sure that any
  loader_conf_files gets done as appropriate.
  
  r360423:
  lualoader: cli: clobber loader_conf_files before proceeding
  
  This makes sure that config.readConfFiles doesn't see a stale
  loader_conf_files from before, in case the newly loaded file doesn't set it.
  
  r360425:
  config.lua(8): "may should" is not proper grammar
  
  r360427:
  config.lua(8): catch up to recently added hooks
  
  While we're here, let's stylize these as functions instead of just raw text.
  A future change may allow arbitrary data arguments to be passed some of
  these, and the distinction is useful.
  
  r360486:
  loader.conf(5): document that loader_conf_files may be clobbered
  
  A future change in lualoader may take some liberties with the
  loader_conf_files in the name of efficiency; namely, it may start omitting
  it from the loader environment entirely so that it doesn't need to worry
  about maintaining any specific value.
  
  This variable has historically been incredibly volatile anyways, as it may
  get set to completely different values in any given configuration file to
  trigger a load of more files.
  
  Document now that we may not maintain it in the future, but perhaps we'll
  reserve the right to change our minds and eventually formally export all of
  the loader configuration files that were read using this variable.
  
  r360505:
  lualoader: config: add a table for restricted environment vars
  
  This new table should be used for transient values that don't need to end up
  in the loader environment. Generally, these will be things that are internal
  details that really aren't needed or interesting outside of the config
  module (e.g. if we changed how ${module}_* directives work, they might use
  this instead).
  
  To start, populate it with loader_conf_files. Any specific value of
  loader_conf_files isn't all that interesting; if we're going to export it,
  we should really instead export a loader_conf_files that indicates all of
  the configuration files we processed. This will be used to reduce
  bookkeeping overhead in a future commit that cleans up readConfFiles.
  
  r360506:
  lualoader: config: improve readConfFiles, rename to readConf
  
  The previous interface was pretty bad, and required the caller to get some
  implementation details correct that it really shouldn't need to (e.g.
  loader_conf_files handling) and pass in an empty table for it to use.
  
  The new and much improved interface, readConf, is much less of a hack;
  hiding these implementation details and just doing the right thing.
  config.lua will now use it to process /boot/defaults/loader.conf and the
  subsequent loader_conf_files from there, and read-conf will also use it.
  
  This improvement submitted by Olivier (cited below), loader_conf_files
  handling from the original patch was changed to just clobber it before
  processing and not bother restoring it after the fact following r360505
  where it's now guaranteed to evade the loader environment.
  
  PR:		244640

Modified:
  stable/11/stand/defaults/loader.conf.5
  stable/11/stand/lua/cli.lua
  stable/11/stand/lua/config.lua
  stable/11/stand/lua/config.lua.8
Directory Properties:
  stable/11/   (props changed)

Changes in other areas also in this revision:
Modified:
  stable/12/stand/defaults/loader.conf.5
  stable/12/stand/lua/cli.lua
  stable/12/stand/lua/config.lua
  stable/12/stand/lua/config.lua.8
Directory Properties:
  stable/12/   (props changed)

Modified: stable/11/stand/defaults/loader.conf.5
==============================================================================
--- stable/11/stand/defaults/loader.conf.5	Sun May  3 03:44:58 2020	(r360595)
+++ stable/11/stand/defaults/loader.conf.5	Sun May  3 03:53:38 2020	(r360596)
@@ -23,7 +23,7 @@
 .\" SUCH DAMAGE.
 .\"
 .\" $FreeBSD$
-.Dd May 16, 2019
+.Dd May 2, 2020
 .Dt LOADER.CONF 5
 .Os
 .Sh NAME
@@ -91,6 +91,10 @@ independently.
 .It Ar loader_conf_files
 Defines additional configuration files to be processed right after the
 present file.
+.Ar loader_conf_files
+should be treated as write-only.
+One cannot depend on any value remaining in the loader environment or carried
+over into the kernel environment.
 .It Ar kernel
 Name of the kernel to be loaded.
 If no kernel name is set, no additional

Modified: stable/11/stand/lua/cli.lua
==============================================================================
--- stable/11/stand/lua/cli.lua	Sun May  3 03:44:58 2020	(r360595)
+++ stable/11/stand/lua/cli.lua	Sun May  3 03:53:38 2020	(r360596)
@@ -126,6 +126,11 @@ cli['boot-conf'] = function(...)
 	core.autoboot(argstr)
 end
 
+cli['read-conf'] = function(...)
+	local _, argv = cli.arguments(...)
+	config.readConf(assert(core.popFrontTable(argv)))
+end
+
 cli['reload-conf'] = function(...)
 	config.reload()
 end

Modified: stable/11/stand/lua/config.lua
==============================================================================
--- stable/11/stand/lua/config.lua	Sun May  3 03:44:58 2020	(r360595)
+++ stable/11/stand/lua/config.lua	Sun May  3 03:53:38 2020	(r360596)
@@ -61,6 +61,17 @@ local QVALREPL = QVALEXPR:gsub('%%', '%%%%')
 local WORDEXPR = "([%w]+)"
 local WORDREPL = WORDEXPR:gsub('%%', '%%%%')
 
+-- Entries that should never make it into the environment; each one should have
+-- a documented reason for its existence, and these should all be implementation
+-- details of the config module.
+local loader_env_restricted_table = {
+	-- loader_conf_files should be considered write-only, and consumers
+	-- should not rely on any particular value; it's a loader implementation
+	-- detail.  Moreover, it's not a particularly useful variable to have in
+	-- the kenv.  Save the overhead, let it get fetched other ways.
+	loader_conf_files = true,
+}
+
 local function restoreEnv()
 	-- Examine changed environment variables
 	for k, v in pairs(env_changed) do
@@ -88,14 +99,31 @@ local function restoreEnv()
 	env_restore = {}
 end
 
+-- XXX This getEnv/setEnv should likely be exported at some point.  We can save
+-- the call back into loader.getenv for any variable that's been set or
+-- overridden by any loader.conf using this implementation with little overhead
+-- since we're already tracking the values.
+local function getEnv(key)
+	if loader_env_restricted_table[key] ~= nil or
+	    env_changed[key] ~= nil then
+		return env_changed[key]
+	end
+
+	return loader.getenv(key)
+end
+
 local function setEnv(key, value)
+	env_changed[key] = value
+
+	if loader_env_restricted_table[key] ~= nil then
+		return 0
+	end
+
 	-- Track the original value for this if we haven't already
 	if env_restore[key] == nil then
 		env_restore[key] = {value = loader.getenv(key)}
 	end
 
-	env_changed[key] = value
-
 	return loader.setenv(key, value)
 end
 
@@ -340,34 +368,6 @@ local function loadModule(mod, silent)
 	return status
 end
 
-local function readConfFiles(loaded_files)
-	local f = loader.getenv("loader_conf_files")
-	if f ~= nil then
-		for name in f:gmatch("([%w%p]+)%s*") do
-			if loaded_files[name] ~= nil then
-				goto continue
-			end
-
-			local prefiles = loader.getenv("loader_conf_files")
-
-			print("Loading " .. name)
-			-- These may or may not exist, and that's ok. Do a
-			-- silent parse so that we complain on parse errors but
-			-- not for them simply not existing.
-			if not config.processFile(name, true) then
-				print(MSG_FAILPARSECFG:format(name))
-			end
-
-			loaded_files[name] = true
-			local newfiles = loader.getenv("loader_conf_files")
-			if prefiles ~= newfiles then
-				readConfFiles(loaded_files)
-			end
-			::continue::
-		end
-	end
-end
-
 local function readFile(name, silent)
 	local f = io.open(name)
 	if f == nil then
@@ -488,6 +488,40 @@ function config.parse(text)
 	return status
 end
 
+function config.readConf(file, loaded_files)
+	if loaded_files == nil then
+		loaded_files = {}
+	end
+
+	if loaded_files[file] ~= nil then
+		return
+	end
+
+	print("Loading " .. file)
+
+	-- The final value of loader_conf_files is not important, so just
+	-- clobber it here.  We'll later check if it's no longer nil and process
+	-- the new value for files to read.
+	setEnv("loader_conf_files", nil)
+
+	-- These may or may not exist, and that's ok. Do a
+	-- silent parse so that we complain on parse errors but
+	-- not for them simply not existing.
+	if not config.processFile(file, true) then
+		print(MSG_FAILPARSECFG:format(file))
+	end
+
+	loaded_files[file] = true
+
+	-- Going to process "loader_conf_files" extra-files
+	local loader_conf_files = getEnv("loader_conf_files")
+	if loader_conf_files ~= nil then
+		for name in loader_conf_files:gmatch("[%w%p]+") do
+			config.readConf(name, loaded_files)
+		end
+	end
+end
+
 -- other_kernel is optionally the name of a kernel to load, if not the default
 -- or autoloaded default from the module_path
 function config.loadKernel(other_kernel)
@@ -596,12 +630,7 @@ function config.load(file, reloading)
 		file = "/boot/defaults/loader.conf"
 	end
 
-	if not config.processFile(file) then
-		print(MSG_FAILPARSECFG:format(file))
-	end
-
-	local loaded_files = {file = true}
-	readConfFiles(loaded_files)
+	config.readConf(file)
 
 	checkNextboot()
 

Modified: stable/11/stand/lua/config.lua.8
==============================================================================
--- stable/11/stand/lua/config.lua.8	Sun May  3 03:44:58 2020	(r360595)
+++ stable/11/stand/lua/config.lua.8	Sun May  3 03:53:38 2020	(r360596)
@@ -26,7 +26,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd June 9, 2018
+.Dd April 30, 2020
 .Dt CONFIG.LUA 8
 .Os
 .Sh NAME
@@ -59,6 +59,24 @@ to
 A lookup will be done as needed to determine what value
 .Ev idx
 actually corresponds to.
+.It Fn config.readConf file loaded_files
+Process
+.Pa file
+as a configuration file
+.Po e.g., as
+.Pa loader.conf
+.Pc
+and then processing files listed in
+.Ev loader_conf_files
+variable
+.Po see
+.Xr loader.conf 5
+.Pc .
+The caller may optionally pass in a table as the
+.Ev loaded_files
+argument, which uses filenames as keys and any non-nil value to
+indicate that the file named by the key has already been loaded and
+should not be loaded again.
 .It Fn config.processFile name silent
 Process and parse
 .Ev name
@@ -171,8 +189,10 @@ commands.
 The following hooks are defined in
 .Nm :
 .Bl -tag -width "config.reloaded" -offset indent
-.It config.loaded
-.It config.reloaded
+.It Fn config.loaded
+.It Fn config.reloaded
+.It Fn kernel.loaded
+.It Fn modules.loaded
 .El
 .Sh SEE ALSO
 .Xr loader.conf 5 ,


More information about the svn-src-all mailing list