#2 Backport patch for bug 68
Closed 5 years ago by eclipseo. Opened 5 years ago by eclipseo.
Unknown source master  into  master

@@ -0,0 +1,435 @@

+ From 1debd9a68b0f7cbfeef99428b6d9c3dfacdc6976 Mon Sep 17 00:00:00 2001

+ From: Valentin Blot <freedesktop-devel@valentinblot.org>

+ Date: Thu, 28 Jun 2018 10:28:57 +0200

+ Subject: [PATCH 1/2] agent: Unimplement unused interface GAsyncInitable

+ 

+ The GAsyncInitable interface that was implemented by the agent was not

+ useful. This commit removes this interface implementation to simplify the

+ code and allow the use of user_data for other things.

+ ---

+  demo/agent.c               |  25 +-------

+  demo/gclue-service-agent.c | 119 ++++++++++---------------------------

+  demo/gclue-service-agent.h |  10 +---

+  3 files changed, 37 insertions(+), 117 deletions(-)

+ 

+ diff --git a/demo/agent.c b/demo/agent.c

+ index 23f9c6d..8e2a3ef 100644

+ --- a/demo/agent.c

+ +++ b/demo/agent.c

+ @@ -44,32 +44,15 @@ static GOptionEntry entries[] =

+          { NULL }

+  };

+  

+ -GDBusConnection *connection;

+ -GMainLoop *main_loop;

+  GClueServiceAgent *agent = NULL;

+  

+ -static void

+ -on_service_agent_ready (GObject      *source_object,

+ -                        GAsyncResult *res,

+ -                        gpointer      user_data)

+ -{

+ -        GError *error = NULL;

+ -

+ -        agent = gclue_service_agent_new_finish (res, &error);

+ -        if (agent == NULL) {

+ -                g_critical ("Failed to launch agent service: %s", error->message);

+ -                g_error_free (error);

+ -

+ -                exit (-3);

+ -        }

+ -}

+ -

+  static void

+  on_get_bus_ready (GObject      *source_object,

+                    GAsyncResult *res,

+                    gpointer      user_data)

+  {

+          GError *error = NULL;

+ +        GDBusConnection *connection;

+  

+          connection = g_bus_get_finish (res, &error);

+          if (connection == NULL) {

+ @@ -80,10 +63,7 @@ on_get_bus_ready (GObject      *source_object,

+                  exit (-2);

+          }

+  

+ -        gclue_service_agent_new_async (connection,

+ -                                       NULL,

+ -                                       on_service_agent_ready,

+ -                                       NULL);

+ +        agent = gclue_service_agent_new (connection);

+  }

+  

+  #define ABS_PATH ABS_SRCDIR "/agent"

+ @@ -91,6 +71,7 @@ on_get_bus_ready (GObject      *source_object,

+  int

+  main (int argc, char **argv)

+  {

+ +        GMainLoop *main_loop;

+          GError *error = NULL;

+          GOptionContext *context;

+  

+ diff --git a/demo/gclue-service-agent.c b/demo/gclue-service-agent.c

+ index c4573a7..bd0563c 100644

+ --- a/demo/gclue-service-agent.c

+ +++ b/demo/gclue-service-agent.c

+ @@ -31,18 +31,20 @@

+  

+  #define AGENT_PATH "/org/freedesktop/GeoClue2/Agent"

+  

+ +#define SERVICE           "org.freedesktop.GeoClue2"

+ +#define MANAGER_PATH      "/org/freedesktop/GeoClue2/Manager"

+ +#define MANAGER_INTERFACE "org.freedesktop.GeoClue2.Manager"

+ +

+  static void

+  gclue_service_agent_agent_iface_init (GClueAgentIface *iface);

+  static void

+ -gclue_service_agent_async_initable_init (GAsyncInitableIface *iface);

+ +gclue_service_agent_constructed (GObject *object);

+  

+  G_DEFINE_TYPE_WITH_CODE (GClueServiceAgent,

+                           gclue_service_agent,

+                           GCLUE_TYPE_AGENT_SKELETON,

+                           G_IMPLEMENT_INTERFACE (GCLUE_TYPE_AGENT,

+ -                                                gclue_service_agent_agent_iface_init)

+ -                         G_IMPLEMENT_INTERFACE (G_TYPE_ASYNC_INITABLE,

+ -                                                gclue_service_agent_async_initable_init))

+ +                                                gclue_service_agent_agent_iface_init))

+  

+  struct _GClueServiceAgentPrivate

+  {

+ @@ -114,6 +116,7 @@ gclue_service_agent_class_init (GClueServiceAgentClass *klass)

+          object_class->finalize = gclue_service_agent_finalize;

+          object_class->get_property = gclue_service_agent_get_property;

+          object_class->set_property = gclue_service_agent_set_property;

+ +        object_class->constructed = gclue_service_agent_constructed;

+  

+          g_type_class_add_private (object_class, sizeof (GClueServiceAgentPrivate));

+  

+ @@ -141,22 +144,11 @@ on_add_agent_ready (GObject      *source_object,

+                      GAsyncResult *res,

+                      gpointer      user_data)

+  {

+ -        GTask *task = G_TASK (user_data);

+ -        GVariant *results;

+          GError *error = NULL;

+  

+ -        results = g_dbus_proxy_call_finish (G_DBUS_PROXY (source_object),

+ -                                            res,

+ -                                            &error);

+ -        if (results == NULL) {

+ -                g_task_return_error (task, error);

+ -                g_object_unref (task);

+ -                return;

+ -        }

+ -

+ -        g_task_return_boolean (task, TRUE);

+ -

+ -        g_object_unref (task);

+ +        g_dbus_proxy_call_finish (G_DBUS_PROXY (source_object),

+ +                                  res,

+ +                                  &error);

+  }

+  

+  static void

+ @@ -201,19 +193,21 @@ on_manager_proxy_ready (GObject      *source_object,

+                          GAsyncResult *res,

+                          gpointer      user_data)

+  {

+ -        GTask *task = G_TASK (user_data);

+          GClueAgent *agent;

+          GDBusProxy *proxy;

+          GError *error = NULL;

+  

+          proxy = g_dbus_proxy_new_for_bus_finish (res, &error);

+          if (proxy == NULL) {

+ -                g_task_return_error (task, error);

+ -                g_object_unref (task);

+ +                g_critical ("Failed to create proxy to %s: %s",

+ +                            MANAGER_PATH,

+ +                            error->message);

+ +                g_error_free (error);

+ +

+                  return;

+          }

+  

+ -        agent = GCLUE_AGENT (g_task_get_source_object (task));

+ +        agent = GCLUE_AGENT (user_data);

+          gclue_agent_set_max_accuracy_level (agent, GCLUE_ACCURACY_LEVEL_EXACT);

+  

+          g_dbus_proxy_call (proxy,

+ @@ -222,55 +216,37 @@ on_manager_proxy_ready (GObject      *source_object,

+                                            "geoclue-demo-agent"),

+                             G_DBUS_CALL_FLAGS_NONE,

+                             -1,

+ -                           g_task_get_cancellable (task),

+ +                           NULL,

+                             on_add_agent_ready,

+ -                           task);

+ +                           NULL);

+          print_in_use_info (proxy);

+          g_signal_connect (proxy,

+                            "g-properties-changed",

+                            G_CALLBACK (on_manager_props_changed),

+ -                          agent);

+ +                          NULL);

+  }

+  

+  static void

+ -gclue_service_agent_init_async (GAsyncInitable     *initable,

+ -                                int                 io_priority,

+ -                                GCancellable       *cancellable,

+ -                                GAsyncReadyCallback callback,

+ -                                gpointer            user_data)

+ +gclue_service_agent_constructed (GObject *object)

+  {

+ -        GTask *task;

+          GError *error = NULL;

+ -

+ -        task = g_task_new (initable, cancellable, callback, user_data);

+ -

+          if (!g_dbus_interface_skeleton_export

+ -                (G_DBUS_INTERFACE_SKELETON (initable),

+ -                 GCLUE_SERVICE_AGENT (initable)->priv->connection,

+ +                (G_DBUS_INTERFACE_SKELETON (object),

+ +                 GCLUE_SERVICE_AGENT (object)->priv->connection,

+                   AGENT_PATH,

+                   &error)) {

+ -                g_task_return_error (task, error);

+ -                g_object_unref (task);

+                  return;

+          }

+  

+          g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM,

+                                    G_DBUS_PROXY_FLAGS_NONE,

+                                    NULL,

+ -                                  "org.freedesktop.GeoClue2",

+ -                                  "/org/freedesktop/GeoClue2/Manager",

+ -                                  "org.freedesktop.GeoClue2.Manager",

+ -                                  cancellable,

+ +                                  SERVICE,

+ +                                  MANAGER_PATH,

+ +                                  MANAGER_INTERFACE,

+ +                                  NULL,

+                                    on_manager_proxy_ready,

+ -                                  task);

+ -}

+ -

+ -static gboolean

+ -gclue_service_agent_init_finish (GAsyncInitable *initable,

+ -                                 GAsyncResult   *result,

+ -                                 GError        **error)

+ -{

+ -        return g_task_propagate_boolean (G_TASK (result), error);

+ +                                  object);

+  }

+  

+  typedef struct

+ @@ -405,42 +381,11 @@ gclue_service_agent_agent_iface_init (GClueAgentIface *iface)

+          iface->handle_authorize_app = gclue_service_agent_handle_authorize_app;

+  }

+  

+ -static void

+ -gclue_service_agent_async_initable_init (GAsyncInitableIface *iface)

+ -{

+ -        iface->init_async = gclue_service_agent_init_async;

+ -        iface->init_finish = gclue_service_agent_init_finish;

+ -}

+ -

+ -void

+ -gclue_service_agent_new_async (GDBusConnection    *connection,

+ -                               GCancellable       *cancellable,

+ -                               GAsyncReadyCallback callback,

+ -                               gpointer            user_data)

+ -{

+ -        g_async_initable_new_async (GCLUE_TYPE_SERVICE_AGENT,

+ -                                    G_PRIORITY_DEFAULT,

+ -                                    cancellable,

+ -                                    callback,

+ -                                    user_data,

+ -                                    "connection", connection,

+ -                                    NULL);

+ -}

+ -

+  GClueServiceAgent *

+ -gclue_service_agent_new_finish (GAsyncResult *res,

+ -                                GError      **error)

+ +gclue_service_agent_new (GDBusConnection *connection)

+  {

+ -        GObject *object;

+ -        GObject *source_object;

+ -

+ -        source_object = g_async_result_get_source_object (res);

+ -        object = g_async_initable_new_finish (G_ASYNC_INITABLE (source_object),

+ -                                              res,

+ -                                              error);

+ -        g_object_unref (source_object);

+ -        if (object != NULL)

+ -                return GCLUE_SERVICE_AGENT (object);

+ -        else

+ -                return NULL;

+ +        return g_object_new (GCLUE_TYPE_SERVICE_AGENT,

+ +                             "connection",

+ +                             connection,

+ +                             NULL);

+  }

+ diff --git a/demo/gclue-service-agent.h b/demo/gclue-service-agent.h

+ index 019a6a3..a1944a7 100644

+ --- a/demo/gclue-service-agent.h

+ +++ b/demo/gclue-service-agent.h

+ @@ -53,14 +53,8 @@ struct _GClueServiceAgentClass

+          GClueAgentSkeletonClass parent_class;

+  };

+  

+ -GType gclue_service_agent_get_type                 (void) G_GNUC_CONST;

+ -void  gclue_service_agent_new_async                (GDBusConnection    *connection,

+ -                                                    GCancellable       *cancellable,

+ -                                                    GAsyncReadyCallback callback,

+ -                                                    gpointer            user_data);

+ -GClueServiceAgent * gclue_service_agent_new_finish (GAsyncResult       *res,

+ -                                                    GError            **error);

+ -

+ +GType               gclue_service_agent_get_type (void) G_GNUC_CONST;

+ +GClueServiceAgent * gclue_service_agent_new      (GDBusConnection *connection);

+  

+  G_END_DECLS

+  

+ -- 

+ 2.17.1

+ 

+ 

+ From c8d7689687189a4b9b1676dca58e7fcf4ef21783 Mon Sep 17 00:00:00 2001

+ From: Valentin Blot <freedesktop-devel@valentinblot.org>

+ Date: Thu, 28 Jun 2018 10:50:49 +0200

+ Subject: [PATCH 2/2] agent: Register the agent whenever geoclue starts

+ 

+ The agent watches on d-bus and registers whenever it sees geoclue getting

+ alive.

+ 

+ https://gitlab.freedesktop.org/geoclue/geoclue/issues/68

+ ---

+  demo/gclue-service-agent.c | 70 ++++++++++++++++++++++++++------------

+  1 file changed, 48 insertions(+), 22 deletions(-)

+ 

+ diff --git a/demo/gclue-service-agent.c b/demo/gclue-service-agent.c

+ index bd0563c..6f2e227 100644

+ --- a/demo/gclue-service-agent.c

+ +++ b/demo/gclue-service-agent.c

+ @@ -49,6 +49,7 @@ G_DEFINE_TYPE_WITH_CODE (GClueServiceAgent,

+  struct _GClueServiceAgentPrivate

+  {

+          GDBusConnection *connection;

+ +        GDBusProxy *manager_proxy;

+  };

+  

+  enum

+ @@ -193,24 +194,21 @@ on_manager_proxy_ready (GObject      *source_object,

+                          GAsyncResult *res,

+                          gpointer      user_data)

+  {

+ -        GClueAgent *agent;

+ -        GDBusProxy *proxy;

+ +        GClueServiceAgent *agent;

+          GError *error = NULL;

+ +        agent = GCLUE_SERVICE_AGENT (user_data);

+  

+ -        proxy = g_dbus_proxy_new_for_bus_finish (res, &error);

+ -        if (proxy == NULL) {

+ +        agent->priv->manager_proxy = g_dbus_proxy_new_for_bus_finish (res,

+ +                                                                      &error);

+ +        if (agent->priv->manager_proxy == NULL) {

+                  g_critical ("Failed to create proxy to %s: %s",

+                              MANAGER_PATH,

+                              error->message);

+                  g_error_free (error);

+ -

+                  return;

+          }

+  

+ -        agent = GCLUE_AGENT (user_data);

+ -        gclue_agent_set_max_accuracy_level (agent, GCLUE_ACCURACY_LEVEL_EXACT);

+ -

+ -        g_dbus_proxy_call (proxy,

+ +        g_dbus_proxy_call (agent->priv->manager_proxy,

+                             "AddAgent",

+                             g_variant_new ("(s)",

+                                            "geoclue-demo-agent"),

+ @@ -219,25 +217,19 @@ on_manager_proxy_ready (GObject      *source_object,

+                             NULL,

+                             on_add_agent_ready,

+                             NULL);

+ -        print_in_use_info (proxy);

+ -        g_signal_connect (proxy,

+ +        print_in_use_info (agent->priv->manager_proxy);

+ +        g_signal_connect (agent->priv->manager_proxy,

+                            "g-properties-changed",

+                            G_CALLBACK (on_manager_props_changed),

+                            NULL);

+  }

+  

+  static void

+ -gclue_service_agent_constructed (GObject *object)

+ +on_name_appeared (GDBusConnection *connection,

+ +                  const gchar     *name,

+ +                  const gchar     *name_owner,

+ +                  gpointer         user_data)

+  {

+ -        GError *error = NULL;

+ -        if (!g_dbus_interface_skeleton_export

+ -                (G_DBUS_INTERFACE_SKELETON (object),

+ -                 GCLUE_SERVICE_AGENT (object)->priv->connection,

+ -                 AGENT_PATH,

+ -                 &error)) {

+ -                return;

+ -        }

+ -

+          g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM,

+                                    G_DBUS_PROXY_FLAGS_NONE,

+                                    NULL,

+ @@ -246,7 +238,41 @@ gclue_service_agent_constructed (GObject *object)

+                                    MANAGER_INTERFACE,

+                                    NULL,

+                                    on_manager_proxy_ready,

+ -                                  object);

+ +                                  user_data);

+ +}

+ +

+ +static void

+ +on_name_vanished (GDBusConnection *connection,

+ +                  const gchar     *name,

+ +                  gpointer         user_data)

+ +{

+ +        GClueServiceAgent *agent = GCLUE_SERVICE_AGENT (user_data);

+ +

+ +        g_clear_object (&agent->priv->manager_proxy);

+ +}

+ +

+ +static void

+ +gclue_service_agent_constructed (GObject *object)

+ +{

+ +        GError *error = NULL;

+ +        GClueServiceAgent *agent = GCLUE_SERVICE_AGENT (object);

+ +        if (!g_dbus_interface_skeleton_export

+ +                (G_DBUS_INTERFACE_SKELETON (agent),

+ +                 agent->priv->connection,

+ +                 AGENT_PATH,

+ +                 &error)) {

+ +                return;

+ +        }

+ +        gclue_agent_set_max_accuracy_level (GCLUE_AGENT (agent),

+ +                                            GCLUE_ACCURACY_LEVEL_EXACT);

+ +        g_bus_watch_name (G_BUS_TYPE_SYSTEM,

+ +                          SERVICE,

+ +                          G_BUS_NAME_WATCHER_FLAGS_NONE,

+ +                          on_name_appeared,

+ +                          on_name_vanished,

+ +                          agent,

+ +                          NULL);

+ +

+  }

+  

+  typedef struct

+ -- 

+ 2.17.1

+ 

file modified
+9 -2
@@ -1,12 +1,15 @@

  Name:           geoclue2

  Version:        2.4.10

- Release:        2%{?dist}

+ Release:        3%{?dist}

  Summary:        Geolocation service

  

  License:        GPLv2+

  URL:            http://www.freedesktop.org/wiki/Software/GeoClue/

  Source0:        http://www.freedesktop.org/software/geoclue/releases/2.4/geoclue-%{version}.tar.xz

  

+ # https://gitlab.freedesktop.org/geoclue/geoclue/issues/68

+ Patch0:         0001-agent-Register-the-agent-whenever-geoclue-starts.patch

+ 

  BuildRequires:  avahi-glib-devel

  BuildRequires:  glib2-devel

  BuildRequires:  gobject-introspection-devel
@@ -68,7 +71,7 @@

  

  %prep

  %setup -q -n geoclue-%{version}

- 

+ %patch0 -p1 -b .register_the_agent_whenever_geoclue_starts

  

  %build

  %configure --with-dbus-service-user=geoclue --enable-demo-agent
@@ -141,6 +144,10 @@

  %{_datadir}/applications/geoclue-where-am-i.desktop

  

  %changelog

+ * Thu Jul 05 2018 Robert-André Mauchin <zebob.m@gmail.com> - 2.4.10-3

+ - Backport patch for bug 68

+ - Fixes RHBZ#1585970

+ 

  * Fri Jul 13 2018 Fedora Release Engineering <releng@fedoraproject.org> - 2.4.10-2

  - Rebuilt for https://fedoraproject.org/wiki/Fedora_29_Mass_Rebuild

  

rebased onto f13ccde

5 years ago

Can we move this forward please?

cc @hadess

Thank you! I'd also very much like to see this moved forward. But there seem to be a conflict introduced since submitting the pull request. Maybe author needs to resolve it before maintainer can merge.

Thank you! I'd also very much like to see this moved forward. But there seem to be a conflict introduced since submitting the pull request. Maybe author needs to resolve it before maintainer can merge.

Ha yes it is due to the mass rebuild, I rebased it.

rebased onto 8019961

5 years ago

If anybody has an RPM I'll be happy to test it out.

Simple Koji Ci results are on the right here. Get RPM from there. Those would be f29 RPMs. If they are uninstallable, I'll be happy to make a f28 scratchbuild.

$ rpm -qa geoclue2
geoclue2-2.4.10-2.fc29.x86_64
$ rpm -qa redshift
redshift-1.11-8.fc28.x86_64

$ redshift
Trying location provider `geoclue2'...
Using provider `geoclue2'.
Unable to start GeoClue client: GDBus.Error:org.freedesktop.DBus.Error.NoReply: Message recipient disconnected from message bus without replying.
Unable to connect to GeoClue.
Unable to get location from provider.

Any ideas? I did systemctl daemon-reload && systemctl stop geoclue && systemctl start geoclue.

Hi, can you please check your logs to see any messages from geoclue? If there aren't any, see if you can start geoclue manually as 'geoclue' user with G_MESSAGES_DEBUG=Gecolue env.

I now understand there is a geoclue agent that needs to be started by user. I killed existing copy and started a new one. I think it is working now:

$ ps -ef | grep geo
$ /usr/libexec/geoclue-2.0/demos/agent & disown
$ ps -ef | grep geo
user   14166  8553  0 15:41 ?        00:00:00 /usr/libexec/geoclue-2.0/demos/agent
$ redshift
Trying location provider `geoclue2'...
Using provider `geoclue2'.
Using method `randr'.

One issue is that agent asks every time whether to allow redshift to use location instead of remembering the choice. Another thing is that if I'm slow to allow usage, then redshift dies.

IMO there is more work needed to make interaction between redshift an geoclue nicer. But this patch is big step in the right direction.

Thank you.

@akostadi No, the agent should not need to be started manually but if starting it manually fixes the issue, then you've found the problem.

The agent in question wasn't originally meant for anything more than a simple demo but since no OS other than GNOME implemented an agent, we decided to make that the agent for non-GNOME for now.

@zeenix , I'm running XFCE and agent starts automatically. I just needed to restart it apparently for the RPM update to take place without restart.

In the log above I made sure to start the agent without having geoclue process running. As far as I understood the issue was when agent started before geoclue.

It would be really helpful if agent writes to some place in user home to remember allow/deny user decisions.

@akostadi launching the agent should actually automatically launch the service. Is the dbus autolaunch not working on your system for some reason?

About agent remembering the user's choice, I agree that a real agent really should but this is just a work around. XFCE, KDE and others really should provide an agent if they care at all about user's privacy. geoclue2 has been around for many years now.

@zeenix , actually it works. I think I demonstrated in log that only agent is running when I ran redshift. Thank you for eplanation.

wrt agents, why every DE should implement an agent when one agent in project can cover them all?

@akostadi Great.

About agents, it's because it needs proper integration with/from the desktop. Just showing a yes/no notification is not enough. The agent has to provide a UI to enable/disable geolocation globally and per-app bases, which includes (temporary) revocation of location access for apps after the app has been given access before.

Pull-Request has been closed by eclipseo

5 years ago