You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Sahlberg <da...@gmail.com> on 2021/08/01 20:53:18 UTC
Re: 1.14.x test failure under USE_HTTPV1=1: ra-test 13 commit_empty_last_change
Den tors 22 juli 2021 kl 22:36 skrev Daniel Sahlberg <
daniel.l.sahlberg@gmail.com>:
> I'll re-use this thread's second half instead of starting a new thread
> since I realise Daniel has already seen the same error message as I have
> found.
>
> Den fre 16 juli 2021 kl 15:01 skrev Daniel Shahaf <d.s@daniel.shahaf.name
> >:
>
>> In 1.14.1 and branches/1.14.x@HEAD:
>>
>> % make -s davautocheck APACHE_MPM=event USE_HTTPV1=1
>> TESTS="subversion/tests/libsvn_ra/ra-test"
>> ⋮
>> % cat fails.log
>> [[[
>> /home/daniel/src/svn/14x/./subversion/tests/libsvn_ra/ra-test.c:1691,
>> /home/daniel/src/svn/14x/./subversion/libsvn_ra_serf/commit.c:1536,
>> /home/daniel/src/svn/14x/./subversion/libsvn_ra_serf/commit.c:406,
>> /home/daniel/src/svn/14x/./subversion/libsvn_ra_serf/commit.c:344,
>> /home/daniel/src/svn/14x/./subversion/libsvn_ra_serf/commit.c:284,
>> /home/daniel/src/svn/14x/./subversion/libsvn_ra_serf/util.c:1035,
>> /home/daniel/src/svn/14x/./subversion/libsvn_ra_serf/util.c:984,
>> /home/daniel/src/svn/14x/./subversion/libsvn_ra_serf/util.c:949,
>> /home/daniel/src/svn/14x/./subversion/libsvn_ra_serf/multistatus.c:558:
>> (apr_err=SVN_ERR_FS_CONFLICT)
>> svn_tests: E160024: resource out of date; try updating
>> FAIL: ra-test 13: check how last change applies to empty commit
>> ]]]
>>
>> In trunk I got an unrelated error message —
>> .
>> AH00526: Syntax error on line 161 of
>> …/subversion/tests/cmdline/httpd-…/cfg:
>> AuthzSVNAccessFile and AuthzSVNReposRelativeAccessFile directives are
>> mutually exclusive.
>> .
>> — and didn't investigate further.
>>
>
> While investigating danielsh's patch [1] I stumbled on the same error
> message and did some digging.
>
> Line 161 seems related to r1883838. It pulls in a block of configuration
> using a function location_common(), among other things AuthzSVNAccessFile.
> In the location /authz-test-work/in-repos-authz the directive
> AuthzSVNReposRelativeAccessFile is also added. There is a check in
> mod_authz_svn.c returning the error message above.
>
> Assuming that the error message is correct (it is at least 10 years older
> than the new test), we would have to add the common block without the
> AuthzSVNAccessFile.
>
> A naïve way of doing it might be:
>
> [[[
> Index: subversion/tests/cmdline/davautocheck.sh
> ===================================================================
> --- subversion/tests/cmdline/davautocheck.sh (revision 1891735)
> +++ subversion/tests/cmdline/davautocheck.sh (working copy)
> @@ -547,10 +547,9 @@
>
> <Location /svn-test-work/repositories>
> __EOF__
> -location_common() {
> +location_common_without_authz() {
> cat >> "$HTTPD_CFG" <<__EOF__
> DAV svn
> - AuthzSVNAccessFile
> "$ABS_BUILDDIR/subversion/tests/cmdline/svn-test-work/authz"
> AuthType Basic
> AuthName "Subversion Repository"
> AuthUserFile $HTTPD_USERS
> @@ -560,6 +559,12 @@
> SVNBlockRead ${BLOCK_READ_SETTING}
> __EOF__
> }
> +location_common() {
> +location_common_without_authz
> +cat >> "$HTTPD_CFG" <<__EOF__
> + AuthzSVNAccessFile
> "$ABS_BUILDDIR/subversion/tests/cmdline/svn-test-work/authz"
> +__EOF__
> +}
> location_common
> cat >> "$HTTPD_CFG" <<__EOF__
> SVNParentPath
> "$ABS_BUILDDIR/subversion/tests/cmdline/svn-test-work/repositories"
> @@ -612,7 +617,7 @@
> </Location>
> <Location /authz-test-work/in-repos-authz>
> __EOF__
> -location_common
> +location_common_without_authz
> cat >> "$HTTPD_CFG" <<__EOF__
> SVNParentPath
> "$ABS_BUILDDIR/subversion/tests/cmdline/svn-test-work/repositories"
> Require valid-user
> ]]]
>
> Kind regards,
> Daniel Sahlberg
>
> [1]
> https://mail-archives.apache.org/mod_mbox/subversion-dev/202107.mbox/%3C20210716135352.GA2413%40tarpaulin.shahaf.local2%3E
>
>
Did anyone have the time to review this?
I realise another way to do it would be to add an argument to
location_common. Maybe a bit clearer, possibly even reducing to a single
call to cat if the AuthzSVNAccessFile is put in a variable, performance..!
Let me know if you would prefer that way of solving it.
Kind regards,
Daniel Sahlberg
Re: 1.14.x test failure under USE_HTTPV1=1: ra-test 13 commit_empty_last_change
Posted by Daniel Sahlberg <da...@gmail.com>.
Den sön 8 aug. 2021 kl 22:07 skrev Stefan Sperling <st...@elego.de>:
> On Sun, Aug 01, 2021 at 10:53:18PM +0200, Daniel Sahlberg wrote:
> > Did anyone have the time to review this?
> >
> > I realise another way to do it would be to add an argument to
> > location_common. Maybe a bit clearer, possibly even reducing to a single
> > call to cat if the AuthzSVNAccessFile is put in a variable,
> performance..!
> > Let me know if you would prefer that way of solving it.
>
> This slipped through my testing because I don't use davautocheck.sh.
>
> The proposed fix seems fine to me. I would suggest that you commit the
> patch. As far as I can tell nobody has objected to your proposal.
> The code could still be tweaked later in follow-up commits if anyone
> comes up with new ideas.
>
Thanks for reviewing. Committed as r1892121 (plus some messing around with
a backport that didn't go as planned and thus reverted, sorry for spamming
commits@).
/Daniel
Re: 1.14.x test failure under USE_HTTPV1=1: ra-test 13
commit_empty_last_change
Posted by Stefan Sperling <st...@elego.de>.
On Sun, Aug 01, 2021 at 10:53:18PM +0200, Daniel Sahlberg wrote:
> Did anyone have the time to review this?
>
> I realise another way to do it would be to add an argument to
> location_common. Maybe a bit clearer, possibly even reducing to a single
> call to cat if the AuthzSVNAccessFile is put in a variable, performance..!
> Let me know if you would prefer that way of solving it.
This slipped through my testing because I don't use davautocheck.sh.
The proposed fix seems fine to me. I would suggest that you commit the
patch. As far as I can tell nobody has objected to your proposal.
The code could still be tweaked later in follow-up commits if anyone
comes up with new ideas.
Cheers,
Stefan