[angle] Rename libs to avoid conflict with system OpenGL. #27701

BillyONeal · 2022-11-08T00:28:17Z

Note that we already rename the headers in

Lines 77 to 96 in e6a79ac

    
           file(GLOB_RECURSE angle_includes "${CURRENT_PACKAGES_DIR}/include") 
        
           file(COPY ${angle_includes} DESTINATION "${CURRENT_PACKAGES_DIR}/include/angle") 
        
           set(_double_files 
        
               include/GLES/egl.h 
        
               include/GLES/gl.h 
        
               include/GLES/glext.h 
        
               include/GLES/glplatform.h 
        
               include/GLES2/gl2.h 
        
               include/GLES2/gl2ext.h 
        
               include/GLES2/gl2platform.h 
        
               include/GLES3/gl3.h 
        
               include/GLES3/gl31.h 
        
               include/GLES3/gl32.h 
        
               include/GLES3/gl3platform.h) 
        
           foreach(_file ${_double_files}) 
        
               if(EXISTS "${CURRENT_PACKAGES_DIR}/${_file}") 
        
                   file(REMOVE "${CURRENT_PACKAGES_DIR}/${_file}") 
        
               endif() 
        
           endforeach()

Also remove what appears to be phantom dependency in qt5-base when not Windows, @Neumann-A reports over Discord https://discord.com/channels/400588936151433218/687365466422902841/1038241263494893649 that they just need any OpenGL implementation.

This hopefully resolves build baseline issues, including but not limited to: https://dev.azure.com/vcpkg/public/_build/results?buildId=80502

REGRESSION: cmake:x64-linux cascaded, but it is required to pass. (/agent/_work/1/s/scripts/azure-pipelines/../ci.baseline.txt).
REGRESSION: qt:x64-linux cascaded, but it is required to pass. (/agent/_work/1/s/scripts/azure-pipelines/../ci.baseline.txt).
REGRESSION: qtbase:x64-linux failed with BUILD_FAILED. If expected, add qtbase:x64-linux=fail to /agent/_work/1/s/scripts/azure-pipelines/../ci.baseline.txt.

@Neumann-A

Note that we already rename the headers in https://github.com/microsoft/vcpkg/blob/e6a79ac0183043994ab21a2c4e0f9cdad8a5421c/ports/angle/portfile.cmake#L77 Also remove what appears to be phantom dependency in qt5-base when not Windows, @Neumann-A reports over Discord https://discord.com/channels/400588936151433218/687365466422902841/1038241263494893649 that they just need any OpenGL implementation.

github-actions

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

ports/angle/vcpkg.json

Valid values for the license field can be found in the documentation

BillyONeal · 2022-11-08T01:11:36Z

To clarify, I believe the baseline issue is that qt et al. try to do a check_source_compiles where they get the system OpenGL headers, but end up linking with ANGLE's OpenGL implementation libraries == linker doom.

Neumann-A · 2022-11-08T08:30:35Z

check_source_compiles where they get the system OpenGL headers, but end up linking with ANGLE's OpenGL implementation libraries == linker doom.

a) It doesn't matter if you use system headers or headers from angle. the OpenGL API is tightly specified so you will basically get the same headers with the same symbols. (Maybe some vendor specific extension are missing but those are typically not used at all)
b) vcpkg builds static libs on linux
c) static libs miss extra linkage so testing them typically requires additional libs like e.g. -lGL -lAngle -lX11 -lXext (or whatever extra libs the linker complains about;)
d) downstream typically assume EGL / GLESv2 to be a shared library which doesn't require c) which is why the checks in https://github.com/qt/qtbase/blob/2ec77f4f11b5e01e25307a068cd3b360b37011e0/cmake/FindGLESv2.cmake#L27 and https://github.com/qt/qtbase/blob/2ec77f4f11b5e01e25307a068cd3b360b37011e0/cmake/3rdparty/extra-cmake-modules/find-modules/FindEGL.cmake#L127 fail for angle
e) Question: Should linking angle the be default for EGL/GLESv2 on linux? Probably not considering that mesa/libglvnd is the default. As such ports in vcpkg shouldn't be able to find angle libs by default.

Neumann-A · 2022-11-08T08:31:17Z

ports/angle/CMakeLists.txt

@@ -625,6 +625,7 @@ list(APPEND LIBGLESV2_SOURCES
 )
 add_library(libGLESv2 ${LIBGLESV2_SOURCES})
 target_link_libraries(libGLESv2 PRIVATE angle::common angle::libANGLE)
+set_property(TARGET libGLESv2 PROPERTY OUTPUT_NAME libGLESv2_angle)


Only on Linux

If so should we also avoid renaming the headers on Windows?

If so should we also avoid renaming the headers on Windows?

If you want to update (opengl|egl)-registry make angle use the officially updated khronos headers and only keep the implementation specific angle headers, feel free to do so but i doubt it is worth the effort ;)

Neumann-A · 2022-11-08T08:33:16Z

ports/angle/CMakeLists.txt

@@ -669,6 +670,7 @@ add_library(libEGL
 )

 target_link_libraries(libEGL PRIVATE angle::common angle::libANGLE libGLESv2)
+set_property(TARGET libEGL PROPERTY OUTPUT_NAME libEGL_angle)


Only on Linux

github-actions

You have modified or added at least one vcpkg.json where you should check the license field.

If you feel able to do so, please consider adding a "license" field to the following files:

ports/angle/vcpkg.json

Valid values for the license field can be found in the documentation

BillyONeal · 2022-11-08T19:15:08Z

a) It doesn't matter if you use system headers or headers from angle. the OpenGL API is tightly specified so you will basically get the same headers with the same symbols. (Maybe some vendor specific extension are missing but those are typically not used at all)

Just because in some cases the headers could match doesn't mean they should be used that way.

d) downstream typically assume EGL / GLESv2 to be a shared library which doesn't require c) which is why the checks in https://github.com/qt/qtbase/blob/2ec77f4f11b5e01e25307a068cd3b360b37011e0/cmake/FindGLESv2.cmake#L27 and https://github.com/qt/qtbase/blob/2ec77f4f11b5e01e25307a068cd3b360b37011e0/cmake/3rdparty/extra-cmake-modules/find-modules/FindEGL.cmake#L127 fail for angle

Thanks for clarifying!

e) Question: Should linking angle the be default for EGL/GLESv2 on linux? Probably not considering that mesa/libglvnd is the default. As such ports in vcpkg shouldn't be able to find angle libs by default.

It indeed seems a system dependency to me.

…ates) (#27444) * [angle] Update to chromium/5249 - Refactor build system based on WebKit's CMake build system for libANGLE - Add maintainer-notes.md * [angle] Fetch & generate more files in portfile * [angle] Move maintainer-notes.md * [angle] Port changes from #27701 * Merge install tweak from #28547 * [angle] Tweak PlatformLinux * Run x-add-version * [angle] Update to chromium/5414 * Run x-add-version * Fix version database. * Fix version database --------- Co-authored-by: Billy Robert O'Neal III <bion@microsoft.com> Co-authored-by: Jonliu1993 <13720414433@163.com>

BillyONeal added category:port-bug The issue is with a library, which is something the port should already support info:internal This PR or Issue was filed by the vcpkg team. category:infrastructure Pertaining to the CI/Testing infrastrucutre labels Nov 8, 2022

github-actions bot previously approved these changes Nov 8, 2022

View reviewed changes

Cheney-W assigned FrankXie05 Nov 8, 2022

Neumann-A reviewed Nov 8, 2022

View reviewed changes

Don't rename on Windows.

7110628

BillyONeal dismissed github-actions[bot]’s stale review via 7110628 November 8, 2022 18:59

github-actions bot approved these changes Nov 8, 2022

View reviewed changes

vicroms approved these changes Nov 8, 2022

View reviewed changes

BillyONeal merged commit 76a79d9 into microsoft:master Nov 8, 2022

BillyONeal deleted the fix-qt branch November 8, 2022 19:49

FrankXie05 added the info:reviewed Pull Request changes follow basic guidelines label Nov 9, 2022

past-due added a commit to past-due/vcpkg that referenced this pull request Jan 27, 2023

[angle] Port changes from microsoft#27701

d33a71f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[angle] Rename libs to avoid conflict with system OpenGL. #27701

[angle] Rename libs to avoid conflict with system OpenGL. #27701

BillyONeal commented Nov 8, 2022

github-actions bot left a comment

BillyONeal commented Nov 8, 2022

Neumann-A commented Nov 8, 2022

Neumann-A Nov 8, 2022

BillyONeal Nov 8, 2022 •

edited

Loading

Neumann-A Nov 8, 2022

Neumann-A Nov 8, 2022

github-actions bot left a comment

BillyONeal commented Nov 8, 2022

	file(GLOB_RECURSE angle_includes "${CURRENT_PACKAGES_DIR}/include")
	file(COPY ${angle_includes} DESTINATION "${CURRENT_PACKAGES_DIR}/include/angle")

	set(_double_files
	include/GLES/egl.h
	include/GLES/gl.h
	include/GLES/glext.h
	include/GLES/glplatform.h
	include/GLES2/gl2.h
	include/GLES2/gl2ext.h
	include/GLES2/gl2platform.h
	include/GLES3/gl3.h
	include/GLES3/gl31.h
	include/GLES3/gl32.h
	include/GLES3/gl3platform.h)
	foreach(_file ${_double_files})
	if(EXISTS "${CURRENT_PACKAGES_DIR}/${_file}")
	file(REMOVE "${CURRENT_PACKAGES_DIR}/${_file}")
	endif()
	endforeach()

[angle] Rename libs to avoid conflict with system OpenGL. #27701

[angle] Rename libs to avoid conflict with system OpenGL. #27701

Conversation

BillyONeal commented Nov 8, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

BillyONeal commented Nov 8, 2022

Neumann-A commented Nov 8, 2022

Neumann-A Nov 8, 2022

Choose a reason for hiding this comment

BillyONeal Nov 8, 2022 • edited Loading

Choose a reason for hiding this comment

Neumann-A Nov 8, 2022

Choose a reason for hiding this comment

Neumann-A Nov 8, 2022

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

BillyONeal commented Nov 8, 2022

BillyONeal Nov 8, 2022 •

edited

Loading