Skip to content

[openmvs] add new port#6505

Merged
Rastaban merged 2 commits intomicrosoft:masterfrom
cenit:dev/cenit/openmvs
May 23, 2019
Merged

[openmvs] add new port#6505
Rastaban merged 2 commits intomicrosoft:masterfrom
cenit:dev/cenit/openmvs

Conversation

@cenit
Copy link
Copy Markdown
Contributor

@cenit cenit commented May 17, 2019

it required fixing cgal along the road. The bug is known upstream CGAL/cgal#3922

@cdcseacave
Copy link
Copy Markdown
Contributor

thx for helping with this @cenit but if you can wait few more days I am about to release a new version

@cenit
Copy link
Copy Markdown
Contributor Author

cenit commented May 17, 2019

We will update it later also ;)
In the meantime I am working on building it. It’s already ok on Linux, I will make it sure it works also on other architectures later

@Rastaban
Copy link
Copy Markdown
Contributor

mpir is failing because our CI machines don't have m4 installed. Because of this the openmvs port is being skipped. I will install that on the machines and rerun the tests.

@cdcseacave
Copy link
Copy Markdown
Contributor

@cenit I see lots of custom steps in the OpenMVS port; can you pls fix the CMake file in OpenMVS rather then doing this steps here and post a PR?

@cenit
Copy link
Copy Markdown
Contributor Author

cenit commented May 17, 2019

There’s also a trace of “all” feature that should disappear... it breaks non-static builds... sorry I pushed in a hurry :(

@cdcseacave not at all, the build went very smooth on Linux. No special step that would be done on your upstream repo better! I will let you know for Windows in case any patch will become necessary!

@Rastaban
Copy link
Copy Markdown
Contributor

I see the following in the CI system:

Starting package 158/158: openmvs:x64-linux
Building package openmvs[core]:x64-linux...
Could not locate cached archive: /home/vcpkg/myagent/_work/3/s/archives/70/702afc6c971622d8dad04a8b3d39a42a6b614569.zip
-- Downloading https://github.com/cdcseacave/openMVS/archive/v0.9.tar.gz...
-- Extracting source /home/vcpkg/myagent/_work/3/s/downloads/cdcseacave-openMVS-v0.9.tar.gz
-- Using source at /home/vcpkg/myagent/_work/3/s/buildtrees/openmvs/src/v0.9-477bc0388b
-- Configuring x64-linux-dbg
-- Configuring x64-linux-rel
-- Building x64-linux-dbg
-- Building x64-linux-rel
CMake Error at scripts/cmake/vcpkg_fixup_cmake_targets.cmake:45 (message):
  
  '/home/vcpkg/myagent/_work/3/s/packages/openmvs_x64-linux/debug/lib/cmake/OpenMVS'
  does not exist.
Call Stack (most recent call first):
  ports/openmvs/portfile.cmake:19 (vcpkg_fixup_cmake_targets)
  scripts/ports.cmake:71 (include)

@cenit
Copy link
Copy Markdown
Contributor Author

cenit commented May 18, 2019

😯 i can’t reproduce it... I have cmake configs in that folder! I will look into it!

@cenit
Copy link
Copy Markdown
Contributor Author

cenit commented May 18, 2019

@cdcseacave I had to lock build to static linking under windows, otherwise the library does not export all requested symbols. I also had to produce a small patch for correct glfw3 integration, which you can import upstream if you want.
If you have any suggestion please let me know

@cdcseacave
Copy link
Copy Markdown
Contributor

I'm not sure I understand. Now it will only work if all vcpkg libs are using x64-windows-static? I think we should find why it does not export needed simbols. Not only that, but the apps should not be built vy vcpkg. The whole point of this is to use OpenMVS as a third party lib, so the apps are not needed.

@cenit
Copy link
Copy Markdown
Contributor Author

cenit commented May 20, 2019

No, it means that openmvs works only if it is built with static linking. Dependencies are fine even if built with dynamic linking.
On the other hand, why not building .exe apps? It is useful to build them, maybe I can wrap them inside a optional [tools] feature, if it’s easy to disable them

Edit: if you want to debug missing symbols, it’s “easy”. Just disable the “only_static_linking” line and observe the results. I fixed apps using boost but not linking with it, which was easy, but then I received complains about the missing export of throw_exception (which you explicitly disabled as inherited by boost but was not replaced in the exports) and then more...
We can work together on it in a PR on your repo, but for sure for you it would be much much easier and much quicker, maybe even before next release?

@cdcseacave
Copy link
Copy Markdown
Contributor

I understand what you mean; indeed, using a lib dynamically on Windows requires explicit exposure of the library content, opposite to linux which exports everything no matter what. I don't think doing this will be easy, so I do not plan implementing it any time soon.

@cenit
Copy link
Copy Markdown
Contributor Author

cenit commented May 20, 2019

so no problem, it just means that OpenMVS will be built statically ;)
we have plenty of ports doing that, it's not a problem really!

@Rastaban Rastaban merged commit 0bcd82c into microsoft:master May 23, 2019
@cenit cenit deleted the dev/cenit/openmvs branch May 31, 2019 12:32
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