byuccl / bfasst

Tools for FPGA Assurance Flows
Apache License 2.0
12 stars 5 forks source link

Add implicit dependencies for python scripts used by rules in ninja build snippet templates #336

Open KeenanRileyFaulkner opened 11 months ago

KeenanRileyFaulkner commented 11 months ago

If a ninja build snippet will cause a python script to be invoked (for example, one in ninja_utils or anything in third_party), the script file path should be an implicit dependency of the rule as seen here.

To do so, review the ninja rule templates found in the ninja_tools subdirectory. For any rules that invoke Python, make sure that all associated ninja build template files have implicit dependencies declared, as in the above link.

jgoeders commented 10 months ago

@KeenanRileyFaulkner When should a python be script be placed as a dependency in the ninja rule versus being added by add_ninja_deps? Should it be in both places? Or just one of them?

KeenanRileyFaulkner commented 10 months ago

The ninja deps have to do with rebuilding the build.ninja file when they change. The implicit dependencies will cause other snippets to be rerun. So if a Python script should cause the build file to be recreated (which I think is almost always true), it should be included in the add ninja deps method. But I also think that a Python script should almost always be an implicit dependency of the build snippet that calls it so that the snippet reruns if it changes.

jgoeders commented 10 months ago

... cause the build file to be recreated (which I think is almost always true)...but I also think that a Python script should almost always be an implicit dependency of the build snippet that calls it so that the snippet reruns if it changes.

Does recreating the build file invalidate all output products and cause everything to be re-run? (in which case the latter wouldn't be required)

KeenanRileyFaulkner commented 10 months ago

No, it doesn't invalidate outputs unless the build snippet that created the output has dependencies that change. This gets checked when the build file is redone and executed. If none of the dependencies for rules other than the configure rule change, there is still no work to do.

KeenanRileyFaulkner commented 10 months ago

Part of the reason we need that is so that not everything needs to be rebuilt just because the build file changes. For example, the phys netlist tool already has its Python script as an implicit dependency for that build snippet and added as a "ninja dep". That means that if it is changed, the build file will be recreated and the phys netlist will be regenerated but the synthesis and implementation outputs will still be valid since those files didn't change.

jgoeders commented 10 months ago

So it sounds like in almost all cases you would want the python files listed in both places.

In that case, perhaps we should standardize on a process for this to remove some code duplication, perhaps having a standard variable name in the mustache file where add_ninja_deps are placed.