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.

Sign up for GitHub

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

Jump to bottom

[vcpkg.cmake] fix find_package not restoring variables correctly. #27689

Merged
merged 8 commits into from
Nov 16, 2022
Merged

[vcpkg.cmake] fix find_package not restoring variables correctly. #27689

merged 8 commits into from
Nov 16, 2022

Conversation

Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Nov 7, 2022 •
edited
Loading

closes #27498
closes #19030

github-actions[bot]
github-actions bot previously approved these changes Nov 7, 2022
View reviewed changes
github-actions[bot]
github-actions bot previously approved these changes Nov 7, 2022
View reviewed changes
@Neumann-A
Copy link
Contributor Author

@BillyONeal: another angle related failure in x64-linux here. ninja: error: '/mnt/vcpkg-ci/installed/x64-linux/lib/libEGL.a', needed by 'exe', missing and no known rule to make it.

@dg0yt: I assume you already encountered that one.

@LilyWangLL LilyWangLL added the category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) label Nov 7, 2022
@Neumann-A
Copy link
Contributor Author

@BillyONeal: I also request a full rebuild here. (which will automatically resolve the x64-linux error)

@Neumann-A Neumann-A mentioned this pull request Nov 7, 2022
Closed
@BillyONeal BillyONeal mentioned this pull request Nov 8, 2022
Merged
@LilyWangLL LilyWangLL added the info:reviewed Pull Request changes follow basic guidelines label Nov 14, 2022
BillyONeal
BillyONeal approved these changes Nov 14, 2022
View reviewed changes
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

@ras0219-msft This LGTM, can you double check?

@BillyONeal
Copy link
Member

Note to self: If Robert doesn't merge by EOD Wednesday, merge.

ras0219-msft
ras0219-msft approved these changes Nov 16, 2022
View reviewed changes
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

LGTM, I think we could use slightly shorter variable names but all the logic looks correct.

Thank you very much for fixing this issue!

@ras0219-msft ras0219-msft merged commit 90b616f into microsoft:master Nov 16, 2022
@Neumann-A Neumann-A deleted the args_find_package branch November 16, 2022 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BillyONeal BillyONeal BillyONeal approved these changes

@ras0219-msft ras0219-msft ras0219-msft approved these changes

@github-actions github-actions[bot] github-actions[bot] approved these changes

@LilyWangLL LilyWangLL LilyWangLL approved these changes

Assignees

@LilyWangLL LilyWangLL

Labels
category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
4 participants
@Neumann-A @BillyONeal @ras0219-msft @LilyWangLL