From 9e23534ec365158768bb68699724b9ed559d1331 Mon Sep 17 00:00:00 2001
From: Matthias Schiffer <mschiffer@universe-factory.net>
Date: Tue, 26 May 2020 22:40:12 +0200
Subject: [PATCH] build: rework config generation

So far, we were using a sort operation on the generated .config to
implement precedence of =y packages over =m, and =m over unset.
Unfortunately, this sort not only used for packages, but for all config
lines. This made it impossible to override settings from targets/generic
in a target config when the new setting was sorted before the generic
setting.

To fix this, track configurations by their keys, so we can properly
override config keys that were set before. Value-based precedence is
only preserved for package configuration.

The config() and try_config() calls always take key and value as
separate arguments now. Strings are quoted automatically; the values
true, nil and false map to y, m and unset for tristate options. config()
can take an optional third argument to override the error message to
display when the setting fails to apply.

All existing target configs generate the same .config with the old and the
new code. The new code is also a bit faster on targets with many devices.
---
 scripts/target_config.lua       | 25 ++-----------
 scripts/target_config_check.lua | 58 +++++++-----------------------
 scripts/target_config_lib.lua   | 43 ++++++++++++++++------
 scripts/target_lib.lua          | 45 ++++++++++++++++++++---
 targets/ar71xx-generic          |  4 +--
 targets/ar71xx-mikrotik         |  2 +-
 targets/ar71xx-nand             |  2 +-
 targets/ar71xx-tiny             |  2 +-
 targets/generic                 | 64 +++++++++++++++------------------
 targets/ramips-rt305x           |  4 +--
 targets/x86.inc                 |  4 +--
 11 files changed, 127 insertions(+), 126 deletions(-)

diff --git a/scripts/target_config.lua b/scripts/target_config.lua
index 452d4f190..9696f5cee 100755
--- a/scripts/target_config.lua
+++ b/scripts/target_config.lua
@@ -1,24 +1,5 @@
-local funcs = {}
+local lib = dofile('scripts/target_config_lib.lua')()
 
-function funcs.config_message(config, _, ...)
-	config(...)
-end
-
-function funcs.config_package(config, pkg, value)
-	config('CONFIG_PACKAGE_%s=%s', pkg, value)
-end
-
-local lib = dofile('scripts/target_config_lib.lua')(funcs)
-
-
-local output = {}
-
-for config in pairs(lib.configs) do
-	table.insert(output, config)
-end
-
--- The sort will make =y entries override =m ones
-table.sort(output)
-for _, line in ipairs(output) do
-	io.stdout:write(line, '\n')
+for _, config in pairs(lib.configs) do
+	io.stdout:write(config:format(), '\n')
 end
diff --git a/scripts/target_config_check.lua b/scripts/target_config_check.lua
index 373cf430b..c5690556d 100755
--- a/scripts/target_config_check.lua
+++ b/scripts/target_config_check.lua
@@ -1,16 +1,12 @@
-local errors = {}
+local errors = false
 
-
-local function fail(...)
-	if not next(errors) then
+local function fail(msg)
+	if not errors then
+		errors = true
 		io.stderr:write('Configuration failed:', '\n')
 	end
 
-	local msg = string.format(...)
-	if not errors[msg] then
-		errors[msg] = true
-		io.stderr:write(' * ', msg, '\n')
-	end
+	io.stderr:write(' * ', msg, '\n')
 end
 
 local function match_config(f)
@@ -23,49 +19,21 @@ local function match_config(f)
 	return false
 end
 
-local function check_config(pattern)
-	return match_config(function(line) return line == pattern end)
-end
-
-local function check_config_prefix(pattern)
-	return match_config(function(line) return string.sub(line, 1, -2) == pattern end)
-end
-
-
-local funcs = {}
-
-function funcs.config_message(_, message, ...)
-	local pattern = string.format(...)
-
-	if not check_config(pattern) then
-		fail('%s', message)
-	end
+local function check_config(config)
+	return match_config(function(line) return line == config end)
 end
 
-function funcs.config_package(_, pkg, value)
-	local pattern = string.format('CONFIG_PACKAGE_%s=%s', pkg, value)
-	local res
-	if value == 'y' then
-		res = check_config(pattern)
-	else
-		res = check_config_prefix(string.sub(pattern, 1, -2))
-	end
-
-	if not res then
-		fail("unable to enable package '%s'", pkg)
-	end
-end
 
-local lib = dofile('scripts/target_config_lib.lua')(funcs)
+local lib = dofile('scripts/target_config_lib.lua')()
 
-for config, v in pairs(lib.configs) do
-	if v == 2 then
-		if not check_config(config) then
-			fail("unable to set '%s'", config)
+for _, config in pairs(lib.configs) do
+	if config.required then
+		if not check_config(config:format()) then
+			fail(config.required)
 		end
 	end
 end
 
-if next(errors) then
+if errors then
 	os.exit(1)
 end
diff --git a/scripts/target_config_lib.lua b/scripts/target_config_lib.lua
index ee2bf0069..7004ac1b7 100644
--- a/scripts/target_config_lib.lua
+++ b/scripts/target_config_lib.lua
@@ -40,7 +40,7 @@ local function compact_list(list, keep_neg)
 	return ret
 end
 
-return function(funcs)
+return function()
 	local lib = dofile('scripts/target_lib.lua')
 	local env = lib.env
 
@@ -102,12 +102,29 @@ END_MAKE
 		return pkgs
 	end
 
+	local enabled_packages = {}
+	-- Arguments: package name and config value (true: y, nil: m, false: unset)
+	-- Ensures precedence of y > m > unset
+	local function config_package(pkg, v)
+		if v == false then
+			if not enabled_packages[pkg] then
+				lib.try_config('CONFIG_PACKAGE_' .. pkg, false)
+			end
+			return
+		end
+
+		if v == true or not enabled_packages[pkg] then
+			lib.config('CONFIG_PACKAGE_' .. pkg, v, string.format("unable to enable package '%s'", pkg))
+			enabled_packages[pkg] = true
+		end
+	end
+
 	local function handle_target_pkgs(pkgs)
 		for _, pkg in ipairs(pkgs) do
 			if string.sub(pkg, 1, 1) == '-' then
-				lib.try_config('# CONFIG_PACKAGE_%s is not set', string.sub(pkg, 2))
+				config_package(string.sub(pkg, 2), false)
 			else
-				funcs.config_package(lib.config, pkg, 'y')
+				config_package(pkg, true)
 			end
 		end
 	end
@@ -118,8 +135,8 @@ END_MAKE
 	lib.check_devices()
 
 	if not lib.opkg then
-		lib.config '# CONFIG_SIGNED_PACKAGES is not set'
-		lib.config 'CONFIG_CLEAN_IPKG=y'
+		lib.config('CONFIG_SIGNED_PACKAGES', false)
+		lib.config('CONFIG_CLEAN_IPKG', true)
 		lib.packages {'-opkg'}
 	end
 
@@ -133,7 +150,7 @@ END_MAKE
 			local function handle_pkgs(pkgs)
 				for _, pkg in ipairs(pkgs) do
 					if string.sub(pkg, 1, 1) ~= '-' then
-						funcs.config_package(lib.config, pkg, 'm')
+						config_package(pkg, nil)
 					end
 					device_pkgs = append_to_list(device_pkgs, pkg)
 				end
@@ -144,11 +161,15 @@ END_MAKE
 			handle_pkgs(dev.options.packages or {})
 			handle_pkgs(site_packages(dev.image))
 
-			funcs.config_message(lib.config, string.format("unable to enable device '%s'", profile),
-				'CONFIG_TARGET_DEVICE_%s_DEVICE_%s=y', openwrt_config_target, profile)
-			lib.config('CONFIG_TARGET_DEVICE_PACKAGES_%s_DEVICE_%s="%s"',
-				openwrt_config_target, profile,
-				table.concat(device_pkgs, ' '))
+			lib.config(
+				string.format('CONFIG_TARGET_DEVICE_%s_DEVICE_%s', openwrt_config_target, profile),
+				true,
+				string.format("unable to enable device '%s'", profile)
+			)
+			lib.config(
+				string.format('CONFIG_TARGET_DEVICE_PACKAGES_%s_DEVICE_%s', openwrt_config_target, profile),
+				table.concat(device_pkgs, ' ')
+			)
 		end
 	else
 		-- x86 fallback: no devices
diff --git a/scripts/target_lib.lua b/scripts/target_lib.lua
index 0fc2d8b61..cdaa4b761 100644
--- a/scripts/target_lib.lua
+++ b/scripts/target_lib.lua
@@ -150,14 +150,51 @@ local function add_image(image)
 	table.insert(M.images[device], setmetatable(image, image_mt))
 end
 
-function F.try_config(...)
-	M.configs[string.format(...)] = 1
+
+local function format_config(k, v)
+	local format
+	if type(v) == 'string' then
+		format = '%s=%q'
+	elseif v == true then
+		format = '%s=y'
+	elseif v == nil then
+		format = '%s=m'
+	elseif v == false then
+		format = '# %s is not set'
+	else
+		format = '%s=%d'
+	end
+	return string.format(format, k, v)
 end
 
-function F.config(...)
-	M.configs[string.format(...)] = 2
+local config_mt = {
+	__index = {
+		format = function(config)
+			return format_config(config.key, config.value)
+		end,
+	}
+}
+
+local function do_config(k, v, required)
+	M.configs[k] = setmetatable({
+		key = k,
+		value = v,
+		required = required,
+	}, config_mt)
 end
 
+function F.try_config(k, v)
+	do_config(k, v)
+end
+
+function F.config(k, v, message)
+	if not message then
+		message = string.format("unable to set '%s'", format_config(k, v))
+	end
+	do_config(k, v, message)
+end
+
+
 function F.packages(pkgs)
 	for _, pkg in ipairs(pkgs) do
 		table.insert(M.target_packages, pkg)
diff --git a/targets/ar71xx-generic b/targets/ar71xx-generic
index 04d798aba..dda722170 100644
--- a/targets/ar71xx-generic
+++ b/targets/ar71xx-generic
@@ -1,5 +1,5 @@
-config 'CONFIG_GLUON_SPECIALIZE_KERNEL=y'
-config 'CONFIG_TARGET_SQUASHFS_BLOCK_SIZE=64'
+config('CONFIG_GLUON_SPECIALIZE_KERNEL', true)
+config('CONFIG_TARGET_SQUASHFS_BLOCK_SIZE', 64)
 
 local ATH10K_PACKAGES = {
 	'kmod-ath10k',
diff --git a/targets/ar71xx-mikrotik b/targets/ar71xx-mikrotik
index 6a783b5e9..8921b6f66 100644
--- a/targets/ar71xx-mikrotik
+++ b/targets/ar71xx-mikrotik
@@ -1,4 +1,4 @@
-config 'CONFIG_GLUON_SPECIALIZE_KERNEL=y'
+config('CONFIG_GLUON_SPECIALIZE_KERNEL', true)
 
 defaults {
 	factory = false,
diff --git a/targets/ar71xx-nand b/targets/ar71xx-nand
index e52b82e01..866e97094 100644
--- a/targets/ar71xx-nand
+++ b/targets/ar71xx-nand
@@ -1,4 +1,4 @@
-config 'CONFIG_GLUON_SPECIALIZE_KERNEL=y'
+config('CONFIG_GLUON_SPECIALIZE_KERNEL', true)
 
 local ATH10K_PACKAGES = {'kmod-ath10k', '-kmod-ath10k-ct', 'ath10k-firmware-qca988x', '-ath10k-firmware-qca988x-ct'}
 
diff --git a/targets/ar71xx-tiny b/targets/ar71xx-tiny
index bc9f03c88..61da28eda 100644
--- a/targets/ar71xx-tiny
+++ b/targets/ar71xx-tiny
@@ -1,4 +1,4 @@
-config 'CONFIG_GLUON_SPECIALIZE_KERNEL=y'
+config('CONFIG_GLUON_SPECIALIZE_KERNEL', true)
 
 no_opkg()
 
diff --git a/targets/generic b/targets/generic
index 91ab783c6..42502b768 100644
--- a/targets/generic
+++ b/targets/generic
@@ -1,21 +1,21 @@
 assert(env.GLUON_LANGS)
 
 
-config('CONFIG_GLUON_SITEDIR="%s"', env.GLUON_SITEDIR)
-config('CONFIG_GLUON_RELEASE="%s"', env.GLUON_RELEASE)
-try_config('CONFIG_GLUON_BRANCH="%s"', env.GLUON_BRANCH or '')
+config('CONFIG_GLUON_SITEDIR', env.GLUON_SITEDIR)
+config('CONFIG_GLUON_RELEASE', env.GLUON_RELEASE)
+try_config('CONFIG_GLUON_BRANCH', env.GLUON_BRANCH or '')
 
 for lang in string.gmatch(env.GLUON_LANGS, '%S+') do
-	try_config('CONFIG_GLUON_WEB_LANG_%s=y', lang)
+	try_config('CONFIG_GLUON_WEB_LANG_' .. lang, true)
 end
 
-config('CONFIG_TARGET_%s=y', env.BOARD)
+config('CONFIG_TARGET_' .. env.BOARD, true)
 if env.SUBTARGET ~= '' then
-	config('CONFIG_TARGET_%s_%s=y', env.BOARD, env.SUBTARGET)
+	config(string.format('CONFIG_TARGET_%s_%s', env.BOARD, env.SUBTARGET), true)
 end
 
 -- Disable non-default feeds in distfeeds.conf
-config('# CONFIG_FEED_gluon_base is not set')
+config('CONFIG_FEED_gluon_base', false)
 
 local default_feeds = {}
 for feed in string.gmatch(exec_capture_raw('. scripts/default_feeds.sh && echo "$DEFAULT_FEEDS"'), '%S+') do
@@ -24,52 +24,46 @@ end
 
 for feed in string.gmatch(exec_capture_raw('. scripts/modules.sh && echo -n "$FEEDS"'), '%S+') do
 	if not default_feeds[feed] then
-		config('# CONFIG_FEED_%s is not set', feed)
+		config('CONFIG_FEED_' .. feed, false)
 	end
 end
 
 
-config '# CONFIG_TARGET_ROOTFS_INITRAMFS is not set'
+config('CONFIG_TARGET_ROOTFS_INITRAMFS', false)
 
-config 'CONFIG_DEVEL=y'
-config 'CONFIG_ALL_NONSHARED=y'
+config('CONFIG_DEVEL', true)
+config('CONFIG_ALL_NONSHARED', true)
 
-config '# CONFIG_PACKAGE_usbip is not set' -- fails to build
-config '# CONFIG_PACKAGE_kmod-jool is not set' -- fails to build
+config('CONFIG_PACKAGE_usbip', false) -- fails to build
+config('CONFIG_PACKAGE_kmod-jool', false) -- fails to build
 
-config 'CONFIG_BUSYBOX_CUSTOM=y'
-config '# CONFIG_BUSYBOX_CONFIG_FEATURE_PREFER_IPV4_ADDRESS is not set'
+config('CONFIG_BUSYBOX_CUSTOM', true)
+config('CONFIG_BUSYBOX_CONFIG_FEATURE_PREFER_IPV4_ADDRESS', false)
 
-config 'CONFIG_PACKAGE_ATH_DEBUG=y'
+config('CONFIG_PACKAGE_ATH_DEBUG', true)
 
-try_config 'CONFIG_TARGET_SQUASHFS_BLOCK_SIZE=256'
+try_config('CONFIG_TARGET_SQUASHFS_BLOCK_SIZE', 256)
 
-config '# CONFIG_KERNEL_IP_MROUTE is not set'
-config '# CONFIG_KERNEL_IPV6_MROUTE is not set'
+config('CONFIG_KERNEL_IP_MROUTE', false)
+config('CONFIG_KERNEL_IPV6_MROUTE', false)
 
-try_config 'CONFIG_TARGET_MULTI_PROFILE=y'
-try_config 'CONFIG_TARGET_PER_DEVICE_ROOTFS=y'
+try_config('CONFIG_TARGET_MULTI_PROFILE', true)
+try_config('CONFIG_TARGET_PER_DEVICE_ROOTFS', true)
 
-if istrue(env.GLUON_MULTIDOMAIN) then
-	config 'CONFIG_GLUON_MULTIDOMAIN=y'
-end
+config('CONFIG_GLUON_MULTIDOMAIN', istrue(env.GLUON_MULTIDOMAIN))
 
-if istrue(env.GLUON_AUTOREMOVE) then
-	config 'CONFIG_AUTOREMOVE=y'
-end
+config('CONFIG_AUTOREMOVE', istrue(env.GLUON_AUTOREMOVE))
 
 if istrue(env.GLUON_DEBUG) then
-	config 'CONFIG_DEBUG=y'
-	config 'CONFIG_NO_STRIP=y'
-	config '# CONFIG_USE_STRIP is not set'
-	config '# CONFIG_USE_SSTRIP is not set'
+	config('CONFIG_DEBUG', true)
+	config('CONFIG_NO_STRIP', true)
+	config('CONFIG_USE_STRIP', false)
+	config('CONFIG_USE_SSTRIP', false)
 
-	try_config 'CONFIG_TARGET_ROOTFS_PARTSIZE=500'
+	try_config('CONFIG_TARGET_ROOTFS_PARTSIZE', 500)
 end
 
-if not istrue(env.GLUON_MINIFY) then
-	config '# CONFIG_GLUON_MINIFY is not set'
-end
+config('CONFIG_GLUON_MINIFY', istrue(env.GLUON_MINIFY))
 
 packages {
 	'-kmod-ipt-offload',
diff --git a/targets/ramips-rt305x b/targets/ramips-rt305x
index a4026e247..9226fb06e 100644
--- a/targets/ramips-rt305x
+++ b/targets/ramips-rt305x
@@ -1,5 +1,5 @@
-config '# CONFIG_KERNEL_KALLSYMS is not set'
-config 'CONFIG_GLUON_SPECIALIZE_KERNEL=y'
+config('CONFIG_KERNEL_KALLSYMS', false)
+config('CONFIG_GLUON_SPECIALIZE_KERNEL', true)
 
 no_opkg()
 
diff --git a/targets/x86.inc b/targets/x86.inc
index 9536e805b..947e82e96 100644
--- a/targets/x86.inc
+++ b/targets/x86.inc
@@ -1,5 +1,5 @@
-config 'CONFIG_VDI_IMAGES=y'
-config 'CONFIG_VMDK_IMAGES=y'
+config('CONFIG_VDI_IMAGES', true)
+config('CONFIG_VMDK_IMAGES', true)
 
 packages {
 	'kmod-3c59x',
-- 
GitLab