Skip to content

[bgfx, bx, bimg] Add new ports#9163

Closed
Outurnate wants to merge 5 commits intomicrosoft:masterfrom
Outurnate:master
Closed

[bgfx, bx, bimg] Add new ports#9163
Outurnate wants to merge 5 commits intomicrosoft:masterfrom
Outurnate:master

Conversation

@Outurnate
Copy link
Copy Markdown
Contributor

This is a resubmit of #7763 with some changes

  • GENiE common logic has been moved to a common file
  • tinystl includes the BSD license it's under with its include directory. This is a separate project, but it has the same license as bx

@PhoebeHui PhoebeHui requested a review from JackBoosY December 2, 2019 07:49
Comment thread ports/bgfx/portfile.cmake Outdated
Comment thread ports/bgfx/portfile.cmake Outdated
Comment thread ports/bimg/portfile.cmake Outdated
Comment thread ports/bimg/portfile.cmake Outdated
Comment thread ports/bx/portfile.cmake Outdated
Comment thread ports/bx/portfile.cmake Outdated
Comment thread scripts/cmake/vcpkg_configure_genie.cmake Outdated
@grdowns grdowns self-assigned this Dec 2, 2019
@grdowns grdowns changed the title [bgfx] New ports: bgfx, bx, bimg [bgfx, bx, bimg] Add new ports Dec 2, 2019
@Outurnate
Copy link
Copy Markdown
Contributor Author

All the necessary changes were made. I took the time to update the commit refs from the upstream. I've also added a step to include the shader compiler tool which is necessary for projects using bgfx

There is a patch file included - I've submitted bkaradzic/bgfx#1958 to upstream this

@Outurnate Outurnate requested a review from JackBoosY December 3, 2019 02:23
@Outurnate
Copy link
Copy Markdown
Contributor Author

Can anyone provide some guidance on retrieving logs from Azure DevOps? One of the steps seems to reference storing the results in a container, but I can't figure out where that is

Comment thread ports/bgfx/portfile.cmake
endif()

# Post-build test for cmake libraries
vcpkg_test_cmake(PACKAGE_NAME ${PORT})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vcpkg_test_cmake is now disabled in vcpkg, please remove this line.

Comment thread ports/bimg/portfile.cmake
vcpkg_install_cmake(TARGET bimg_encode/all)
vcpkg_install_cmake(TARGET bimg_decode/all)
# GENie does not generate an install target, so we install explicitly
file(INSTALL "${SOURCE_DIR}/include/bimg" DESTINATION "${CURRENT_PACKAGES_DIR}/include")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer to add a patch to fix these installation issues instead of adding the installation code directly in portfile.cmake.

Comment thread ports/bx/portfile.cmake
)
vcpkg_install_cmake(TARGET bx/all)
# GENie does not generate an install target, so we install explicitly
file(INSTALL
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer to add a patch to fix these installation issues instead of adding the installation code directly in portfile.cmake.

@JackBoosY
Copy link
Copy Markdown
Contributor

JackBoosY commented Dec 3, 2019

@Outurnate You can click the Details link and view the log in Test Modified Port.

Linux error:

CMake Error at ports/bx/portfile.cmake:13 (vcpkg_configure_genie):
  vcpkg_configure_genie Function invoked with incorrect arguments for
  function named: vcpkg_configure_genie

@Outurnate
Copy link
Copy Markdown
Contributor Author

@JackBoosY upstream has declined to apply the patch. The CMake files generated by GENie don't link shaderc & friends to libdl. According to the maintainer of bgfx and vcpkg, CMake generation isn't supported (ref bgfx#1958)

Appreciate the pointer to the detailed logs. Since this change represents a total rewrite, I'll be closing this PR and resubmitting

@Outurnate Outurnate closed this Dec 17, 2019
@Outurnate Outurnate mentioned this pull request Dec 19, 2019
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