[SRU] SDL_syswm.h can't find mir_toolkit/mir_client_library.h

Bug #1306629 reported by Scott Howard
40
This bug affects 7 people
Affects Status Importance Assigned to Milestone
Mir
Invalid
Undecided
Unassigned
libsdl2 (Ubuntu)
Fix Released
High
Brandon Schaefer
Trusty
Fix Released
High
Unassigned
Utopic
Fix Released
High
Brandon Schaefer

Bug Description

I believe something is wrong with MIR support. A daily build of openmw just failed with:

/usr/include/SDL2/SDL_syswm.h:97:44: fatal error: mir_toolkit/mir_client_library.h: No such file or directory
 #include <mir_toolkit/mir_client_library.h>

In SDL_syswm.h:
#if defined(SDL_VIDEO_DRIVER_MIR)
#include <mir_toolkit/mir_client_library.h>
#endif

the file is:
/usr/include/mirclient/mir_toolkit/mir_client_library.h
http://packages.ubuntu.com/search?searchon=contents&keywords=mir_client_library.h&mode=exactfilename&suite=trusty&arch=any

openmw buildlog:
https://launchpadlibrarian.net/172588668/buildlog_ubuntu-trusty-i386.openmw_0.29%2Bgit20140411.77-0~ubuntu14.04.1_FAILEDTOBUILD.txt.gz

it built successfully until the fix for bug #1295389 libsdl2 commit hit that enabled mir.

Assigning "high" since it has the potentially to cause a large number of FTBFS in the archive.

SRU Info:

*Note* This patch has landed in libsdl2 upstream:
https://hg.libsdl.org/SDL/rev/ede9d13dad21
(which also reverts https://hg.libsdl.org/SDL/rev/274017846e73)

1) The issue is we include mir_client_library.h, meaning everything that compiles sdl2 apps will need to include this as well. So we need to forward declare the structs so we don't need to include the mir header.
2) A simple way to test the current Mir problem:
http://paste.ubuntu.com/7271077/

3) Regression potential: Low - upstream has had this change for a month without problem

Related branches

Revision history for this message
Scott Howard (showard314) wrote :

ok, so it looks like libsdl2 has to update/use pkgconfig and pass those headers.

This is related to:
https://bugs.launchpad.net/mir/+bug/1161064
https://bugs.launchpad.net/mir/+bug/1161240

I don't know what's the best way to handle this - many packages in the archive don't know about mir, and SDL just pulled it in - and SDL isn't telling the build process where MIR is located. There isn't enough time to find them all and fix it before trusty is released.

Since we're so close to an LTS release, should MIR support be dropped from SDL2?

Revision history for this message
Scott Howard (showard314) wrote :

More info:
1) packages which use cmake (FindSDL.cmake) will FTBFS since they will not know to add includes for mir
2) packages which use pkgconfig will FTBFS since SDL2 doesn't add a requires on mir or adds mir includes to Cflags:

# sdl pkg-config source file

prefix=/usr
exec_prefix=${prefix}
libdir=${prefix}/lib/i386-linux-gnu
includedir=${prefix}/include

Name: sdl2
Description: Simple DirectMedia Layer is a cross-platform multimedia library designed to provide low level access to audio, keyboard, mouse, joystick, 3D hardware via OpenGL, and 2D video framebuffer.
Version: 2.0.2
Requires:
Conflicts:
Libs: -L${libdir} -lSDL2
Libs.private: -lSDL2 -lpthread -Wl,--no-undefined -lm -ldl -lasound -lm -ldl -lpthread -lpulse-simple -lpulse -lX11 -lXext -lXcursor -lXinerama -lXi -lXrandr -lXss -lXxf86vm -lwayland-egl -lwayland-client -lwayland-cursor -lxkbcommon -lts -lpthread -lrt
Cflags: -I${includedir}/SDL2 -D_REENTRANT

Revision history for this message
Scott Howard (showard314) wrote :

update: it looks like only one package depends on libsdl2 in trusty, so this might not be as important. However, it is a problem that SDL requires headers from Mir but SDL does not tell buildsystems the locations of these headers nor passes the correct -I required when built with Mir support.

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in libsdl2 (Ubuntu):
status: New → Confirmed
Revision history for this message
Dennis (dennis.guse) wrote :

I have the same problem (building PJSIP with video-support for packaging it on Trusty):

  /usr/include/SDL2/SDL_syswm.h:97:44: fatal error: mir_toolkit/mir_client_library.h: No such file or directory

Is there a plan to fix the problem?

Changed in libsdl2 (Ubuntu):
assignee: nobody → Brandon Schaefer (brandontschaefer)
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Problem is how mir installes its headers, which isn't the real issue. Its just atm, its looking for <mir_toolkit/mir_client_library.h> from /usr/include, which its in /usr/include/mirclient/mir_toolkit/mir_client_lilbrary.h.

So a fix is to ensure the sdl2.pc file includes the MIR_CFLAGS, only when its enabled.

http://paste.ubuntu.com/7270442/

Ill look into creating a patch to get this fixed ASAP.

Sorry!

Changed in libsdl2 (Ubuntu):
status: Confirmed → In Progress
Changed in mir:
status: New → Invalid
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

So landed the fix upstream, will need to get a patch for 14.04. (So the fix will get to you soon!)

https://hg.libsdl.org/SDL/rev/274017846e73

summary: - SDL_syswm.h can't find mir_toolkit/mir_client_library.h
+ [SRU] SDL_syswm.h can't find mir_toolkit/mir_client_library.h
description: updated
Revision history for this message
Benjamin Xiao (ben-r-xiao) wrote :

Any good workarounds or proposed package that we can install?

Revision history for this message
Scott Howard (showard314) wrote :

A) Workaround is to add: "-I/usr/include/mirclient -I/usr/include/mircommon" to your compiler options.

B) Something is still odd here. Mir support was enabled in SDL2. You now have to install libmirclient-dev and mircommon-dev and explicitly add includes to mir header files in your program in order to compile any SDL2 program, even if you don't use mir or have mir installed. Sure, Mir will be the default next release - but that's next release, and trusty was supposed to be an LTS release.

It's like having to install java in order to compile anything that uses libc. (Hyperbole, but similar idea).

Enabling Mir support is great, but requiring developers to explicitly include headers they don't know about and won't use seems odd.

Revision history for this message
Benjamin Xiao (ben-r-xiao) wrote :

Thanks I'll try the workaround.

It does make sense for them to include Mir support though even if it's not released yet. Developers have to do testing on Mir and Mir is still an installable option on Ubuntu 14.04. SDL, along with many other programs, have support for various things compiled in. Not all of them are used, but they are supported and therefore have to be inside the compiled code.

Revision history for this message
Scott Howard (showard314) wrote :

I agree with everything Benjamin said, but including support for Mir is one thing and making developers include headers is something different.

Look at wayland, for example. SDL2 does not require you to include wayland headers, nor x11 headers - why does Mir require you to include Mir headers when I want to compile a program that uses SDL2? What makes Mir different?

Revision history for this message
Scott Howard (showard314) wrote :

That's my problem:
SDL2 has wayland, Mir, and x11 support. As a developer that uses SDL2, I now have to explicitly include Mir headers but not wayland nor x11. What's wrong with Mir that it makes us do that?

This is what it is:
https://bugs.launchpad.net/mir/+bug/1161064
https://bugs.launchpad.net/mir/+bug/1161240

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

A quick workaround: http://paste.ubuntu.com/7301381/

If you edit the header file your self, all should work. (just be careful :)

This is the real fix, I should have used forward declaration here. (Which I will be fixing that upstream asap, as well as a patch for 14.04).

Revision history for this message
Benjamin Xiao (ben-r-xiao) wrote :

Yes Scott, you're absolutely correct. SDL2 users shouldn't have to include Mir headers explicitly.

Thanks Brandon, I'll test the workaround you posted later tonight.

description: updated
Revision history for this message
Scott Howard (showard314) wrote :

Thanks!
Using
http://paste.ubuntu.com/7271077/
alone I was able to build openmw on trusty.

Revision history for this message
Benjamin Xiao (ben-r-xiao) wrote :

Can confirm that the patch works nicely! Thanks!

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Cool, sorry about that. Thanks for poking the bug to get the right fix :)

Changed in libsdl2 (Ubuntu):
status: In Progress → Triaged
milestone: none → trusty-updates
Revision history for this message
Darren Salt (dsalt) wrote :

This bug is biting regarding Unvanquished. For now, I'm working around it by build-depending on this:

 libsdl2-dev (<< 2.0.2+dfsg1-3ubuntu1) | libsdl2-dev (>> 2.0.2+dfsg1-3ubuntu1) | libsdl1.2-dev

Once I see that this bug is fixed, I'll revert that change.

Revision history for this message
Martin Pitt (pitti) wrote :

Utopic fix uploaded.

Changed in libsdl2 (Ubuntu Trusty):
milestone: none → trusty-updates
Changed in libsdl2 (Ubuntu Utopic):
milestone: trusty-updates → none
status: Triaged → Fix Committed
Revision history for this message
Martin Pitt (pitti) wrote :

Trusty SRU uploaded to review queue. Unsubscribing sponsors.

Changed in libsdl2 (Ubuntu Trusty):
status: New → In Progress
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package libsdl2 - 2.0.2+dfsg1-3ubuntu2

---------------
libsdl2 (2.0.2+dfsg1-3ubuntu2) utopic; urgency=medium

  * New patch: mir_forward_declaration_syswm.diff
    - Forward declare structs so you don't need mir headers
      (LP: #1306629)
 -- Brandon Schaefer <email address hidden> Thu, 01 May 2014 13:03:23 -0400

Changed in libsdl2 (Ubuntu Utopic):
status: Fix Committed → Fix Released
Chris J Arges (arges)
description: updated
description: updated
description: updated
Revision history for this message
Chris J Arges (arges) wrote :

I'm a bit confused I don't see the changes to configure.in as mentioned in:
https://hg.libsdl.org/SDL/rev/274017846e73

This is what the SRU Info above mentioned, yet the patch is different. Can either the SRU template be adjusted (if the patch uploaded is correct) or let us know if the current upload needs to be reworked before it is accepted.

Revision history for this message
Scott Howard (showard314) wrote :

The patch you showed was incorrect, it was corrected by a later patch:
https://hg.libsdl.org/SDL/rev/ede9d13dad21

I'll update the SRU template to reflect that.

description: updated
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Very sorry, forgot certain parts of the bug!

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Thank you for updating the info!

Revision history for this message
Scott Kitterman (kitterman) wrote : Please test proposed package

Hello Scott, or anyone else affected,

Accepted libsdl2 into trusty-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/libsdl2/2.0.2+dfsg1-3ubuntu1.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in libsdl2 (Ubuntu Trusty):
status: In Progress → Fix Committed
tags: added: verification-needed
Revision history for this message
Scott Howard (showard314) wrote :

This works, thanks everyone

tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package libsdl2 - 2.0.2+dfsg1-3ubuntu1.1

---------------
libsdl2 (2.0.2+dfsg1-3ubuntu1.1) trusty-proposed; urgency=medium

  * New patch: mir_forward_declaration_syswm.diff
    - Forward declare structs so you don't need mir headers
      (LP: #1306629)
 -- Brandon Schaefer <email address hidden> Thu, 01 May 2014 13:03:23 -0400

Changed in libsdl2 (Ubuntu Trusty):
status: Fix Committed → Fix Released
Revision history for this message
Scott Kitterman (kitterman) wrote : Update Released

The verification of the Stable Release Update for libsdl2 has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

Mathew Hodson (mhodson)
Changed in libsdl2 (Ubuntu Trusty):
importance: Undecided → High
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.