Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#39898 closed defect (fixed)

webkit-gtk @2.0.4_0+quartz: build fails on X11/xresource.h code dependencies

Reported by: c.herbig@… Owned by: jeremyhu (Jeremy Huddleston Sequoia)
Priority: Normal Milestone:
Component: ports Version: 2.2.0
Keywords: Cc: dbevans (David B. Evans), cooljeanius (Eric Gallager), joris@…
Port: webkit-gtk

Description

If the user has not installed anything that installs xorg-libx11, then building webkit-gtk +quartz will fail. The first place it fails is here:

:info:build In file included from Source/WebCore/bridge/NP_jsobject.cpp:30:
:info:build In file included from ./Source/WebCore/bridge/NP_jsobject.h:31:
:info:build ./Source/WebCore/bridge/npruntime_internal.h:33:14: fatal error: 'X11/Xresource.h' file not found
:info:build     #include <X11/Xresource.h>
:info:build              ^
:info:build In file included from Source/WebCore/bridge/npruntime.cpp:31:
:info:build ./Source/WebCore/bridge/npruntime_internal.h:33:14: fatal error: 'X11/Xresource.h' file not found
:info:build     #include <X11/Xresource.h>
:info:build              ^
:info:build 1 error generated.

In this first case, the offending #include line can be removed and the build will proceed until the next error:

:info:build In file included from Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:27:
:info:build In file included from ./Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.h:35:
:info:build ./Source/WebCore/plugins/PluginView.h:432:9: error: unknown type name 'Pixmap'
:info:build         Pixmap m_drawable;
:info:build         ^
:info:build ./Source/WebCore/plugins/PluginView.h:433:9: error: unknown type name 'Visual'
:info:build         Visual* m_visual;
:info:build         ^
:info:build ./Source/WebCore/plugins/PluginView.h:434:9: error: unknown type name 'Colormap'
:info:build         Colormap m_colormap;
:info:build         ^
:info:build ./Source/WebCore/plugins/PluginView.h:435:9: error: unknown type name 'Display'
:info:build         Display* m_pluginDisplay;
:info:build         ^
:info:build ./Source/WebCore/plugins/PluginView.h:437:25: error: unknown type name 'XEvent'; did you mean 'Event'?
:info:build         void initXEvent(XEvent* event);
:info:build                         ^~~~~~
:info:build                         Event

Unfortunately, the quick fix of removing the offending lines does not work this time. Installing the port: xorg-libX11 will make the build succeed:

variant quartz {
depends_lib-append port:xorg-libX11
...

However, a real solution ought to be sought in the code itself.

Attachments (7)

main.log.bz2 (50.6 KB) - added by c.herbig@… 11 years ago.
Build fail log for first error
2nd-main.log.bz2 (26.8 KB) - added by c.herbig@… 11 years ago.
Log of second error, after applying 1st fix
Portfile-webkit-gtk.diff (432 bytes) - added by c.herbig@… 11 years ago.
xorg-libX11 quick-fix
patch_quartz-Source-WebCore-bridge-npruntime_internal.h.diff (327 bytes) - added by c.herbig@… 11 years ago.
patch_quartz-Source-WebCore-plugins-PluginView.cpp.diff (418 bytes) - added by c.herbig@… 11 years ago.
patch_quartz-Source-WebCore-plugins-PluginView.h.diff (450 bytes) - added by c.herbig@… 11 years ago.
Portfile-webkit-gtk.2.diff (757 bytes) - added by c.herbig@… 11 years ago.
applies +quartz patches

Download all attachments as: .zip

Change History (36)

comment:1 Changed 11 years ago by larryv (Lawrence Velázquez)

Cc: devans@… added
Owner: changed from macports-tickets@… to jeremyhu@…
Summary: Webkit-gtk +quartz fails on X11/xresource.h code dependencieswebkit-gtk +quartz: build fails on X11/xresource.h code dependencies
  • Please Cc port maintainers (port info --maintainers) when you open tickets.
  • Please include the version of the port in the summary.
  • Please always set the version of MacPorts you are using. Sometimes it’s relevant, sometimes it isn’t. That’s for us to decide.
  • Please always attach a complete log. Don’t assume you know which parts are relevant; we usually end up asking for the full log anyway because it contains a great deal of ancillary information.

comment:2 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)

The fix is not to depend on X11. The fix is to stop webkit-gtk +quartz from using X11.

Looks like bugs are in Source/WebCore/plugins/PluginView.h and Source/WebCore/bridge/npruntime_internal.h

comment:3 in reply to:  1 ; Changed 11 years ago by c.herbig@…

Replying to larryv@…:

  • Please Cc port maintainers (port info --maintainers) when you open tickets.

Sorry, slipped my mind.

  • Please include the version of the port in the summary.

2.0.4_0

  • Please always set the version of MacPorts you are using. Sometimes it’s relevant, sometimes it isn’t. That’s for us to decide.

2.2.0

  • Please always attach a complete log. Don’t assume you know which parts are relevant; we usually end up asking for the full log anyway because it contains a great deal of ancillary information.

I assumed that since the webkit logs end up being so long and large (40+ MB) that you wouldn't want to deal with them.

comment:4 in reply to:  2 Changed 11 years ago by c.herbig@…

Replying to jeremyhu@…:

The fix is not to depend on X11. The fix is to stop webkit-gtk +quartz from using X11.

Looks like bugs are in Source/WebCore/plugins/PluginView.h and Source/WebCore/bridge/npruntime_internal.h

I suppose I should specify, the quick fix of appending the X11 dep will make the port build successfully, but I don't know if it causes any runtime errors. It seems to work well enough for gnome yelp, but I don't know beyond that.

comment:5 in reply to:  3 ; Changed 11 years ago by larryv (Lawrence Velázquez)

Summary: webkit-gtk +quartz: build fails on X11/xresource.h code dependencieswebkit-gtk @2.0.4_0+quartz: build fails on X11/xresource.h code dependencies
Version: 2.2.0

Replying to c.herbig@…:

  • Please always attach a complete log. Don’t assume you know which parts are relevant; we usually end up asking for the full log anyway because it contains a great deal of ancillary information.

I assumed that since the webkit logs end up being so long and large (40+ MB) that you wouldn't want to deal with them.

If a log is overly large, it’s fine to compress it before attaching.

Changed 11 years ago by c.herbig@…

Attachment: main.log.bz2 added

Build fail log for first error

Changed 11 years ago by c.herbig@…

Attachment: 2nd-main.log.bz2 added

Log of second error, after applying 1st fix

comment:6 in reply to:  5 Changed 11 years ago by c.herbig@…

Replying to larryv@…:

If a log is overly large, it’s fine to compress it before attaching.

I've attached the complete logs that were generated.

comment:7 Changed 11 years ago by cooljeanius (Eric Gallager)

Cc: egall@… added

Cc Me!

comment:8 Changed 11 years ago by c.herbig@…

Meanwhile, would you consider adding this patch to the portfile? I know it is not favorable to make it depend on X11, but until a real fix is found, this will at least allow for a working webkit installation.

Changed 11 years ago by c.herbig@…

Attachment: Portfile-webkit-gtk.diff added

xorg-libX11 quick-fix

comment:9 in reply to:  8 ; Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Replying to c.herbig@…:

Meanwhile, would you consider adding this patch to the portfile? I know it is not favorable to make it depend on X11, but until a real fix is found, this will at least allow for a working webkit installation.

No. I expect that will cause problems and would prefer the correct fix.

comment:10 in reply to:  9 ; Changed 11 years ago by c.herbig@…

Replying to jeremyhu@…:

No. I expect that will cause problems and would prefer the correct fix.

That actually has been on my mind, masquerading some subtle bug or something. But how long is it going to take to fix this? Is this something that should be sent to the webkit-gtk developers?

comment:11 in reply to:  10 ; Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Replying to c.herbig@…:

Replying to jeremyhu@…:

No. I expect that will cause problems and would prefer the correct fix.

That actually has been on my mind, masquerading some subtle bug or something. But how long is it going to take to fix this? Is this something that should be sent to the webkit-gtk developers?

They are primarily focused on Linux and thus gtk+quartz is not something that they would likely work on directly. They'll need a patch from the community.

I don't use gtk+quartz either, so this needs to come from one of the maintainers or community members that uses that configuration. Once we have a fix, I'll certainly send it upstream.

comment:12 in reply to:  11 Changed 11 years ago by c.herbig@…

Replying to jeremyhu@…:

They are primarily focused on Linux and thus gtk+quartz is not something that they would likely work on directly. They'll need a patch from the community.

Ironically, that makes this issue more interesting to me, because as I understand it, the big distros are trying to get Wayland up and running as their default environment "soon", and having gtk or qt take care of all the backends rather than having direct calls in the programs themselves. I could be wrong though.

I don't use gtk+quartz either, so this needs to come from one of the maintainers or community members that uses that configuration. Once we have a fix, I'll certainly send it upstream.

I might play with it a bit.

comment:13 Changed 11 years ago by c.herbig@…

The first problem can be fixed by patching out include <X11/Xresource.h>. The second problem is in this code in PluginView.h

#if defined(XP_UNIX) || PLATFORM(GTK)
        void setNPWindowIfNeeded();
#elif defined(XP_MACOSX)
        NP_CGContext m_npCgContext;
        OwnPtr<Timer<PluginView> > m_nullEventTimer;
        NPDrawingModel m_drawingModel;
        NPEventModel m_eventModel;
        CGContextRef m_contextRef;
        WindowRef m_fakeWindow;
#if PLATFORM(QT)
        QPixmap m_pixmap;
#endif

        Point m_lastMousePos;
        void setNPWindowIfNeeded();
        void nullEventTimerFired(Timer<PluginView>*);
        Point globalMousePosForPlugin() const;
        Point mousePosForPlugin(MouseEvent* event = 0) const;
#endif

#if defined(XP_UNIX) && ENABLE(NETSCAPE_PLUGIN_API)
        bool m_hasPendingGeometryChange;
        Pixmap m_drawable;
        Visual* m_visual;
        Colormap m_colormap;
        Display* m_pluginDisplay;

        void initXEvent(XEvent* event);
#endif

So, without changing the code, it seems there needs to be a way to set ENABLE_NETSCAPE_PLUGIN_API=0 and see if that is the only remaining build error. However, I've tried configure.env-append --ENABLE_NETSCAPE_PLUGIN_API=0 but it doesn't actually change it, and that's all I know how to do.

comment:14 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Try changing "defined(XP_UNIX)" in that last block to "PLATFORM(X11)"

comment:15 Changed 11 years ago by c.herbig@…

That is also the keyword in the first X11 build error:

--- Source/WebCore/bridge/npruntime_internal-orig.h	2013-07-19 00:04:01.000000000 -0700
+++ Source/WebCore/bridge/npruntime_internal.h	2013-08-09 17:54:08.000000000 -0700
@@ -30,8 +30,6 @@
 #include "npruntime.h"
 
 #ifdef XP_UNIX
-    #include <X11/Xresource.h>
-
     #undef None
     #undef Above
     #undef Below

Should I try changing that one as well, or just keep Xresource.h out?

comment:16 in reply to:  15 Changed 11 years ago by c.herbig@…

Replying to c.herbig@…:

Should I try changing that one as well, or just keep Xresource.h out?

Actually, I'm going to go ahead and do that.

comment:17 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)

You should change that "#ifdef XP_UNIX" to "#if PLATFORM(X11)" as well

comment:18 in reply to:  17 Changed 11 years ago by c.herbig@…

Replying to jeremyhu@…:

You should change that "#ifdef XP_UNIX" to "#if PLATFORM(X11)" as well

Ok, it built successfully, but I don't know how to test it beyond that. Here is what I have for the Portfile:

# see bug #24622
variant quartz {
    configure.args-append --with-target=quartz

    # TODO: See if this will build with OpenGL.framework
    configure.args-delete --enable-webgl
    depends_lib-delete \
        port:mesa \
        port:xorg-libXt
        

    # quartz-include-widgetbackingstorecairo.patch
    # https://trac.macports.org/ticket/38203
    # https://bugs.webkit.org/show_bug.cgi?id=111598
    patchfiles-append quartz-duplicate-symbols.patch \
                      quartz-include-widgetbackingstorecairo.patch \
                      patch_quartz-Source-WebCore-bridge-npruntime_internal.h.diff \
                      patch_quartz-Source-WebCore-plugins-PluginView.h.diff \
                      patch_quartz-Source-WebCore-plugins-PluginView.cpp.diff
}

I didn't make a diff because I still have things like --disable-spellcheck.

comment:19 Changed 11 years ago by c.herbig@…

It works for webkit-gtk3 too.

comment:20 Changed 11 years ago by c.herbig@…

Finally got back to making a proper .diff for the port file.

Changed 11 years ago by c.herbig@…

Attachment: Portfile-webkit-gtk.2.diff added

applies +quartz patches

comment:21 Changed 11 years ago by c.herbig@…

Has anyone had a chance to look at this yet? Are there any other problems with it? The only thing I can say is that it works well enough to use yelp. Should I test it with Midori or something else?

comment:22 Changed 11 years ago by c.herbig@…

I modified the portfile to use

if {![variant_isset quartz]} {
    default_variants +video
}

to deal with #40282

if there is nothing wrong with this, can the following be committed along with this: patch_quartz-Source-WebCore-bridge-npruntime_internal.h.diff​, patch_quartz-Source-WebCore-plugins-PluginView.cpp.diff​, patch_quartz-Source-WebCore-plugins-PluginView.h.diff​, and the ticket can be closed.

comment:23 Changed 11 years ago by larryv (Lawrence Velázquez)

Cc: joris@… added

Has duplicate #40475.

comment:24 in reply to:  23 Changed 11 years ago by c.herbig@…

Replying to larryv@…:

Has duplicate #40475.

Can we do something about this then?

comment:25 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Do you want to apply for a developer account to maintain it? It would certainly be welcome.

comment:26 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)

Resolution: fixed
Status: newclosed

comment:27 in reply to:  25 Changed 11 years ago by c.herbig@…

Replying to jeremyhu@…:

Do you want to apply for a developer account to maintain it? It would certainly be welcome.

Not sure I'm feeling up to that level of responsibility yet... I still make mistakes with portfiles. Perhaps when I am even more versed in the documentation and correct practices.

comment:28 in reply to:  26 Changed 11 years ago by c.herbig@…

Replying to jeremyhu@…:

r111060

Thank you for committing the changes, and especially for pointing me in the right directions to fix and test the real underlying issue. Wouldn't have been able to do it without such help.

comment:29 Changed 11 years ago by jeremyhu (Jeremy Huddleston Sequoia)

No, thanks for the patches. I'm sorry I didn't notice it was ready 3 weeks ago.

Note: See TracTickets for help on using tickets.