-
Notifications
You must be signed in to change notification settings - Fork 8
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
common bazel cxx toolchain for Linux and Mac #1
Conversation
Thanks for the updates, I was able to get parts of |
README.md
Outdated
## Prequisites |
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 that prequisite
might not be a word
scripts/chromium_repo.py
Outdated
response = requests.get(url, stream=stream) | ||
if response.status_code != 200: | ||
print >> sys.stderr, "Unexpected status code", response.status_code | ||
raise |
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.
do you want to raise a specific exception here? (Python might fail on no argument 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.
My Bazel-fu is not strong, but this looks LGTM — very nice
👏 🎈 sorry for taking 84 years to respond
After removing the health-check code from
Was able to successfully build prediction-api on OSX! 🎉 🎉 🎉 🎉 |
TEST(CompilerMeta, AppleBuild) { | ||
#if defined(__apple_build_version__) | ||
// __apple_build_version__ shouldn't be defined in the Chromium toolchain | ||
ASSERT_TRUE(false); |
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.
BTW, gtest has a FAIL()
macro just for such a case.
https://travis-ci.com/vsco/bazel-toolchains/builds/50882186
Still some cleanup to do:
not sure I'm getting a clean link on linux (should be using the sysroot on the link line).give scripts/generate_workspace.py an arg parsergo through the visibility rules under tools/ (they are all public rn)doesn't use the Chromium copy of libc++ on mac, but I don't think Chromium does either.[edit: this is a lie. we need to use their tree's copy of libc++ too. https://docs.google.com/document/d/1cFVCLYqpVV0pn2aX09SALcE3BN351sFN_rTcPcHRg3U/edit#]
only used on linux: https://cs.chromium.org/chromium/src/build/config/c%2B%2B/c%2B%2B.gni?type=cs&q=use_custom_libcxx&l=11