Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setup: Make validate_config to check for specified archs #97

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
setup: Make validate_config to check for specified archs
Up until now we could have a CONFIG_ being set on an archtecture and not
on others and then it would pass the checks, only complaining for a missing
symbol later on the setup phase.

IIRC this used to work in the past, but somehow it got broken. Added a test
to ensure that we catch such cases in the future.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
marcosps committed Mar 7, 2025
commit 18f83d53e6675b50bfab96754e3d008372ea2843
16 changes: 13 additions & 3 deletions klpbuild/klplib/codestream.py
Original file line number Diff line number Diff line change
@@ -244,14 +244,24 @@ def get_all_configs(self, conf):

return configs


def validate_config(self, conf, mod):
def validate_config(self, archs, conf, mod):
configs = {}
cs_config = self.get_all_configs(conf)

# Validate only the specified architectures, but check if the codestream
# is supported on that arch (like RT that is currently supported only on
# x86_64)
for arch, conf_entry in self.get_all_configs(conf).items():
for arch in archs:
# Check if the desired CONFIG entry is set on the codestreams's supported
# architectures, by iterating on the specified architectures from the setup command.
if arch not in self.archs:
continue

try:
conf_entry = cs_config.pop(arch)
except KeyError as exc:
raise RuntimeError(f"{self.name()}: {conf} not set on {arch}. Aborting") from exc
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

humm is it really necessary to abort here? Perhaps it would be better to just skip the codestream+arch combination.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, but it might require additional work on the TemplateGen, or maybe in how are handle it. Maybe we could just remove the arch from self.archs? I'm not sure, but yeah, sounds a better idea.


if conf_entry == "m" and mod == "vmlinux":
raise RuntimeError(f"{self.name()}:{arch} ({self.kernel}): Config {conf} is set as module, but no module was specified")
if conf_entry == "y" and mod != "vmlinux":
3 changes: 2 additions & 1 deletion klpbuild/plugins/setup.py
Original file line number Diff line number Diff line change
@@ -142,6 +142,7 @@ def setup_codestreams(lp_name, data):
patched_cs=new_patched_cs, cve=data['cve'])
return codestreams


def setup_project_files(lp_name, codestreams, ffuncs, archs):
utils.get_workdir(lp_name).mkdir(exist_ok=True)

@@ -163,7 +164,7 @@ def setup_project_files(lp_name, codestreams, ffuncs, archs):
for f, fdata in cs.files.items():

mod = fdata["module"]
cs.validate_config(fdata["conf"], mod)
cs.validate_config(archs, fdata["conf"], mod)

sdir = cs.get_src_dir()
if not Path(sdir, f).is_file():
32 changes: 32 additions & 0 deletions tests/test_setup.py
Original file line number Diff line number Diff line change
@@ -135,3 +135,35 @@ def test_valite_conf_mod_file_funcs():
assert sch["module"] == "sch_qfq"
assert bts["conf"] == "CONFIG_BT_HCIBTSDIO"
assert bts["module"] == "btsdio"


def test_valite_conf_unsupported_arch():
# Make sure we error out in the case of a configuration entry that is not enabled
# on a codestream
lp = "bsc_" + inspect.currentframe().f_code.co_name

# CONFIG_HID is not enabled on s390x, so setup should fail here
LP_DEFAULT_DATA = {"cve": None, "lp_filter": CS, "conf": "CONFIG_HID", "no_check": False}
with pytest.raises(RuntimeError, match=rf"{CS}: CONFIG_HID not set on s390x"):
setup_args = {
"lp_name": lp,
"archs": utils.ARCHS,
"module": "vmlinux",
"file_funcs": [["drivers/hid/hid-core.c", "hid_alloc_report_buf"]],
"mod_file_funcs": [],
"conf_mod_file_funcs": [],
**LP_DEFAULT_DATA
}
setup(**setup_args)

# It shoudl succeed when s390x is removed from the setup command
setup_args = {
"lp_name": lp,
"archs": ["x86_64", "ppc64le"],
"module": "vmlinux",
"file_funcs": [["drivers/hid/hid-core.c", "hid_alloc_report_buf"]],
"mod_file_funcs": [],
"conf_mod_file_funcs": [],
**LP_DEFAULT_DATA
}
setup(**setup_args)