Use pkg-config instead of sdl2-config
ClosedPublic
Actions

Authored by s0600204 on Jun 18 2018, 9:17 PM.

Details

Reviewers

echotangoecho

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP21865: Use pkg-config instead of sdl2-config

Trac Tickets

#5157
#5152

Summary

Resolves build issue on Arch linux.

As noted in the attached trac tickets, setting the environment variable SDL2_CONFIG to pkg-config sdl2 appears to work as a work-around to the issue reported in both the tickets and on the forums (https://wildfiregames.com/forum/index.php?/topic/24180-icu-and-stdlib-error-svn/). (It is also the solution/work-around used by the Arch package maintainer.)

Looking into why it works, we can see the outputs of both sdl2-config --cflags (which is used if the environmental variable is not set) and the work-around:

$> sdl2-config --cflags
-I/usr/include/SDL2 -I/usr/include -D_REENTRANT

$> pkg-config sdl2 --cflags
-D_REENTRANT -I/usr/include/SDL2

The missing /usr/include from the second output (the absence of which makes the compile work), is a "standard system include" and thus is implicitly included in the includes-search-list. When sdl2-config explicitly includes it, it appears to be added to the includes-search-list too early, thus causing an issue with gcc's #include_next macro.

Test Plan

This will need testing on various Linux distros. (And on BSD if anyone uses that...)

It should not need testing on OSX, as update_workspaces.sh sets the SDL2_CONFIG environment variable on that system.
It will also not need testing on Windows as there is no change to the logic-path used for that system.

Run make clean, update-workspaces.sh, then make in the appropriate folders.

Running clean-workspaces.sh should not be necessary.

(Alternatively, instead of running make clean, delete the ./build/workspaces/gcc/obj/scriptinterface_Release folder, to just rebuild that module.)

Please report linux distro name & version, gcc version, and whether you're using the premake4 or premake5 script.

Diff Detail

Repository

rP 0 A.D. Public Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

s0600204 created this revision.Jun 18 2018, 9:17 PM

Owners added a subscriber: Restricted Owners Package.Jun 18 2018, 9:17 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/655/display/redirect

Harbormaster completed remote builds in B6255: Diff 6764.Jun 18 2018, 9:22 PM

elexis awarded a token.Jun 18 2018, 11:38 PM

Works:

Ubuntu 16.04.4 LTS 64-Bit
gcc version 5.4.0 20160609
using premake5 ✔

@Imarok Good to know, thanks!

lua4 file too?

Compiled for wes-fole-dog too on Parabola GNU/Linux.

In D1582#63842, @elexis wrote:

lua4 file too?

Hmm. premake4 doesn't have this problem, so changes are not required to get it to compile.

Our lua code for premake4 takes the output from sdl2-config and relays the -I{path} bits unchanged.

Our lua code for premake5 takes the same input and essentially translates -I{path} to -isystem {path}. This behaviour might be - if not incorrect then - perhaps questionable, but is done to fix an apparent problem with OSX if it doesn't happen: https://code.wildfiregames.com/D72#16402. Maybe @wraitii could clarify further.

Either way, adding -I/usr/include or -isystem /usr/include to the includes search path is incorrect. The former causes gcc (when set to "verbose") to emit a warning[^1][^2], whilst the latter causes the system includes to be in the wrong order leading to a failed compile.

Although its not strictly necessary (as it works as-is), I'll make the change (so we're "more correct", and so a warning (that doesn't appear anyway) never has cause to be emitted).

[^1] - "ignoring duplicate directory "/usr/include" as it is a non-system directory that duplicates a system directory"

[^2] - Incidentally, the same warning is given concerning "/usr/local/include", which is added manually in premake[4|5].lua in the function project_add_x11_dirs. To be honest, most of the directory entries added by this function cause gcc (when in "verbose" mode) to emit "ignoring nonexistent directory {dir}" warnings. I think this function could possibly be removed and the x11 entry in the extern_libs[4|5].lua expanded with calls to pkg-config.