edalize
edalize copied to clipboard
Command Generator Enhancements
This is an initial attempt to support phony targets as discussed in #250. It seems likely that something like the Target
class will be needed rather than just a string as is currently used, but it seems awkward to have self.EdaCommands.Target
all over. I'm open to alternatives!
I took a first pass at converting Quartus to use the command generator. First, it has a case that uses multiple commands to generate a target, so I've added this to the command generator.
The Quartus Makefiles also uses variables. I thought about adding a add_variable
method to the command generator, but while the Quartus case just uses simply expanded variables other backends use other types, making this less trivial. For now I've just appended variables to the header, which is likely poor form. I'm certainly open to other suggestions on how to handle this. The ninja_syntax
Python API for Ninja just uses a variable method, but I also think it only has one type of variable!
The failing tests are unrelated to this work. See #254 for a reporting test that works with the new Pandas 1.3. I rebased this branch on top of that one and all seems well.
Thanks for this. I agree that it looks a bit clumsy to have self.EdaCommands.Target
everywhere. Could we perhaps add a flag instead to add_command
to mark the rule as generating phony targets. That won't work if the rule creates a mix of phony and regular targets but perhaps that's not a real issue. Would need to dig into that, or perhaps you know?
Regarding variables, as far as I can tell they made a deliberate choice in ninja to downplay the use of variables so that they are just simple aliases. This makes me suspect that a) we need to be careful if we want to make this portable between command executioners and b) perhaps we can get around the need for variables in most (all?) situations. The ones I'm most worried about are the cases that rely on environment variables. Come to think of it, does that even work on Windows as it is implemented today? No idea
Taking another look at this, long overdue. Wouldn't it be possible to solve this by simple creating an extra target, like we do here https://github.com/olofk/edalize/blob/master/edalize/tools/icetime.py#L60
That doesn't create a phony target per se, but we could either regard all targets without a real command as phony, or if that doesn't work, add a phony plag to the add function. Would that solve your issue?
Also, do we want to pull in the first panda commit as is?