-
Notifications
You must be signed in to change notification settings - Fork 69
LLVM 4.0.1 #452
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
LLVM 4.0.1 #452
Conversation
|
/cc @tmpsantos - this improves upon the packages you added. |
|
@daniel-j-h tagged you for review on this since I know you've been interested in the process of creating these packages being documented. The docs are now at https://github.com/mapbox/mason/blob/d6dbdad607b4a61ce4b70a119aec4147db3e262b/scripts/llvm/base/README.md. Would appreciate your 👍 before merging. |
|
|
||
| LLVM_VERSION="4.0.2" | ||
| docker run -it --volume $(pwd):/home/travis/build/mapbox/mason mason-llvm \ | ||
| ./mason build llvm ${LLVM_VERSION} && ./utils/llvm.sh build ${LLVM_VERSION} |
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 think this misses the code-blocks backticks
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.
👍 added in c5596d5
| RUN apt-get update -y && \ | ||
| apt-get install -y vim python build-essential bash curl git-core ca-certificates software-properties-common python-software-properties --no-install-recommends | ||
|
|
||
| RUN add-apt-repository -y ppa:ubuntu-toolchain-r/test && \ |
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.
Hm I guess pull this ppa in for a recent C++ stdlib version. But why exactly? Does building llvm or any of the binaries need a recent stdlib? We do not link to the stdlib in the binaries anyway, no? I don't quite follow here.
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.
Nice catch! This is only needed for libstdc++6: an updated version of that is needed at runtime by cmake. It is not a compile/link dependency of llvm, so I've added comments about this to the Dockerfile to make this clear: 1c73eb6.
daniel-j-h
left a comment
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.
Wow, thanks for making this more transparent!! 🙇 Looks good from my side - some minor comments on the changeset.
Context
The llvm package contains a nearly complete, cross-platform, toolchain for building and linking C++ code. Normally this would be provided by the operating system (g++ toolchain on linux or XCode on mac). Instead we package the llvm suite of compilers in mason to enable upgrading your systems toolchain quickly via mason. The advantages of this are:
This PR
Adds llvm 4.0.1 and related sub-packages, closing #438. This improves upon #451, by additionally adding:
llvm/base/common.shto allow pointing at a custom gitsha for a given sub-package - needed in the case of include-what-you-use which did not have a tag that worked yet with llvm 4.x.