Merge pull request #2007 from freifunk-gluon/class-package-order

Fix package selection precedence

Makefile

@@ -55,7 +55,8 @@ endef
 GLUON_VARS = \
 	GLUON_RELEASE GLUON_REGION GLUON_MULTIDOMAIN GLUON_DEBUG GLUON_DEPRECATED GLUON_DEVICES \
 	GLUON_TARGETSDIR GLUON_PATCHESDIR GLUON_TMPDIR GLUON_IMAGEDIR GLUON_PACKAGEDIR \
-	GLUON_SITEDIR GLUON_RELEASE GLUON_BRANCH GLUON_LANGS GLUON_BASE_FEEDS BOARD SUBTARGET
+	GLUON_SITEDIR GLUON_RELEASE GLUON_BRANCH GLUON_LANGS GLUON_BASE_FEEDS \
+	GLUON_TARGET BOARD SUBTARGET
 
 unexport $(GLUON_VARS)
 GLUON_ENV = $(foreach var,$(GLUON_VARS),$(var)=$(call escape,$($(var))))
@@ -135,31 +136,6 @@ lint-lua: FORCE
 lint-sh: FORCE
 	scripts/lint-sh.sh
 
-define merge_lists
-  $(1) :=
-  $(foreach pkg,$(2),
-    $(1) := $$(strip $$(filter-out -$$(patsubst -%,%,$(pkg)) $$(patsubst -%,%,$(pkg)),$$(value $(1))) $(pkg))
-  )
-endef
-
-define feature_packages
-  $(1) := $(shell scripts/features.sh '$(2)' || echo '__ERROR__')
-endef
-
-$(eval $(call merge_lists,GLUON_FEATURE_LIST_standard,$(GLUON_FEATURES) $(GLUON_FEATURES_standard)))
-$(eval $(call merge_lists,GLUON_FEATURE_LIST_tiny,$(GLUON_FEATURES) $(GLUON_FEATURES_tiny)))
-
-$(eval $(call feature_packages,GLUON_FEATURE_PACKAGES_standard,$(GLUON_FEATURE_LIST_standard)))
-$(eval $(call feature_packages,GLUON_FEATURE_PACKAGES_tiny,$(GLUON_FEATURE_LIST_tiny)))
-
-ifneq ($(filter __ERROR__,$(GLUON_FEATURES_standard) $(GLUON_FEATURES_tiny)),)
-  $(error Error while evaluating features)
-endif
-
-$(eval $(call merge_lists,GLUON_DEFAULT_PACKAGES,$(GLUON_DEFAULT_PACKAGES) $(GLUON_SITE_PACKAGES)))
-$(eval $(call merge_lists,GLUON_CLASS_PACKAGES_standard,$(GLUON_FEATURE_PACKAGES_standard) $(GLUON_SITE_PACKAGES_standard)))
-$(eval $(call merge_lists,GLUON_CLASS_PACKAGES_tiny,$(GLUON_FEATURE_PACKAGES_tiny) $(GLUON_SITE_PACKAGES_tiny)))
-
 
 LUA := openwrt/staging_dir/hostpkg/bin/lua
 
@@ -182,22 +158,16 @@ config: $(LUA) FORCE
 		$(call CheckSite,$(conf)); \
 	)
 
-	$(GLUON_ENV) \
-		$(LUA) scripts/target_config.lua '$(GLUON_TARGET)' '$(GLUON_DEFAULT_PACKAGES)' '$(GLUON_CLASS_PACKAGES_standard)' '$(GLUON_CLASS_PACKAGES_tiny)' \
-		> openwrt/.config
+	$(GLUON_ENV) $(LUA) scripts/target_config.lua > openwrt/.config
 	$(OPENWRTMAKE) defconfig
-
-	$(GLUON_ENV) \
-		$(LUA) scripts/target_config_check.lua '$(GLUON_TARGET)' '$(GLUON_DEFAULT_PACKAGES)' '$(GLUON_CLASS_PACKAGES_standard)' '$(GLUON_CLASS_PACKAGES_tiny)'
+	$(GLUON_ENV) $(LUA) scripts/target_config_check.lua
 
 
 all: config
 	+
-	$(GLUON_ENV) \
-		$(LUA) scripts/clean_output.lua
+	$(GLUON_ENV) $(LUA) scripts/clean_output.lua
 	$(OPENWRTMAKE)
-	$(GLUON_ENV) \
-		$(LUA) scripts/copy_output.lua '$(GLUON_TARGET)'
+	$(GLUON_ENV) $(LUA) scripts/copy_output.lua
 
 clean download: config
 	+$(OPENWRTMAKE) $@

scripts/copy_output.lua

 local lib = dofile('scripts/target_lib.lua')
 local env = lib.env
 
+local target = env.GLUON_TARGET
+
+assert(target)
 assert(env.GLUON_IMAGEDIR)
 assert(env.GLUON_PACKAGEDIR)
 
 
-local target = arg[1]
-
 local openwrt_target
 local subtarget = env.SUBTARGET
 if subtarget ~= '' then

scripts/target_config_lib.lua

+-- Split a string into words
+local function split(s)
+	local ret = {}
+	for w in string.gmatch(s, '%S+') do
+		table.insert(ret, w)
+	end
+	return ret
+end
+
+-- Strip leading '-' character
+local function strip_neg(s)
+	if string.sub(s, 1, 1) == '-' then
+		return string.sub(s, 2)
+	else
+		return s
+	end
+end
+
+-- Add an element to a list, removing duplicate entries and handling negative
+-- elements prefixed with a '-'
+local function append_to_list(list, item, keep_neg)
+	local match = strip_neg(item)
+	local ret = {}
+	for _, el in ipairs(list) do
+		if strip_neg(el) ~= match then
+			table.insert(ret, el)
+		end
+	end
+	if keep_neg ~= false or string.sub(item, 1, 1) ~= '-' then
+		table.insert(ret, item)
+	end
+	return ret
+end
+
+local function compact_list(list, keep_neg)
+	local ret = {}
+	for _, el in ipairs(list) do
+		ret  = append_to_list(ret, el, keep_neg)
+	end
+	return ret
+end
+
 return function(funcs)
 	local lib = dofile('scripts/target_lib.lua')
 	local env = lib.env
 
+	local target = env.GLUON_TARGET
+
+	assert(target)
 	assert(env.BOARD)
 	assert(env.SUBTARGET)
 
-	local target = arg[1]
-	local extra_packages = arg[2]
-	local class_packages = {
-		standard = arg[3],
-		tiny = arg[4],
-	}
-
 	local openwrt_config_target
 	if env.SUBTARGET ~= '' then
 		openwrt_config_target = env.BOARD .. '_' .. env.SUBTARGET
@@ -20,81 +58,114 @@ return function(funcs)
 	end
 
 
-	local function site_packages(image)
+	local function site_vars(var)
 		return lib.exec_capture_raw(string.format([[
-	MAKEFLAGS= make print _GLUON_IMAGE_=%s --no-print-directory -s -f - <<'END_MAKE'
+	MAKEFLAGS= make print _GLUON_SITE_VARS_=%s --no-print-directory -s -f - <<'END_MAKE'
 include $(GLUON_SITEDIR)/site.mk
 
 print:
-	echo -n '$(GLUON_$(_GLUON_IMAGE_)_SITE_PACKAGES)'
+	echo -n '$(_GLUON_SITE_VARS_)'
 END_MAKE
-		]], lib.escape(image)))
+		]], lib.escape(var)))
+	end
+
+	local function site_packages(image)
+		return split(site_vars(string.format('$(GLUON_%s_SITE_PACKAGES)', image)))
+	end
+
+	-- TODO: Rewrite features.sh in Lua
+	local function feature_packages(features)
+		-- Ugly hack: Lua doesn't give us the return code of a popened
+		-- command, so we match on a special __ERROR__ marker
+		local pkgs = lib.exec_capture({'scripts/features.sh', features}, '|| echo __ERROR__')
+		assert(string.find(pkgs, '__ERROR__') == nil, 'Error while evaluating features')
+		return pkgs
+	end
+
+	-- This involves running lots of processes to evaluate site.mk, so we
+	-- add a simple cache
+	local class_cache = {}
+	local function class_packages(class)
+		if class_cache[class] then
+			return class_cache[class]
+		end
+
+		local features = site_vars(string.format('$(GLUON_FEATURES) $(GLUON_FEATURES_%s)', class))
+		features = table.concat(compact_list(split(features), false), ' ')
+
+		local pkgs = feature_packages(features)
+		pkgs = pkgs .. ' ' .. site_vars(string.format('$(GLUON_SITE_PACKAGES) $(GLUON_SITE_PACKAGES_%s)', class))
+
+		pkgs = compact_list(split(pkgs))
+
+		class_cache[class] = pkgs
+		return pkgs
 	end
 
 	local function handle_target_pkgs(pkgs)
-		local packages = string.gmatch(pkgs, '%S+')
-		for pkg in packages do
-			lib.packages {pkg}
+		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))
+			else
+				funcs.config_package(lib.config, pkg, 'y')
+			end
 		end
 	end
 
 	lib.include('generic')
-	handle_target_pkgs(extra_packages)
 	lib.include(target)
 
-	if lib.target_class ~= nil then
-		handle_target_pkgs(class_packages[lib.target_class])
-	end
-
 	lib.check_devices()
 
-
 	if not lib.opkg then
 		lib.config '# CONFIG_SIGNED_PACKAGES is not set'
 		lib.config 'CONFIG_CLEAN_IPKG=y'
 		lib.packages {'-opkg'}
 	end
 
+	if #lib.devices > 0 then
+		handle_target_pkgs(lib.target_packages)
 
-	local default_pkgs = ''
-	for _, pkg in ipairs(lib.target_packages) do
-		default_pkgs = default_pkgs .. ' ' .. pkg
+		for _, dev in ipairs(lib.devices) do
+			local profile = dev.options.profile or dev.name
 
-		if string.sub(pkg, 1, 1) == '-' then
-			lib.try_config('# CONFIG_PACKAGE_%s is not set', string.sub(pkg, 2))
-		else
-			funcs.config_package(lib.config, pkg, 'y')
-		end
-	end
+			local device_pkgs = {}
+			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')
+					end
+					device_pkgs = append_to_list(device_pkgs, pkg)
+				end
+			end
 
-	for _, dev in ipairs(lib.devices) do
-		local profile = dev.options.profile or dev.name
-		local device_pkgs = default_pkgs
+			handle_pkgs(lib.target_packages)
+			handle_pkgs(class_packages(dev.options.class))
+			handle_pkgs(dev.options.packages or {})
+			handle_pkgs(site_packages(dev.image))
 
-		local function handle_pkg(pkg)
-			if string.sub(pkg, 1, 1) ~= '-' then
-				funcs.config_package(lib.config, pkg, 'm')
-			end
-			device_pkgs = device_pkgs .. ' ' .. pkg
+			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, ' '))
 		end
+	else
+		-- x86 fallback: no devices
+		local target_pkgs = {}
 		local function handle_pkgs(pkgs)
-			local packages = string.gmatch(pkgs or '', '%S+')
-			for pkg in packages do
-				handle_pkg(pkg)
+			for _, pkg in ipairs(pkgs) do
+				target_pkgs = append_to_list(target_pkgs, pkg)
 			end
 		end
 
-		for _, pkg in ipairs(dev.options.packages or {}) do
-			handle_pkg(pkg)
-		end
-		handle_pkgs(site_packages(dev.image))
-
-		handle_pkgs(class_packages[dev.options.class])
+		-- Just hardcode the class for device-less targets to 'standard'
+		-- - this is x86 only at the moment, and it will have devices
+		-- in OpenWrt 19.07 + 1 as well
+		handle_pkgs(lib.target_packages)
+		handle_pkgs(class_packages('standard'))
 
-		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, device_pkgs)
+		handle_target_pkgs(target_pkgs)
 	end
 
 	return lib

scripts/target_lib.lua

@@ -23,7 +23,6 @@ assert(env.GLUON_DEPRECATED)
 
 M.site_code = assert(assert(dofile('scripts/site_config.lua')('site.conf')).site_code)
 M.target_packages = {}
-M.target_class = nil
 M.configs = {}
 M.devices = {}
 M.images = {}
@@ -40,7 +39,7 @@ local default_options = {
 	aliases = {},
 	manifest_aliases = {},
 	packages = {},
-	class = "standard",
+	class = 'standard',
 	deprecated = false,
 	broken = false,
 }
@@ -99,7 +98,10 @@ function F.escape(s)
 end
 
 local function escape_command(command, raw)
-	local ret = 'exec'
+	local ret = ''
+	if not raw then
+		ret = 'exec'
+	end
 	for _, arg in ipairs(command) do
 		ret = ret .. ' ' .. F.escape(arg)
 	end
@@ -156,10 +158,6 @@ function F.config(...)
 	M.configs[string.format(...)] = 2
 end
 
-function F.class(target_class)
-	M.target_class = target_class
-end
-
 function F.packages(pkgs)
 	for _, pkg in ipairs(pkgs) do
 		table.insert(M.target_packages, pkg)

targets/x86-geode

-class 'standard'
-
 packages {
 	'kmod-3c59x',
 	'kmod-8139cp',

targets/x86.inc

 config 'CONFIG_VDI_IMAGES=y'
 config 'CONFIG_VMDK_IMAGES=y'
 
-class 'standard'
-
 packages {
 	'kmod-3c59x',
 	'kmod-8139cp',