Opened 8 years ago

Closed 8 years ago

#50199 closed defect (fixed)

gnuplot: opportunistically uses gtk2 when building with wxwidgets enabled

Reported by: dbevans (David B. Evans) Owned by: mojca (Mojca Miklavec)
Priority: Normal Milestone:
Component: ports Version:
Keywords: Cc: sfeam@…
Port: gnuplot

Description (last modified by dbevans (David B. Evans))

I ran into this issue when testing p5-graphics-gnuplotif.

When building using the +wxwidgets variant (default), gnuplot will use gtk2 if available and enable "gdk/gtk tweaks" to the configuration. This could be fixed by adding a dependency on gtk2 but that may not be what you want here. The alternative seems to be patching or otherwise overriding configure.in to disable this behavior -- there's no configuration option to do this for you.

Here's the relevant section of configure.in:

if test "${enable_wxwidgets_ok}" = yes ; then
  WX_CXXFLAGS="`$WX_CONFIG --cxxflags | sed 's/-fno-exceptions//'` $CAIROPANGO_CFLAGS"
  WX_LIBS="`$WX_CONFIG --libs` $CAIROPANGO_LIBS"

  dnl Check for fork(), used for the 'persist' effect
  AC_FUNC_FORK

  dnl Check for gtk (raise/lower tweaks)
  PKG_CHECK_MODULES(GTK, [gtk+-2.0], have_gtk=yes, have_gtk=no)
  if  test "${have_gtk}" = yes ; then
    AC_DEFINE(HAVE_GTK, 1, [Define to use gtk/gdk tweaks])
    WX_CXXFLAGS="$WX_CXXFLAGS $GTK_CFLAGS"
    WX_LIBS="$WX_LIBS $GTK_LIBS"
  fi

Change History (10)

comment:1 Changed 8 years ago by dbevans (David B. Evans)

Description: modified (diff)

comment:2 Changed 8 years ago by mojca (Mojca Miklavec)

Cc: sfeam@… added

I'm CC-ing one of the gnuplot developers. I'll test and submit a report upstream. (It' weird that I never noticed that.)

I can easily patch that for MacPorts, but it would be much much easier and cleaner if this was fixed upstream.

comment:3 Changed 8 years ago by mojca (Mojca Miklavec)

I checked what difference it makes when HAVE_GTK is defined. From the point of view of functionality it doesn't seem to make any difference:

/* if the gtk headers are available, use them to tweak some behaviours */
#if defined(__WXGTK__)&&defined(HAVE_GTK)
# define USE_GTK
#endif

but there is still a problem with:

    WX_CXXFLAGS="$WX_CXXFLAGS $GTK_CFLAGS"
    WX_LIBS="$WX_LIBS $GTK_LIBS"

which shouldn't have been there.

David, do you have any suggestions for a proper fix? The only thing that comes to my might would be either this:

# when using Cocoa:
> wx-config --list

    Default config is osx_cocoa-unicode-3.0

  Default config will be used for output
# when using GTK:
> wx-config --list

    Default config is gtk3-unicode-3.0

  Default config will be used for output

or actually trying to compile a minimal example with wxWidgets and checking whether __WXGTK__ is defined or not with one of AC_RUN_IFELSE (or other AC_<FOO> functions).

comment:4 Changed 8 years ago by mojca (Mojca Miklavec)

Here's Ethan's response (I'm not sure why it didn't get automatically added to Trac):

The autoconfigure script assume pkg-config will correctly provide the include and lib directories for whatever version of gtk is appropriate. If not, then according to ./configure --help you can set

   GTK_CFLAGS  C compiler flags for GTK, overriding pkg-config
   GTK_LIBS    linker flags for GTK, overriding pkg-config

Ethan, pkg-config works ok and finds the right flags for GTK 2. The problem is that users here don't want to link against GTK at all because wxWigets use the Cocoa backend on OS X rather than GTK. As a consequence gnuplot gets linked against GTK 2 without using any GTK functionality at all.

But even in some extreme cases when OS X users would demand the use of wxGTK, the current check is still wrong because users might have wxWidgets built against GTK 3 as in the example I pointed out above. I can test it, but I believe this would then break badly. The following code from src/wxterminal/wxt_gui.h would be true:

#if defined(__WXGTK__)&&defined(HAVE_GTK)
# define USE_GTK
#endif

and then gnuplot would try to link against both GTK 2 and GTK 3 at the same time. WX_LIBS has the following value:

-framework IOKit -framework CoreServices -framework System -framework ApplicationServices -lwx_gtk3u_xrc-3.0 -lwx_gtk3u_html-3.0 -lwx_gtk3u_qa-3.0 -lwx_gtk3u_adv-3.0 -lwx_gtk3u_core-3.0 -lwx_baseu_xml-3.0 -lwx_baseu_net-3.0 -lwx_baseu-3.0

and GTK 2 libs would be added on top of that.

Last edited 8 years ago by mojca (Mojca Miklavec) (previous) (diff)

comment:5 Changed 8 years ago by mojca (Mojca Miklavec)

Unless the configure would grep for GTK version in the string "Default config is gtk3-unicode-3.0", we would probably need to check for

#ifdef __WXGTK2__

and

#ifdef __WXGTK3__

and test both versions of GTK on "both" OS-es (Mac and Linux).

Version 0, edited 8 years ago by mojca (Mojca Miklavec) (next)

comment:6 Changed 8 years ago by mojca (Mojca Miklavec)

I tried some relatively naive code for configure.ac:

  dnl Check for gtk (raise/lower tweaks)
  AC_MSG_CHECKING(if using wxGTK)
  AC_LANG_PUSH([C++])
  CXXFLAGS_OLD=$CXXFLAGS
  CXXFLAGS="$CXXFLAGS $WX_CXXFLAGS"

  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <wx/wx.h>]],
    [[int a = __WXGTK__]])],
    [AC_MSG_RESULT(yes)
     have_gtk=yes
     AC_MSG_CHECKING(if using wxGTK3)
     AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <wx/wx.h>]],
       [[int a = __WXGTK3__]])],
       [AC_MSG_RESULT(yes)
        have_gtk3=yes],
       [AC_MSG_RESULT(no)
        have_gtk3=no])
     ],
    [AC_MSG_RESULT(no)
     have_gtk=no
     have_gtk3=no])

  CXXFLAGS=$CXXFLAGS_OLD
  AC_LANG_POP([C++])

  if test "${have_gtk}" = yes ; then
    if test "${have_gtk3}" = yes ; then
      PKG_CHECK_MODULES(GTK, [gtk+-3.0],, [have_gtk=no have_gtk3=no])
      AC_DEFINE(HAVE_GTK, 1, [Define to use gtk/gdk tweaks])
      WX_CXXFLAGS="$WX_CXXFLAGS $GTK_CFLAGS"
      WX_LIBS="$WX_LIBS $GTK_LIBS"
    else
      PKG_CHECK_MODULES(GTK, [gtk+-2.0],, have_gtk=no)
      AC_DEFINE(HAVE_GTK, 1, [Define to use gtk/gdk tweaks])
      WX_CXXFLAGS="$WX_CXXFLAGS $GTK_CFLAGS"
      WX_LIBS="$WX_LIBS $GTK_LIBS"
    fi
  fi

but when I build against GTK 3 this fails to build:

	mv -f $depbase.Tpo $depbase.Po
../../gnuplot/src/wxterminal/wxt_gui.cpp:1537:20: error: use of undeclared identifier 'gdk_window_foreign_new'
                gdk_window_raise(gdk_window_foreign_new(windowid));
                                 ^
../../gnuplot/src/wxterminal/wxt_gui.cpp:1538:20: error: use of undeclared identifier 'gdk_window_foreign_new'
                gdk_window_focus(gdk_window_foreign_new(windowid), GDK_CURRENT_TIME);
                                 ^
../../gnuplot/src/wxterminal/wxt_gui.cpp:3170:48: error: no member named 'window' in '_GtkWidget'
                gdk_window_raise(window->frame->GetHandle()->window);
                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~  ^
../../gnuplot/src/wxterminal/wxt_gui.cpp:3183:47: error: no member named 'window' in '_GtkWidget'
        gdk_window_lower(window->frame->GetHandle()->window);
                         ~~~~~~~~~~~~~~~~~~~~~~~~~~  ^
4 errors generated.

It looks like the patches are specific to GTK version 2.0 only.

I need to add that the above code needs extremely long to finish the test. I guess there must be some more efficient way (even if just grepping for "gtk" in wx-config --list).

Last edited 8 years ago by mojca (Mojca Miklavec) (previous) (diff)

comment:7 Changed 8 years ago by mojca (Mojca Miklavec)

I committed r144202 (hoping that it fixes the problem for MacPorts; please let me know if I'm wrong). But this needs a proper upstream patch. I didn't revbump gnuplot, assuming that hardly anyone cares about some extra linking and the port will get upgraded sooner or later anyway. If you want to revbump the port, feel free to do so, I'm not against it.

comment:8 Changed 8 years ago by dbevans (David B. Evans)

This patch really is a shotgun approach because only one of configure or configure.in will actually be effective in any given case. Your patch for configure.in should work but you need to use autoreconf in this case to create a corresponding configure file. If you patch configure then it will work if you DON'T use autoreconf or it will get over-written.

I usually patch configure.in (or configure.ac in more modern ports) and autoreconf as the code is easier to understand without all the expanded macros in the way.

I do think that a rev bump is appropriate as this port is not distributable and people current port may link or not with gtk2 depending on what was active build time. With the patch, you guarantee that they are not linking with gtk2. I noticed the problem because, my gnuplot was built with gtk2 but I inadvertantly deactivated it during testing of p5-graphics-gnuplotif and got run time errors concerning the missing gtk2 libraries and their dependencies.

I think this issue has been beaten into submission at this point.

comment:9 in reply to:  8 ; Changed 8 years ago by mojca (Mojca Miklavec)

Replying to devans@…:

This patch really is a shotgun approach because only one of configure or configure.in will actually be effective in any given case. Your patch for configure.in should work but you need to use autoreconf in this case to create a corresponding configure file. If you patch configure then it will work if you DON'T use autoreconf or it will get over-written.

I usually patch configure.in (or configure.ac in more modern ports) and autoreconf as the code is easier to understand without all the expanded macros in the way.

Did you actually test or are you speculating? I didn't patch configure.in (at least not intentionally as I don't want to run autoconf if there's no need to do so), I just tried to find the least intrusive patch that hopefully fixes the problem (I could have removed the whole chunk of code testing for GTK2, but this would require way more massive patching of configure).

I do think that a rev bump is appropriate as this port is not distributable and people current port may link or not with gtk2 depending on what was active build time. With the patch, you guarantee that they are not linking with gtk2. I noticed the problem because, my gnuplot was built with gtk2 but I inadvertantly deactivated it during testing of p5-graphics-gnuplotif and got run time errors concerning the missing gtk2 libraries and their dependencies.

If you are happy with the patch, feel free to revbump. If you are not, I want further explanation :)

I think this issue has been beaten into submission at this point.

What does that mean?

comment:10 in reply to:  9 Changed 8 years ago by dbevans (David B. Evans)

Resolution: fixed
Status: newclosed

Replying to mojca@…:

Replying to devans@…:

This patch really is a shotgun approach because only one of configure or configure.in will actually be effective in any given case. Your patch for configure.in should work but you need to use autoreconf in this case to create a corresponding configure file. If you patch configure then it will work if you DON'T use autoreconf or it will get over-written.

I usually patch configure.in (or configure.ac in more modern ports) and autoreconf as the code is easier to understand without all the expanded macros in the way.

Did you actually test or are you speculating? I didn't patch configure.in (at least not intentionally as I don't want to run autoconf if there's no need to do so), I just tried to find the least intrusive patch that hopefully fixes the problem (I could have removed the whole chunk of code testing for GTK2, but this would require way more massive patching of configure).

I do think that a rev bump is appropriate as this port is not distributable and people current port may link or not with gtk2 depending on what was active build time. With the patch, you guarantee that they are not linking with gtk2. I noticed the problem because, my gnuplot was built with gtk2 but I inadvertantly deactivated it during testing of p5-graphics-gnuplotif and got run time errors concerning the missing gtk2 libraries and their dependencies.

If you are happy with the patch, feel free to revbump. If you are not, I want further explanation :)

I tested it and verified that it builds without reference to gtk2 even if it is present. You are patching configure but the patchfile also includes changes to configure.in that have been commented out (so not functional). I was confused.

I think this issue has been beaten into submission at this point.

What does that mean?

I was being too colloquial, sorry. I mean the subject has been discussed thoroughly. At your invitation, revision incremented in r144230. Case closed.

Note: See TracTickets for help on using tickets.