Fix NVC version detection #927
Conversation
nickg
force-pushed
the
fix-nvc-version
branch
2 times, most recently
from
cc35e3f
to
e88111c
Compare
vunit/sim_if/nvc.py
Outdated
| @@ -261,8 +264,7 @@ def simulate(self, output_path, test_suite_name, config, elaborate_only): | |||
|
|
|||
| if not elaborate_only: | |||
| cmd += ["--no-save"] | |||
| if self._supports_jit: | |||
| cmd += ["--jit"] | |||
| cmd += ["--jit"] | |||
There was a problem hiding this comment.
This implies that VUnit can only work with NVC >= 1.9, and that's why you added the runtimeerror above?
Just willing to clarify, because this feels a second change on top of improving the version detection. Might be worth having two commits, even if we merge them at the same time.
There was a problem hiding this comment.
Most of the examples worked with earlier releases but there were also many acceptance test failures so it seems better to specify a minimum version that we know is fully working. But I agree it's a logically separate change so I'll do it in a separate PR after this one is merged.
I bumped the version on the master branch to 1.10-devel but the comparison here is using floating point so 1.10 >= 1.9 is false. Changed determine_version to return a (major, minor) tuple instead.
|
The CI failures don't appear to be caused by this patch. |
There was a problem hiding this comment.
FTR, this PR modifies determine_version. The same function/method exists in the GHDL class, and the returned type is not consistent. Actually, GHDL uses a float while this PR is changing NVC to a tuple.
However, that is not a problem because each simulator class uses a different mechanism to detect the version, and we don't currently have a consistent API to "get the version of the tool" independently of the specific tool.
I bumped the version on the master branch to 1.10-devel but the comparison here is using floating point so 1.10 >= 1.9 is false. Changed determine_version to return a (major, minor) tuple instead.