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.

Sign up for GitHub

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

Jump to bottom

Fixed CLI override function #663

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixed CLI override function #663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions f4pga/flows/argparser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ def p_parse_cli_value(s: str):
return s.replace("\\", "")


def get_cli_flow_config(args: Namespace, part: str):
def get_cli_flow_config(args: Namespace):
def create_defdict():
return {
"dependencies": {},
Expand All @@ -261,4 +261,4 @@ def add_entries(arglist: "list[str]", dict_name: str):
add_entries(args.dep, "dependencies")
add_entries(args.val, "values")

return {part: part_flow_config}
return part_flow_config
2 changes: 1 addition & 1 deletion f4pga/flows/commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def cmd_build(args: Namespace):
if (project_flow_cfg is None) and part_name is None:
fatal(-1, "No configuration was provided. Use `--flow`, and/or " "`--part` to configure flow.")

override_prj_flow_cfg_by_cli(project_flow_cfg, get_cli_flow_config(args, part_name))
project_flow_cfg = override_prj_flow_cfg_by_cli(project_flow_cfg, get_cli_flow_config(args))

flow_cfg = make_flow_config(project_flow_cfg, part_name)

Expand Down
52 changes: 7 additions & 45 deletions f4pga/flows/flow_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,51 +131,13 @@ def get_dependency_platform_overrides(self, part: str):


def override_prj_flow_cfg_by_cli(cfg: ProjectFlowConfig, cli_d: "dict[str, dict[str, dict]]"):
for part_name, part_cfg in cli_d.items():
print(f"OVERRIDING CONFIG FOR {part_name}")
p_cfg = cfg.flow_cfg.get(part_name)
if p_cfg is None:
p_cfg = {}
cfg.flow_cfg[part_name] = p_cfg
cli_p_values = part_cfg.get("values")
cli_p_dependencies = part_cfg.get("dependencies")
p_values = p_cfg.get("values")
p_dependencies = p_cfg.get("dependencies")
if cli_p_values is not None:
if p_values is None:
p_values = {}
part_cfg["values"] = p_values
p_values.update(cli_p_values)
if cli_p_dependencies is not None:
if p_dependencies is None:
p_dependencies = {}
part_cfg["dependencies"] = p_dependencies
p_dependencies.update(cli_p_dependencies)

for stage_name, cli_stage_cfg in part_cfg.items():
if stage_name in KWORDS:
continue

stage_cfg = part_cfg.get(stage_name)
if stage_cfg is None:
stage_cfg = {}
part_cfg[stage_name] = stage_cfg

stage_values = stage_cfg.get("values")
stage_dependencies = stage_cfg.get("dependencies")
cli_stage_values = cli_stage_cfg.get("values")
cli_stage_dependencies = cli_stage_cfg.get("dependencies")

if cli_stage_values is not None:
if stage_values is None:
stage_values = {}
stage_cfg["values"] = stage_values
stage_values.update(cli_stage_values)
if cli_stage_dependencies is not None:
if stage_dependencies is None:
stage_dependencies = {}
stage_cfg["dependencies"] = stage_dependencies
stage_dependencies.update(cli_stage_dependencies)
Comment on lines -155 to -178
Copy link
Collaborator

Choose a reason for hiding this comment

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

I appreciate how you simplified the code for global dependency overrides, but you dropped an entire part for overriding dependencies defined per-stage.

Copy link
Author

Choose a reason for hiding this comment

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

Oops, thanks for letting me know. I was probably a little too quick to open this pull request. I will try to fix this when I have time.

if "dependencies" not in cfg.flow_cfg:
cfg.flow_cfg["dependencies"] = {}
if "values" not in cfg.flow_cfg:
cfg.flow_cfg["values"] = {}
cfg.flow_cfg["dependencies"] = {**cfg.flow_cfg["dependencies"], **cli_d["dependencies"]}
cfg.flow_cfg["values"] = {**cfg.flow_cfg["values"], **cli_d["values"]}
return cfg


class FlowConfig:
Expand Down
Loading