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

Remove superfluous pthread flag on OS X. #5333

Merged
merged 1 commit into from
Feb 27, 2017

Conversation

david-german-tri
Copy link
Contributor

@david-german-tri david-german-tri commented Feb 27, 2017

Contributes to #5183.

@liangfok for feature review


This change is Reviewable

@liangfok
Copy link
Contributor

When I first tested this using a dirty workspace, the pthread warnings persisted. However, after cleaning the workspace using bazel clean --expunge, the warnings continued to show up. Did I do something wrong? I am using 559ee99.


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


Comments from Reviewable

@liangfok
Copy link
Contributor

Here is what I see: https://gist.github.com/liangfok/c194d35efff74d8249ff2c5ad6a9d5c3


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


Comments from Reviewable

@liangfok
Copy link
Contributor

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


Comments from Reviewable

@david-german-tri
Copy link
Contributor Author

Yep, I missed some - thanks! done.


Review status: 1 of 3 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@liangfok
Copy link
Contributor

:lgtm: I tested this on my OSX machine and it works perfectly. Thanks!


Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@david-german-tri
Copy link
Contributor Author

+@jwnimmer-tri platform


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


Comments from Reviewable

@jwnimmer-tri
Copy link
Collaborator

:lgtm:


Review status: all files reviewed at latest revision, 1 unresolved discussion.


a discussion (no related file):
BTW So this is fine, but now that tools/otherlib.BUILD files are using @//tools:linux conditions, they are not really separable from Drake anyway, so I wonder if a deps = ["@//tools:pthread"] (and a cc_library rule with the flag intools/BUILD) in lieu of linkopts=... would be worth it to de-dup the select logic?


Comments from Reviewable

@david-german-tri david-german-tri merged commit 19d6d53 into RobotLocomotion:master Feb 27, 2017
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.

None yet

3 participants