[Merge] lp:~lool/ubuntu-location-provider-here/dbus-activation-fix into lp:ubuntu-location-provider-here

Manuel de la Peña manuel.delapena at canonical.com
Wed Oct 22 18:59:26 UTC 2014


Review: Needs Fixing



Diff comments:

> === modified file 'usr/bin/herepositioning-license-accepted'
> --- usr/bin/herepositioning-license-accepted	2014-09-17 14:18:33 +0000
> +++ usr/bin/herepositioning-license-accepted	2014-10-22 18:20:44 +0000
> @@ -33,15 +33,19 @@
>  USERNAME="$1"
>  USERNAME="${USERNAME:-phablet}"
>  
> -if ! uid="$(id -u $USERNAME)"; then
> -    die "User $USERNAME not found" >&2
> +# NB: this triggers DBus activation in case Account Service wasn't running
> +if ! user="$(ofa_dbus /org/freedesktop/Accounts \
> +                      org.freedesktop.Accounts.FindUserByName \
> +                      "string:$USERNAME")"; then
> +    die "Couldn't find user $USERNAME in AccountService"
>  fi
>  
> -if ! data="$(dbus_do org.freedesktop.Accounts \
> -                     /org/freedesktop/Accounts/User$uid \
> -                     org.freedesktop.DBus.Properties.GetAll \
> -                     string:com.ubuntu.location.providers.here.AccountsService)"; then
> -    die "Couldn't read AccountService properties for uid $uid"
> +userpath="$(echo "$user" | dbus_object_pathes)"
> +
> +if ! data="$(ofa_dbus "$userpath" \
> +                      org.freedesktop.DBus.Properties.GetAll \
> +                      string:com.ubuntu.location.providers.here.AccountsService)"; then
> +    die "Couldn't read HERE AccountService properties for user path $userpath"
>  fi
>  
>  if [ "$(get_dbus_prop "$data" "boolean" "LicenseAccepted")" = "true" ]; then
> 
> === modified file 'usr/share/ubuntu-location-provider-here/functions'
> --- usr/share/ubuntu-location-provider-here/functions	2014-09-17 14:18:33 +0000
> +++ usr/share/ubuntu-location-provider-here/functions	2014-10-22 18:20:44 +0000
> @@ -18,6 +18,15 @@
>      dbus_do "org.ofono" "$@"
>  }
>  
> +ofa_dbus() {
> +    dbus_do "org.freedesktop.Accounts" "$@"
> +}
> +
> +# stdin: DBus reply, stdout: DBus object pathes, one per line
> +dbus_object_pathes() {

Small type, should be dbus_object_paths, right?

> +    sed -n 's/^ *object path "\(.*\)"$/\1/p'
> +}
> +
>  # DBus object pathes of oFono modems, one per line; bails out if oFono isn't
>  # available
>  get_modems() {
> @@ -27,7 +36,7 @@
>          return 1
>      fi
>      # convert to /ril_0 etc. object pathes
> -    echo "$modems" | sed -n 's/^ *object path "\(.*\)"$/\1/p'
> +    echo "$modems" | dbus_object_pathes
>  }
>  
>  # extracts value of a property from a DBus reply; variant is the type
> 


-- 
https://code.launchpad.net/~lool/ubuntu-location-provider-here/dbus-activation-fix/+merge/239252
Your team Ubuntu Phablet Team is subscribed to branch lp:ubuntu-location-provider-here.



More information about the Ubuntu-reviews mailing list