[PATCH] fix libsvn_auth_gnome_keyring.pc w/libsecret

[Joe Orton <jo...@redhat.com>]

libsvn_auth_gnome_keyring.pc is wrong in 1.10.0; when built against 
libsecret it still references the gnome-keyring library.

...
Requires:  apr-1 gnome-keyring-1
Requires.private:  libsvn_subr

The dep should also be private; fix attached OK?

[[[
Fixes libsvn_auth_gnome_keyring.pc when configured with libsecret, and
make dependencies private.

* configure.in, build/ac-macros/libsecret.m4:
  Set SVN_GNOME_KEYRING_PCLIBS to libsecret/gnome-keyring as appropriate.

* build.conf (gnome-keyring): 
  Substitute SVN_GNOME_KEYRING_PCLIBS as private deps of 
  libsvn_auth_gnome_keyring in generated pkg-config file.
]]]

Re: [PATCH] fix libsvn_auth_gnome_keyring.pc w/libsecret

[Joe Orton <jo...@redhat.com>]

On Thu, May 03, 2018 at 08:32:56PM +0100, Philip Martin wrote:
> Your patch is an improvement so I would be happy to see it on
> trunk/1.10.  Going further and removing some .pc files is probably also
> suitable for 1.10.  Changing the DSO names and installation location is
> probably not suitable for 1.10.

OK, committed to trunk in r1830882.  Thanks for reviewing!

Regards, Joe

Re: [PATCH] fix libsvn_auth_gnome_keyring.pc w/libsecret

[Philip Martin <ph...@codematters.co.uk>]

Joe Orton <jo...@redhat.com> writes:

> I thought about that, but the SVN buildsystem is building & installing 
> them like libraries not like DSOs (which should be linked with libtool's 
> -module argument and not installed directly in $libdir),

I suspect that is accidental.  The RA/FS option --enable-dso, which
became --enable-runtime-module-search, was added a long time ago and the
names of the libraries/modules do not change when switching between
shared library and DSO.  When the auth DSOs were added they just
followed the same naming scheme as the RA/FS DSOs.

> and there are 
> symbols exposed in the headers like svn_auth_gnome_keyring_version().  

I'm not sure why those functions are public, we should probably
deprecate them and make them private functions.  I don't think a 3rd
party can do anything useful with the public gnome/kwallet API.  Even a
3rd party implementing a custom auth provider would not use the
gnome/kwallet part of the svn_auth_ API.

> So it looks a bit weird if they are really not intended to be used as 
> libraries?

Your patch is an improvement so I would be happy to see it on
trunk/1.10.  Going further and removing some .pc files is probably also
suitable for 1.10.  Changing the DSO names and installation location is
probably not suitable for 1.10.

-- 
Philip

Re: [PATCH] fix libsvn_auth_gnome_keyring.pc w/libsecret

[Joe Orton <jo...@redhat.com>]

On Wed, May 02, 2018 at 04:32:26PM +0100, Philip Martin wrote:
> Philip Martin <ph...@codematters.co.uk> writes:
> 
> >> Fixes libsvn_auth_gnome_keyring.pc when configured with libsecret, and
> >> make dependencies private.
> >
> > Looks OK to me.
> 
> Perhaps we should delete the .pc files for gnome/kwallet auth libraries.
> The corresponding libraries do not really have a public API and I think
> nodody will ever attempt to link against them.  In Subversion the
> libraries are loaded at runtime using APR's DSO functions so even we do
> not link against them.

I thought about that, but the SVN buildsystem is building & installing 
them like libraries not like DSOs (which should be linked with libtool's 
-module argument and not installed directly in $libdir), and there are 
symbols exposed in the headers like svn_auth_gnome_keyring_version().  
So it looks a bit weird if they are really not intended to be used as 
libraries?

Regards, Joe

Re: [PATCH] fix libsvn_auth_gnome_keyring.pc w/libsecret

[Stefan Sperling <st...@apache.org>]

On Thu, May 03, 2018 at 09:11:33AM +0100, Philip Martin wrote:
> Philip Martin <ph...@codematters.co.uk> writes:
> 
> > Philip Martin <ph...@codematters.co.uk> writes:
> >
> >>> Fixes libsvn_auth_gnome_keyring.pc when configured with libsecret, and
> >>> make dependencies private.
> >>
> >> Looks OK to me.
> >
> > Perhaps we should delete the .pc files for gnome/kwallet auth libraries.
> > The corresponding libraries do not really have a public API and I think
> > nodody will ever attempt to link against them.  In Subversion the
> > libraries are loaded at runtime using APR's DSO functions so even we do
> > not link against them.
> 
> We should probably remove the .pc files for anything that doesn't have a
> public API, i.e. the RA backends, the FS backends and fs_util in
> addition to gnome/kwallet auth libraries.

+1

Re: [PATCH] fix libsvn_auth_gnome_keyring.pc w/libsecret

[Philip Martin <ph...@codematters.co.uk>]

Philip Martin <ph...@codematters.co.uk> writes:

> Philip Martin <ph...@codematters.co.uk> writes:
>
>>> Fixes libsvn_auth_gnome_keyring.pc when configured with libsecret, and
>>> make dependencies private.
>>
>> Looks OK to me.
>
> Perhaps we should delete the .pc files for gnome/kwallet auth libraries.
> The corresponding libraries do not really have a public API and I think
> nodody will ever attempt to link against them.  In Subversion the
> libraries are loaded at runtime using APR's DSO functions so even we do
> not link against them.

We should probably remove the .pc files for anything that doesn't have a
public API, i.e. the RA backends, the FS backends and fs_util in
addition to gnome/kwallet auth libraries.

-- 
Philip

Re: [PATCH] fix libsvn_auth_gnome_keyring.pc w/libsecret

[Philip Martin <ph...@codematters.co.uk>]

Philip Martin <ph...@codematters.co.uk> writes:

>> Fixes libsvn_auth_gnome_keyring.pc when configured with libsecret, and
>> make dependencies private.
>
> Looks OK to me.

Perhaps we should delete the .pc files for gnome/kwallet auth libraries.
The corresponding libraries do not really have a public API and I think
nodody will ever attempt to link against them.  In Subversion the
libraries are loaded at runtime using APR's DSO functions so even we do
not link against them.

-- 
Philip

Re: [PATCH] fix libsvn_auth_gnome_keyring.pc w/libsecret

[Philip Martin <ph...@codematters.co.uk>]

Joe Orton <jo...@redhat.com> writes:

> libsvn_auth_gnome_keyring.pc is wrong in 1.10.0; when built against 
> libsecret it still references the gnome-keyring library.
>
> ...
> Requires:  apr-1 gnome-keyring-1
> Requires.private:  libsvn_subr
>
> The dep should also be private; fix attached OK?
>
> [[[
> Fixes libsvn_auth_gnome_keyring.pc when configured with libsecret, and
> make dependencies private.

Looks OK to me.

-- 
Philip