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

[rbdl] Add new port #13884

Merged
merged 29 commits into from Oct 12, 2020
Merged

[rbdl] Add new port #13884

merged 29 commits into from Oct 12, 2020

Conversation

RDCH106
Copy link
Contributor

@RDCH106 RDCH106 commented Oct 5, 2020

RBDL - Rigid Body Dynamics Library

  • What does your PR fix?

    • This PR adds a new port for the RDBL for Windows and Linux. It builds the library downloading it from official repository and making compilation through CMake according to Packaging Zipfiles guide.

    • The port only builds the core library and not the optional addons (future work for features).

    • Port was tested in vcpkg 2020.07 with RBDL 2.6.0

  • Which triplets are supported/not supported?

    • Expected supported triplets: Triplets tested by CI baseline
  • Have you updated the CI baseline?

    • Not updated the CI baseline.
  • Does your PR follow the maintainer guide?

@ghost
Copy link

ghost commented Oct 5, 2020

CLA assistant check
All CLA requirements met.

@RDCH106 RDCH106 marked this pull request as ready for review October 6, 2020 11:56
@NancyLi1013 NancyLi1013 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Oct 9, 2020
ports/rbdl/CONTROL Show resolved Hide resolved
ports/rbdl/CONTROL Outdated Show resolved Hide resolved
ports/rbdl/portfile.cmake Outdated Show resolved Hide resolved
ports/rbdl/portfile.cmake Outdated Show resolved Hide resolved
ports/rbdl/portfile.cmake Outdated Show resolved Hide resolved
ports/rbdl/portfile.cmake Outdated Show resolved Hide resolved
ports/rbdl/portfile.cmake Outdated Show resolved Hide resolved
scripts/ci.baseline.txt Outdated Show resolved Hide resolved
RDCH106 and others added 6 commits October 9, 2020 10:19
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
ports/rbdl/portfile.cmake Outdated Show resolved Hide resolved
RDCH106 and others added 3 commits October 9, 2020 11:08
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
@RDCH106
Copy link
Contributor Author

RDCH106 commented Oct 9, 2020

I am going to update Supports field. It works perfectly for Windows and I am using on Windows.
Linux and OSX remains as future work. ok?

@RDCH106
Copy link
Contributor Author

RDCH106 commented Oct 9, 2020

I don't understand... 🤔

The previous build was: https://dev.azure.com/vcpkg/public/_build/results?buildId=43808&view=logs&j=d3670d0c-cc82-50c9-762c-eccde036829f

And the new one: https://dev.azure.com/vcpkg/public/_build/results?buildId=43818&view=logs&j=a83b7983-e539-5449-9e09-337ff5f52283

  • Between 2 builds starts failing x86-windows when in the previous one was marked as success
  • I exclude linux and osx in Supports field and using vcpkg_fail_port_install... they continue failing in CI

@NancyLi1013 any idea or recommendation about that?

Thanks in advance.

@NancyLi1013
Copy link
Contributor

Hi @RDCH106
I checked the build results here https://dev.azure.com/vcpkg/public/_build/results?buildId=43808&view=logs&j=d3670d0c-cc82-50c9-762c-eccde036829f and found that:

The failures on Linux and osx are due to this:

c++: error: /bigobj: No such file or directory

Since /bigobj is only used in MSVC.

The failures on uwp are due to this:

CMake Error at ports/rbdl/portfile.cmake:29 (file):
  file COPY cannot find "D:/buildtrees/rbdl/x64-uwp-dbg/rbdl.dll": No error.
Call Stack (most recent call first):
  scripts/ports.cmake:79 (include)

Generally, CMake will install the targets to the related directory automatically and we don't need to handle it manually.
So this part might not be needed.

if(VCPKG_LIBRARY_LINKAGE STREQUAL dynamic)
    if(VCPKG_TARGET_IS_WINDOWS)
        if(NOT DEFINED VCPKG_BUILD_TYPE OR VCPKG_BUILD_TYPE STREQUAL "debug")
            file(COPY ${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg/rbdl.dll DESTINATION ${CURRENT_PACKAGES_DIR}/debug/bin)
        endif()
        if(NOT DEFINED VCPKG_BUILD_TYPE OR VCPKG_BUILD_TYPE STREQUAL "release")
            file(COPY ${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-rel/rbdl.dll DESTINATION ${CURRENT_PACKAGES_DIR}/bin)
        endif()
    endif()
endif()

You can make a patch to add
RUNTIME DESTINATION ${CMAKE_INSTALL_LIBDIR} to install targets in dynamic build.

Based on the above situation, I think this port supports all platform. So we can remove Supports field now and try to fix the failures and rebuild this.

Note: You can merge the master branch to this PR when you do the changes.

ports/rbdl/portfile.cmake Outdated Show resolved Hide resolved
ports/rbdl/portfile.cmake Outdated Show resolved Hide resolved
@RDCH106
Copy link
Contributor Author

RDCH106 commented Oct 10, 2020

Just one more question to be more self-reliant next time. The error due /bigobj only supported by MSVC is not showed here: https://dev.azure.com/vcpkg/public/_build/results?buildId=43808&view=logs&j=d3670d0c-cc82-50c9-762c-eccde036829f

imagen

But how can I inspect /mnt/vcpkg-ci/buildtrees/rbdl/install-x64-linux-dbg-out.log? Is it any way to access the log produced by the CI to see that file?

Thanks in advance and sorry for my many questions

RDCH106 and others added 2 commits October 10, 2020 09:49
…patch.diff

Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
RDCH106 and others added 3 commits October 10, 2020 09:58
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
@RDCH106
Copy link
Contributor Author

RDCH106 commented Oct 10, 2020

@NancyLi1013 removing Supports field and vcpkg_fail_port_install function produces global failure. It have no sense for me...

@RDCH106
Copy link
Contributor Author

RDCH106 commented Oct 10, 2020

I begin to understand... The successfully builds are because they are skipping CMake building 😅, but I don't know how to resolve it because have no idea how to see CMake logs in Azure DevOps.

@NancyLi1013
Copy link
Contributor

Hi @RDCH106

The failures are caused by the patch. It seems you didn't update the patch. So It failed to apply.

CMake Error at scripts/cmake/vcpkg_apply_patches.cmake:54 (message):
  Applying patch failed.  error: corrupt patch at line 16

Call Stack (most recent call first):
  scripts/cmake/vcpkg_extract_source_archive_ex.cmake:141 (vcpkg_apply_patches)
  scripts/cmake/vcpkg_from_github.cmake:139 (vcpkg_extract_source_archive_ex)
  ports/rbdl/portfile.cmake:7 (vcpkg_from_github)
  scripts/ports.cmake:79 (include)

You need to make a new patch.

@NancyLi1013
Copy link
Contributor

@RDCH106
Copy link
Contributor Author

RDCH106 commented Oct 10, 2020

I think it's ready 😁

@NancyLi1013, thank you again for your huge job and patient with me.

@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Oct 12, 2020
@NancyLi1013
Copy link
Contributor

Thanks for your efforts to this PR. @RDCH106
LGTM now.

@dan-shaw dan-shaw merged commit 30d39c8 into microsoft:master Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants