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

[fcl] Explicity handle FCL_USE_X64_SSE CMake option #11406

Merged
merged 2 commits into from
May 20, 2020

Conversation

traversaro
Copy link
Contributor

The FCL_USE_X64_SSE option is enabled by default in fcl, but it only make sense to enable SSE flags when targeting either x86 or x64.

Related upstream PR: flexible-collision-library/fcl#470 .

  • What does your PR fix? There is no relative issue.

  • Which triplets are supported/not supported? Have you updated the CI baseline? The CI baseline should not change.

  • Does your PR follow the maintainer guide? Yes

The FCL_USE_X64_SSE option is enabled by default in fcl,
but it only make sense to enable SSE flags when targeting
either x86 or x64.
Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@traversaro, thanks for the PR!

Could you also update the Version to '0.6.0-1' in CONTROL file?

@traversaro
Copy link
Contributor Author

@traversaro, thanks for the PR!

Could you also update the Version to '0.6.0-1' in CONTROL file?

Done in traversaro@a02bab8 .

Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the udpates!

@PhoebeHui PhoebeHui added info:reviewed Pull Request changes follow basic guidelines and removed waiting for response labels May 19, 2020
@PhoebeHui
Copy link
Contributor

The CI failed on 'git checkout' step, it doesn't related to this change, @strega-nil, could you help have a look?

@strega-nil
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strega-nil
Copy link
Contributor

Cools, LGTM! Thanks @traversaro :)

I'll wait for the CI to finish and then merge.

@PhoebeHui
Copy link
Contributor

The CI failures doesn't relate to this changes.
Protobuf with arm/uwp failed due to it requires x86-windows protoc to be available, this is expected, PR #11459 would fix it.

Failures:
CMake Error at ports/protobuf/portfile.cmake:21 (message):
Cross-targetting protobuf requires the x86-windows protoc to be available.
Please install protobuf:x86-windows first.

@strega-nil
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strega-nil strega-nil merged commit 3b79a2d into microsoft:master May 20, 2020
@strega-nil
Copy link
Contributor

Alright, it passed! Thanks @traversaro :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants