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

Use explicit instead of autoconfigured Bazel CROSSTOOL. #4772

Merged
merged 2 commits into from
Jan 26, 2017

Conversation

david-german-tri
Copy link
Contributor

@david-german-tri david-german-tri commented Jan 12, 2017

This change is Reviewable

@david-german-tri
Copy link
Contributor Author

@drake-jenkins-bot linux-gcc-bazel-experimental please

@david-german-tri
Copy link
Contributor Author

@drake-jenkins-bot retest this please

1 similar comment
@david-german-tri
Copy link
Contributor Author

@drake-jenkins-bot retest this please

@david-german-tri
Copy link
Contributor Author

-(status: do not merge) -(status: do not review) +@jwnimmer-tri for both levels of review


Review status: 0 of 7 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@david-german-tri
Copy link
Contributor Author

Most of the LOC in this PR are third_party imports, either from the Bazel source tree, or from the outputs of cc_configure.bzl on our supported platforms: https://github.com/bazelbuild/bazel/blob/master/tools/cpp/cc_configure.bzl


Review status: 0 of 7 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@jwnimmer-tri
Copy link
Collaborator

Checkpoint.


Reviewed 6 of 7 files at r1.
Review status: 6 of 7 files reviewed at latest revision, 2 unresolved discussions.


drake/doc/bazel.rst, line 52 at r1 (raw file):

  bazel test //...                      # Build and test the entire project.
  bazel build --compiler=gcc-4.9 //...  # Build using gcc 4.9 on Trusty.
  bazel build --compiler=gcc-5 //...    # Build using gcc 5.x on Xenial.

Does this PR need to coincide with a drake-ci configuration change as well, then?


tools/third_party/bazel/tools/cpp/osx_cc_wrapper.sh, line 1 at r1 (raw file):

#!/bin/bash

BTW Is this file pristine, or modified in Drake vs Bazel? If the latter, please rebase the commits so that the pristine vendor copy comes it in its own commit, and our diffs are separated out.


Comments from Reviewable

@david-german-tri
Copy link
Contributor Author

Review status: 6 of 7 files reviewed at latest revision, 2 unresolved discussions.


drake/doc/bazel.rst, line 52 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Does this PR need to coincide with a drake-ci configuration change as well, then?

Yes, #4920


tools/third_party/bazel/tools/cpp/osx_cc_wrapper.sh, line 1 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

BTW Is this file pristine, or modified in Drake vs Bazel? If the latter, please rebase the commits so that the pristine vendor copy comes it in its own commit, and our diffs are separated out.

It's pristine.


Comments from Reviewable

@jwnimmer-tri
Copy link
Collaborator

Reviewed 1 of 7 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


tools/CROSSTOOL, line 1 at r1 (raw file):

# This CROSSTOOL was produced by copying the Bazel auto-generated CROSSTOOL for

Please add a comment explaining how to re-run or re-create this step.


tools/CROSSTOOL, line 2 at r1 (raw file):

# This CROSSTOOL was produced by copying the Bazel auto-generated CROSSTOOL for
# our supported platforms, and then making some Drake-specific tweaks.

BTW At least some tweaks are marked "specific to Drake". Are they all marked as such? Might be a good idea -- and then if so, explain that marking here.


Comments from Reviewable

@jwnimmer-tri
Copy link
Collaborator

:lgtm:


Review status: all files reviewed at latest revision, 2 unresolved discussions.


Comments from Reviewable

@david-german-tri
Copy link
Contributor Author

Review status: 6 of 7 files reviewed at latest revision, 2 unresolved discussions.


tools/CROSSTOOL, line 1 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Please add a comment explaining how to re-run or re-create this step.

Done.


tools/CROSSTOOL, line 2 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

BTW At least some tweaks are marked "specific to Drake". Are they all marked as such? Might be a good idea -- and then if so, explain that marking here.

Done.


Comments from Reviewable

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.

2 participants