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

Enhancements for Dependent packages check advisor #54

Open
Tracked by #55
NingW101 opened this issue Dec 26, 2023 · 2 comments
Open
Tracked by #55

Enhancements for Dependent packages check advisor #54

NingW101 opened this issue Dec 26, 2023 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@NingW101
Copy link
Contributor

image

Currently, dep_check advisor will just remove the find_package(XXX) statement but ignore other operations related to this emscripten ported library , which may cause the errors block the building process. We should make enhancements to avoid this problem.

  • For emscripten ported libraries (as shown in the above pictures), webinizer will add -sUSE_LIBXXX into the compiler & linker flags, and emscripten will take responsibilities of linking libraries and including header files directories, so except for find_package statement, the include_directories(${XX_INCLUDE_DIRS}), target_link_libraries(target ${XX_LIBRARIES}) and other operations related to this library are not necessary any more.
  • For the dependencies added from webinizer UI, should also remove the find_package(DEP) statement from the main project, besides, should adjust the statement pattern to target_link_libraries(target -ldepName)
@NingW101 NingW101 mentioned this issue Dec 26, 2023
3 tasks
@NingW101 NingW101 added the enhancement New feature or request label Dec 26, 2023
@NingW101
Copy link
Contributor Author

NingW101 commented Jan 15, 2024

For emscripten ported libraries (as shown in the above pictures), webinizer will add -sUSE_LIBXXX into the compiler & linker flags, and emscripten will take responsibilities of linking libraries and including header files directories, so except for find_package statement, the include_directories(${XX_INCLUDE_DIRS}), target_link_libraries(target ${XX_LIBRARIES}) and other operations related to this library are not necessary any more.

enhanced with #60

Solution updates:

Following the action for removing find_package(XXX REQUIRED) and the action for adding -s USE_XXX flag, we will add action to make file change to remove the ${XXX_LIBRARIES}) and ${XXX_INCLUDE_DIRS} strings in the build_root/CMakeLists.txt file.

Most of these related statements went well after delete like the modification target_include_directories( ${SDL2_INCLUDE_DIRS} src ) --> target_include_directories( src), except the statement string(STRIP ${SDL2_LIBRARIES} SDL2_INCLUDE_DIRS) -> string(STRIP SDL2_INCLUDE_DIRS) will throw the error as following in our projects testing journey so far.

  CMake Error at CMakeLists.txt:18 (string):
     string sub-command STRIP requires two arguments.

To handle this error, on the base of dep_check advisor enhancement, we enhance the stripAdvisor at the same time to handle this cmake error.

Updates

image

@NingW101 NingW101 self-assigned this Jan 15, 2024
@NingW101
Copy link
Contributor Author

For the dependencies added from webinizer UI, should also remove the find_package(DEP) statement from the main project, besides, should adjust the statement pattern to target_link_libraries(target -ldepName)

There are some common scenarios that the find_package(XXX REQUIRED) is defined in sub-cmake file and included in main cmakelists.txt file, and this package is also added as dependency in main projects. In this case, find_package statement (in sub-cmake file) is unnecessary anymore and should be removed or commented, otherwise it will lead to errors occurring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant