Skip to content

[osgearth] Fix x64-windows-static-md#15375

Merged
vicroms merged 26 commits intomicrosoft:masterfrom
ankurvdev:ankurv/osgearth
Mar 25, 2021
Merged

[osgearth] Fix x64-windows-static-md#15375
vicroms merged 26 commits intomicrosoft:masterfrom
ankurvdev:ankurv/osgearth

Conversation

@ankurvdev
Copy link
Contributor

Describe the pull request

  • Fix osgearth for x64-windows-static-md

@JackBoosY JackBoosY self-assigned this Dec 30, 2020
@JackBoosY JackBoosY added the category:port-bug The issue is with a library, which is something the port should already support label Dec 30, 2020
@ankurvdev ankurvdev requested a review from JackBoosY January 4, 2021 06:07
@JackBoosY JackBoosY added the category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. label Jan 6, 2021
@JackBoosY JackBoosY added depends:upstream-changes Waiting on a change to the upstream project requires:author-response and removed requires:author-response labels Jan 11, 2021
@ankurvdev ankurvdev marked this pull request as draft January 14, 2021 20:05
@ankurvdev ankurvdev marked this pull request as ready for review January 16, 2021 05:41
@JackBoosY
Copy link
Contributor

Please run command vcpkg x-add-version osgearth then commit the changes.

Thanks.

@JackBoosY
Copy link
Contributor

Waiting for upstream approval.

@plevy
Copy link
Contributor

plevy commented Jan 20, 2021

@ankurverma85 Does this build successfully with -DOSGEARTH_BUILD_TOOLS=OFF? For me, I get an error on Release build:
CMake Error at ports/osgearth/portfile.cmake:60 (file):
file must be called with at least two arguments.
Call Stack (most recent call first):
scripts/ports.cmake:136 (include)
Error: Building package osgearth:x64-windows failed with: BUILD_FAILED

The debug build is successful, but for release, I needed to set -DOSGEARTH_BUILD_TOOLS=ON
Perhaps just set -DOSGEARTH_BUILD_TOOLS=ON as the default as that is a more useful case for most people.

Are you able to successfully build osgearth:x64-windows-static or osgearth:x64-windows-static-md? Are you using Windows and if so, what version of Visual Studio?

@JackBoosY JackBoosY added requires:author-response and removed depends:upstream-changes Waiting on a change to the upstream project labels Jan 21, 2021
@ankurvdev
Copy link
Contributor Author

@ankurverma85 Does this build successfully with -DOSGEARTH_BUILD_TOOLS=OFF? For me, I get an error on Release build:
CMake Error at ports/osgearth/portfile.cmake:60 (file):
file must be called with at least two arguments.
Call Stack (most recent call first):
scripts/ports.cmake:136 (include)
Error: Building package osgearth:x64-windows failed with: BUILD_FAILED

The debug build is successful, but for release, I needed to set -DOSGEARTH_BUILD_TOOLS=ON
Perhaps just set -DOSGEARTH_BUILD_TOOLS=ON as the default as that is a more useful case for most people.

I suspect its your cmake that might be old that could be causing the issue. . Could you try with a fresh clone of vcpkg ?

Some build tools dont quite built properly in x64-windows-static-md and will require patches into the osgearth itself.
Given its not really in the scope of vcpkg libs to use those anyways. I'm disabling building those here.

Are you able to successfully build osgearth:x64-windows-static or osgearth:x64-windows-static-md? Are you using Windows and if so, what version of Visual Studio?

Yes. VS 2019 16.8.4
I'll be turning on the CI-Pipeline build for osgearth in the vcpkg PR pipeline so hopefully any issues should show up

@JackBoosY
Copy link
Contributor

@gwaldron Can you please review this PR?

Thanks.

@JackBoosY
Copy link
Contributor

CMake Error at ports/osgearth/portfile.cmake:62 (file):
  file must be called with at least two arguments.
Call Stack (most recent call first):
  scripts/ports.cmake:128 (include)


Error: Building package osgearth:x64-windows failed with: BUILD_FAILED

@JackBoosY
Copy link
Contributor

LGTM, will rerun the pipeline test after #16088 merged.

@ankurvdev
Copy link
Contributor Author

LGTM,
@JackBoosY,
Thanks for driving the right set of fixes across different ports

@JackBoosY
Copy link
Contributor

CMake Error at CMakeModules/FindOSG.cmake:134 (FIND_LIBRARY):
  Could not find UUID_LIBRARY_DEBUG using the following names: uuid, libuuid
Call Stack (most recent call first):
  C:/a/1/s/scripts/buildsystems/vcpkg.cmake:653 (_find_package)
  CMakeLists.txt:130 (find_package)

Needs re-patch FindOSG.cmake.

@ankurvdev
Copy link
Contributor Author

@JackBoosY ,
Havent seen an update on this for a while and neither on the dependent PR (#16088)
Can you maybe summarize whats the chain of dependencies here ?
Seems like this could be stuck for a while

Thanks again for driving the right set of fixes

@JackBoosY
Copy link
Contributor

@ankurverma85 I was on holiday before, waiting for re-fix PR #16088.

@ankurvdev
Copy link
Contributor Author

ankurvdev commented Feb 24, 2021

@JackBoosY , Not sure how we're tracking on this .
But given this has been hanging around for about a month now.

Would you be open to revert this to last successful state but without the osgearth_tools being included. ?

This will atleast unblock the triplet builds for osgearth: x64-windows-static-md and the triplet will be built and tested in CI/CD pipelines

It can also be successfully consumed in an external project (with a few hacks, that i believe wont be required after your fixes)

you can then drive the right set of changes as a follow up with the triplet being unblocked

@JackBoosY
Copy link
Contributor

@ankurverma85 I understand your thoughts very well, but before this PR, the feature tools can be built successfully.
If the feature tools is removed, some users will lose this feature.
Please wait a while.

@ankurvdev
Copy link
Contributor Author

I totally support and agree what you are saying.
In addition to it being a regression, that fact that we cannot link the static-md libs into an exe and get undefined-ref errors is proof of the different bugs plaguing this port, and so I'm happy to wait

But just for the sake of argument (feel free to ignore if you'd still like me to wait and am happy to, because again, i am convinced you're right):

  1. VCPKG is for libs and headers. Tools are primarily codegen tools like protobuf but the output most is libs and headers. So in some sense while its a regression, perhaps, not that serious in light of what it enables. The tools with osgearth are mostly display tools not really programming tools.
  2. I've tested the libs to link with an exe when used. So in some sense the link errors are not because the libs are wrongly generated, its because the cmake scripting that links the libs into the exe is wrong. So they're good to be used in a developer script with a few additonal lines of hackiness
  3. It enables osgearth:x64-windows-static-md builds and thereby testing all its other dependent ports for the triplet.

@JackBoosY
Copy link
Contributor

cc @ras0219 @ras0219-msft

@ankurvdev
Copy link
Contributor Author

@JackBoosY , I see there's been some progress on this in the last week but I'm still not fully sure what are the series of dependencies that need to come together to get this working.
So I'd like to compile a list of open Pull-Requests and issues blocking this.

Pull-Requests
#16088 (GDAL)
-- #16500 (GDAL)

Open Issues ?

Is GDAL #16088 the only blocking dependency for this ?
Are there other pull requests?
And do we know what are the cascading dependencies for 16088 (gdal) (open issues/pullrequests)?

Are there any open issues that are not under review/pull requests ?

I'm currently using vcpkg from waaay back with my last successful build for the time being.
But if there's some fixes available (in WIP Pull-Requests) that get this working I'd like to start using atleast locally

If not, i'd atleast like to track the list of work items to see when that might be possible

@JackBoosY
Copy link
Contributor

This PR doesn't depend on #16500, we need to patch vcpkg-cmake-wrapper to add the correct dependencies to the port using ffmpeg.
Sorry to stop this PR for so long, I will solve this problem as soon as possible.

@ankurvdev
Copy link
Contributor Author

@JackBoosY I made some scoped changes on top of what you were working on to get things into a clean "all-checks-passing" state
Given this has been hanging around for quite some time.
Can we perhaps get this in as-is and follow this up with any clean-ups you might want. ?

@JackBoosY JackBoosY added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed depends:different-pr This PR or Issue depends on a PR which has been filed labels Mar 21, 2021
@JackBoosY
Copy link
Contributor

@ankurverma85 Thanks for your hard work! I think this PR will not depend on other PR now.

@ankurvdev
Copy link
Contributor Author

@JackBoosY , Can we get this merged ?

@JackBoosY
Copy link
Contributor

ping @vicroms

@vicroms vicroms merged commit 5a10163 into microsoft:master Mar 25, 2021
@ankurvdev ankurvdev deleted the ankurv/osgearth branch March 25, 2021 20:48
@autoantwort
Copy link
Contributor

Now the pipeline fails for osgearth for x64_linux, x64_windows, x64_windows_static and x64_windows_static_md as you can see in #16914, #16879, #16913, #16910, ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. category:port-bug The issue is with a library, which is something the port should already support info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[osg, osgearth] x64-windows-static-md build failure

6 participants