-
Notifications
You must be signed in to change notification settings - Fork 263
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
(Re)move AXI and UART verification components to separate repos #511
base: master
Are you sure you want to change the base?
Conversation
1949add
to
355898c
Compare
Good work. But I'm not sure if it is good to hard-code git as source code management system. |
@tmeissner thanks, and I agree. However, I have the feeling to be reinventing the wheel, because we are implementing a kind of package manager. Hence, I didn't want to make it more complex than necessary. I hope that some other user with better knowledge about the Python ecosystem can suggest a suitable mechanism to:
Currently, vc_uart depends on vc_axi. So, at some point, vc_axi might be updated, but vc_uart might be left behind. As a result, those designs that use latest vc_uart and latest vc_axi might need two copies of the later. Since we do not want to rewrite the sources of the VCs, the solution would be to make vc_uart use the version of vc_axi built into the expected library name and use another library name for the second compilation. Alternatively, we can just block any contribution upstream if corresponding PRs are not yet available downstream, so we would avoid these kind of conflicts completely. Anyway, note that it is not necessary to use the built-in procedure to install the dependencies. For example, vc_uart is tested in a docker container where git is not available: https://github.com/VUnit/vc_uart/blob/master/.travis.yml. Hence, in the example As commented in #480, you can have a fixed location in your own computer (say git clone https://github.com/VUnit/vc_axi $HOME/vunit_vcs/vc_axi
git clone https://github.com/VUnit/vc_uart $HOME/vunit_vcs/vc_uart vu.use_verification_components(vu.add_verification_components(
abspath(join(os.environ['HOME'], 'vunit_vcs'))
), ['UART']) which is exactly the same as: wh = vu.add_verification_components()
wh['third_party'] = abspath(join(os.environ['HOME'], 'vunit_vcs'))
vu.use_verification_components(wh, ['UART']) Note that, although AXI and UART are used, I did only specify UART because AXI is declared as a dependecy. As you see, this allows you to use any tool you want and to manage the checked out versions explicitly. |
I do not think we should add any mechanism to add verification components from url or similar. It should be up to the user to download the code and place whereever they want manually. We do not want to invent a package system. The user should just be able to add the verification component of their choice using a simple add_source_files command prj.add_source_files("path/to/vc/src/*.vhd") |
As commented in Gitter, I agree. We do need I will update this PR so that no default location is provided and an error is shown if the requested dependency is not available.
This is already possible. |
355898c
to
9207853
Compare
I do not think the use_verification_components method is necessary at all and it is just more complexity that we do not need at the moment. Just let the user download/git-submodule the VC they want and add them just like any other file. |
As commented in June 12, 2019 8:12 PM, I think it is important to acknowledge the concept of a parent repository containing multiple VC repos, each of them with a valid Currently, I have a single global location for VUnit, and I can use it (along with all the verification components) with a single function. I think we should allow the new structure to require managing two global locations only, and use two commands to add eveything (core and extras). Of course, the user is free to use any number of locations and/or commands/funcion calls. BTW, this is a general mechanism, not limited neither to known (those in |
c5a9e0c
to
b5b528f
Compare
b5b528f
to
c66047c
Compare
c66047c
to
b74908a
Compare
b74908a
to
0a4cb61
Compare
This PR is built on top of #509. It will be kept as a draft until #509 is merged, at least.
Coming from #480, this PR is related to:
Please, note that in both repos https://github.com/VUnit/vc_uart/blob/master/.travis.yml#L5.
This should be replaced with the master branch of VUnit/vunit when this PR is merged.