Bug 272623

Summary: games/openttd: never uses sdl2 when built in clean environment
Product: Ports & Packages Reporter: Andrey Zakharchenko <mc>
Component: Individual Port(s)Assignee: Alexey Dokuchaev <danfe>
Status: Closed FIXED    
Severity: Affects Only Me Flags: bugzilla: maintainer-feedback? (danfe)
Priority: ---    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Options to select SDL version manually
none
Options to select SDL version manually (corrected) none

Description Andrey Zakharchenko 2023-07-20 14:59:08 UTC
Created attachment 243505 [details]
Options to select SDL version manually

games/openttd/Makefile checks for installed SDL2 and use it if found, SDL 1.2 is used otherwise.

When built in the clean environment (e.g. using ports-mgmt/synth), SDL2 is never found, and SDL 1.2 is used. So one can't build OpenTTD with SDL2 in the package-building system.

It is possible to use the port's options to force using sdl or sdl2, or leave default autodetection intact.
Comment 1 Andrey Zakharchenko 2023-07-20 15:11:13 UTC
Created attachment 243506 [details]
Options to select SDL version manually (corrected)

Sorry, a typo in the patch.
Comment 2 Alexey Dokuchaev freebsd_committer freebsd_triage 2023-07-21 03:46:54 UTC
> So one can't build OpenTTD with SDL2 in the package-building system.
Right, SDL 1.2 is preferred and that's what pulled when building packages.

> It is possible to use the port's options to force using sdl or sdl2
Maybe, albeit I don't see what's the problem.  If you have SDL2 installed, it will be used, otherwise the port will pull good old SDL 1.2.

> or leave default autodetection intact.
Sorry, I don't undestand.  Autodetection is what the port does now and it works as intended.
Comment 3 Andrey Zakharchenko 2023-07-21 06:14:12 UTC
(In reply to Alexey Dokuchaev from comment #2)
> Maybe, albeit I don't see what's the problem.  If you have SDL2 installed, it will be used, otherwise the port will pull good old SDL 1.2.

I HAVE SDL2 installed. But I build packages for 4 machines with ports-mgmt/synth. Synth makes a clean chroot(8) for building, and it doesn't install SDL2 into that chroot unless it is stated as a dependency, and OpenTTD's Makefile pulls SDL 1.2. And SDL 1.2 is unusable for pure wayland environment without Xwayland layer.

> Sorry, I don't undestand.  Autodetection is what the port does now and it works as intended.

So does it with my patch with the default options. But now I can:
- tell the port to use good old SDL 1.2 even if I have SDL2 installed (e.g. for compatibility reasons);
- tell the port to pull SDL2 (e.g. for wayland support) even in synth or poudrier clean building environment.
Comment 4 Jan Beich freebsd_committer freebsd_triage 2023-07-22 12:19:00 UTC
(In reply to Alexey Dokuchaev from comment #2)
> Right, SDL 1.2 is preferred and that's what pulled when building packages.

Looks contrary to upstream intention. OpenTTD added SDL2 in 1.10.0 and made it default (for plain/vendor build and bundled recipes for Windows, Debian, openSUSE). OpenTTD switched to CMake in 1.11.0 but kept SDL2 as default. According to bug 245253 the port kept SDL12 only per maintainer's discretion thus insincere to say in passive voice.

CMakeLists.txt:

            find_package(SDL2)
            if(NOT SDL2_FOUND)
                find_package(SDL)
            endif()

config.lib:

		sdl2_config=""
		if [ -x "`which sdl2-config`" ]; then
			detect_pkg_config "$with_sdl" "sdl2" "sdl2_config" "2.0"
		fi
		if [ -z "$sdl2_config" ]; then
			detect_pkg_config "$with_sdl" "sdl" "sdl_config" "1.2"
		fi

> Autodetection is what the port does now and it works as intended.

Autodetection in ports (unlike vendor build) is discouraged (see ports 7cffd3c8f908 or ports 4da13b8f8e61) and gets in the way of reproducible builds (see D24586).

(In reply to Andrey Zakharchenko from comment #3)
> And SDL 1.2 is unusable for pure wayland environment without Xwayland layer.

devel/sdl12-compat can be used as a workaround. However, sdl12-compat may be more buggy than native sdl2 support and currently requires sdl12 during build to expose API and avoid package conflict due to missing provides/requires in the ports framework.
Comment 5 commit-hook freebsd_committer freebsd_triage 2024-02-22 11:11:44 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=b274c8918b4f9bda160326dc0233524975acd031

commit b274c8918b4f9bda160326dc0233524975acd031
Author:     Alexey Dokuchaev <danfe@FreeBSD.org>
AuthorDate: 2024-02-22 11:09:56 +0000
Commit:     Alexey Dokuchaev <danfe@FreeBSD.org>
CommitDate: 2024-02-22 11:09:56 +0000

    games/openttd: update OpenTTD to the latest version 13.4

    - By popular demand, prefer SDL 2.x by default but leave the option
      to use the previous stable version
    - Drop incomplete work-around for strange Clang bug added in commit
      f2488f960dc8: the problem is not specific to PowerPC but versions
      13.1 and later no longer trigger it (pending investigation)

    PR:             272428, 272623
    Reported by:    portscout, pkg-fallout
    Clang bug:      https://github.com/llvm/llvm-project/issues/49093

 games/openttd/Makefile                   | 30 +++++++++---------------------
 games/openttd/distinfo                   |  6 +++---
 games/openttd/files/patch-CMakeLists.txt | 14 ++++++++++----
 3 files changed, 22 insertions(+), 28 deletions(-)