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

[realsense2] Update to v2.19.0 #5777

Merged
merged 5 commits into from
Jun 17, 2019

Conversation

claudiofantacci
Copy link
Contributor

This PR updates librealsense to version 2.19.0.

It is still WIP and proper check should be made, especially because it partially reverts #4564, and in particular this 6afbceb.

Under windows, the debug/bin files were erroneously deleted and the installation failed.

Closes #5753.

@vicroms vicroms self-assigned this Mar 22, 2019
@vicroms
Copy link
Member

vicroms commented Mar 22, 2019

Regression Status (commit 4e6531a)

platform port master test notes
x64-windows realsense2 Pass Fail Regression
x64-windows-static realsense2 Pass Fail Regression
x86-windows realsense2 Pass Fail Regression

x64-osx had no regressions
arm64-windows had no regressions
x64-linux had no regressions
x64-uwp had no regressions
arm-uwp had no regressions

@claudiofantacci
Copy link
Contributor Author

Hi @vicroms, I don't understand the table 😄
Master passed, but test failed and I'm not allowed (error 401) to check vcpkg-PR-Eager.
Can I have an hint to improve the PR?

As a side note, I'd like to change -DCMAKE_DEBUG_POSTFIX=_d in -DCMAKE_DEBUG_POSTFIX=d to be consistent with the majority of libraries present in vcpkg when compiled in debug mode.

@Rastaban Rastaban added the wip label Mar 25, 2019
@Rastaban
Copy link
Contributor

@claudiofantacci, The port is building in the master branch, but fails in the tests run against this PR. Here is the relevent log for x64-windows:

Starting package 1/1: realsense2:x64-windows
Building package realsense2[core]:x64-windows... 
Could not locate cached archive: C:\vsts\_work\2\s\archives\db\db31abea05c8de6a9d6877d3f52e25a8e71dfb3d.zip
-- Downloading https://github.com/IntelRealSense/librealsense/archive/v2.19.0.tar.gz...
-- Extracting source C:/vsts/_work/2/s/downloads/IntelRealSense-librealsense-v2.19.0.tar.gz
-- Using source at C:/vsts/_work/2/s/buildtrees/realsense2/src/v2.19.0-3a2934e71a
-- Configuring x64-windows
CMake Error at scripts/cmake/vcpkg_execute_required_process.cmake:56 (message):
    Command failed: ninja;-v
    Working Directory: C:/vsts/_work/2/s/buildtrees/realsense2/x64-windows-rel/vcpkg-parallel-configure
    Error code: 1
    See logs for more information:
      C:\vsts\_work\2\s\buildtrees\realsense2\config-x64-windows-out.log

Call Stack (most recent call first):
  scripts/cmake/vcpkg_configure_cmake.cmake:269 (vcpkg_execute_required_process)
  ports/realsense2/portfile.cmake:21 (vcpkg_configure_cmake)
  scripts/ports.cmake:71 (include)


Error: Building package realsense2:x64-windows failed with: BUILD_FAILED
Elapsed time for package realsense2:x64-windows: 23.98 s

config-x64-windows-out.log

If you are interested in seeing any other logs don't hesitate to ask.

@vicroms
Copy link
Member

vicroms commented Mar 25, 2019

It is the same problem as PR #5275.
realsense2 tries to obtain libusb using Git instead of using libusb installed by VCPKG.

@claudiofantacci
Copy link
Contributor Author

@vicroms yes, it is one of the problem I'm trying to understand. The issue involves ExternalProject_Add calls in librealsense's CMake/external_libusb.cmake. It works perfectly when used with VS, but fails using ninja build for some strange permission errors. I found a way to let it work with ninja, but it requires a different folder configuration inside the build tree directory. Anyway, why are the tests run with ninja build even if the port is not configured to do so?

@claudiofantacci
Copy link
Contributor Author

I force pushed an update to this PR.
As described in #5753, I added a patch, removed -DBUILD_WITH_TM2=OFF and we now use ninja.
Let me know test reports 👍

Note that I also changed -DCMAKE_DEBUG_POSTFIX=_d to -DCMAKE_DEBUG_POSTFIX=d to agree with all the orther libraries. Is it ok?

@claudiofantacci
Copy link
Contributor Author

It seems that tests are still failing 😞
@Rastaban may I please have a look at the logs?

@Rastaban
Copy link
Contributor

Sure! the regressions are still on x64-windows, x64-windows-static, and x86-windows.
failureLogs.zip

@claudiofantacci
Copy link
Contributor Author

claudiofantacci commented Mar 27, 2019

Errors:

  1. x64-windows: ExternalProject - error: could not find git for clone of libusb.
  2. x64-windows-static: ExternalProject - error: could not find git for clone of libusb.
  3. x86-windows: ExternalProject - error: could not find git for clone of libusb.

The builds are not able to get libusb, which is required to build the library, form the interent using git.
@Rastaban, It is not related to either the library or a CMake error but, I guess, on the build environment for the tests. Are you aware of any possible constraints related to this?

What I can do is to disable a specific option during CMake configuration and apply another patch to the library. I'm discussing this upstream.

@claudiofantacci
Copy link
Contributor Author

Upstream discussion about librealsense issues with Windows: IntelRealSense/librealsense#3597

@claudiofantacci
Copy link
Contributor Author

@Rastaban and @vicroms, do you have any feedback for me about my last post #5777 (comment)?

@claudiofantacci
Copy link
Contributor Author

In the meantime that I'm discussing upstream for Windows fixes, I pushed a new commit with another patch to see if this works (at least temporarily).

ports/realsense2/portfile.cmake Outdated Show resolved Hide resolved
ports/realsense2/portfile.cmake Outdated Show resolved Hide resolved
@claudiofantacci
Copy link
Contributor Author

A brief update on this: all the problems related to the Windows build have been fixed upstream.
As a consequence, with the next stable release (> 2.21.0) the portfile should be a cleaner, stable and with Ninja build system. For a detailed view of the many fixes applied upstream, you can start from IntelRealSense/librealsense#3611.

@vicroms
Copy link
Member

vicroms commented May 17, 2019

@claudiofantacci
Thanks for the update!

@ras0219-msft ras0219-msft assigned grdowns and unassigned vicroms May 21, 2019
@vicroms
Copy link
Member

vicroms commented May 21, 2019

A pre-release tag for v2.22.0 has been created 🎉

@claudiofantacci
Copy link
Contributor Author

Great! I think we can update the port as soon as we have a stable release 👍

@vicroms
Copy link
Member

vicroms commented Jun 12, 2019

Updated port to version 2.22.0

Waiting for CI results.

@claudiofantacci
Copy link
Contributor Author

Updated port to version 2.22.0

Waiting for CI results.

Great!

Please note that upgrading to 2.22.0 the following option no longer exists:

-DBUILD_WITH_TM2=OFF

You can probably check from the CMake log in the CI 👍

@vicroms vicroms merged commit 302c43f into microsoft:master Jun 17, 2019
@vicroms
Copy link
Member

vicroms commented Jun 17, 2019

@claudiofantacci Thanks for your awesome work!

@claudiofantacci claudiofantacci deleted the upd/librealsense branch June 18, 2019 07:35
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.

Update librealsense to v2.19
5 participants