-
Notifications
You must be signed in to change notification settings - Fork 258
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.
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
Vunit dependencies #593
Vunit dependencies #593
Conversation
WIP because I do not know if this is the intended way to add such a feature |
Ok, this is an interesting feature and I'm definitely open to adding more metadata to the EDAM file. I just want to be careful to not expose too much internal FuseSoC details since Edalize is intended to work without FuseSoC (and it is already used by a bunch of other projects). Adding core is the least controversial in my opinion. I'm less sure about fileset. To me that feels like an internal FuseSoC detail. Do you need that? I don't see it being used in olofk/edalize#358 I see two issues with depend. The first is that we probably need to parse the list to evaluate the flags before sending it to the EDAM. The other thing is that the EDAM file already contains the dependency tree, so if you know which core a file belongs to, you can figure out its dependencies. So with that, I don't think we need to specify dependencies on a file-level as well. So, I'm happy to accept a modified patch that only adds the "core" field. If you still want to include the other stuff, then we need to discuss that. |
481049e
to
d941b36
Compare
@olofk |
Anything else I missed? |
I tested this just now and there is actually a test that breaks with this change. test_capi2_get_files in |
@nopeslide I'm planning to release FuseSoC 2.0 very soon. If we want this patch to go in before that, then we need to fix the failing test and do the VLNV to string conversion. |
d941b36
to
e48ab6a
Compare
e48ab6a
to
7305d9c
Compare
@olofk rebased on latest main |
This completely slipped my mind, and I also realized that this is actually implemented since some time ago, so I'll thank you for the contributions and closing this now. |
added fields to return value of get_files