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

build: prevent accidental builds with clang in builder #48587

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benesch
Copy link
Contributor

@benesch benesch commented May 8, 2020

The default toolchain in the builder image is crashy, but plausibly
useful for msan builds. Teach the Makefile to prohibit use of the crashy
toolchain unless an msan build is explicitly requested.

Fix #16071.

Release note: None

The default toolchain in the builder image is crashy, but plausibly
useful for msan builds. Teach the Makefile to prohibit use of the crashy
toolchain unless an msan build is explicitly requested.

Fix cockroachdb#16071.

Release note: None
@blathers-crl
Copy link

blathers-crl bot commented May 8, 2020

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

I have added a few people who may be able to assist in reviewing:

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@blathers-crl blathers-crl bot added the X-blathers-triaged blathers was able to find an owner label May 8, 2020
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@blathers-crl
Copy link

blathers-crl bot commented May 8, 2020

❌ The GitHub CI (Cockroach) build has failed on 1d0680ab.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@benesch
Copy link
Contributor Author

benesch commented May 8, 2020

Well, this has exposed that CI runs all tests with the crashy toolchain. Surprising that works. Anyway, I'm not planning to debug this further, but hopefully it's useful information for whoever winds up working on this.

@petermattis
Copy link
Collaborator

Well, this has exposed that CI runs all tests with the crashy toolchain. Surprising that works. Anyway, I'm not planning to debug this further, but hopefully it's useful information for whoever winds up working on this.

Thanks for the attempt.

Note that I don't think the clang tool chain is always crashy. My understanding is that crashes only occur when using the clang tool chain and cgosymbolizer. The latter is only used when CPU profiling is enabled. Heap profiles (which are always enabled) are unaffected because the associated stacks are always rooted in Go.

@irfansharif irfansharif removed their request for review May 8, 2020 16:36
@tbg tbg removed their request for review May 19, 2020 08:29
@kenliu kenliu requested review from jlinder and removed request for kenliu May 21, 2020 18:20
@petermattis petermattis marked this pull request as draft December 1, 2020 14:22
@tbg tbg added the X-noremind Bots won't notify about PRs with X-noremind label May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-blathers-triaged blathers was able to find an owner X-noremind Bots won't notify about PRs with X-noremind
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build: use a gcc-based toolchain by default (on linux and in builder)
4 participants