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

Dockerized LLVM #1914

Merged
merged 8 commits into from
Nov 28, 2018
Merged

Dockerized LLVM #1914

merged 8 commits into from
Nov 28, 2018

Conversation

jackcmay
Copy link
Contributor

@jackcmay jackcmay commented Nov 26, 2018

Problem

BPF programs will soon require customized LLVM toolchain

Summary of Changes

Sync and build LLVM in docker and with 2 stage docker image make the binaries available to run directly. This docker is optionally used to build BPF programs.

Further changes are coming to populate this docker with the llvm binaries build and released separately.

Fixes #

@jackcmay jackcmay added work in progress This isn't quite right yet noCI Suppress CI on this Pull Request labels Nov 26, 2018
@jackcmay jackcmay requested a review from mvines November 26, 2018 18:14
@mvines
Copy link
Member

mvines commented Nov 26, 2018

looks good so far, I see the WIP label so I'll refrain from nits or looking too hard

@jackcmay
Copy link
Contributor Author

@mvines. The WIP is more to sync up with what you have in mind. This is working but is setup so that we run llvm from within Docker. With the Travis building and posting binaries in release tags would we still be building in a docker?

@borromeotlhs
Copy link

@jackcmay Running llvm in docker is loosely defined, as you are right: travis would be creating the binaries. However, do you have any ways to verify a built toolchain? As if anyone is building it, the docker file would just download it from there. . .

This type of thing is admirable, but I think after this is committed, the resultant binary (docker container) should be tagged and pushed to dockerhub to be repulled for veracity's sake against the version you successfully built.

Deeper conversation should be had about dedicating the toolchains to a repository and the levels of veracity one places on correctly building to a known good standard. I have not see gpg checksums on built toolchains unless one is downloading them directly or building them from source like bitbake or LEDE on Openwrt.

@mvines is there any discussion for ground up veracity on the resultant blockchain? I wonder what portions you all are building besides the applications? I assume the blockchain computation backend needs compiling?

@mvines
Copy link
Member

mvines commented Nov 26, 2018

With the Travis building and posting binaries in release tags would we still be building in a docker?

Probably not, ideally we can use the native binaries directly. But the docker setup still has value in the short term, in that we'd likely first get Travis building Linux LLVM binaries, so macOS could continue to use the docker image until we get Travis building macOS LLVM binaries. For Windows support I feel that we can just create a new issue for it and one day somebody may be motivated enough to pick it up.

@jackcmay
Copy link
Contributor Author

@borromeotlhs
We are building the programs (smart contracts) with LLVM. We need custom changes to LLVM to support things like BPF shared objects. Up until this point, we've been downloading publically available LLVM binaries. In this PR we are building our own forks of LLVM. To make this easier we are doing this in docker and will be pushing the resulting docker images to docker hub.

@jackcmay jackcmay removed the noCI Suppress CI on this Pull Request label Nov 28, 2018
@jackcmay jackcmay removed the request for review from mvines November 28, 2018 18:20
@jackcmay jackcmay removed the work in progress This isn't quite right yet label Nov 28, 2018
@jackcmay jackcmay requested a review from mvines November 28, 2018 18:48
@echo ' V=1'
@echo ' - Show commands while building: V=1'
@echo ' V=$(V)'
@echo ' - Use LLVM from docker: DOCKER=1'
Copy link
Member

Choose a reason for hiding this comment

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

probably should document somewhere around here that the user needs to docker pull solanalabs/llvm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't docker do that automagically if it does not find the docker locally?

Copy link
Member

Choose a reason for hiding this comment

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

nerp

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about pull as part of the script or is that just wasteful?

#!/usr/bin/env bash
set -ex
SDKPATH="$( cd "$(dirname "$0")" ; pwd -P )"/../../../..
docker pull solanalabs/llvm
docker run --workdir /solana_sdk --volume $SDKPATH:/solana_sdk --rm solanalabs/llvm `basename "$0"` "$@"

Copy link
Member

Choose a reason for hiding this comment

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

ci/docker-run.sh does this, but there's also cases when you don't want to pull (you're on an airplane with no uplink) so if we autopull then we'd also want to add a way to disable it. My vote for now is to just document that the user must pull

sdk/docker-llvm/README.md Outdated Show resolved Hide resolved
Copy link
Member

@mvines mvines left a comment

Choose a reason for hiding this comment

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

r+ with doc nits picked

@jackcmay jackcmay merged commit 0c091c1 into solana-labs:master Nov 28, 2018
@jackcmay jackcmay deleted the llvm-docker branch November 28, 2018 22:42
vkomenda pushed a commit to vkomenda/solana that referenced this pull request Aug 29, 2021
…olana-labs#1914)

Bumps [@babel/cli](https://github.com/babel/babel/tree/HEAD/packages/babel-cli) from 7.14.3 to 7.14.5.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.14.5/packages/babel-cli)

---
updated-dependencies:
- dependency-name: "@babel/cli"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
HaoranYi pushed a commit to HaoranYi/solana that referenced this pull request Jun 28, 2024
* build(deps): bump num-bigint from 0.4.5 to 0.4.6

Bumps [num-bigint](https://github.com/rust-num/num-bigint) from 0.4.5 to 0.4.6.
- [Changelog](https://github.com/rust-num/num-bigint/blob/master/RELEASES.md)
- [Commits](rust-num/num-bigint@num-bigint-0.4.5...num-bigint-0.4.6)

---
updated-dependencies:
- dependency-name: num-bigint
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* Update all Cargo files

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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