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

[sdl2-ttf] Update to 2.20.0 #25803

Closed
wants to merge 45 commits into from

Conversation

Thomas1664
Copy link
Contributor

@Thomas1664 Thomas1664 commented Jul 16, 2022

Describe the pull request

I took over this PR because the old version of sdl2-ttf included a CVE.

@Thomas1664 Thomas1664 marked this pull request as draft July 16, 2022 11:07
github-actions[bot]
github-actions bot previously approved these changes Jul 16, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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/sdl2pp/vcpkg.json

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

github-actions[bot]
github-actions bot previously approved these changes Jul 16, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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/sdl2pp/vcpkg.json

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

ports/sdl2-ttf/fix-find_dependencies.patch Outdated Show resolved Hide resolved
ports/sdl2-ttf/fix-find_dependencies.patch Outdated Show resolved Hide resolved
ports/sdl2-ttf/portfile.cmake Outdated Show resolved Hide resolved
ports/sdl2-ttf/portfile.cmake Outdated Show resolved Hide resolved
ports/sdl2-ttf/portfile.cmake Outdated Show resolved Hide resolved
ports/sdl2pp/fix-dependencies.patch Show resolved Hide resolved
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for sdl2pp have changed but the version was not updated
version: 0.16.1#3
old SHA: 968a4398351ee7c92eff8c7faa589a29bc37c29c
new SHA: 08d7511a09215e789ecc2089da277064d8856c75
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

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/sdl2pp/vcpkg.json

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

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for tgui have changed but the version was not updated
version: 2021-04-19#4
old SHA: 736c8cb50817d42dcc0c89e48500a0d0a4e324db
new SHA: 358a25ecf600e7c91b8f485bff5316c66687ff84
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***
error: checked-in files for sdl2pp have changed but the version was not updated
version: 0.16.1#3
old SHA: 968a4398351ee7c92eff8c7faa589a29bc37c29c
new SHA: 10d63d562dd2ab5cb84b83032eaf46c678a158e5
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

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/sdl2pp/vcpkg.json

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

@Thomas1664
Copy link
Contributor Author

@dg0yt Is tgui CMake suggestion wrong? I don't think the TGUI_X11 part is needed.

tgui provides CMake targets:
    # this is heuristically generated, and may not be correct
    find_package(TGUI CONFIG REQUIRED)
    target_link_libraries(main PRIVATE tgui TGUI_X11)

@dg0yt
Copy link
Contributor

dg0yt commented Jul 16, 2022

@dg0yt Is tgui CMake suggestion wrong? I don't think the TGUI_X11 part is needed.

Welcome to vcpkg known issues: #20190.
If you are looking for potential work items: #20190 (comment)
(This is more important than premature string perfomance micro-optimizations IMO.)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for tgui have changed but the version was not updated
version: 2021-04-19#4
old SHA: 736c8cb50817d42dcc0c89e48500a0d0a4e324db
new SHA: 358a25ecf600e7c91b8f485bff5316c66687ff84
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***
error: checked-in files for sdl2pp have changed but the version was not updated
version: 0.16.1#3
old SHA: 968a4398351ee7c92eff8c7faa589a29bc37c29c
new SHA: 612a3062d45a1aeefdbb00018590e3feb0e7d49b
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

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/sdl2pp/vcpkg.json

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

@Thomas1664
Copy link
Contributor Author

@dg0yt I need some help with how to correctly import SDL2_ttf:

sdl2pp

LINK Pass 1: command "C:\PROGRA~1\MICROS~1\2022\ENTERP~1\VC\Tools\MSVC\1432~1.313\bin\Hostx64\x64\link.exe CMakeFiles\SDL2pp.dir\SDL2pp\AudioDevice.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\AudioLock.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\AudioSpec.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Color.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Exception.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Point.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\RWops.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Rect.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Renderer.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\SDL.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Surface.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\SurfaceLock.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Texture.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\TextureLock.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Wav.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Window.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\SDLTTF.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Font.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\SDLImage.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Chunk.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Mixer.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\Music.cc.obj CMakeFiles\SDL2pp.dir\SDL2pp\SDLMixer.cc.obj /out:SDL2pp.dll /implib:SDL2pp.lib /pdb:SDL2pp.pdb /dll /version:8.3 /machine:x64 /nologo /debug /INCREMENTAL -LIBPATH:D:\installed\x64-windows\debug\lib D:\installed\x64-windows\debug\lib\SDL2_image.lib SDL2::SDL2_ttf.lib D:\installed\x64-windows\debug\lib\SDL2_mixer.lib D:\installed\x64-windows\debug\lib\SDL2d.lib Winmm.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTFILE:CMakeFiles\SDL2pp.dir/intermediate.manifest CMakeFiles\SDL2pp.dir/manifest.res" failed (exit code 1104) with the following output:
LINK : fatal error LNK1104: cannot open file 'SDL2::SDL2_ttf.lib'
ninja: build stopped: subcommand failed.

tgui

LINK Pass 1: command "C:\PROGRA~1\MICROS~1\2022\ENTERP~1\VC\Tools\MSVC\1432~1.313\bin\Hostx64\x64\link.exe @CMakeFiles\tgui.rsp /out:lib\tgui-d.dll /implib:lib\tgui-d.lib /pdb:lib\tgui-d.pdb /dll /version:0.9 /machine:x64 /nologo /debug /INCREMENTAL /MANIFEST /MANIFESTFILE:src\CMakeFiles\tgui.dir/intermediate.manifest src\CMakeFiles\tgui.dir/manifest.res" failed (exit code 1104) with the following output:
LINK : fatal error LNK1104: cannot open file 'SDL2_ttf.lib'
ninja: build stopped: subcommand failed.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for tgui have changed but the version was not updated
version: 2021-04-19#4
old SHA: 736c8cb50817d42dcc0c89e48500a0d0a4e324db
new SHA: 358a25ecf600e7c91b8f485bff5316c66687ff84
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***
error: checked-in files for sdl2pp have changed but the version was not updated
version: 0.16.1#3
old SHA: 968a4398351ee7c92eff8c7faa589a29bc37c29c
new SHA: c873c58d9affb77b15ee26fc08aef6a614d8e8ef
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

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/sdl2pp/vcpkg.json

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

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for tgui have changed but the version was not updated
version: 2021-04-19#4
old SHA: 736c8cb50817d42dcc0c89e48500a0d0a4e324db
new SHA: 358a25ecf600e7c91b8f485bff5316c66687ff84
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***
error: checked-in files for sdl2pp have changed but the version was not updated
version: 0.16.1#3
old SHA: 968a4398351ee7c92eff8c7faa589a29bc37c29c
new SHA: 59937851b307aed4705edc50749539566cbbc51b
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

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/sdl2pp/vcpkg.json

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

FrankXie05
FrankXie05 previously approved these changes Jul 21, 2022
@FrankXie05 FrankXie05 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jul 21, 2022
Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

I still think the PR needs to be improved.
My comments are just hidden in "resolved" conversations... but not resolved IMO.

ports/sdl2-ttf/fix-find_dependencies.patch Outdated Show resolved Hide resolved
ports/sdl2-ttf/fix-find_dependencies.patch Outdated Show resolved Hide resolved
ports/sdl2pp/fix-dependencies.patch Show resolved Hide resolved
ports/sdl2pp/fix-dependencies.patch Show resolved Hide resolved
ports/tgui/fix-dependencies.patch Outdated Show resolved Hide resolved
@FrankXie05 FrankXie05 removed the info:reviewed Pull Request changes follow basic guidelines label Jul 21, 2022
github-actions[bot]
github-actions bot previously approved these changes Jul 21, 2022
@kleszcz
Copy link
Contributor

kleszcz commented Jul 21, 2022

Hi! First of all thank you for updating SDL TTF to 2.20.0. I don't see in new port files any dependency on HarfBuzz. SDL TTF has optional support for HarfBuzz since 2.0.18 but while this version wasn't ported in vcpkg it's easy to miss it. Can you add it as a feature in vcpkg port?

github-actions[bot]
github-actions bot previously approved these changes Jul 21, 2022
github-actions[bot]
github-actions bot previously approved these changes Jul 25, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

error: checked-in files for sdl2pp have changed but the version was not updated
version: 0.16.1#4
old SHA: 0d0efed99dd39ca3bbf35b1b601b7aec4a82b55b
new SHA: 6ce1ada2f6c7eb64cfc6307884abaa1456f5ccaa
Did you remember to update the version or port version?
Use --overwrite-version to bypass this check
***No files were updated***

@dg0yt
Copy link
Contributor

dg0yt commented Jul 26, 2022

Please take ports/sdl2pp/fix-dependencies.patch from master, and apply only the sdl2-ttf changes.

Comment on lines +5 to +39
@@ -164,6 +164,7 @@ set(PC_REQUIRES)
set(BUILD_SHARED_LIBS OFF)

if(SDL2TTF_HARFBUZZ)
+ if(0)
if(SDL2TTF_HARFBUZZ_VENDORED)
message(STATUS "${PROJECT_NAME}: Using vendored harfbuzz library")
# HB_BUILD_UTILS variable is used by harfbuzz
@@ -190,11 +191,14 @@ if(SDL2TTF_HARFBUZZ)
find_package(harfbuzz REQUIRED)
list(APPEND PC_REQUIRES harfbuzz)
endif()
+ endif()
+ find_package(harfbuzz REQUIRED)
target_compile_definitions(SDL2_ttf PRIVATE TTF_USE_HARFBUZZ=1)
target_link_libraries(SDL2_ttf PRIVATE harfbuzz::harfbuzz)
endif()

if(SDL2TTF_FREETYPE)
+ if(0)
if(SDL2TTF_FREETYPE_VENDORED)
message(STATUS "${PROJECT_NAME}: Using vendored freetype library")
# FT_DISABLE_ZLIB variable is used by freetype
@@ -232,7 +236,11 @@ if(SDL2TTF_FREETYPE)
find_package(Freetype REQUIRED)
list(APPEND PC_REQUIRES freetype2)
endif()
+ endif()
+ list(APPEND PC_REQUIRES freetype2)
+ find_package(Freetype REQUIRED)
target_link_libraries(SDL2_ttf PRIVATE Freetype::Freetype)
+
endif()

# Restore BUILD_SHARED_LIBS variable
Copy link
Contributor

Choose a reason for hiding this comment

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

The patching can be reduced to the pkg-config stuff by passing SDL2TTF_VENDORED=OFF via OPTIONS.

@Thomas1664
Copy link
Contributor Author

I close this PR because due to misleading/ incomplete code review and avoidable merge conflicts I'm no longer willing to work on this.

Maybe @FrankXie05 can take over this PR again.

@Thomas1664 Thomas1664 closed this Jul 27, 2022
@dg0yt
Copy link
Contributor

dg0yt commented Jul 27, 2022

I pick this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-update The issue is with a library, which is requesting update new revision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[sdl2-ttf] update to 2.20.0
4 participants