Re: [PATCH] Build against NM 0.8.1 (say RHEL 6)

[Dan Williams <dcbw redhat com>]

On Thu, 2012-10-11 at 22:37 -0300, Murilo Opsfelder Araujo wrote:
> Signed-off-by: Murilo Opsfelder Araujo <muriloo linux vnet ibm com>

I think what we'd rather do here is compat-style code instead of just
reverting changes.  More details inline.

> ---
>  auth-dialog/main.c                              |  9 ++++++---
>  configure.ac                                    |  8 ++++----
>  src/nm-openconnect-service-openconnect-helper.c | 18 ++----------------
>  3 files changed, 12 insertions(+), 23 deletions(-)
> 
> diff --git a/auth-dialog/main.c b/auth-dialog/main.c
> index ea728b6..1c244e5 100644
> --- a/auth-dialog/main.c
> +++ b/auth-dialog/main.c
> @@ -30,6 +30,7 @@
>  #include <string.h>
>  #include <errno.h>
>  #include <unistd.h>
> +#include <fcntl.h>
>  #define _GNU_SOURCE
>  #include <getopt.h>
>  
> @@ -40,7 +41,6 @@
>  
>  #include <gtk/gtk.h>
>  #include <glib/gi18n.h>
> -#include <glib-unix.h>
>  
>  #include "auth-dlg-settings.h"
>  
> @@ -1496,8 +1496,11 @@ static auth_ui_data *init_ui_data (char *vpn_name)
>  		ui_data->cancel_pipes[0] = -1;
>  		ui_data->cancel_pipes[1] = -1;
>  	}
> -	g_unix_set_fd_nonblocking(ui_data->cancel_pipes[0], TRUE, NULL);
> -	g_unix_set_fd_nonblocking(ui_data->cancel_pipes[1], TRUE, NULL);
> +
> +	fcntl(ui_data->cancel_pipes[0], F_SETFL,
> +	      fcntl(ui_data->cancel_pipes[0], F_GETFL) | O_NONBLOCK);
> +	fcntl(ui_data->cancel_pipes[1], F_SETFL,
> +	      fcntl(ui_data->cancel_pipes[1], F_GETFL) | O_NONBLOCK);
>  
>  	ui_data->vpninfo = (void *)openconnect_vpninfo_new("OpenConnect VPN Agent (NetworkManager)",
>  							   validate_peer_cert, write_new_config,

Instead of just removing this code, we can make use of
GLIB_CHECK_VERSION.  So instead of doing these hunks here, you'd do
something like:

#if GLIB_CHECK_VERSION(2,30,0)
#include <glib-unix.h>
#else
#include <fcntl.h>

static gboolean
g_unix_set_fd_nonblocking (gint fd, gboolean nonblock, GError **error)
{
	long flags = 0;
	int ret;

	flags = fcntl (fd, F_GETFL);

	if (nonblock)
		flags |= O_NONBLOCK;
	else
		flags &= ~O_NONBLOCK;

	ret = fcntl(fd, F_SETFL, flags);
	if (ret) {
		g_set_error_literal (error, G_UNIX_ERROR, errno,
                                     g_strerror (errno));
	}
	return ret == 0 ? TRUE : FALSE;
}
#endif  /* GLIB_CHECK_VERSION(2,30,0) */

> diff --git a/configure.ac b/configure.ac
> index a7f864f..e8617eb 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -85,10 +85,10 @@ if test x"$with_gnome" != xno; then
>  fi
>  
>  PKG_CHECK_MODULES(NETWORKMANAGER,
> -                  NetworkManager >= 0.8.6
> -                  libnm-util >= 0.8.6
> -                  libnm-glib  >= 0.8.6
> -                  libnm-glib-vpn >= 0.8.6)
> +                  NetworkManager >= 0.8.1
> +                  libnm-util >= 0.8.1
> +                  libnm-glib  >= 0.8.1
> +                  libnm-glib-vpn >= 0.8.1)

Instead of this, which (in conjunction with the following patch) removes
some functionality, what you really want to do is to just check whether
the things you want to use are defined.  In configure.ac that would be
something like:

AC_MSG_CHECKING([Checking for recent NetworkManager VPN properties])
AC_COMPILE_IFELSE(
	[AC_LANG_PROGRAM(
              [[#include <NetworkManagerVPN.h>]],
              [[const char a = ;]])],
	[have_recent_vpn_props=yes],
	[have_recent_vpn_props=no])
AC_MSG_RESULT($ac_recent_vpn_prpos)
if test "$ac_have_recent_vpn_props" = yes; then
	AC_DEFINE(HAVE_RECENT_VPN_PROPS, 1, [Define if you have NM 0.8.6 or
later])
else
	AC_DEFINE(HAVE_RECENT_VPN_PROPS, 0, [Define if you have NM 0.8.6 or
later])
fi

and then later on:

>  AC_SUBST(NETWORKMANAGER_CFLAGS)
>  AC_SUBST(NETWORKMANAGER_LIBS)
>  
> diff --git a/src/nm-openconnect-service-openconnect-helper.c b/src/nm-openconnect-service-openconnect-helper.c
> index c195d46..861ed6b 100644
> --- a/src/nm-openconnect-service-openconnect-helper.c
> +++ b/src/nm-openconnect-service-openconnect-helper.c
> @@ -136,17 +136,6 @@ uint_to_gvalue (guint32 num)
>  }
>  
>  static GValue *
> -bool_to_gvalue (gboolean b)
> -{
> -	GValue *val;
> -
> -	val = g_slice_new0 (GValue);
> -	g_value_init (val, G_TYPE_BOOLEAN);
> -	g_value_set_boolean (val, b);
> -	return val;
> -}
> -
> -static GValue *
>  addr_to_gvalue (const char *str)
>  {
>  	struct in_addr	temp_addr;
> @@ -373,12 +362,9 @@ main (int argc, char *argv[])
>  
>  	/* Routes */
>  	val = get_routes ();
> -	if (val) {
> +	if (val)
>  		g_hash_table_insert (config, NM_VPN_PLUGIN_IP4_CONFIG_ROUTES, val);
> -		/* If routes-to-include were provided, that means no default route */
> -		g_hash_table_insert (config, NM_VPN_PLUGIN_IP4_CONFIG_NEVER_DEFAULT,
> -		                     bool_to_gvalue (TRUE));
> -	}
> +
>  	/* Banner */
>  	val = str_to_gvalue (getenv ("CISCO_BANNER"), TRUE);
>  	if (val)

instead of just removing this code, protect them with:

#if HAVE_RECENT_VPN_PROPS
  <stuff that needs new defines>
#endif

Dont' forget to #include <config.h> at the top of this file too, so that
you get the new defines.

This way everything works well with both old and new NetworkManager.

Dan