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

[RFC] Add extra_command_templates API #618

Closed
anuejn opened this issue Jun 8, 2021 · 2 comments
Closed

[RFC] Add extra_command_templates API #618

anuejn opened this issue Jun 8, 2021 · 2 comments
Labels
design

Comments

@anuejn
Copy link
Contributor

anuejn commented Jun 8, 2021

This is the RFC following up #515.
To solve the issue I encountered, it would be beneficial to have some way to inject additional build commands into the generated build script.

I propose that this is done by adding properties named command_templates_override, required_tools_override, and file_templates_override to TemplatedPlatform that are initialized to None. If they are not None, TemplatedPlatform should use them instead of their siblings without _override.

If the user desires to e.g. inject additional commands into the build script (as I do), they can copy the command_templates list of the platform to command_templates_override and add their commands.

There are some Pros / Cons of this approach vs. other approaches:

  • Pro: compared to an approach with an extra_command_templates list, this is more flexible: The user can even inject commands in the middle of the build script (to e.g. patch the netlist).
  • Con: rather cumbersome to use and rather easy to misuse: Just add an empty command_templates_override list and your build silently does nothing.
  • Enlarges the nMigen API surface in a not too clean way.

I am happy for any comments and / or other propositions to solve the stated problem. I am also happy to implement this RFC.
@anuejn anuejn changed the title RFC: Add extra_command_templates API [RFC] Add extra_command_templates API Jun 8, 2021
@whitequark
Copy link
Member

I propose that this is done by adding properties named command_templates_override, required_tools_override, and file_templates_override to TemplatedPlatform that are initialized to None. If they are not None, TemplatedPlatform should use them instead of their siblings without _override.

I don't understand why this mechanism is necessary. If you want to override command_templates, you define a property named command_templates that calls back to the superclass:

@property
def command_templates(self):
    return super().command_templates + ["foo_cmd"]

There are a few boards that already use this approach.

@anuejn
Copy link
Contributor Author

anuejn commented Jun 8, 2021

I am not wrapping the platform but rather want to inject the additional commands to any platform.
However, your comment lead to a hack that works good enough for me. Thanks a lot :).

@anuejn anuejn closed this as completed Jun 8, 2021
@anuejn anuejn mentioned this issue Jun 8, 2021
Closed
anuejn added a commit to apertus-open-source-cinema/naps that referenced this issue Jun 8, 2021
@anuejn
soc: add better hack for extra_command_templates
445bca4 
this one does not alter the python sessions in unhygienic ways
see amaranth-lang/amaranth#618
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
design
Milestone
No milestone
Development

No branches or pull requests
2 participants
@whitequark @anuejn