Skip to content
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

Add Dockerfile #641

Closed
wants to merge 2 commits into from
Closed

Add Dockerfile #641

wants to merge 2 commits into from

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Aug 9, 2020

Added Dockerfile. It's possible that it is missing some dependencies, or should simply be switched to a Ubuntu base. Curious to learn your review and thoughts. The Dockerfile is in use in this CI system: https://gitlab.com/virgohardware/core/-/pipelines

Creating it, I opted to use as many precompiled binaries as possible to reduce the image build time. It's also possible to restructure the setup scripts in a docker format, if that will work out better.

Related issue:

#109

Type of change: bug fix | new feature | other enhancement

Add a Dockerfile for easier setup and orientation.

Impact: rtl change | software change | unknown | other

Other

Release Notes

Added Dockerfile. It's possible that it is missing some dependencies, or should simply be switched to a Ubuntu base.
@jerryz123
Copy link
Contributor

Thanks for looking at this. I have a few comments..

  • Chipyard is not guaranteed to be compatible with packaged versions of various risc-v tools (for example, FESVR). For various reasons, we find it more flexible to build and deploy specific versions of these tools tracked in Chipyard.
  • We don't encourage setting the RISCV path to /usr/bin. The chipyard build-toolchains.sh script should generate a tools-install directory and a bash script to set RISCV.

We already have a Dockerfile for CI that installs all prerequisites of Chipyard (without installing Chipyard itself). This is in .circleci/images. I think a "deploy" image should inherit from that Dockerfile.

@faddat
Copy link
Contributor Author

faddat commented Aug 9, 2020

Thanks very much for your comments :).

I'll give the same task a try with your recommendations applied, including inheriting from the Dockerfile in .circleci/images.

Should I close this PR and submit another later, or just update my Dockerfile?

Cheers!

@faddat
Copy link
Contributor Author

faddat commented Aug 10, 2020

Have some more comments, but will move them to #109

I've updated this so that:

* It gets maven, gradle, ant, jq and sbt from apt
* It is no longer configured for docker in docker
* It allows the apt versions in debian Sid to be standard
* Build steps that have been omitted have been commented out for visibility

I will use it to build all of the cores that we've got in our CI system-- every "config" found in this repo, without exception.  

What else should I be testing?
@faddat
Copy link
Contributor Author

faddat commented Aug 10, 2020

I've updated this so that:

  • It gets maven, gradle, ant, jq and sbt from apt
  • It is no longer configured for docker in docker
  • It allows the apt versions in debian Sid to be standard
  • Build steps that have been omitted have been commented out for visibility

I will use it to build all of the cores that we've got in our CI system-- every "config" found in Chipyard.

What else should I be testing?

I reckon that this will work, but of course we want a bit more than just a reckoning :).

Comment on lines +208 to +210
ENV RISCV="$HOME/riscv-tools-install"
ENV LD_LIBRARY_PATH="$RISCV/lib"
ENV PATH="$RISCV/bin:$PATH"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these still need to be set here?

Copy link
Contributor Author

@faddat faddat Aug 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are not currently sourcing env.sh, yes. They ensure that when the Docker image is used, environment variables are set even without sourcing env.sh.

I should probably fire up the image to make sure that CI defaults to using user riscvuser, and it may be preferable to just source env.sh when using this in CI.

For users who aren't using CI, and are developing on their local machine, these env vars ensure that they don't need to source env.sh when using the image.

goal: This Docker image should allow anyone to play with Chipyard quickly and easily, using the exact same image that we use in the Tendermint Hardware CI system so that:

  • questions about differing build environments are eliminated
  • time spent configuring a build environment is drastically reduced
  • need to own/rent a build machine is eliminated

all of those were a big problems for me when I first began researching Open Source Semiconductors.

  • users should be able to modify a core, create a branch in our repo, and build/test it so that the pace of development, especially for non-professionals, increases

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the goals you specified.

I believe that setting RISCV here in the Dockerfile does not actually adjust the location of the eventual riscv-tools install, as the build-toolchains.sh hard-code a path ({riscv/esp}-tools-install) unless the --prefix argument is given. You will probably observe this when you inspect the final image.

Since Chipyard provides two toolchain options for users, I think its best to leave RISCV unset in final Dockerimage. The Chipyard makefiles will already provide a informative error if running RISCV unset, and the documentation additionally stresses that RISCV must be set by sourcing one of the env scripts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your advice!

I'll take that out, and see what I find :).

@faddat
Copy link
Contributor Author

faddat commented Aug 13, 2020

This is currently pretty much broken; I only thought it was working :/.

Will continue to bang on it until it's working.

I am going to try: totally abandon the Docker image used for CI mentioned before. Build the Docker image from scripts/ubuntu-req.sh + Ubuntu 20.04

It seems to be getting stuck on a submodule

fpu.c:(.text._gfortrani_get_fpu_trap_exceptions+0x4): warning: fegetexcept is not implemented and will always fail
Makefile:867: warning: overriding recipe for target 'all-multi'
Makefile:861: warning: ignoring old recipe for target 'all-multi'
Makefile:867: warning: overriding recipe for target 'all-multi'
Makefile:861: warning: ignoring old recipe for target 'all-multi'
Makefile:867: warning: overriding recipe for target 'all-multi'
Makefile:861: warning: ignoring old recipe for target 'all-multi'
Cloning into '/home/riscvuser/chipyard/toolchains/riscv-tools/riscv-isa-sim'...

I'll keep at it.

Update: we get past that now, but still fail.

Past:
https://gitlab.com/virgohardware/core/-/jobs/685479198#L6470

Fail:
https://gitlab.com/virgohardware/core/-/jobs/685479198#L6526

@abejgonzalez
Copy link
Contributor

Im going to close this in favor of #771 since that PR seems to work nicely and achieves the same result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants