-
Notifications
You must be signed in to change notification settings - Fork 10
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
Split UHDM package into three packages #117
base: master
Are you sure you want to change the base?
Conversation
8a22cbe
to
3be3668
Compare
More files were missing in verilator-uhdm package. I'm rewriting recipe to use standard 'make install' target instead of copying selected files. |
7994861
to
b0327b7
Compare
b3947ef
to
5e9b782
Compare
Signed-off-by: Tomasz Jurtsch <[email protected]>
5e9b782
to
04a32e9
Compare
ba01311
to
000306c
Compare
This way we ensure that python will be included in the Conda package build string. Signed-off-by: Tomasz Jurtsch <[email protected]>
000306c
to
0cd9a10
Compare
All *-uhdm packages were built successfully. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think this looks okay to merge.
@@ -52,6 +52,7 @@ export GIT_SSL_NO_VERIFY=1 | |||
export GITREV="$(git describe --long 2>/dev/null || echo "unknown")" | |||
export CONDA_BUILD_ARGS=$PACKAGE | |||
export CONDA_OUT="$(conda render --output $CONDA_BUILD_ARGS 2> /dev/null | grep conda-bld | grep tar.bz2 | sed -e's/-[0-9]\+\.tar/*.tar/' -e's/-git//' | tr '\n' ';')" | |||
export UHDM_INTEGRATION_REV="$(git ls-remote https://github.com/alainmarcel/uhdm-integration.git HEAD | awk '{ print $1}')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, why do you need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to make sure we don't have any race conditions in package compatibility, for example uhdm surelog build starts -> in the meantime somebody upgrades the submodules in uhdm integration -> only then uhdm verilator build starts with different submodules. Checking the revision once and using it for all packages ensures this won't happen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you have solved the problem you think you have solved here... Give me a while to think about it....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tgorochowik - Do you know what the state of conda build prepare
that @PiotrZierhoffer was managing? That was designed to solve exactly this problem by separating the "generating / freezing the conda recipe" from the build / test / etc steps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tgorochowik @tjurtsch @mithro This is kinda unrelated to conda-build-prepare.
The problem here is that we can have different revisions for different uhdm packages - Travis will not preserve the value across packages. They are built in different VMs.
Conda-build-prepare will help you debug/alter the code before building, but this is a totally different scope.
I think that you can share the git versions between packages in an external storage, no other way for that in Travis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a review where I wanted to add a comment. It's below
This PR splits uhdm-integration package into 3 packages:
It also uses "make install" targets to install files in conda prefix instead of copying binary files to prefix, one-by-one.
This PR solves two problems: