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

[osgearth] Fix errors that occur in the osgearth config file on some platforms #26024

Merged
merged 3 commits into from
Aug 4, 2022

Conversation

cbrl
Copy link
Contributor

@cbrl cbrl commented Jul 28, 2022

Describe the pull request

  • What does your PR fix?

    Fixes the exported target in osgEarthConfig.cmake. Though it seems to work on Windows, on other platforms (e.g. Linux) the capitalization of the find_path argument causes the operation to fail, and the lack of the library definition causes an error upon attempting to set that target's properties.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    Unchanged

  • Does your PR follow the maintainer guide?

    Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes

@ghost
Copy link

ghost commented Jul 28, 2022

CLA assistant check
All CLA requirements met.

@Cheney-W Cheney-W added the category:port-bug The issue is with a library, which is something the port should already support label Jul 28, 2022
github-actions[bot]
github-actions bot previously approved these changes Jul 28, 2022
@Cheney-W
Copy link
Contributor

Local testing in progress.

@Cheney-W Cheney-W changed the title Fix errors that occur in the osgearth config file on some platforms [osgearth] Fix errors that occur in the osgearth config file on some platforms Jul 28, 2022
@Cheney-W Cheney-W added the info:reviewed Pull Request changes follow basic guidelines label Jul 29, 2022
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.

Thanks for the PR! LGTM with the one suggestion applied.

ports/osgearth/fix-osgearth-config.patch Outdated Show resolved Hide resolved
@Cheney-W
Copy link
Contributor

Cheney-W commented Aug 1, 2022

versions/o-/osgearth.json#L2
versions/o-/osgearth.json(2,): error : While reading versions for port osgearth from file: version/o-/osgearth.json
File declares version 3.3#1 with SHA: 1198ef439cf8f6851e6b3accbe668c34a46c8ba6
But local port with the same version has a different SHA: 1091743df235c6472d30b157723f82e6d3473cc9
Please update the port's version fields and then run:

vcpkg x-add-version osgearth
git add versions
git commit -m "Update version database"

to add a new version.

@Cheney-W Cheney-W added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Aug 1, 2022
@Cheney-W
Copy link
Contributor

Cheney-W commented Aug 2, 2022

Please don't change port-version twice in one PR, you need to change the value of port-version back to 1, and then execute the following command:

1. <modify file>
2. git add .
3. git commit -m "xxx"
4. .\vcpkg.exe x-add-version osgearth --overwrite-version
5. git add .
6. git commit --amend --no-edit
7. git push

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
@cbrl
Copy link
Contributor Author

cbrl commented Aug 2, 2022

Sorry about that. I amended the modification with the proper port version.

@Cheney-W Cheney-W added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Aug 3, 2022
@BillyONeal BillyONeal dismissed ras0219-msft’s stale review August 4, 2022 22:48

Suggestion was applied

@BillyONeal BillyONeal merged commit 5e77198 into microsoft:master Aug 4, 2022
@BillyONeal
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants