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

Revert "[ROCm][MIGraphX] for googletest dep, set OVERRIDE_FIND_PACKAGE (#16715)" #17523

Merged
merged 2 commits into from
Sep 13, 2023

Conversation

snnn
Copy link
Member

@snnn snnn commented Sep 12, 2023

This reverts commit bb136f8, then re-implement it in a different way.
I reverted the original change, then added a version constraint to the find_package args.

If you still found it picks up wrong gtest version after this change, you may disable find_package by setting 'FETCHCONTENT_TRY_FIND_PACKAGE_MODE' to NEVER. For example, the latest gtest version is 1.14.0. If at a later time Google releases a new version of gtest and that one is incompatible with the ONNX Runtime source code you get today and your dev environment already installed the new version and you do not want to create a new clean build environment that is without the package, you can add --cmake_extra_defines FETCHCONTENT_TRY_FIND_PACKAGE_MODE=NEVER to your build command to solve the problem.

@snnn snnn requested review from cloudhan, a team and pranavsharma September 12, 2023 22:57
@snnn
Copy link
Member Author

snnn commented Sep 12, 2023

@cbourjau, would this change solve the problem you found?

@snnn
Copy link
Member Author

snnn commented Sep 13, 2023

The change I made here is not specific to ROCM/MIGRAPHX EPs.
However, I just realized that the two EPs do require extra caution: they do not work with any find_package usages. :-(

@snnn snnn merged commit 24a3c74 into main Sep 13, 2023
@snnn snnn deleted the snnn/revert3 branch September 13, 2023 05:39
centwang pushed a commit that referenced this pull request Sep 13, 2023
#16715)" (#17523)

This reverts commit bb136f8, then
re-implement it in a different way.
I reverted the original change, then added a version constraint to the
find_package args.

If you still found it picks up wrong gtest version after this change,
you may disable `find_package` by setting
'FETCHCONTENT_TRY_FIND_PACKAGE_MODE' to NEVER. For example, the latest
gtest version is 1.14.0. If at a later time Google releases a new
version of gtest and that one is incompatible with the ONNX Runtime
source code you get today and your dev environment already installed the
new version and you do not want to create a new clean build environment
that is without the package, you can add `--cmake_extra_defines
FETCHCONTENT_TRY_FIND_PACKAGE_MODE=NEVER` to your build command to solve
the problem.
kleiti pushed a commit to kleiti/onnxruntime that referenced this pull request Mar 22, 2024
microsoft#16715)" (microsoft#17523)

This reverts commit bb136f8, then
re-implement it in a different way.
I reverted the original change, then added a version constraint to the
find_package args.

If you still found it picks up wrong gtest version after this change,
you may disable `find_package` by setting
'FETCHCONTENT_TRY_FIND_PACKAGE_MODE' to NEVER. For example, the latest
gtest version is 1.14.0. If at a later time Google releases a new
version of gtest and that one is incompatible with the ONNX Runtime
source code you get today and your dev environment already installed the
new version and you do not want to create a new clean build environment
that is without the package, you can add `--cmake_extra_defines
FETCHCONTENT_TRY_FIND_PACKAGE_MODE=NEVER` to your build command to solve
the problem.
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.

3 participants