You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2007/12/17 15:54:56 UTC
static linking broken in trunk since commit of clientstring feature
Hi,
linking statically on Linux/BSD seems to have been broken since
r28503 (custom client string feature):
subversion/libsvn_ra_neon/session.c:831: undefined reference to `svn_ra_get_client_namestring'
"svn log -r28503 | head -n3":
------------------------------------------------------------------------
r28503 | epg | 2007-12-15 21:33:37 +0100 (Sat, 15 Dec 2007) | 32 lines
Allow the user-agent string sent over http to be defined by the client.
Quoting #svn-dev:
(16:38:34) maxb: Oh, gaaagh
(16:38:50) maxb: Someone's gone and introduced circular linkage again, haven't they?
(16:39:09) stsp: it worked last week :)
(16:39:13) maxb: Yup
(16:39:16) maxb: Bah
(16:39:19) sussman [n=sussman@72.14.229.1] entered the room.
(16:39:19) mode (+o sussman ) by ChanServ
(16:39:34) stsp: maxb: do you know how to fix this?
(16:40:10) maxb: Go point out to whoever committed the namestring stuff that circular linkage is not portable, and they need to redesign their patch
(16:40:28) maxb: And meanwhile, if it's giving other's a pain in development, revert it :-)
(16:41:30) stsp: can you explain exactly what is circular?
(16:41:44) stsp: I don't get it :/
(16:41:52) maxb: libsvn_ra depends on libsvn_ra_neon depends on libsvn_ra
(16:41:58) maxb: ditto serf
(16:42:06) stsp: ah, ok
Anyone else OK with reverting it?
Eric, would you be OK with this?
--
Stefan Sperling <st...@elego.de> Software Developer
elego Software Solutions GmbH HRB 77719
Gustav-Meyer-Allee 25, Gebaeude 12 Tel: +49 30 23 45 86 96
13355 Berlin Fax: +49 30 23 45 86 95
http://www.elego.de Geschaeftsfuehrer: Olaf Wagner
Re: static linking broken in trunk since commit of clientstring feature
Posted by Stefan Küng <to...@gmail.com>.
C. Michael Pilato wrote:
> Eric Gillespie wrote:
>>> I don't think that will work. Last time I wanted to add something to
>>> svn_client_ctx_t and use it in the ra-layers, it didn't work because
>>> svn_client_ctx_t is *not* passed down there (at least not as far down as
>>> I needed it to). And I don't like to change APIs or introduce a whole
>> I don't know what you were doing before, but in this case, you're
>> just copying the function pointer from the ctx to the callbacks
>> in libsvn_client/ra.c:svn_client__open_ra_session_internal .
>
> Maybe this can serve as a good head start for you, Stefan? It's untested,
> but does reflect the basic idea Eric and I are thinking of.
Thanks.
I'll try to get this done sometimes this week. Don't have much time so
close to the holidays - the office is keeping me busy.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: static linking broken in trunk since commit of clientstring feature
Posted by "C. Michael Pilato" <cm...@collab.net>.
Eric Gillespie wrote:
>> I don't think that will work. Last time I wanted to add something to
>> svn_client_ctx_t and use it in the ra-layers, it didn't work because
>> svn_client_ctx_t is *not* passed down there (at least not as far down as
>> I needed it to). And I don't like to change APIs or introduce a whole
>
> I don't know what you were doing before, but in this case, you're
> just copying the function pointer from the ctx to the callbacks
> in libsvn_client/ra.c:svn_client__open_ra_session_internal .
Maybe this can serve as a good head start for you, Stefan? It's untested,
but does reflect the basic idea Eric and I are thinking of.
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 28513)
+++ subversion/include/svn_client.h (working copy)
@@ -857,6 +857,10 @@
svn_wc_conflict_resolver_func_t conflict_func;
void *conflict_baton;
+ /** Custom client name string, or @c null.
+ * @since New in 1.5. */
+ const char *client_name;
+
} svn_client_ctx_t;
/** @} end group: Client context management */
Index: subversion/include/svn_ra.h
===================================================================
--- subversion/include/svn_ra.h (revision 28513)
+++ subversion/include/svn_ra.h (working copy)
@@ -117,6 +117,16 @@
(void *session_baton,
svn_revnum_t *latest_revnum);
+/** A function type which allows the RA layer to ask about any
+ * customizations to the client name string. This is primarily used
+ * by HTTP-based RA layers wishing to extend the string reported to
+ * Apache/mod_dav_svn via the User-agent HTTP header.
+ */
+typedef svn_error_t *(*svn_ra_get_client_string_func_t)(void *baton,
+ const char **name,
+ apr_pool_t *pool);
+
+
/**
* A callback function type for use in @c get_file_revs.
* @a baton is provided by the caller, @a path is the pathname of the file
@@ -488,10 +498,16 @@
*/
svn_cancel_func_t cancel_func;
+ /** Client string customization callback function
+ * @since New in 1.5
+ */
+ svn_ra_get_client_string_func_t get_client_string;
+
} svn_ra_callbacks2_t;
/** Similar to svn_ra_callbacks2_t, except that the progress
- * notification function and baton is missing.
+ * notification function and baton, cancellation function, and client
+ * string customization function are missing.
*
* @deprecated Provided for backward compatibility with the 1.2 API.
*/
Index: subversion/libsvn_client/ra.c
===================================================================
--- subversion/libsvn_client/ra.c (revision 28513)
+++ subversion/libsvn_client/ra.c (working copy)
@@ -268,6 +268,17 @@
return (b->ctx->cancel_func)(b->ctx->cancel_baton);
}
+
+static svn_error_t *
+get_client_string(void *baton,
+ const char **name,
+ apr_pool_t *pool)
+{
+ svn_client__callback_baton_t *b = baton;
+ *name = apr_pstrdup(pool, b->ctx->client_name);
+ return SVN_NO_ERROR;
+}
+
svn_error_t *
svn_client__open_ra_session_internal(svn_ra_session_t **ra_session,
const char *base_url,
@@ -291,6 +302,7 @@
cbtable->progress_func = ctx->progress_func;
cbtable->progress_baton = ctx->progress_baton;
cbtable->cancel_func = ctx->cancel_func ? cancel_callback : NULL;
+ cbtable->get_client_string = get_client_string;
cb->base_dir = base_dir;
cb->base_access = base_access;
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: static linking broken in trunk since commit of clientstring feature
Posted by Stefan Küng <to...@gmail.com>.
Eric Gillespie wrote:
> =?ISO-8859-1?Q?Stefan_K=FCng?= <to...@gmail.com> writes:
>
>> What exactly is 'icky' about moving this to svn_subr?
>
> "Oh, but this one RA interfaces is in svn-subr. Sorry."
>
> Ick.
>
>>> Stefan, care to work up a patch to do this with a new field in
>>> the callback struct?
>> I don't think that will work. Last time I wanted to add something to
>> svn_client_ctx_t and use it in the ra-layers, it didn't work because
>> svn_client_ctx_t is *not* passed down there (at least not as far down as
>> I needed it to). And I don't like to change APIs or introduce a whole
>
> I don't know what you were doing before, but in this case, you're
> just copying the function pointer from the ctx to the callbacks
> in libsvn_client/ra.c:svn_client__open_ra_session_internal .
That's what I mean: the svn_client_ctx_t is not passed down to the ra
layers. For every new entry there another one has to be created in the
svn_ra_callbacks2_t struct. And then the new members of that struct must
be copied yet to another struct (session, connection) to pass those
values further down to where they're actually needed.
Do you really think this is the way to go? Putting that one ra-util
function in svn_subr shouldn't be such a big problem considering the
amount of changes required otherwise.
In case you're wondering what I needed that for last time: I once tried
to implement better progress information callbacks. But since
svn_client_ctx_t is not passed down directly to the other libraries I
gave up after changing two libraries: way too much data copying was
needed for a simple 'global' variable to get incremented in smaller
steps (the progress variable).
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: static linking broken in trunk since commit of clientstring feature
Posted by Eric Gillespie <ep...@pretzelnet.org>.
=?ISO-8859-1?Q?Stefan_K=FCng?= <to...@gmail.com> writes:
> What exactly is 'icky' about moving this to svn_subr?
"Oh, but this one RA interfaces is in svn-subr. Sorry."
Ick.
> > Stefan, care to work up a patch to do this with a new field in
> > the callback struct?
>
> I don't think that will work. Last time I wanted to add something to
> svn_client_ctx_t and use it in the ra-layers, it didn't work because
> svn_client_ctx_t is *not* passed down there (at least not as far down as
> I needed it to). And I don't like to change APIs or introduce a whole
I don't know what you were doing before, but in this case, you're
just copying the function pointer from the ctx to the callbacks
in libsvn_client/ra.c:svn_client__open_ra_session_internal .
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: static linking broken in trunk since commit of clientstring feature
Posted by Stefan Küng <to...@gmail.com>.
Eric Gillespie wrote:
> Eric Gillespie <ep...@pretzelnet.org> writes:
>
>> "David Glasser" <gl...@davidglasser.net> writes:
>>
>>> Some discussion on the IRC channel suggested adding a callback to the
>>> svn_ra_callbacks table, and a field to the svn_client_ctx_t structure,
>>> to allow pushing this data through to the RA layer without needing a
>>> static variable.
>>>
>>> This would be much more invasive, of course.
>> I think it's overkill. This is one-time initialization, so it
>> shouldn't be difficult not to call it from multiple threads.
>
> When I was about to add the note to svn_ra.h that these two
> interfaces are actually in svn-subr, I lost my nerve. It's too
> icky after all.
Sorry for the late reply, I almost missed this thread...
What exactly is 'icky' about moving this to svn_subr?
If you think the static stuff is icky: the same static stuff is used for
the .svn/_svn foldername changes.
> Stefan, care to work up a patch to do this with a new field in
> the callback struct?
I don't think that will work. Last time I wanted to add something to
svn_client_ctx_t and use it in the ra-layers, it didn't work because
svn_client_ctx_t is *not* passed down there (at least not as far down as
I needed it to). And I don't like to change APIs or introduce a whole
lot of new ones (such patches from me have a chance of almost zero to
get committed, so I would just waste my time).
> I'm reverting this in the mean-time.
Ok.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: static linking broken in trunk since commit of clientstring feature
Posted by Eric Gillespie <ep...@pretzelnet.org>.
Eric Gillespie <ep...@pretzelnet.org> writes:
> "David Glasser" <gl...@davidglasser.net> writes:
>
> > Some discussion on the IRC channel suggested adding a callback to the
> > svn_ra_callbacks table, and a field to the svn_client_ctx_t structure,
> > to allow pushing this data through to the RA layer without needing a
> > static variable.
> >
> > This would be much more invasive, of course.
>
> I think it's overkill. This is one-time initialization, so it
> shouldn't be difficult not to call it from multiple threads.
When I was about to add the note to svn_ra.h that these two
interfaces are actually in svn-subr, I lost my nerve. It's too
icky after all.
Stefan, care to work up a patch to do this with a new field in
the callback struct?
I'm reverting this in the mean-time.
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: static linking broken in trunk since commit of clientstring feature
Posted by Eric Gillespie <ep...@pretzelnet.org>.
"David Glasser" <gl...@davidglasser.net> writes:
> Some discussion on the IRC channel suggested adding a callback to the
> svn_ra_callbacks table, and a field to the svn_client_ctx_t structure,
> to allow pushing this data through to the RA layer without needing a
> static variable.
>
> This would be much more invasive, of course.
I think it's overkill. This is one-time initialization, so it
shouldn't be difficult not to call it from multiple threads.
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: static linking broken in trunk since commit of clientstring feature
Posted by David Glasser <gl...@davidglasser.net>.
On Dec 17, 2007 10:53 AM, Eric Gillespie <ep...@pretzelnet.org> wrote:
> "David James" <ja...@cs.toronto.edu> writes:
>
> > Instead of reverting r28503, why don't you just move
> > subversion/libsvn_ra/clientstring.c to subversion/libsvn_subr ? This
> > will eliminate the circular dependency. libsvn_subr is a grab bag of
> > utilities anyway.
> >
> > If it turns out later that libsvn_ra_neon and libsvn_ra_serf need to
> > share more functions (and not just these two simple functions), we'll
> > want to go the whole way and create a "libsvn_ra_util" library,
> > similar to libsvn_fs_util. Right now, that seems like overkill though,
> > so I think the simple fix above will do.
> >
> > We can document the above thinking by adding a short comment to the
> > top of clientstring.c, so that folks will remember to create a new
> > library later if it is needed.
> >
> > (I've CC'd Max and epg to see if they have better ideas.)
>
> I agree there's no sense creating ra-util just yet. I'll move it today.
Some discussion on the IRC channel suggested adding a callback to the
svn_ra_callbacks table, and a field to the svn_client_ctx_t structure,
to allow pushing this data through to the RA layer without needing a
static variable.
This would be much more invasive, of course.
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: static linking broken in trunk since commit of clientstring feature
Posted by Eric Gillespie <ep...@pretzelnet.org>.
"David James" <ja...@cs.toronto.edu> writes:
> Instead of reverting r28503, why don't you just move
> subversion/libsvn_ra/clientstring.c to subversion/libsvn_subr ? This
> will eliminate the circular dependency. libsvn_subr is a grab bag of
> utilities anyway.
>
> If it turns out later that libsvn_ra_neon and libsvn_ra_serf need to
> share more functions (and not just these two simple functions), we'll
> want to go the whole way and create a "libsvn_ra_util" library,
> similar to libsvn_fs_util. Right now, that seems like overkill though,
> so I think the simple fix above will do.
>
> We can document the above thinking by adding a short comment to the
> top of clientstring.c, so that folks will remember to create a new
> library later if it is needed.
>
> (I've CC'd Max and epg to see if they have better ideas.)
I agree there's no sense creating ra-util just yet. I'll move it today.
--
Eric Gillespie <*> epg@pretzelnet.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: static linking broken in trunk since commit of clientstring feature
Posted by David James <ja...@cs.toronto.edu>.
Hi Stefan,
Instead of reverting r28503, why don't you just move
subversion/libsvn_ra/clientstring.c to subversion/libsvn_subr ? This
will eliminate the circular dependency. libsvn_subr is a grab bag of
utilities anyway.
If it turns out later that libsvn_ra_neon and libsvn_ra_serf need to
share more functions (and not just these two simple functions), we'll
want to go the whole way and create a "libsvn_ra_util" library,
similar to libsvn_fs_util. Right now, that seems like overkill though,
so I think the simple fix above will do.
We can document the above thinking by adding a short comment to the
top of clientstring.c, so that folks will remember to create a new
library later if it is needed.
(I've CC'd Max and epg to see if they have better ideas.)
Cheers,
David
On Dec 17, 2007 7:54 AM, Stefan Sperling <st...@elego.de> wrote:
> Hi,
>
> linking statically on Linux/BSD seems to have been broken since
> r28503 (custom client string feature):
>
> subversion/libsvn_ra_neon/session.c:831: undefined reference to `svn_ra_get_client_namestring'
>
> "svn log -r28503 | head -n3":
>
> ------------------------------------------------------------------------
> r28503 | epg | 2007-12-15 21:33:37 +0100 (Sat, 15 Dec 2007) | 32 lines
>
> Allow the user-agent string sent over http to be defined by the client.
>
>
> Quoting #svn-dev:
>
> (16:38:34) maxb: Oh, gaaagh
> (16:38:50) maxb: Someone's gone and introduced circular linkage again, haven't they?
> (16:39:09) stsp: it worked last week :)
> (16:39:13) maxb: Yup
> (16:39:16) maxb: Bah
> (16:39:19) sussman [n=sussman@72.14.229.1] entered the room.
> (16:39:19) mode (+o sussman ) by ChanServ
> (16:39:34) stsp: maxb: do you know how to fix this?
> (16:40:10) maxb: Go point out to whoever committed the namestring stuff that circular linkage is not portable, and they need to redesign their patch
> (16:40:28) maxb: And meanwhile, if it's giving other's a pain in development, revert it :-)
> (16:41:30) stsp: can you explain exactly what is circular?
> (16:41:44) stsp: I don't get it :/
> (16:41:52) maxb: libsvn_ra depends on libsvn_ra_neon depends on libsvn_ra
> (16:41:58) maxb: ditto serf
> (16:42:06) stsp: ah, ok
>
>
> Anyone else OK with reverting it?
> Eric, would you be OK with this?
>
> --
> Stefan Sperling <st...@elego.de> Software Developer
> elego Software Solutions GmbH HRB 77719
> Gustav-Meyer-Allee 25, Gebaeude 12 Tel: +49 30 23 45 86 96
> 13355 Berlin Fax: +49 30 23 45 86 95
> http://www.elego.de Geschaeftsfuehrer: Olaf Wagner
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org