[PATCH] Re: SVN-4783 KWallet assumes KDE5 when Qt5 present

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Re: SVN-4783 KWallet assumes KDE5 when Qt5 present

Satya Mishra
Hi,

As requested in the bug report, here’s a patch that doesn’t hard-code the path to kf5-config, but instead assumes it’s in the path.

> The test for kwallet assumes KDE5 when Qt5 present, but that is not necessarily the case. For example CentOS7 has KDE4 and Qt5. The correct test should test for kf5-config in addition to Qt5.


Regards,
Satya



kwallet-m4.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Re: SVN-4783 KWallet assumes KDE5 when Qt5 present

Julian Foad-6
Satya Mishra wrote on 2018-11-19:
> As requested in the bug report, here’s a patch that doesn’t hard-code
> the path to kf5-config, but instead assumes it’s in the path.

Thanks! It looks plausible to me.

Is anyone here able to test and commit it?

SVN-4783: https://issues.apache.org/jira/browse/SVN-4783

--
- Julian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Re: SVN-4783 KWallet assumes KDE5 when Qt5 present

Julian Foad-5
Ping! Daniel or anyone, are you able to review/test/commit this?

Julian Foad wrote on 2018-11-23:
> Satya Mishra wrote on 2018-11-19:
> > As requested in the bug report, here’s a patch that doesn’t hard-code
> > the path to kf5-config, but instead assumes it’s in the path.
>
> Thanks! It looks plausible to me.
>
> Is anyone here able to test and commit it?
>
> SVN-4783: https://issues.apache.org/jira/browse/SVN-4783
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Re: SVN-4783 KWallet assumes KDE5 when Qt5 present

Daniel Shahaf-5
Julian Foad wrote on Mon, Jan 28, 2019 at 17:07:27 +0000:
> Ping! Daniel or anyone, are you able to review/test/commit this?

I'm afraid I'm not.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Re: SVN-4783 KWallet assumes KDE5 when Qt5 present

James McCoy-3
In reply to this post by Satya Mishra
On Mon, Nov 19, 2018 at 02:19:12PM -0600, Satya Mishra wrote:
> Hi,
>
> As requested in the bug report, here’s a patch that doesn’t hard-code the path to kf5-config, but instead assumes it’s in the path.
>
> > The test for kwallet assumes KDE5 when Qt5 present, but that is not
> > necessarily the case. For example CentOS7 has KDE4 and Qt5. The
> > correct test should test for kf5-config in addition to Qt5.

There are 3 ways that --with-kwallet can be used.

* --with-kwallet, everything is detected from the environment,
  kf5-config should be in $PATH.  AC_PATH_PROG() is used to find it.
* --with-kwallet=/basedir, kf5-config is expected to be under
  /basedir/bin
* --with-kwallet=/incdir:/libdir, kf5-config is not used

> --- subversion-1.10.3/build/ac-macros/kwallet.m4        2018-10-24 11:50:47.000000000 -0700
> +++ subversion-1.10.3/build/ac-macros/kwallet.m4        2018-10-24 11:55:37.000000000 -0700
> @@ -45,7 +45,8 @@
>            if test -n "$PKG_CONFIG"; then
>              if test "$HAVE_DBUS" = "yes"; then
> +              AC_CHECK_PROG(KF5_CHECK,kf5-config,yes)
>                AC_MSG_CHECKING([for Qt])
> -              if $PKG_CONFIG --exists Qt5Core Qt5DBus Qt5Gui; then
> +              if test x"$KF5_CHECK" = x"yes" && $PKG_CONFIG --exists Qt5Core Qt5DBus Qt5Gui ; then
>                  AC_MSG_RESULT([yes, Qt5])
>                  qt_pkg_config_names="Qt5Core Qt5DBus Qt5Gui"
>                  kde_config_name="kf5-config"

This patch should help with the first case above (aside from duplicating
the search for kf5-config).  However, it seems like it would affect the
second case if /basedir wasn't part of $PATH.

Maybe some of the later logic that handles $svn_lib_kwallet's different
forms should be hoisted up here.

Cheers,
--
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB