Skip to content

GCC11/Libstdc++11 Compilation fixes #7599

Merged
snnn merged 4 commits intomicrosoft:masterfrom
ankurvdev:gcc11fixes
May 10, 2021
Merged

GCC11/Libstdc++11 Compilation fixes #7599
snnn merged 4 commits intomicrosoft:masterfrom
ankurvdev:gcc11fixes

Conversation

@ankurvdev
Copy link
Copy Markdown
Contributor

@ankurvdev ankurvdev commented May 6, 2021

Fixes bug # 7583

@ankurvdev ankurvdev requested a review from a team as a code owner May 6, 2021 21:35
@ankurvdev ankurvdev mentioned this pull request May 6, 2021
Closed
@ytaous ytaous requested a review from snnn May 6, 2021 22:20
@snnn
Copy link
Copy Markdown
Contributor

snnn commented May 7, 2021

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux CPU x64 NoContribops CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, MacOS NoContribops CI Pipeline

@snnn
Copy link
Copy Markdown
Contributor

snnn commented May 7, 2021

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule, orttraining-ortmodule-distributed

snnn
snnn previously approved these changes May 7, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@snnn snnn left a comment

Choose a reason for hiding this comment

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

LGTM.

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented May 7, 2021

Azure Pipelines successfully started running 9 pipeline(s).

1 similar comment
@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented May 7, 2021

Azure Pipelines successfully started running 9 pipeline(s).

@snnn
Copy link
Copy Markdown
Contributor

snnn commented May 7, 2021

How about this: you exclude the changes on cmake/CMakeLists.txt and let us merge the other changes first. Then we handle the gtest issue separately.

@snnn
Copy link
Copy Markdown
Contributor

snnn commented May 7, 2021 •
edited
Loading

For example, if we update our gtest version to the latest ,probably the warning will be gone. But I know there will be other issues.

@ankurvdev
Copy link
Copy Markdown
Contributor Author

ankurvdev commented May 7, 2021

For example, if we update our gtest version to the latest ,probably the warning will be gone. But I know there will be other issues.

Sounds good i'll revert back the cmake changes.
Also , Yes i can confirm . It goes away
I just wasnt sure how open you'll be to the prospect of moving to a non-release floating commit from a release commit of gtest

@snnn
Copy link
Copy Markdown
Contributor

snnn commented May 10, 2021

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux CPU x64 NoContribops CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux Nuphar CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, MacOS NoContribops CI Pipeline

@snnn
Copy link
Copy Markdown
Contributor

snnn commented May 10, 2021

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows WebAssembly CI Pipeline, orttraining-amd-gpu-ci-pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule, orttraining-ortmodule-distributed

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented May 10, 2021

Azure Pipelines successfully started running 9 pipeline(s).

@snnn
Copy link
Copy Markdown
Contributor

snnn commented May 10, 2021

I just wasnt sure how open you'll be to the prospect of moving to a non-release floating commit from a release commit of gtest

I think it's fine. We don't ship gtest.

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented May 10, 2021

Azure Pipelines successfully started running 9 pipeline(s).

@snnn snnn merged commit de4089f into microsoft:master May 10, 2021
@snnn snnn mentioned this pull request May 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@snnn snnn snnn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ankurvdev @snnn