microsoft / vcpkg

C++ Library Manager for Windows, Linux, and MacOS
MIT License
23.26k stars 6.41k forks source link

[angelscript] include the addons sources #42074

Open deadlocklogic opened 6 days ago

deadlocklogic commented 6 days ago

Is your feature request related to a problem? Please describe.

Related to #32024. Current implementation copies the feature addon files to the installed directory but doesn't include them in the build system.

Proposed solution

This is heuristically generated, and may not be correct. Change this: https://github.com/microsoft/vcpkg/blob/813a241fb83adad503a391facaa6aa634631accc/ports/angelscript/portfile.cmake#L25-L28

if("addons" IN_LIST FEATURES)
     file(INSTALL "${SOURCE_PATH}/add_on/" DESTINATION "${CURRENT_PACKAGES_DIR}/include/angelscript" FILES_MATCHING PATTERN "*.h" PATTERN "*.cpp")

     file(GLOB_RECURSE ADDON_SOURCES "${CURRENT_PACKAGES_DIR}/include/angelscript/*.cpp")
     list(REMOVE_ITEM ADDON_SOURCES "${CURRENT_PACKAGES_DIR}/include/angelscript/autowrapper/generator/generateheader.cpp")
     target_sources(${ANGELSCRIPT_LIBRARY_NAME} PRIVATE ${ADDON_SOURCES})
 endif()

Describe alternatives you've considered

No response

Additional context

No response

FrankXie05 commented 4 days ago

@deadlocklogic IMO, this library does not have a feature in the cmake CMakeLists.txt. This feature addon is only applicable to vcpkg to meet the issue https://github.com/microsoft/vcpkg/issues/32024 . And the feature addon is not compiled in the upstream code. Add_on only exists as a folder of tool functions.

dg0yt commented 4 days ago

@deadlocklogic Neither this issue nor #32024 explain how the extra sources are expected to be used. There is no point in changing anything if nobody can explain how the change can be verified.

deadlocklogic commented 4 days ago

So basically the addons feature is practically useless in its current form, there is no point copying sources to the installation directory if there aren't included in the build system. All I am suggesting is adding the sources to the CMake target in order to link them later as a dependency, otherwise link errors will occur rendering this feature unusable. @FrankXie05 the addons folder contains modules of helpful boilerplate code, many of them are a must have. @dg0yt:

@deadlocklogic Neither this issue nor https://github.com/microsoft/vcpkg/issues/32024 explain how the extra sources are expected to be used. There is no point in changing anything if nobody can explain how the change can be verified.

I gave a suggestion on how these sources could be included.

dg0yt commented 4 days ago

I gave a suggestion on how these sources could be included.

32024 also gave a vague suggestion, and you see the result.

Your suggestion is untested code which mixes portfile script mode (if("addons" IN_LIST FEATURES)) with CMake project mode (target_sources(...)). It is bound to fail.

But my question was not for how to achieve a particular installation effect, but for what kind of effect is intented by upstream. If they don't add it to a library, how are users expected to make use of it? Which observable (testable) result is expected from the port?