[RFC]: make IcedTea-web to be compatible with RHEL5 libraries
Dr Andrew John Hughes
ahughes at redhat.com
Sat Aug 27 21:10:10 PDT 2011
On 15:35 Fri 26 Aug , Jiri Vanek wrote:
snip...
> Based also on attached irc discussion:
>
> 2011-08-26 Jiri Vanek<jvanek at redhat.com>
> Added functionality to allow icedtea web to be buildable with
> rhel5 libraries
> *acinclude.m4: added block to test version of glib installed and
> setting variable GLIB214 to -DGLIB214 if version is <2.15
> * plugin/icedteanp/IcedTeaNPPlugin.cc: added replacements for incompatible
> functions, added #define sections for use this function instead of glib ones
> *Makefile.am: ($(PLUGIN_DIR)/%.o): using GLIB214 setted by configure
> to define GLIB214
> ($(PLUGIN_DIR)/IcedTeaPlugin.so): same as ^
> *configure.ac: added IT_GET_GLIBVERSION
>
Comments below.
> <jvanek_home> dbhole, I can not reply your email now (I'm not on working environment) .. but to clarify : you wrote that iterator appeared in 2.15. It si not true - documentation said 2.16, but on 2.12.5 it is working also....
> <dbhole> jvanek_home: Hmm, it was added in commit d741d3e7:
> <dbhole> commit d741d3e7a344622e5a534c34e9e2c0488b2fea4d
> <dbhole> Author: Matthias Clasen <mclasen at redhat.com>
> <dbhole> Date: Sat Dec 15 03:54:09 2007 +0000
> <dbhole> Add hash table iterators. (#500507, Jean-Yves Lefort)
> <jvanek_home> dbhole, if you are enough with (probably incorrect) check for version, you can look into http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2011-August/015519.html
> <jvanek_home> dbhole, I see it compiling right now
> <jvanek_home> ldd (GNU libc) 2.13
> <jvanek_home> Copyright (C) 2011 Free Software Foundation, Inc.
> <jvanek_home> This is free software; see the source for copying conditions. There is NO
> <jvanek_home> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> <jvanek_home> Written by Roland McGrath and Ulrich Drepper.
> <dbhole> jvanek_home: If it is incorrect, I am in favor of your approach.. I am not just sure if it is, thats all :) I am going by git history which shows it was added before 2.15
> <dbhole> jvanek_home: That is glibc
> <dbhole> jvanek_home: This function is in glib
I've already pointed this out once when you posted this patch in an earlier form.
> <jvanek_home> omg
> <dbhole> they are different
> <jvanek_home> dbhole, So you are fine with glib-config --version and sedding similar like in http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2011-August/015519.html ok?
> <jvanek_home> with version >2.15
> <dbhole> jvanek_home: Yes, assuming it is correct.. do you have reason to believe it is not?
> <jvanek_home> now ot make sens
> * jvanek_home feels like complete idiot
> <dbhole> jvanek_home: Ah, cool .. no worries :) Just glad it works!
> <jvanek_home> dbhole, yy, i will check it if the versions match on all available systems and then (probably) will make the version check. Thanx for advice... will create the patch tomorrow
> <dbhole> jvanek_home: There is code in the plugin right now that checks for xulrunner versions >= something, and it compiled conditionally.. we can do the same for glibc.. just pass the glibc version via a -D and then have the code do the rest
> <dbhole> jvanek_home: np, thanks!
> <jvanek_home> dbhole, glib-config --version
> <jvanek_home> 1.2.10
> <dbhole> jvanek_home: The package is named glib2.. there may be a different .pc file for it
> <dbhole> jvanek_home: one sec, trying to get a hold of a rhel5 system
> <dbhole> jvanek_home: glib2 and glib are installable in parallel (or they used to be, rather)
> <dbhole> jvanek_home: doh! don't have a rhel5 vm :/ .. provisioning one on to-openjdk1 now
> ....
$ pkg-config --modversion glib-2.0
as you probably found out. Surprised you even have glib 1.2...
> <jvanek_home> dbhole, Is there any better way how to get glib version then by rpm?
> <jvanek_home> rpm -qa | grep ^glib2-
> <dbhole> jvanek_home: You can just use autotools: http://www.flameeyes.eu/autotools-mythbuster/pkgconfig/pkg_check_modules.html
> <jvanek_home> dbhole, ok
>
>
> diff -r 36270c76a533 Makefile.am
> --- a/Makefile.am Wed Aug 24 15:17:46 2011 -0400
> +++ b/Makefile.am Fri Aug 26 15:23:27 2011 +0200
> @@ -210,6 +210,7 @@
> mkdir -p $(PLUGIN_DIR) && \
> cd $(PLUGIN_DIR) && \
> $(CXX) $(CXXFLAGS) \
> + $(GLIB214) \
> -DJDK_UPDATE_VERSION="\"$(JDK_UPDATE_VERSION)\"" \
> -DPLUGIN_NAME="\"IcedTea-Web Plugin\"" \
> -DPLUGIN_VERSION="\"$(PLUGIN_VERSION)\"" \
> @@ -225,6 +226,7 @@
> $(PLUGIN_DIR)/IcedTeaPlugin.so: $(addprefix $(PLUGIN_DIR)/,$(PLUGIN_OBJECTS))
> cd $(PLUGIN_DIR) && \
> $(CXX) $(CXXFLAGS) \
> + $(GLIB214) \
> $(PLUGIN_OBJECTS) \
> $(GLIB_LIBS) \
> $(GTK_LIBS) \
This should use $(DEFS). See below.
Why do we need this twice? I think the second one is unnecessary, as we are
just linking object files.
> diff -r 36270c76a533 acinclude.m4
> --- a/acinclude.m4 Wed Aug 24 15:17:46 2011 -0400
> +++ b/acinclude.m4 Fri Aug 26 15:23:27 2011 +0200
> @@ -484,6 +484,18 @@
> AC_SUBST(PKGVERSION)
> ])
>
> +AC_DEFUN_ONCE([IT_GET_GLIBVERSION],
> +[
> +PKG_CHECK_MODULES([GLIB2_V_216],[glib-2.0 >= 2.16],
> +[
> +GLIB214=""
> +],[
> +GLIB214=" -DGLIB214 "
> +])
> +AC_SUBST([GLIB214])
> +]
> +)
> +
Use AC_DEFINE([LEGACY_GLIB],1); that's the standard way to do
it and the naming isn't then tied to the test.
Why are you testing for >= 2.16 when the discussion implied
the functions appeared in 2.15?
You don't need the AC_SUBST as you don't add anything to the
Makefile.am, although it doesn't hurt.
> AC_DEFUN([IT_CHECK_WITH_GCJ],
> [
> AC_MSG_CHECKING([whether to compile ecj natively])
> diff -r 36270c76a533 configure.ac
> --- a/configure.ac Wed Aug 24 15:17:46 2011 -0400
> +++ b/configure.ac Fri Aug 26 15:23:27 2011 +0200
> @@ -84,7 +84,7 @@
> #
> # Find optional depedencies
> #
> -
> +IT_GET_GLIBVERSION
> IT_FIND_OPTIONAL_JAR([rhino], RHINO,
> [/usr/share/java/js.jar /usr/share/rhino-1.6/lib/js.jar])
> IT_FIND_OPTIONAL_JAR([junit], JUNIT,
This is the wrong place. These aren't optional dependencies.
And that empty line shouldn't be removed.
> diff -r 36270c76a533 plugin/icedteanp/IcedTeaNPPlugin.cc
> --- a/plugin/icedteanp/IcedTeaNPPlugin.cc Wed Aug 24 15:17:46 2011 -0400
> +++ b/plugin/icedteanp/IcedTeaNPPlugin.cc Fri Aug 26 15:23:27 2011 +0200
> @@ -47,6 +47,10 @@
> #include <sys/types.h>
> #include <unistd.h>
>
> +#ifdef GLIB214
> +#include <glib.h>
> +#endif
> +
This seems wrong. How does this work if glib.h is not included?
Have you checked this still works without the define?
> // Liveconnect extension
> #include "IcedTeaScriptablePluginObject.h"
> #include "IcedTeaNPPlugin.h"
> @@ -869,6 +873,16 @@
> PLUGIN_DEBUG ("ITNP_URLNotify return\n");
> }
>
> +#ifdef GLIB214
> +// Returns key from first item stored in hashtable
> +gboolean
> +find_first_item_in_hash_table(gpointer key, gpointer value, gpointer user_data)
> +{
> + user_data = key;
> + return (gboolean)TRUE;
> +}
> +#endif
> +
> NPError
> get_cookie_info(const char* siteAddr, char** cookieString, uint32_t* len)
> {
> @@ -913,19 +927,33 @@
> // valid. So we just pick the first valid one and use it. Proxy/Cookie
> // information is not instance specific anyway, it is URL specific.
>
> +#ifdef GLIB214
> if (browser_functions.getvalueforurl)
> {
> - GHashTableIter iter;
> gpointer id, instance;
>
> - g_hash_table_iter_init (&iter, instance_to_id_map);
> - g_hash_table_iter_next (&iter, &instance, &id);
> + g_hash_table_find(instance_to_id_map, (GHRFunc)find_first_item_in_hash_table, &instance);
>
> return browser_functions.getvalueforurl((NPP) instance, NPNURLVCookie, siteAddr, cookieString, len);
> } else
> {
> return NPERR_GENERIC_ERROR;
> }
> +#else
> + if (browser_functions.getvalueforurl)
> + {
> + GHashTableIter iter;
> + gpointer id, instance;
> +
> + g_hash_table_iter_init (&iter, instance_to_id_map);
> + g_hash_table_iter_next (&iter, &instance, &id);
> +
> + return browser_functions.getvalueforurl((NPP) instance, NPNURLVCookie, siteAddr, cookieString, len);
> + } else
> + {
> + return NPERR_GENERIC_ERROR;
> + }
> +#endif
This would be a cleaner patch if the test was inverted i.e. #ifndef LEGACY_GLIB
>
> #endif
>
> @@ -1363,21 +1391,38 @@
>
> #else
>
> +#ifdef GLIB214
> if (browser_functions.getvalueforurl)
> {
>
> - // As in get_cookie_info, we use the first active instance
> - GHashTableIter iter;
> + // As in get_cookie_info, we use the first active instance
> gpointer id, instance;
>
> - g_hash_table_iter_init (&iter, instance_to_id_map);
> - g_hash_table_iter_next (&iter, &instance, &id);
> + g_hash_table_find(instance_to_id_map, (GHRFunc)find_first_item_in_hash_table, &instance);
>
> browser_functions.getvalueforurl((NPP) instance, NPNURLVProxy, siteAddr, proxy, len);
> } else
> {
> return NPERR_GENERIC_ERROR;
> }
> +#else
> + if (browser_functions.getvalueforurl)
> + {
> +
> + // As in get_cookie_info, we use the first active instance
> + GHashTableIter iter;
> + gpointer id, instance;
> +
> + g_hash_table_iter_init (&iter, instance_to_id_map);
> + g_hash_table_iter_next (&iter, &instance, &id);
> +
> + browser_functions.getvalueforurl((NPP) instance, NPNURLVProxy, siteAddr, proxy, len);
> + } else
> + {
> + return NPERR_GENERIC_ERROR;
> + }
> +#endif
> +
Likewise.
Could this not be factored out into a common function or define so we don't have this duplication?
> #endif
>
> return NPERR_NO_ERROR;
> @@ -1403,6 +1448,17 @@
> return FALSE;
> }
>
> +#ifdef GLIB214
> +int
> +strcmp0(char *str1, char *str2)
> +{
> + if (str1 != NULL)
> + return str2 != NULL ? strcmp(str1, str2) : 1;
> + else // str1 == NULL
> + return str2 != NULL ? 1 : 0;
> +}
> +#endif
> +
Just define g_strcmp0 here and the function below should work unchanged.
> // remove all components from LD_LIBRARY_PATH, which start with
> // MOZILLA_FIVE_HOME; firefox has its own NSS based security provider,
> // which conflicts with the one configured in nss.cfg.
> @@ -1424,7 +1480,11 @@
> components = g_strsplit (path_old, ":", -1);
> for (i1 = 0, i2 = 0; components[i1] != NULL; i1++)
> {
> +#ifdef GLIB214
> + if (strcmp0 (components[i1], moz_home) == 0
> +#else
> if (g_strcmp0 (components[i1], moz_home) == 0
> +#endif
> || g_str_has_prefix (components[i1], moz_home))
> components[i2] = components[i1];
> else
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list