Have not read through the whole thread, but why not make intent (that
keyword substitution is desired) explicit rather than implicit (based on
type of the client). E.g. send a flag with request indicating that
substitution is desired on the server side: keyword=substitute, or if you
wanted to get more fancy list of keywords to substitute. Server then can
even return error if the feature is not enabled server side. It should not
matter what type of client makes a request, otherwise a dumb client will
not be able to get raw content even if it wanted one, which is the current
behavior.
P.S. This is just general design advice, I cannot speak as whether the
above approach would be more acceptable or not for inclusion.
My 0.02 UAH,
Vladimir
On Sun, Feb 10, 2013 at 11:20 AM, jinfroster <jinfroster_at_mail.ru> wrote:
> Hello,
>
> CMP> On 02/04/2013 09:55 AM, C. Michael Pilato wrote:
> >> On 02/04/2013 06:30 AM, Philip Martin wrote:
> >>> jinfroster <jinfroster_at_mail.ru> writes:
> >>>
> >>>> Add "SVNKeywordSubstitution" per-directory (repository) mod_dav_svn
> >>>> configuration parameter (default is "Off"). Implement keywords
> >>>> substitution.
> >>>
> >>>> * subversion/mod_dav_svn/repos.c
> >>>> (set_headers):
> >>>> If parameter SVNKeywordSubstitution is On, don't send
> >>>> "Content-length". We can't guess the size of expanded stream at
> >>>> the moment (..is that bad?)
> >>>> (deliver):
> >>>> If parameter SVNKeywordSubstitution is On, perform keywords
> >>>> substitution, like client-side utilities do.
> >>>
> >>> Does your Subversion client use neon? I think this causes the server
> to
> >>> send expanded keywords in response to GET requests and so will break
> >>> Subversion clients using serf.
> >>
> >> Yes, that's precisely what it does. But the problem isn't limited to
> Serf
> >> clients. Any call to svn_ra_get_file() -- for example, to support 'svn
> cat'
> >> -- will suffer. So, yeah, cool idea, but unfortunately the patch is
> >> unacceptable as-is.
>
> CMP> Sorry, hit send too fast. That should be "... Any call to
> svn_ra_get_file()
> CMP> with Neon ..."
>
> You are right! With this patch SVN client complains on bad checksums...
> Sorry, didn't test that well.
> But SVN clients do keywords substitution themselves. The idea was to
> implement substitution for dumb HTTP clients which are missing it.
>
> Would it be sufficent to check 'is_svn_client'?
>
> if (dav_svn__get_keyword_substitution_flag(resource->info->r)
> && !resource->info->repos->is_svn_client)
> {
> ...
>
> If there are more troubles/questions, please point on them to me.
> I'm willing to work on this patch if it has chances to be accepted :)
>
> --
> Best regards,
> jinfroster mailto:jinfroster_at_mail.ru
>
Received on 2013-02-10 17:55:36 CET